On 2/3/21 1:09 PM, Marcelo Ricardo Leitner wrote: > On Wed, Feb 03, 2021 at 01:30:00PM +0800, we...@ucloud.cn wrote: > ... >> @@ -1641,6 +1644,10 @@ netdev_tc_flow_put(struct netdev *netdev, struct >> match *match, >> } >> >> if (mask->ct_state) { >> + if (mask->ct_state & TC_UNSUPP_OVS_CS_FLAGS) { >> + return EOPNOTSUPP; >> + } >> + > > Hi Wenxu, > > Paul's proposal on the other thread is more aligned with how the rest > of the code is doing such kind of validation.
Well, it's more aligned, but still not good enough. Keeping lists of supported things is error prone in both cases. I think, we need to just disable flags at the point where these flags were consumed, e.g. --- if (mask->ct_state & OVS_CS_F_ESTABLISHED) { if (key->ct_state & OVS_CS_F_ESTABLISHED) { flower.key.ct_state |= TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED; } flower.mask.ct_state |= TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED; + mask->ct_state &= ~OVS_CS_F_ESTABLISHED; } --- And we should not touch 'mask->ct_state' anywhere else, so test_key_and_mask() will catch all the unknown bits. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev