Re: [PATCH net-next] net: dsa: mv88e6xxx: Allow dynamic reconfiguration of tag protocol

[Tobias Waldekranz <tobias@waldekranz.com>]

On Wed, Mar 24, 2021 at 16:03, Vladimir Oltean <olteanv@gmail.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".