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 ;) > > 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”? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
