netdev - Re: [patch net-next v2 08/12] dsa: implement ndo_get_devlink

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:   Wed, 27 Mar 2019 13:13:00 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, mlxsw@...lanox.com,
        idosch@...lanox.com, jakub.kicinski@...ronome.com, andrew@...n.ch,
        vivien.didelot@...il.com, michael.chan@...adcom.com
Subject: Re: [patch net-next v2 08/12] dsa: implement ndo_get_devlink_port

On 3/27/19 1:01 PM, Jiri Pirko wrote:
> Wed, Mar 27, 2019 at 08:54:41PM CET, f.fainelli@...il.com wrote:
>> On 3/26/2019 5:03 AM, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@...lanox.com>
>>>
>>> In order for devlink compat functions to work, implement
>>> ndo_get_devlink_port. Legacy slaves does not have devlink port instances
>>> created for themselves.
>>
>> Not a big fan of de-duplicating the entire set of netdev_ops for legacy
>> vs. non-legacy, can we just check that the devlink instance was r
> 
> In nfp, I make legacy and non-legacy ndos to be shared. However there,
> they are doing to eventually all use devlink ports. In dsa on the other
> hand, I don't think that legacy is going to use devlink port, would it?

The legacy probing method has not been updated to match what
net/dsa/dsa2.c does, but there is no technical reason for not supporting
devlink instances over ports registered through the legacy interface. So
eventually all of what you did here will be thrown away (not sure by
which timeframe, probably after mv88e6060 finally gets converted to the
new binding).

> I wan't to eventually have WARN_ON in devlink code in case devlink_port
> exists and phys_port_name ndo is present at the same time.
> 

No sure, but you could always do something like:

if (!dp->devlink_registered)
	return dsa_slave_get_phys_port_name();

return &dp->devlink_port;

is what I had in mind. This just requires adding a boolean to track the
registration of devlink ports within net/dsa/dsa2.c which should be
fewer lines of code and easier to remove in the future. No strong
objections though.
-- 
Florian