[ovs-dev] [PATCH] dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.

[Roi Dayan]

On 2020-03-11 7:39 AM, wenxu at ucloud.cn wrote:
> From: wenxu <wenxu at ucloud.cn>
> 
> The tc modify flow put always delete the original flow first and
> then add the new flow. If the modfiy flow put operation failed,
> the flow put operation will change from modify to create if success
> to delete the original flow in tc (which will be always failed with
> ENOENT, the flow is already be deleted before add the new flow in tc).
> Finally, the modify flow put will failed to add in kernel datapath.
> 
> Signed-off-by: wenxu <wenxu at ucloud.cn>
> ---
>  lib/dpif-netlink.c      | 7 ++++++-
>  lib/netdev-offload-tc.c | 2 +-
>  lib/netdev-offload.h    | 3 +++
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 5b5c96d..b8d08a6 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -2091,6 +2091,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
>      info.tunnel_csum_on = csum_on;
>      info.recirc_id_shared_with_tc = (dpif->user_features
>                                       & OVS_DP_F_TC_RECIRC_SHARING);
> +    info.tc_modify_flow_deleted = false;
>      err = netdev_flow_put(dev, &match,
>                            CONST_CAST(struct nlattr *, put->actions),
>                            put->actions_len,
> @@ -2141,7 +2142,11 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
>  out:
>      if (err && err != EEXIST && (put->flags & DPIF_FP_MODIFY)) {
>          /* Modified rule can't be offloaded, try and delete from HW */
> -        int del_err = netdev_flow_del(dev, put->ufid, put->stats);
> +        int del_err = 0;
> +
> +        if (!info.tc_modify_flow_deleted) {
> +            del_err = netdev_flow_del(dev, put->ufid, put->stats);
> +        }
>  
>          if (!del_err) {
>              /* Delete from hw success, so old flow was offloaded.
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 550e440..5e7b873 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1727,7 +1727,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>      if (get_ufid_tc_mapping(ufid, &id) == 0) {
>          VLOG_DBG_RL(&rl, "updating old handle: %d prio: %d",
>                      id.handle, id.prio);
> -        del_filter_and_ufid_mapping(&id, ufid);
> +        info->tc_modify_flow_deleted = !del_filter_and_ufid_mapping(&id, ufid);
>      }
>  
>      prio = get_prio_for_tc_flower(&flower);
> diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
> index cd6dfdf..b4b882a 100644
> --- a/lib/netdev-offload.h
> +++ b/lib/netdev-offload.h
> @@ -74,6 +74,9 @@ struct offload_info {
>       * it will be in the pkt meta data.
>       */
>      uint32_t flow_mark;
> +
> +    bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success
> +                                  * to delete the original flow. */
>  };
>  
>  int netdev_flow_flush(struct netdev *);
> 

Acked-by: Roi Dayan <roid at mellanox.com>