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] [day] [month] [year] [list]

Message-ID: <20190327234654.GI14297@nanopsycho>
Date:   Thu, 28 Mar 2019 00:46:54 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Florian Fainelli <f.fainelli@...il.com>
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

Wed, Mar 27, 2019 at 09:13:00PM CET, f.fainelli@...il.com wrote:
>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).

Okay. In that case, when you say legacy will be eventually remove,
Will do the same as I did for nfp. Thanks!

>
>> 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