Re: [PATCH RFC net-next 05/19] net: dsa: tag_ar9331: add GRO callbacks

[Alexander Lobakin]

Florian Fainelli wrote 15.01.2020 00:56:
> On 1/13/20 2:28 AM, Vladimir Oltean wrote:
> > On Mon, 13 Jan 2020 at 11:46, Alexander Lobakin <alobakin@xxxxxxxx> 
> > wrote:
> > > Vladimir Oltean wrote 13.01.2020 12:42:
> > > > Hi Alexander,
> > > >
> > > > On Mon, 13 Jan 2020 at 11:22, Alexander Lobakin <alobakin@xxxxxxxx>
> > > > wrote:
> > > > > CPU ports can't be bridged anyway
> > > > >
> > > > > Regards,
> > > > > á á á á á á
> > > >
> > > > The fact that CPU ports can't be bridged is already not ideal.
> > > > One can have a DSA switch with cascaded switches on each port, so it
> > > > acts like N DSA masters (not as DSA links, since the taggers are
> > > > incompatible), with each switch forming its own tree. It is 
> > > > desirable
> > > > that the ports of the DSA switch on top are bridged, so that
> > > > forwarding between cascaded switches does not pass through the CPU.
> > >
> > > Oh, I see. But currently DSA infra forbids the adding DSA masters to
> > > bridges IIRC. Can't name it good or bad decision, but was introduced
> > > to prevent accidental packet flow breaking on DSA setups.
> >
> > I just wanted to point out that some people are going to be looking at
> > ways by which the ETH_P_XDSA handler can be made to play nice with the
> > master's rx_handler, and that it would be nice to at least not make
> > the limitation worse than it is by converting everything to
> > rx_handlers (which "currently" can't be stacked, from the comments in
> > netdevice.h).
>
> I am not sure this would change the situation much, today we cannot 
> have
> anything but switch tags travel on the DSA master network device,
> whether we accomplish the RX tap through a special skb->protocol value
> or via rx_handler, it probably does not functionally matter, but it
> could change the performance.

As for now, I think that we should keep this RFC as it is so
developers working with different DSA switches could test it or
implement GRO offload for other taggers like DSA and EDSA, *but*
any future work on this should come only when we'll revise/reimagine
basic DSA packet flow, as we already know (at least me and Florian
reproduce it well) that the current path through unlikely branches
in eth_type_trans() and frame capturing through packet_type is so
suboptimal that nearly destroys overall performance on several
setups.
Switching to net_device::rx_handler() is just one of all the possible
variants, I'm sure we'll find the best solution together.

Regards,
á á á á á á