On Wed, Mar 17, 2021 at 12:02:43PM +0800, we...@ucloud.cn wrote:
> From: wenxu <we...@ucloud.cn>
>
> The ct_state validate should not only check the mask bit and also
> check mask_bit & key_bit..
> For the +new+est case example, The 'new' and 'est' bits should be
> set in both state_mask and state flags. Or the -new-est case also
> will be reject by kernel.
> When Openvswitch with two flows
> ct_state=+trk+new,action=commit,forward
> ct_state=+trk+est,action=forward
>
> A packet go through the kernel and the contrack state is invalid,
> The ct_state will be +trk-inv. Upcall to the ovs-vswitchd, the
> finally dp action will be drop with -new-est+trk.
>
> Fixes: 1bcc51ac0731 ("net/sched: cls_flower: Reject invalid ct_state flags
> rules")
> Fixes: 3aed8b63336c ("net/sched: cls_flower: validate ct_state for invalid
> and reply flags")
> Signed-off-by: wenxu <we...@ucloud.cn>
> ---
> net/sched/cls_flower.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index d097b5c..c69a4ba 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -1451,7 +1451,7 @@ static int fl_set_key_ct(struct nlattr **tb,
> &mask->ct_state, TCA_FLOWER_KEY_CT_STATE_MASK,
> sizeof(key->ct_state));
>
> - err = fl_validate_ct_state(mask->ct_state,
> + err = fl_validate_ct_state(key->ct_state & mask->ct_state,
Or that, yes. The thing I was wondering on this is if it would be a
problem to have something like
key = trk,inv
mask = trk,new,est,inv
because in essence, this is +trk+inv-new-est, and it's worrying about
bits that shouldn't be considered if +inv is there.
I don't see a reason for it to be that restrictive, though, and it
will work as expected.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leit...@gmail.com>
> tb[TCA_FLOWER_KEY_CT_STATE_MASK],
> extack);
> if (err)
> --
> 1.8.3.1
>
