Re: [PATCH net-next 2/3] net: sched: set IPS_CONFIRMED in tmpl status only when commit is set in act_ct

[Aaron Conole <aconole@redhat.com>]

Xin Long <lucien.xin@gmail.com> writes:

> With the following flows, the packets will be dropped if OVS TC offload is
> enabled.
>
>   'ip,ct_state=-trk,in_port=1 actions=ct(zone=1)'
>   'ip,ct_state=+trk+new+rel,in_port=1 actions=ct(commit,zone=1)'
>   'ip,ct_state=+trk+new+rel,in_port=1 actions=ct(commit,zone=2),normal'
>
> In the 1st flow, it finds the exp from the hashtable and removes it then
> creates the ct with this exp in act_ct. However, in the 2nd flow it goes
> to the OVS upcall at the 1st time. When the skb comes back from userspace,
> it has to create the ct again without exp(the exp was removed last time).
> With no 'rel' set in the ct, the 3rd flow can never get matched.
>
> In OVS conntrack, it works around it by adding its own exp lookup function
> ovs_ct_expect_find() where it doesn't remove the exp. Instead of creating
> a real ct, it only updates its keys with the exp and its master info. So
> when the skb comes back, the exp is still in the hashtable.
>
> However, we can't do this trick in act_ct, as tc flower match is using a
> real ct, and passing the exp and its master info to flower parsing via
> tc_skb_cb is also not possible (tc_skb_cb size is not big enough).
>
> The simple and clear fix is to not remove the exp at the 1st flow, namely,
> not set IPS_CONFIRMED in tmpl when commit is not set in act_ct.
>
> Reported-by: Shuang Li <shuali@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>