diff mbox series

[ovs-dev] netdev-linux: Don't remove ingress when not configured

Message ID 1554440019-4034-1-git-send-email-xiangxia.m.yue@gmail.com
State Not Applicable
Headers show
Series [ovs-dev] netdev-linux: Don't remove ingress when not configured | expand

Commit Message

Tonghao Zhang April 5, 2019, 4:53 a.m. UTC 
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

In some case, we may not use the openvswitch tc to limit the ingress
police rate. And before we add the port to openvswitch bridge, we may
set the ingress policer, so don't remove the ingress when we configured
it in openvswitch.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/netdev-linux.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Ben Pfaff April 8, 2019, 8:21 p.m. UTC | #1 
On Fri, Apr 05, 2019 at 12:53:39AM -0400, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> In some case, we may not use the openvswitch tc to limit the ingress
> police rate. And before we add the port to openvswitch bridge, we may
> set the ingress policer, so don't remove the ingress when we configured
> it in openvswitch.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This seems wrong to me, because it looks like it makes it impossible to
disable policing with Open vSwitch, even if Open vSwitch was what
enabled it.
Tonghao Zhang April 9, 2019, 6:16 a.m. UTC | #2 
On Tue, Apr 9, 2019 at 4:21 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Fri, Apr 05, 2019 at 12:53:39AM -0400, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > In some case, we may not use the openvswitch tc to limit the ingress
> > police rate. And before we add the port to openvswitch bridge, we may
> > set the ingress policer, so don't remove the ingress when we configured
> > it in openvswitch.
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This seems wrong to me, because it looks like it makes it impossible to
> disable policing with Open vSwitch, even if Open vSwitch was what
> enabled it.
I forgot that case, v2 will be sent. thanks
diff mbox series

Patch

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index deedc69..8ea4383 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2447,7 +2447,7 @@  netdev_linux_set_policing(struct netdev *netdev_,
     struct netdev_linux *netdev = netdev_linux_cast(netdev_);
     const char *netdev_name = netdev_get_name(netdev_);
     int ifindex;
-    int error;
+    int error = 0;
 
     kbits_burst = (!kbits_rate ? 0       /* Force to 0 if no rate specified. */
                    : !kbits_burst ? 8000 /* Default to 8000 kbits if 0. */
@@ -2469,10 +2469,7 @@  netdev_linux_set_policing(struct netdev *netdev_,
         netdev->cache_valid &= ~VALID_POLICING;
     }
 
-    error = get_ifindex(netdev_, &ifindex);
-    if (error) {
-        goto out;
-    }
+    COVERAGE_INC(netdev_set_policing);
 
     /* Use matchall for policing when offloadling ovs with tc-flower. */
     if (netdev_is_flow_api_enabled()) {
@@ -2484,16 +2481,20 @@  netdev_linux_set_policing(struct netdev *netdev_,
         return error;
     }
 
-    COVERAGE_INC(netdev_set_policing);
-    /* Remove any existing ingress qdisc. */
-    error = tc_add_del_ingress_qdisc(ifindex, false, 0);
-    if (error) {
-        VLOG_WARN_RL(&rl, "%s: removing policing failed: %s",
-                     netdev_name, ovs_strerror(error));
-        goto out;
-    }
-
     if (kbits_rate) {
+        error = get_ifindex(netdev_, &ifindex);
+        if (error) {
+            goto out;
+        }
+
+        /* Remove any existing ingress qdisc. */
+        error = tc_add_del_ingress_qdisc(ifindex, false, 0);
+        if (error) {
+            VLOG_WARN_RL(&rl, "%s: removing policing failed: %s",
+                         netdev_name, ovs_strerror(error));
+            goto out;
+        }
+
         error = tc_add_del_ingress_qdisc(ifindex, true, 0);
         if (error) {
             VLOG_WARN_RL(&rl, "%s: adding policing qdisc failed: %s",
@@ -2504,7 +2505,7 @@  netdev_linux_set_policing(struct netdev *netdev_,
         error = tc_add_policer(netdev_, kbits_rate, kbits_burst);
         if (error){
             VLOG_WARN_RL(&rl, "%s: adding policing action failed: %s",
-                    netdev_name, ovs_strerror(error));
+                         netdev_name, ovs_strerror(error));
             goto out;
         }
     }