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]

Message-ID: <87a6qulybz.fsf@waldekranz.com>
Date:   Tue, 23 Mar 2021 15:49:20 +0100
From:   Tobias Waldekranz <tobias@...dekranz.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     davem@...emloft.net, kuba@...nel.org, vivien.didelot@...il.com,
        f.fainelli@...il.com, olteanv@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: Allow dynamic reconfiguration of tag protocol

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.