diff mbox series

[ovs-dev,v2,2/2] * netdev-dpdk: Add support for userspace port-based ingress packet-per-second policing.

Message ID SY4PR01MB843839E2A6117E7845597CFBCD949@SY4PR01MB8438.ausprd01.prod.outlook.com
State Changes Requested
Headers show
Series netdev-dpdk: Add support for userspace port-based packet-per-second policing. | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

miter April 9, 2023, 11:15 a.m. UTC 
From: Lin Huang <linhuang@ruijie.com.cn>

OvS has supported packet-per-second policer which can be set at ingress and egress
side in kernel datapath. But the userspace datapath dosen't support for ingress and
egress packet-per-second policing now.

So, this patch add support for userspace ingress pps policing by using native ovs
token bucket library. Token bucket is accumulated by 'rate' tokens per millisecond
and store maxiumim tokens at 'burst' bucket size. One token in the bucket means
one packet (1 kpkts * millisecond) which will drop or pass by policer.

This patch reuse 'ingress_policing_kpkts_rate' and 'ingress_policing_kpkts_burst'
options at interface table. Now userspace ingress policer supports setting
packet-per-second limits in addition to the previously configurable byte rate
settings.

Examples:
$ ovs-vsctl set interface dpdk0 ingress_policing_rate=12300
$ ovs-vsctl set interface dpdk0 ingress_policing_burst=12300
$ ovs-vsctl set interface dpdk0 ingress_policing_kpkts_rate=123
$ ovs-vsctl set interface dpdk0 ingress_policing_kpkts_burst=123

Add some unit tests for ingress packet-per-second policing.

Signed-off-by: Lin Huang <linhuang@ruijie.com.cn>
---
 NEWS                 |   4 ++
 lib/netdev-dpdk.c    |  99 +++++++++++++++++++++-------
 tests/system-dpdk.at | 152 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 230 insertions(+), 25 deletions(-)

Comments

Simon Horman April 20, 2023, 9:08 a.m. UTC | #1 
On Sun, Apr 09, 2023 at 07:15:09PM +0800, miterv@outlook.com wrote:
> From: Lin Huang <linhuang@ruijie.com.cn>
> 
> OvS has supported packet-per-second policer which can be set at ingress and egress
> side in kernel datapath. But the userspace datapath dosen't support for ingress and
> egress packet-per-second policing now.
> 
> So, this patch add support for userspace ingress pps policing by using native ovs
> token bucket library. Token bucket is accumulated by 'rate' tokens per millisecond
> and store maxiumim tokens at 'burst' bucket size. One token in the bucket means
> one packet (1 kpkts * millisecond) which will drop or pass by policer.
> 
> This patch reuse 'ingress_policing_kpkts_rate' and 'ingress_policing_kpkts_burst'
> options at interface table. Now userspace ingress policer supports setting
> packet-per-second limits in addition to the previously configurable byte rate
> settings.
> 
> Examples:
> $ ovs-vsctl set interface dpdk0 ingress_policing_rate=12300
> $ ovs-vsctl set interface dpdk0 ingress_policing_burst=12300
> $ ovs-vsctl set interface dpdk0 ingress_policing_kpkts_rate=123
> $ ovs-vsctl set interface dpdk0 ingress_policing_kpkts_burst=123
> 
> Add some unit tests for ingress packet-per-second policing.
> 
> Signed-off-by: Lin Huang <linhuang@ruijie.com.cn>

Hi Lin Huang,

thanks for your patch.

...

> @@ -3546,23 +3560,43 @@ netdev_dpdk_policer_construct(uint32_t rate, uint32_t burst)
>      policer = xmalloc(sizeof *policer);
>      rte_spinlock_init(&policer->policer_lock);
>  
> -    /* rte_meter requires bytes so convert kbits rate and burst to bytes. */
> -    rate_bytes = rate * 1000ULL / 8;
> -    burst_bytes = burst * 1000ULL / 8;
> +    if (rate) {
> +        /* rte_meter requires bytes so convert kbits rate and burst to bytes. */
> +        rate_bytes = rate * 1000ULL / 8;
> +        burst_bytes = burst * 1000ULL / 8;
>  
> -    policer->app_srtcm_params.cir = rate_bytes;
> -    policer->app_srtcm_params.cbs = burst_bytes;
> -    policer->app_srtcm_params.ebs = 0;
> -    err = rte_meter_srtcm_profile_config(&policer->in_prof,
> -                                         &policer->app_srtcm_params);
> -    if (!err) {
> -        err = rte_meter_srtcm_config(&policer->in_policer,
> -                                     &policer->in_prof);
> +        policer->app_srtcm_params.cir = rate_bytes;
> +        policer->app_srtcm_params.cbs = burst_bytes;
> +        policer->app_srtcm_params.ebs = 0;
> +        err = rte_meter_srtcm_profile_config(&policer->in_prof,
> +                                             &policer->app_srtcm_params);
> +        if (!err) {
> +            err = rte_meter_srtcm_config(&policer->in_policer,
> +                                         &policer->in_prof);
> +        }
> +        if (err) {
> +            VLOG_ERR("Could not create rte meter for ingress policer");

If this is an error, then probably the function should
clean up and return an error.

If it's not an error then VLOG_ERR seems inappropriate.


> +        } else {
> +            policer->type |= POLICER_BPS;
> +        }
>      }
> -    if (err) {
> -        VLOG_ERR("Could not create rte meter for ingress policer");
> +
> +    if (kpkts_rate) {
> +        /*
> +        * By default token is added per millisecond.
> +        * Rate in kilo-packets/second, bucket 1000 packets.
> +        * msec * kilo-packets/sec = 1 packets.
> +        */
> +        kpkts_burst *= 1000;
> +
> +        token_bucket_init(&policer->tb, kpkts_rate, kpkts_burst);
> +        policer->type |= POLICER_PKTPS;
> +    }
> +
> +    if (!policer->type) {
> +        /* Neither kbps nor kpkts policer is configured. */
>          free(policer);
> -        return NULL;
> +        policer = NULL;
>      }
>  
>      return policer;

...
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index c41c6adf6..53adbed82 100644
--- a/NEWS
+++ b/NEWS
@@ -30,6 +30,10 @@  Post-v3.1.0
        ovs-vsctl set port vhost-user0 qos=@newqos -- \
           --id=@newqos create qos type=egress-policer \
           other-config:kpkts_rate=123 other-config:kpkts_burst=123
+     * Added support for ingress packet-per-second policing.
+       Examples:
+       $ ovs-vsctl set interface dpdk0 ingress_policing_kpkts_rate=123
+       $ ovs-vsctl set interface dpdk0 ingress_policing_kpkts_burst=123
 
 
 v3.1.0 - 16 Feb 2023
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2fd8bc68d..ca0f22287 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -411,6 +411,8 @@  struct ingress_policer {
     struct rte_meter_srtcm_params app_srtcm_params;
     struct rte_meter_srtcm in_policer;
     struct rte_meter_srtcm_profile in_prof;
+    struct token_bucket tb;
+    enum policer_type type;
     rte_spinlock_t policer_lock;
 };
 
@@ -494,6 +496,9 @@  struct netdev_dpdk {
         uint32_t policer_rate;
         uint32_t policer_burst;
 
+        uint32_t policer_kpkts_rate;
+        uint32_t policer_kpkts_burst;
+
         /* Array of vhost rxq states, see vring_state_changed. */
         bool *vhost_rxq_enabled;
     );
@@ -1313,6 +1318,8 @@  common_construct(struct netdev *netdev, dpdk_port_t port_no,
     ovsrcu_init(&dev->ingress_policer, NULL);
     dev->policer_rate = 0;
     dev->policer_burst = 0;
+    dev->policer_kpkts_rate = 0;
+    dev->policer_kpkts_burst = 0;
 
     netdev->n_rxq = 0;
     netdev->n_txq = 0;
@@ -2372,9 +2379,15 @@  ingress_policer_run(struct ingress_policer *policer, struct rte_mbuf **pkts,
     int cnt = 0;
 
     rte_spinlock_lock(&policer->policer_lock);
-    cnt = srtcm_policer_run_single_packet(&policer->in_policer,
-                                          &policer->in_prof,
-                                          pkts, pkt_cnt, should_steal);
+    if (policer->type & POLICER_BPS) {
+        cnt = srtcm_policer_run_single_packet(&policer->in_policer,
+                                              &policer->in_prof,
+                                              pkts, pkt_cnt, should_steal);
+    }
+    if (policer->type & POLICER_PKTPS) {
+        cnt = pkts_policer_run_single_packet(&policer->tb, pkts, pkt_cnt,
+                                             should_steal);
+    }
     rte_spinlock_unlock(&policer->policer_lock);
 
     return cnt;
@@ -3536,7 +3549,8 @@  netdev_dpdk_get_features(const struct netdev *netdev,
 }
 
 static struct ingress_policer *
-netdev_dpdk_policer_construct(uint32_t rate, uint32_t burst)
+netdev_dpdk_policer_construct(uint32_t rate, uint32_t burst,
+                              uint32_t kpkts_rate, uint32_t kpkts_burst)
 {
     struct ingress_policer *policer = NULL;
     uint64_t rate_bytes;
@@ -3546,23 +3560,43 @@  netdev_dpdk_policer_construct(uint32_t rate, uint32_t burst)
     policer = xmalloc(sizeof *policer);
     rte_spinlock_init(&policer->policer_lock);
 
-    /* rte_meter requires bytes so convert kbits rate and burst to bytes. */
-    rate_bytes = rate * 1000ULL / 8;
-    burst_bytes = burst * 1000ULL / 8;
+    if (rate) {
+        /* rte_meter requires bytes so convert kbits rate and burst to bytes. */
+        rate_bytes = rate * 1000ULL / 8;
+        burst_bytes = burst * 1000ULL / 8;
 
-    policer->app_srtcm_params.cir = rate_bytes;
-    policer->app_srtcm_params.cbs = burst_bytes;
-    policer->app_srtcm_params.ebs = 0;
-    err = rte_meter_srtcm_profile_config(&policer->in_prof,
-                                         &policer->app_srtcm_params);
-    if (!err) {
-        err = rte_meter_srtcm_config(&policer->in_policer,
-                                     &policer->in_prof);
+        policer->app_srtcm_params.cir = rate_bytes;
+        policer->app_srtcm_params.cbs = burst_bytes;
+        policer->app_srtcm_params.ebs = 0;
+        err = rte_meter_srtcm_profile_config(&policer->in_prof,
+                                             &policer->app_srtcm_params);
+        if (!err) {
+            err = rte_meter_srtcm_config(&policer->in_policer,
+                                         &policer->in_prof);
+        }
+        if (err) {
+            VLOG_ERR("Could not create rte meter for ingress policer");
+        } else {
+            policer->type |= POLICER_BPS;
+        }
     }
-    if (err) {
-        VLOG_ERR("Could not create rte meter for ingress policer");
+
+    if (kpkts_rate) {
+        /*
+        * By default token is added per millisecond.
+        * Rate in kilo-packets/second, bucket 1000 packets.
+        * msec * kilo-packets/sec = 1 packets.
+        */
+        kpkts_burst *= 1000;
+
+        token_bucket_init(&policer->tb, kpkts_rate, kpkts_burst);
+        policer->type |= POLICER_PKTPS;
+    }
+
+    if (!policer->type) {
+        /* Neither kbps nor kpkts policer is configured. */
         free(policer);
-        return NULL;
+        policer = NULL;
     }
 
     return policer;
@@ -3571,8 +3605,8 @@  netdev_dpdk_policer_construct(uint32_t rate, uint32_t burst)
 static int
 netdev_dpdk_set_policing(struct netdev* netdev, uint32_t policer_rate,
                          uint32_t policer_burst,
-                         uint32_t policer_kpkts_rate OVS_UNUSED,
-                         uint32_t policer_kpkts_burst OVS_UNUSED)
+                         uint32_t policer_kpkts_rate,
+                         uint32_t policer_kpkts_burst)
 {
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
     struct ingress_policer *policer;
@@ -3585,13 +3619,23 @@  netdev_dpdk_set_policing(struct netdev* netdev, uint32_t policer_rate,
                      : !policer_burst ? 8000
                      : policer_burst);
 
+    /*
+     * Force to 0 if no rate specified,
+     * default to rate value if burst is 0,
+     * else stick with user-specified value.
+     */
+    policer_kpkts_burst = (!policer_kpkts_rate ? 0
+                           : !policer_kpkts_burst ? policer_kpkts_rate
+                           : policer_kpkts_burst);
     ovs_mutex_lock(&dev->mutex);
 
     policer = ovsrcu_get_protected(struct ingress_policer *,
-                                    &dev->ingress_policer);
+                                   &dev->ingress_policer);
 
     if (dev->policer_rate == policer_rate &&
-        dev->policer_burst == policer_burst) {
+        dev->policer_burst == policer_burst &&
+        dev->policer_kpkts_rate == policer_kpkts_rate &&
+        dev->policer_kpkts_burst == policer_kpkts_burst) {
         /* Assume that settings haven't changed since we last set them. */
         ovs_mutex_unlock(&dev->mutex);
         return 0;
@@ -3602,14 +3646,19 @@  netdev_dpdk_set_policing(struct netdev* netdev, uint32_t policer_rate,
         ovsrcu_postpone(free, policer);
     }
 
-    if (policer_rate != 0) {
-        policer = netdev_dpdk_policer_construct(policer_rate, policer_burst);
-    } else {
+    if (!policer_rate && !policer_kpkts_rate) {
+        /* both policers are zero. */
         policer = NULL;
+    } else {
+        policer = netdev_dpdk_policer_construct(policer_rate, policer_burst,
+                                                policer_kpkts_rate,
+                                                policer_kpkts_burst);
     }
     ovsrcu_set(&dev->ingress_policer, policer);
     dev->policer_rate = policer_rate;
     dev->policer_burst = policer_burst;
+    dev->policer_kpkts_rate = policer_kpkts_rate;
+    dev->policer_kpkts_burst = policer_kpkts_burst;
     ovs_mutex_unlock(&dev->mutex);
 
     return 0;
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index ab527d406..05ad5adc1 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -426,6 +426,158 @@  AT_CLEANUP
 dnl --------------------------------------------------------------------------
 
 
+
+dnl --------------------------------------------------------------------------
+dnl Ingress policing (kpkts) create delete phy port
+AT_SETUP([OVS-DPDK - Ingress policing (kpkts) create delete phy port])
+AT_KEYWORDS([dpdk])
+
+OVS_DPDK_PRE_PHY_SKIP()
+OVS_DPDK_START()
+
+dnl Add userspace bridge and attach it to OVS and add policer
+AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
+AT_CHECK([ovs-vsctl add-port br10 phy0 -- set Interface phy0 type=dpdk options:dpdk-devargs=$(cat PCI_ADDR)], [], [stdout], [stderr])
+AT_CHECK([ovs-vsctl set interface phy0 ingress_policing_kpkts_rate=10000 ingress_policing_kpkts_burst=1000])
+AT_CHECK([ovs-vsctl show], [], [stdout])
+sleep 2
+
+dnl Remove policer
+AT_CHECK([ovs-vsctl set interface phy0 ingress_policing_kpkts_rate=0 ingress_policing_kpkts_burst=0])
+
+dnl Check policer was removed correctly
+AT_CHECK([ovs-vsctl list interface phy0], [], [stdout])
+AT_CHECK([grep -E 'ingress_policing_kpkts_burst: 0' stdout], [], [stdout])
+
+AT_CHECK([ovs-vsctl list interface phy0], [], [stdout])
+AT_CHECK([grep -E 'ingress_policing_kpkts_rate: 0' stdout], [], [stdout])
+
+dnl Clean up
+AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
+OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
+AT_CLEANUP
+dnl --------------------------------------------------------------------------
+
+
+
+dnl --------------------------------------------------------------------------
+dnl Ingress policing (kpkts) create delete vport port
+AT_SETUP([OVS-DPDK - Ingress policing (kpkts) create delete vport port])
+AT_KEYWORDS([dpdk])
+
+OVS_DPDK_PRE_CHECK()
+OVS_DPDK_START()
+
+dnl Add userspace bridge and attach it to OVS and add ingress policer
+AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface dpdkvhostuserclient0 type=dpdkvhostuserclient options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [], [stdout], [stderr])
+AT_CHECK([ovs-vsctl set interface dpdkvhostuserclient0 ingress_policing_kpkts_rate=10000 ingress_policing_kpkts_burst=1000])
+AT_CHECK([ovs-vsctl show], [], [stdout])
+sleep 2
+
+dnl Remove ingress policer
+AT_CHECK([ovs-vsctl set interface dpdkvhostuserclient0 ingress_policing_kpkts_rate=0 ingress_policing_kpkts_burst=0])
+
+dnl Check ingress policer was removed correctly
+AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout])
+AT_CHECK([grep -E 'ingress_policing_kpkts_burst: 0' stdout], [], [stdout])
+
+AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout])
+AT_CHECK([grep -E 'ingress_policing_kpkts_rate: 0' stdout], [], [stdout])
+
+dnl Parse log file
+AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) vhost-user client: socket created" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "vHost User device 'dpdkvhostuserclient0' created in 'client' mode, using client socket" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) reconnecting..." ovs-vswitchd.log], [], [stdout])
+
+dnl Clean up
+AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
+OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
+\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
+])")
+AT_CLEANUP
+dnl --------------------------------------------------------------------------
+
+
+
+dnl --------------------------------------------------------------------------
+dnl Ingress policing (kpkts) no policing rate
+AT_SETUP([OVS-DPDK - Ingress policing (kpkts) no policing rate])
+AT_KEYWORDS([dpdk])
+
+OVS_DPDK_PRE_CHECK()
+OVS_DPDK_START()
+
+dnl Add userspace bridge and attach it to OVS and add ingress policer
+AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface dpdkvhostuserclient0 type=dpdkvhostuserclient options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [], [stdout], [stderr])
+AT_CHECK([ovs-vsctl set interface dpdkvhostuserclient0 ingress_policing_kpkts_burst=1000])
+AT_CHECK([ovs-vsctl show], [], [stdout])
+sleep 2
+
+dnl Check ingress policer was created correctly
+AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout])
+AT_CHECK([grep -E 'ingress_policing_kpkts_burst: 1000' stdout], [], [stdout])
+
+AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout])
+AT_CHECK([grep -E 'ingress_policing_kpkts_rate: 0' stdout], [], [stdout])
+
+
+dnl Parse log file
+AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) vhost-user client: socket created" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "vHost User device 'dpdkvhostuserclient0' created in 'client' mode, using client socket" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) reconnecting..." ovs-vswitchd.log], [], [stdout])
+
+dnl Clean up
+AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
+OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
+\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
+])")
+AT_CLEANUP
+dnl --------------------------------------------------------------------------
+
+
+
+dnl --------------------------------------------------------------------------
+dnl Ingress policing (kpkts) no policing burst
+AT_SETUP([OVS-DPDK - Ingress policing (kpkts) no policing burst])
+AT_KEYWORDS([dpdk])
+
+OVS_DPDK_PRE_CHECK()
+OVS_DPDK_START()
+
+dnl Add userspace bridge and attach it to OVS and add ingress policer
+AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface dpdkvhostuserclient0 type=dpdkvhostuserclient options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [], [stdout], [stderr])
+AT_CHECK([ovs-vsctl set interface dpdkvhostuserclient0 ingress_policing_kpkts_rate=10000])
+AT_CHECK([ovs-vsctl show], [], [stdout])
+sleep 2
+
+dnl Fail if ingress policer could not be created
+AT_FAIL_IF([grep "Could not create rte meter for ingress policer" ovs-vswitchd.log], [], [stdout])
+
+dnl Check ingress policer was created correctly
+AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout])
+AT_CHECK([grep -E 'ingress_policing_kpkts_burst: 0' stdout], [], [stdout])
+
+AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout])
+AT_CHECK([grep -E 'ingress_policing_kpkts_rate: 10000' stdout], [], [stdout])
+
+dnl Parse log file
+AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) vhost-user client: socket created" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "vHost User device 'dpdkvhostuserclient0' created in 'client' mode, using client socket" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) reconnecting..." ovs-vswitchd.log], [], [stdout])
+
+dnl Clean up
+AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
+OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
+\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
+])")
+AT_CLEANUP
+dnl --------------------------------------------------------------------------
+
+
+
 dnl --------------------------------------------------------------------------
 dnl QoS create delete phy port
 AT_SETUP([OVS-DPDK - QoS create delete phy port])