Re: [PATCH net-next 1/3] net/sched: Introduce action ct

From: Cong Wang <xiyou.wangcong@gmail.com>
To: Paul Blakey <paulb@mellanox.com>
Cc: "Marcelo Ricardo Leitner" <marcelo.leitner@gmail.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Jiri Pirko" <jiri@mellanox.com>, "Roi Dayan" <roid@mellanox.com>,
	"Yossi Kuperman" <yossiku@mellanox.com>,
	"Oz Shlomo" <ozsh@mellanox.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"David Miller" <davem@davemloft.net>,
	"Aaron Conole" <aconole@redhat.com>,
	"Zhike Wang" <wangzhike@jd.com>,
	"Rony Efraim" <ronye@mellanox.com>,
	"nst-kernel@redhat.com" <nst-kernel@redhat.com>,
	"John Hurley" <john.hurley@netronome.com>,
	"Simon Horman" <simon.horman@netronome.com>,
	"Justin Pettit" <jpettit@ovn.org>,
	"Kevin Darbyshire-Bryant" <kevin@darbyshire-bryant.me.uk>
Subject: Re: [PATCH net-next 1/3] net/sched: Introduce action ct
Date: Mon, 24 Jun 2019 10:46:55 -0700	[thread overview]
Message-ID: <CAM_iQpX5Ti2F23BTEs7RqDZW_sbWFAT5Fak2vbdBsjGAp-WmpQ@mail.gmail.com> (raw)
In-Reply-To: <db10725e-d31a-efda-e57e-9978fd680c92@mellanox.com>

On Thu, Jun 20, 2019 at 12:32 AM Paul Blakey <paulb@mellanox.com> wrote:
>
>
> On 6/18/2019 7:03 PM, Cong Wang wrote:
> > On Fri, Jun 14, 2019 at 12:24 PM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> >> On Fri, Jun 14, 2019 at 11:07:37AM -0700, Cong Wang wrote:
> >>> On Tue, Jun 11, 2019 at 9:44 AM Marcelo Ricardo Leitner
> >>> <marcelo.leitner@gmail.com> wrote:
> >>>> I had suggested to let act_ct handle the above as well, as there is a
> >>>> big chunk of code on both that is pretty similar. There is quite some
> >>>> boilerplate for interfacing with conntrack which is duplicated.
> >>> Why do you want to mix retrieving conntrack info with executing
> >>> conntrack?
> >> To save on the heavy boilerplate for interfacing with conntrack.
> >>
> >>> They are totally different things to me, act_ctinfo merely retrieves
> >>> information from conntrack, while this one, act_ct, is supposed to
> >>> move packets to conntrack.
> >> Seems we have a different understanding for "move packets to
> >> conntrack": conntrack will not consume the packets after this.
> >> But after act_ct is executed, if not with the clear flag, skb will now
> >> have the skb->_nfct entry available, on which flower then will be able
> >> to match. So in essence, it is also fetching information from
> >> conntrack.
> > Interesting. Is it because cls_flower uses conntrack for flow dissection?
> > What's the reason behind?
> >
> > Again, I am still not convinced to do L3 operations in L2, skb->_nfct
> > belongs to conntrack which is L3, no matter the packet is consumed
> > or not.
> >
> > Thanks.
>
> I'm not sure what you mean, the reason behind what?

Yes, which should be the most important info in changelog.

>
> We use conntrack to track, mark the packet with conntrack info, and
> execute nat, then we push the
>
> headers back to continue processing the next action. This action will
> probably be followed by
>
> goto chain or reclassify and then cls_flower can be used to match on
> conntrack state and metadata via the new flow dissector change.
>

Sounds cool, but again why do we have to do this in L2?

Also, I am not sure if cls_flower really matches packets with any
conntrack state, from my quick glance of its code. Is this feature
merged in upstream?

Is this for ingress only? For egress, packets coming down from L3
so they should already have conntrack state as long as it is enabled?

Sorry for asking many questions here, because your changelog is too
short. :-/

Thanks.