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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Mar 2021 09:53:35 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Tobias Waldekranz <tobias@...dekranz.com>,
        Andrew Lunn <andrew@...n.ch>
Cc:     davem@...emloft.net, kuba@...nel.org, vivien.didelot@...il.com,
        olteanv@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: Allow dynamic
 reconfiguration of tag protocol



On 3/23/2021 7:49 AM, Tobias Waldekranz wrote:
> On Tue, Mar 23, 2021 at 13:41, Andrew Lunn <andrew@...n.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