netdev - Re: consequences of setting net_device_ops ndo_change

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:   Sat, 4 Aug 2018 13:47:20 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     "Robert P. J. Day" <rpjday@...shcourse.ca>
Cc:     Linux kernel ntedev mailing list <netdev@...r.kernel.org>
Subject: Re: consequences of setting net_device_ops ndo_change_carrier()?

Sat, Aug 04, 2018 at 01:06:58PM CEST, rpjday@...shcourse.ca wrote:
>
>  i'll try to keep this (relatively) short as there may be a simple
>answer to this, or it could just be a stupid question -- sort of
>related to previous question (thank you, florian).
>
>  currently messing with networking device involving FPGA and some
>quad-port transceivers, and noticed that, when one unplugs or plugs a
>device into one of the ports, there is no change in the contents of
>the corresponding sysfs files /sys/class/net/<ifname>/carrier (or
>operstate, for that matter, which might be related to this as well).
>doing this with a "regular" port on my linux laptop certainly
>confirmed that the carrier file would switch between 0 and 1, and
>operstate would switch between up and down, so i know what behaviour i
>was *expecting* if things were ostensibly working properly.
>
>  long story short, i pawed through the driver code only to stumble

What driver? Has to be out of tree as I don't see any in the existing
kernel using .ndo_change_carrier (aside of team and dummy)


>over this in the ethernet driver for the device:
>
>  static const struct net_device_ops netdev_netdev_ops = {
>  ... snip ...
>        .ndo_change_carrier     = netdev_change_carrier,
>  ... snip ...
>  };
>
>and
>
>  static int
>  netdev_change_carrier(struct net_device *dev, bool new_carrier)
>  {
>        if (new_carrier)
>                netif_carrier_on(dev);
>        else
>                netif_carrier_off(dev);
>        return 0;
>  }
>
>as i mentioned before, i am really new to kernel networking code, so i
>did a quick search and found this in netdevice.h:
>
>* int (*ndo_change_carrier)(struct net_device *dev, bool new_carrier);
> *      Called to change device carrier. Soft-devices (like dummy, team, etc)
> *      which do not represent real hardware may define this to allow their
> *      userspace components to manage their virtual carrier state. Devices
> *      that determine carrier state from physical hardware properties (eg
> *      network cables) or protocol-dependent mechanisms (eg
> *      USB_CDC_NOTIFY_NETWORK_CONNECTION) should NOT implement this function.
> *
>
>although i still don't fully understand the purpose of that field, it
>makes me *very* nervous to read that that routine is for "soft"
>devices, and ***not*** for devices that attempt to determine carrier
>state from physical hardware properties. i searched the kernel code
>base for other drivers that set that field, and found only what is
>mentioned in that comment -- dummy.c, of_dummy_mac.c and team.c.
>
>  the testers for this unit are complaining that they are somehow not
>being notified when they plug and unplug devices from the ports -- is
>this why? what would be the purpose of assigning a routine to that
>field? as i read it (and i could be wrong), my impression is that you
>can have the driver *either* determine the carrier state from physical
>properties, *or* allow userspace control, but not both, is that
>correct?

Correct. Your device is physical device, it knows how to get the state
of the carrier itself.


>
>  i'm about to ask the original authors why they did the above, but

I guess that the reason is that they had no clue what they are doing :)


>i'd like to feel that it's not a stupid question if there's something
>really clever going on here. is this just a development debugging
>feature that would normally be removed at production? or what?
>
>rday
>
>-- 
>
>========================================================================
>Robert P. J. Day                                 Ottawa, Ontario, CANADA
>                  http://crashcourse.ca/dokuwiki
>
>Twitter:                                       http://twitter.com/rpjday
>LinkedIn:                               http://ca.linkedin.com/in/rpjday
>========================================================================