netdev - Re: No traffic with Marvell switch and latest linux-next

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:   Sun, 24 Feb 2019 13:26:50 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: No traffic with Marvell switch and latest linux-next

On February 24, 2019 9:04:55 AM PST, Andrew Lunn <andrew@...n.ch> wrote:
>> The added difficulty here and the reason why Andrew went with the
>> approach that is used by the code currently is because neither do the
>> CPU or DSA ports are backed by a net_device. It is somewhere on my
>TODO
>> to permit the use of PHYLINK without the need of a net_device to
>cover
>> those specific DSA cases unless we just brute force the whole thing
>and
>> allocate a net_device structure but not register that net_device? Yes
>in
>> fact, why don't we do that?
>
>Hi Florian
>
>At the moment, we are using a phydev which is not connected to a
>MAC. That is rather odd, but the phylib maintainers mostly know about
>this, and keep an eye out for changes which might break any
>assumptions. And the phylib API is quite small.

I would argue that this very thread is a proof against your argument since we all failed to predict that Heiner's changes would change those assumptions. Having a certain of assumptions is fine but given all the recent PHYLIB helpers that have been added I am not sure how well that will scale.

>
>How many assumptions are going to break with a netdev which is not
>registered? The API is much bigger, more people hack on it, and it is
>going to be much harder to review changes to make sure assumptions are
>not changed.

A non registered net_device appears easier to manage and debug since there is state tracking all over the network stack for those cases.

>
>If we are going to do something odd, we should keep the scope as small
>as possible.

Hence my suggestion to allocate a dummy net_device object just so calls to netif_carrier_{on,off} (and possibly more in the future) do nothing. I don't think that teaching either PHYLIB or PHYLINK about a NULL net_device is going to scale really well over time nor make it easier  for respective maintainers. If we make the net_device optional, it will be harder to review changes as well as make sure that we do not create locking/object interactions  issues.

Another approach could be to define a minimal network port object (struct devlink, maybe?) which could be used independently from a net_device, or a lightweight net_device with no visibility into existing namespaces. None of these ideas are new though and would probably require several cycles to get done right.

Heiner, Russell, which approach would you take?
-- 
Florian