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

Ben Pfaff blp at ovn.org
Fri Apr 12 22:24:43 UTC 2019


On Tue, Apr 09, 2019 at 12:37:48PM -0400, xiangxia.m.yue at gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue at 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 not configured
> it in openvswitch.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com>
> ---
>  lib/netdev-linux.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 0fce217..ad8a244 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2448,6 +2448,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
>      const char *netdev_name = netdev_get_name(netdev_);
>      int ifindex;
>      int error;
> +    bool should_cleanup_ingress = false;
>  
>      kbits_burst = (!kbits_rate ? 0       /* Force to 0 if no rate specified. */
>                     : !kbits_burst ? 8000 /* Default to 8000 kbits if 0. */
> @@ -2466,14 +2467,10 @@ netdev_linux_set_policing(struct netdev *netdev_,
>              /* Assume that settings haven't changed since we last set them. */
>              goto out;
>          }
> +        should_cleanup_ingress = true;
>          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. */

Thanks for working on this.

This new version uses the logic that if and only if there's a cached
policing setting, then OVS must have set it up, and only in that case
should OVS clean it up.  But the assumption is wrong.  The setting cache
is just a cache and it gets reset if the kernel notifies us of a change
to a device.  It also doesn't persist from one OVS run to another, so
this wouldn't allow OVS to turn off policing if it set it and then
segfaulted and restarted (or the admin stopped and started it).


More information about the dev mailing list