[ovs-dev] [PATCH v2 10/10] netdev-offloads-tc: Probe recirc tc sharing feature on first recirc_id rule

[Marcelo Ricardo Leitner]

On Wed, Nov 27, 2019 at 02:55:16PM +0200, Roi Dayan wrote:
> @@ -1367,7 +1387,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>      uint32_t block_id = 0;
>      struct nlattr *nla;
>      struct tc_id id;
> -    uint32_t chain;
> +    uint32_t chain = 0;
>      size_t left;
>      int prio = 0;
>      int ifindex;
> @@ -1382,7 +1402,13 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>  
>      memset(&flower, 0, sizeof flower);
>  
> -    chain = key->recirc_id;
> +    if (key->recirc_id) {
> +        if (recirc_id_sharing_support(info->dpif)) {
> +            chain = key->recirc_id;
> +        } else {
> +            return EOPNOTSUPP;
> +        }
> +    }
>      mask->recirc_id = 0;

It may be a personal preference, but I find this easier to read:

     chain = key->recirc_id;
     mask->recirc_id = 0;
+    if (unlikely(chain && !recirc_id_sharing_support(info->dpif))) {
+        return EOPNOTSUPP;
+    }

As it avoids the cascading conditions. Just a suggestion..

>  
>      if (flow_tnl_dst_is_set(&key->tunnel)) {
> @@ -1630,7 +1656,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>          action = &flower.actions[flower.action_count];
>          if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
>              odp_port_t port = nl_attr_get_odp_port(nla);
> -            struct netdev *outdev = netdev_ports_get(port, info->dpif_class);
> +            struct netdev *outdev = netdev_ports_get(port,
> +                                                     info->dpif->dpif_class);
>  
>              action->out.ifindex_out = netdev_get_ifindex(outdev);
>              action->out.ingress = is_internal_port(netdev_get_type(outdev));
> @@ -1696,6 +1723,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>              action->ct.clear = true;
>              flower.action_count++;
>          } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_RECIRC) {
> +            if (!recirc_id_sharing_support(info->dpif)) {
> +                return EOPNOTSUPP;
> +            }
>              action->type = TC_ACT_GOTO;
>              action->chain = nl_attr_get_u32(nla);
>              flower.action_count++;