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