Message-ID: <87mtuslemq.fsf@waldekranz.com>
Date:   Wed, 24 Mar 2021 17:07:09 +0100
From:   Tobias Waldekranz <tobias@...dekranz.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     davem@...emloft.net, kuba@...nel.org, andrew@...n.ch,
        vivien.didelot@...il.com, f.fainelli@...il.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: Allow dynamic reconfiguration of tag protocol

On Wed, Mar 24, 2021 at 17:08, Vladimir Oltean <olteanv@...il.com> wrote:
> On Wed, Mar 24, 2021 at 04:02:52PM +0100, Tobias Waldekranz wrote:
>> On Wed, Mar 24, 2021 at 16:03, Vladimir Oltean <olteanv@...il.com> wrote:
>> > On Tue, Mar 23, 2021 at 10:17:30PM +0100, Tobias Waldekranz wrote:
>> >> > I don't see any place in the network stack that recalculates the FCS if
>> >> > NETIF_F_RXALL is set. Additionally, without NETIF_F_RXFCS, I don't even
>> >> > know how could the stack even tell a packet with bad FCS apart from one
>> >> > with good FCS. If NETIF_F_RXALL is set, then once a packet is received,
>> >> > it's taken for granted as good.
>> >> 
>> >> Right, but there is a difference between a user explicitly enabling it
>> >> on a device and us enabling it because we need it internally in the
>> >> kernel.
>> >> 
>> >> In the first scenario, the user can hardly complain as they have
>> >> explicitly requested to see all packets on that device. That would not
>> >> be true in the second one because there would be no way for the user to
>> >> turn it off. It feels like you would end up in a similar situation as
>> >> with the user- vs. kernel- promiscuous setting.
>> >> 
>> >> It seems to me if we enable it, we are responsible for not letting crap
>> >> through to the port netdevs.
>> >
>> > I think there exists an intermediate approach between processing the
>> > frames on the RX queue and installing a soft parser.
>> >
>> > The BMI of FMan RX ports has a configurable pipeline through Next
>> > Invoked Actions (NIA). Through the FMBM_RFNE register (Rx Frame Next
>> > Engine), it is possible to change the Next Invoked Action from the
>> > default value (which is the hardware parser). You can choose to make the
>> > Buffer Manager Interface enqueue the packet directly to the Queue
>> > Manager Interface (QMI). This will effectively bypass the hardware
>> > parser, so DSA frames will never be sent to the error queue if they have
>> > an invalid EtherType/Length field.
>> >
>> > Additionally, frames with a bad FCS should still be discarded, as that
>> > is done by the MAC (an earlier stage compared to the BMI).
>> 
>> Yeah this sounds like the perfect middle ground. I guess that would then
>> be activated with an `if (netdev_uses_dsa(dev))`-guard in the driver,
>> like how Florian solved it for stmmac? Since it is not quite "rx-all".
>
> I think this would have to be guarded by netdev_uses_dsa for now, yes.
> Also, it is far from being a "perfect" middle ground, because if you
> disable the hardware parser, you also lose the ability to do frame
> classification and hashing/flow steering to multiple RX queues on that
> port, I think.

But even if the parser was enabled, it would never get anywhere since
the Ethertype would look like random garbage. Unless we have the soft
parser, but then it is not the middle ground anymore :)

I suppose you would like to test for netdev_uses_dsa_and_violates_8023,
that way you could still do RSS on DSA devices using regular 1Q-tags for
example. Do we want to add this property to the taggers so that we do
not degrade performance for any existing users?

Powered by blists - more mailing lists