On Thu, Feb 24, 2022 at 03:20:23PM +0100, Eelco Chaudron wrote:
>
>
> On 23 Feb 2022, at 17:23, Marcelo Ricardo Leitner wrote:
>
> > On Tue, Feb 22, 2022 at 04:26:10PM +0100, Eelco Chaudron wrote:
> >> This patch checks for none offloadable ct_state match flag combinations.
> >> If they exist force the +trk flag down to TC Flower
> >>
> >> Signed-off-by: Eelco Chaudron <[email protected]>
> >> ---
> >> v3:
> >>  - Instead of warning about an invalid flag combination fix it.
> >>
> >>  lib/netdev-offload-tc.c |    6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> >> index 0105d883f..3d2c1d844 100644
> >> --- a/lib/netdev-offload-tc.c
> >> +++ b/lib/netdev-offload-tc.c
> >> @@ -1541,6 +1541,12 @@ parse_match_ct_state_to_flower(struct tc_flower 
> >> *flower, struct match *match)
> >>              flower->key.ct_state &= ~(TCA_FLOWER_KEY_CT_FLAGS_NEW);
> >>              flower->mask.ct_state &= ~(TCA_FLOWER_KEY_CT_FLAGS_NEW);
> >>          }
> >> +
> >> +        if (flower->key.ct_state &&
> >> +            !(flower->key.ct_state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED)) {
> >> +            flower->key.ct_state |= TCA_FLOWER_KEY_CT_FLAGS_TRACKED;
> >> +            flower->mask.ct_state |= TCA_FLOWER_KEY_CT_FLAGS_TRACKED;
> >> +        }
> >
> > Instead, what about:
>
> I guess we still need the patch in OVS, as older kernels will exist ;)

Hah, indeed!

>
> >
> > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> > index 1a9b1f140f9e..b8bfb5733f9e 100644
> > --- a/net/sched/cls_flower.c
> > +++ b/net/sched/cls_flower.c
> > @@ -1407,12 +1407,6 @@ static int fl_set_enc_opt(struct nlattr **tb,
> > struct fl_flow_key *key,
> >  static int fl_validate_ct_state(u16 state, struct nlattr *tb,
> >                             struct netlink_ext_ack *extack)
> >  {
> > -   if (state && !(state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED)) {
> > -           NL_SET_ERR_MSG_ATTR(extack, tb,
> > -                               "no trk, so no other flag can be set");
> > -           return -EINVAL;
> > -   }
> > -
> >     if (state & TCA_FLOWER_KEY_CT_FLAGS_NEW &&
> >         state & TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED) {
> >             NL_SET_ERR_MSG_ATTR(extack, tb,
> >
> > I really don't see an issue in having flower to match only on +trk
> > without having +trk together in there.
>
> The kernel change looks fine to me, but I guess in your description the first 
> +trk was supposed to be something like “any flag but +trk”?
>

Oh oh, yes. On my previous email,
  s/match only on +trk/match only on +est/
sorry about that.

  Marcelo

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to