Re: [PATCH net 1/2] net: sched: drop ct for the packets toward ingress only in act_mirred

On Wed, Sep 22, 2021 at 11:43 AM Cong Wang [off-list ref] wrote:

On Tue, Sep 21, 2021 at 12:02 AM Xin Long [off-list ref] wrote:

quoted

On Tue, Sep 21, 2021 at 2:31 AM Cong Wang [off-list ref] wrote:

quoted

On Mon, Sep 20, 2021 at 7:12 AM Xin Long [off-list ref] wrote:

quoted

nf_reset_ct() called in tcf_mirred_act() is supposed to drop ct for
those packets that are mirred or redirected to only ingress, not
ingress and egress.

Any reason behind this? I think we at least need to reset it when
redirecting from ingress to egress as well? That is, when changing
directions?

For the reason why ct should be reset, it's said in
d09c548dbf3b ("net: sched: act_mirred: Reset ct info when mirror").
The user case is OVS HWOL using TC to do NAT and then redirecting
the NATed skb back to the ingress of one local dev, it's ingress only, this
patch is more like to minimize the side effect of d09c548dbf3b IF there is.

What is the side effect here? Or what is wrong with resetting CT on
egress side?

quoted

Not sure if it's too much to do for that from ingress to egress.
What I was thinking is this should happen on rx path(ingress), like it
does in internal_dev_recv() in the OVS kernel module. But IF there is
any user case needing doing this for ingress to egress, I would add it.

If that is the case, then this patch is completely unnecessary. So
instead of going back and forth, please elaborate on why resetting
CT for egress is a problem here.

What I'm afraid is: after resetting CT for the packets redirected to egress,
the tc rules on the next dev egress that may need these CT will break.

I can't give a use case right now, and just don't want to introduce extra
change for which we don't see a use. If you think that's safe, I'm fine
to do these resets when the direction is changed.