[ovs-dev] [PATCH 2/3] netdev-offload-tc: Probe for support for any of the ct_state flags

Paul Blakey paulb at nvidia.com
Thu Mar 4 09:07:17 UTC 2021




On Wed, 3 Mar 2021, Marcelo Ricardo Leitner wrote:

> Hi,
> 
> On Wed, Mar 03, 2021 at 02:15:35PM +0200, Paul Blakey wrote:
> > Upstream kernel now rejects unsupported ct_state flags.
> > Earlier kernels, ignored it but still echoed back the requested ct_state,
> > if ct_state was supported. ct_state initial support had trk, new, est,
> > and rel flags.
> > 
> > If kernel echos back ct_state, assume support for trk, new, est, and
> > rel. If kernel rejects a specific unsupported flag, continue and
> > use reject mechanisim to probe for flags rep and inv.
> > 
> > Disallow inserting rules with unnsupported ct_state flags.
> > 
> > Signed-off-by: Paul Blakey <paulb at nvidia.com>
> > Acked-by: Roi Dayan <roid at nvidia.com>
> > ---
> >  lib/netdev-offload-tc.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 96 insertions(+)
> > 
> > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > index 04b5e21..8940409 100644
> > --- a/lib/netdev-offload-tc.c
> > +++ b/lib/netdev-offload-tc.c
> > @@ -48,6 +48,7 @@ static struct hmap ufid_to_tc = HMAP_INITIALIZER(&ufid_to_tc);
> >  static struct hmap tc_to_ufid = HMAP_INITIALIZER(&tc_to_ufid);
> >  static bool multi_mask_per_prio = false;
> >  static bool block_support = false;
> > +static uint16_t ct_state_support;
> >  
> >  struct netlink_field {
> >      int offset;
> > @@ -1709,6 +1710,10 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
> >      }
> >  
> >      if (mask->ct_state) {
> > +        if ((ct_state_support & mask->ct_state) != mask->ct_state) {
> > +            return -EOPNOTSUPP;
> > +        }
> > +
> >          if (mask->ct_state & OVS_CS_F_NEW) {
> >              if (key->ct_state & OVS_CS_F_NEW) {
> >                  flower.key.ct_state |= TCA_FLOWER_KEY_CT_FLAGS_NEW;
> 
> Considering some ct_state support is a must for CT with TC, it can use
> a similar check on other fields as well, like (c&p'ed) the below, to
> not try to offload flows with anything related to CT and not just
> ct_state. Thoughts?

Yes ill add these to v2.

> 
> @@ -1739,13 +1739,13 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>          }
>      }
> 
> -    if (mask->ct_zone) {
> +    if (mask->ct_zone && ct_state_support) {
> 
> (either this way or return right away as you did above)
> 
>          flower.key.ct_zone = key->ct_zone;
>          flower.mask.ct_zone = mask->ct_zone;
>          mask->ct_zone = 0;
>      }
> 
> -    if (mask->ct_mark) {
> +    if (mask->ct_mark && ct_state_support) {
>          flower.key.ct_mark = key->ct_mark;
>          flower.mask.ct_mark = mask->ct_mark;
>          mask->ct_mark = 0;
> @@ -1837,6 +1837,10 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
>              const struct nlattr *ct = nl_attr_get(nla);
>              const size_t ct_len = nl_attr_get_size(nla);
> 
> +           if (!ct_state_support) {
> +                   return -EOPNOTSUPP;
> +           }
> +
>              err = parse_put_flow_ct_action(&flower, action, ct, ct_len);
>              if (err) {
>                  return err;
> 
> 
> > @@ -2029,6 +2034,96 @@ out:
> >      tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS);
> >  }
> >  
> > +
> > +static int
> > +probe_insert_ct_state_rule(int ifindex, uint16_t ct_state, struct tcf_id *id)
> > +{
> > +    int prio = TC_RESERVED_PRIORITY_MAX + 1;
> > +    struct tc_flower flower;
> > +
> > +    memset(&flower, 0, sizeof flower);
> > +    flower.key.ct_state = ct_state;
> > +    flower.mask.ct_state = ct_state;
> > +    flower.tc_policy = TC_POLICY_SKIP_HW;
> > +    flower.key.eth_type = htons(ETH_P_IP);
> > +    flower.mask.eth_type = OVS_BE16_MAX;
> > +
> > +    *id = tc_make_tcf_id(ifindex, 0, prio, TC_INGRESS);
> > +    return tc_replace_flower(id, &flower);
> > +}
> > +
> > +static void
> > +probe_ct_state_support(int ifindex)
> > +{
> > +    struct tc_flower flower;
> > +    uint16_t ct_state;
> > +    struct tcf_id id;
> > +    int error;
> > +
> > +    error = tc_add_del_qdisc(ifindex, true, 0, TC_INGRESS);
> > +    if (error) {
> > +        return;
> > +    }
> > +
> > +    /* Test for base ct_state match support */
> > +    ct_state = TCA_FLOWER_KEY_CT_FLAGS_NEW | TCA_FLOWER_KEY_CT_FLAGS_TRACKED;
> > +    error = probe_insert_ct_state_rule(ifindex, ct_state, &id);
> > +    if (error) {
> > +        goto out;
> > +    }
> > +
> > +    error = tc_get_flower(&id, &flower);
> > +    if (error || flower.mask.ct_state != ct_state) {
> > +        goto out_del;
> > +    }
> > +
> > +    tc_del_filter(&id);
> > +    ct_state_support = TCA_FLOWER_KEY_CT_FLAGS_NEW |
> > +                       TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED |
> > +                       TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
> > +                       TCA_FLOWER_KEY_CT_FLAGS_RELATED;
> > +
> > +    /* Test for reject, ct_state >= MAX */
> > +    ct_state = ~0;
> > +    error = probe_insert_ct_state_rule(ifindex, ct_state, &id);
> > +    if (!error) {
> > +        /* No reject, can't continue probing other flags */
> > +        goto out_del;
> > +    }
> > +
> > +    tc_del_filter(&id);
> > +
> > +    /* Test for ct_state INVALID support */
> > +    memset(&flower, 0, sizeof flower);
> > +    ct_state = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
> > +               TCA_FLOWER_KEY_CT_FLAGS_INVALID;
> > +    error = probe_insert_ct_state_rule(ifindex, ct_state, &id);
> > +    if (error) {
> > +        goto out;
> > +    }
> > +
> > +    tc_del_filter(&id);
> > +    ct_state_support |= TCA_FLOWER_KEY_CT_FLAGS_INVALID;
> > +
> > +    /* Test for ct_state REPLY support */
> > +    memset(&flower, 0, sizeof flower);
> > +    ct_state = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
> > +               TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED |
> > +               TCA_FLOWER_KEY_CT_FLAGS_REPLY;
> > +    error = probe_insert_ct_state_rule(ifindex, ct_state, &id);
> > +    if (error) {
> > +        goto out;
> > +    }
> > +
> > +    ct_state_support |= TCA_FLOWER_KEY_CT_FLAGS_REPLY;
> > +
> > +out_del:
> > +    tc_del_filter(&id);
> > +out:
> > +    tc_add_del_qdisc(ifindex, false, 0, TC_INGRESS);
> > +    VLOG_INFO("probe tc: supported ct_state bits: 0x%x", ct_state_support);
> > +}
> > +
> >  static void
> >  probe_tc_block_support(int ifindex)
> >  {
> > @@ -2103,6 +2198,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
> >          block_id = get_block_id_from_netdev(netdev);
> >  
> >          probe_multi_mask_per_prio(ifindex);
> > +        probe_ct_state_support(ifindex);
> >          ovsthread_once_done(&once);
> >      }
> >  
> > -- 
> > 1.8.3.1
> > 
> 
> 


More information about the dev mailing list