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

[Florian Fainelli <f.fainelli@gmail.com>]

On 3/23/2021 7:49 AM, Tobias Waldekranz wrote:
> On Tue, Mar 23, 2021 at 13:41, Andrew Lunn <andrew@lunn.ch> wrote:
>> On Tue, Mar 23, 2021 at 11:23:26AM +0100, Tobias Waldekranz wrote:
>>> All devices are capable of using regular DSA tags. Support for
>>> Ethertyped DSA tags sort into three categories:
>>>
>>> 1. No support. Older chips fall into this category.
>>>
>>> 2. Full support. Datasheet explicitly supports configuring the CPU
>>>    port to receive FORWARDs with a DSA tag.
>>>
>>> 3. Undocumented support. Datasheet lists the configuration from
>>>    category 2 as "reserved for future use", but does empirically
>>>    behave like a category 2 device.
>>>
>>> Because there are ethernet controllers that do not handle regular DSA
>>> tags in all cases, it is sometimes preferable to rely on the
>>> undocumented behavior, as the alternative is a very crippled
>>> system. But, in those cases, make sure to log the fact that an
>>> undocumented feature has been enabled.
>>
>> Hi Tobias
>>
>> I wonder if dynamic reconfiguration is the correct solution here. By
>> default it will be wrong for this board, and you need user space to
>> flip it.
>>
>> Maybe a DT property would be better. Extend dsa_switch_parse_of() to
>> look for the optional property dsa,tag-protocol, a string containing
>> the name of the tag ops to be used.
> 
> This was my initial approach. It gets quite messy though. Since taggers
> can be modules, there is no way of knowing if a supplied protocol name
> is garbage ("asdf"), or just part of a module in an initrd that is not
> loaded yet when you are probing the tree. Even when the tagger is
> available, there is no way to verify if the driver is compatible with
> it. So I think we would have to:
> 
> - Keep the list of protcol names compiled in with the DSA module, such
>   that "edsa" can be resolved to DSA_TAG_PROTO_EDSA without having the
>   tagger module loaded.
> 
> - Add (yet) another op so that we can ask the driver if the given
>   protocol is acceptable. Calling .change_tag_protocol will not work as
>   drivers will assume that the driver's .setup has already executed
>   before it is called.
> 
> - Have each driver check (during .setup?) if it should configure the
>   device to use its preferred protocol or if the user has specified
>   something else.
> 
> That felt like a lot to take on board just to solve a corner case like
> this. I am happy to be told that there is a much easier way to do it, or
> that the above would be acceptable if there isn't one.
> 

The other problem with specifying the tag within the Device Tree is that
you are half way between providing a policy (which tag to use) and
describing how the hardware works (which tag is actually supported).
FWIW, the b53/bcm_sf2 binding allows one to specify whether an internal
port should have Broadcom tags enabled because the accelerator behind
that port would require it (brcm,use-bcm-hdr).
-- 
Florian