[ovs-dev] [PATCH ovs-dev v1] netdev-offload: Flush offloaded rules when ports removed

Roi Dayan roid at mellanox.com
Sun Jun 14 05:30:58 UTC 2020



On 2020-06-11 1:36 PM, xiangxia.m.yue at gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue at gmail.com>
> 
> When ports were removed from bridge, we should flush the
> offload rules on the ports. The main reason is two factors:
> 
> * The ports removed from bridge, will be managed by OvS, so
>   flush the rules which installed by OvS.
> * If using the TC flower offload, for example, tc rules still
>   are on the ports. And the information still are maintained by
>   OvS, such as the mapping for tc and ufid.
>   Then if adding the port to bridge and installing the rules
>   to it again, *del_filter_and_ufid_mapping will be invoked,
>   and delete the tc rule using tc handle which may not exist
>   (offload init api flushed them.) on kernel or is used by other
>   previous rules (if so, that rules will be deleted that is not
>   we expected.).
> 
> Cc: Simon Horman <simon.horman at netronome.com>
> Cc: Paul Blakey <paulb at mellanox.com>
> Cc: Roi Dayan <roid at mellanox.com>
> Cc: Ben Pfaff <blp at ovn.org>
> Cc: William Tu <u9012063 at gmail.com>
> Cc: Ilya Maximets <i.maximets at ovn.org>
> Tested-at: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.com%2Fgithub%2Fovn-open-virtual-networks%2Fovs%2Fbuilds%2F170832624&data=02%7C01%7Croid%40mellanox.com%7Cf0bc38aa88074693105108d80df36979%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637274686378147436&sdata=Dlq4jnSclj0vXQoldO7Oq2Wwx%2BGEHjK%2FftPUDHnJcKM%3D&reserved=0
> Co-authored-by: Wengang Hou <houwengang at didiglobal.com>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com>
> ---
>  lib/netdev-offload.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
> index ab97a292ebac..964566caab1e 100644
> --- a/lib/netdev-offload.c
> +++ b/lib/netdev-offload.c
> @@ -593,6 +593,7 @@ netdev_ports_remove(odp_port_t port_no, const struct dpif_class *dpif_class)
>      data = netdev_ports_lookup(port_no, dpif_class);
>      if (data) {
>          dpif_port_destroy(&data->dpif_port);
> +        netdev_flow_flush(data->netdev); /* flush offloaded rules. */
>          netdev_close(data->netdev); /* unref and possibly close */
>          hmap_remove(&port_to_netdev, &data->portno_node);
>          hmap_remove(&ifindex_to_port, &data->ifindex_node);
> 


it looks ok but I wonder if it's redundant?
I tested and when I removed a port with tc rules on it,
all tc rules got deleted using the flow api flow del,
we get into netdev_tc_flow_del() for each flow.
So you shouldn't have a tc rule left or ufid mapping.
How did you test that you saw tc rules were still on the port?


More information about the dev mailing list