[ovs-dev,v2] tc: Use skip_hw flag when probing tc features

There is no need to pass tc rules to hw when just probing
for tc features. this will avoid redundant errors from hw drivers
that may happen.

Signed-off-by: Roi Dayan <roid@mellanox.com>
Acked-By: Vlad Buslov <vladbu@mellanox.com>
---

Notes:
    v2
    - Add missing ack from Vlad
    - Style fix for comment
    - Add build assertion for TC_POLICY_NONE

 lib/netdev-offload-tc.c |  2 ++
 lib/tc.c                | 13 ++++++-------
 lib/tc.h                | 10 ++++++++++
 3 files changed, 18 insertions(+), 7 deletions(-)

On Tue, Aug 4, 2020 at 2:37 PM Roi Dayan <roid@mellanox.com> wrote:
>
> There is no need to pass tc rules to hw when just probing
> for tc features. this will avoid redundant errors from hw drivers
> that may happen.
>
> Signed-off-by: Roi Dayan <roid@mellanox.com>
> Acked-By: Vlad Buslov <vladbu@mellanox.com>
Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>
> Notes:
>     v2
>     - Add missing ack from Vlad
>     - Style fix for comment
>     - Add build assertion for TC_POLICY_NONE
>
>  lib/netdev-offload-tc.c |  2 ++
>  lib/tc.c                | 13 ++++++-------
>  lib/tc.h                | 10 ++++++++++
>  3 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 2c9c6f4cae8b..18ff380f9861 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1918,6 +1918,7 @@ probe_multi_mask_per_prio(int ifindex)
>
>      memset(&flower, 0, sizeof flower);
>
> +    flower.tc_policy = TC_POLICY_SKIP_HW;
>      flower.key.eth_type = htons(ETH_P_IP);
>      flower.mask.eth_type = OVS_BE16_MAX;
>      memset(&flower.key.dst_mac, 0x11, sizeof flower.key.dst_mac);
> @@ -1965,6 +1966,7 @@ probe_tc_block_support(int ifindex)
>
>      memset(&flower, 0, sizeof flower);
>
> +    flower.tc_policy = TC_POLICY_SKIP_HW;
>      flower.key.eth_type = htons(ETH_P_IP);
>      flower.mask.eth_type = OVS_BE16_MAX;
>      memset(&flower.key.dst_mac, 0x11, sizeof flower.key.dst_mac);
> diff --git a/lib/tc.c b/lib/tc.c
> index c96d095381d7..8761304c92bb 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -65,12 +65,6 @@ VLOG_DEFINE_THIS_MODULE(tc);
>
>  static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5);
>
> -enum tc_offload_policy {
> -    TC_POLICY_NONE,
> -    TC_POLICY_SKIP_SW,
> -    TC_POLICY_SKIP_HW
> -};
> -
>  static enum tc_offload_policy tc_policy = TC_POLICY_NONE;
>
>  struct tc_pedit_key_ex {
> @@ -2757,6 +2751,7 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
>      bool is_vlan = eth_type_vlan(flower->key.eth_type);
>      bool is_qinq = is_vlan && eth_type_vlan(flower->key.encap_eth_type[0]);
>      bool is_mpls = eth_type_mpls(flower->key.eth_type);
> +    enum tc_offload_policy policy = flower->tc_policy;
>      int err;
>
>      /* need to parse acts first as some acts require changing the matching
> @@ -2882,7 +2877,11 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
>          }
>      }
>
> -    nl_msg_put_u32(request, TCA_FLOWER_FLAGS, tc_get_tc_cls_policy(tc_policy));
> +    if (policy == TC_POLICY_NONE) {
> +        policy = tc_policy;
> +    }
> +
> +    nl_msg_put_u32(request, TCA_FLOWER_FLAGS, tc_get_tc_cls_policy(policy));
>
>      if (flower->tunnel) {
>          nl_msg_put_flower_tunnel(request, flower);
> diff --git a/lib/tc.h b/lib/tc.h
> index 028eed5d0658..281231c0d3f1 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -312,6 +312,14 @@ is_tcf_id_eq(struct tcf_id *id1, struct tcf_id *id2)
>             && id1->chain == id2->chain;
>  }
>
> +enum tc_offload_policy {
> +    TC_POLICY_NONE = 0,
> +    TC_POLICY_SKIP_SW,
> +    TC_POLICY_SKIP_HW
> +};
> +
> +BUILD_ASSERT_DECL(TC_POLICY_NONE == 0);
> +
>  struct tc_flower {
>      struct tc_flower_key key;
>      struct tc_flower_key mask;
> @@ -337,6 +345,8 @@ struct tc_flower {
>      bool needs_full_ip_proto_mask;
>
>      enum tc_offloaded_state offloaded_state;
> +    /* Used to force skip_hw when probing tc features. */
> +    enum tc_offload_policy tc_policy;
>  };
>
>  /* assert that if we overflow with a masked write of uint32_t to the last byte
> --
> 2.8.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

On Tue, Aug 04, 2020 at 05:33:33PM +0800, Tonghao Zhang wrote:
> On Tue, Aug 4, 2020 at 2:37 PM Roi Dayan <roid@mellanox.com> wrote:
> >
> > There is no need to pass tc rules to hw when just probing
> > for tc features. this will avoid redundant errors from hw drivers
> > that may happen.
> >
> > Signed-off-by: Roi Dayan <roid@mellanox.com>
> > Acked-By: Vlad Buslov <vladbu@mellanox.com>
>
> Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Thanks, pushed to the master branch.