consequences of setting net_device_ops ndo_change_carrier()?

consequences of setting net_device_ops ndo_change_carrier()?

[Robert P. J. Day]

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

  i'm about to ask the original authors why they did the above, but
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
========================================================================

Re: consequences of setting net_device_ops ndo_change_carrier()?

[Jiri Pirko]

Sat, Aug 04, 2018 at 01:06:58PM CEST, rpjday@crashcourse.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
>========================================================================

Re: consequences of setting net_device_ops ndo_change_carrier()?

[Robert P. J. Day]

On Sat, 4 Aug 2018, Jiri Pirko wrote:

> Sat, Aug 04, 2018 at 01:06:58PM CEST, rpjday@crashcourse.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)

  yes, currently proprietary and in-house under development, so i have
to be a little vague about certain details.

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

  that's what i *thought*, good to have confirmation.

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

  given that i've been immersed in networking code for only a few
days, i was not about to draw any conclusion like that. :-) i'm going
to continue perusing the code just to feel more confident about my
eventual conclusion, but it would seem that there is no compelling
reason for setting ndo_change_carrier() for actual physical devices,
and that is quite possibly the cause of the weird behaviour the
testers are seeing.  thanks muchly.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Re: consequences of setting net_device_ops ndo_change_carrier()?

[Stephen Hemminger]

On Sat, 4 Aug 2018 07:57:56 -0400 (EDT)
"Robert P. J. Day" <rpjday@crashcourse.ca> wrote:

> On Sat, 4 Aug 2018, Jiri Pirko wrote:
> 
> > Sat, Aug 04, 2018 at 01:06:58PM CEST, rpjday@crashcourse.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)  
> 
>   yes, currently proprietary and in-house under development, so i have
> to be a little vague about certain details.
> 
> > >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.  
> 
>   that's what i *thought*, good to have confirmation.
> 
> > >
> > >  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
> > :)  
> 
>   given that i've been immersed in networking code for only a few
> days, i was not about to draw any conclusion like that. :-) i'm going
> to continue perusing the code just to feel more confident about my
> eventual conclusion, but it would seem that there is no compelling
> reason for setting ndo_change_carrier() for actual physical devices,
> and that is quite possibly the cause of the weird behaviour the
> testers are seeing.  thanks muchly.
> 
> rday

ndo_change_carrier is not the droid your looking for.

The purpose of ndo_change_carrier was for testing network devices
(ie dummy), and also for cases like network tunnels where the
sofrware carrier state may be controlled by a userspace daemon.

Real network devices  call netif_carrier_on and netif_carrier_off
when they notice change in carrier state in hardware. Typically,
this is when an interrupt happens.

Re: consequences of setting net_device_ops ndo_change_carrier()?

[Robert P. J. Day]

On Sat, 4 Aug 2018, Stephen Hemminger wrote:

... big snip ...

> ndo_change_carrier is not the droid your looking for.
>
> The purpose of ndo_change_carrier was for testing network devices
> (ie dummy), and also for cases like network tunnels where the
> sofrware carrier state may be controlled by a userspace daemon.
>
> Real network devices call netif_carrier_on and netif_carrier_off
> when they notice change in carrier state in hardware. Typically,
> this is when an interrupt happens.

  i had actually come to just that conclusion, as i was digging
through the code, and couldn't immediately see why setting
ndo_change_carrier() would cause a problem. in fact, to help my
admittedly painful newbie-level debugging, i started a wiki page to
track this (i document *everything* on wiki pages):

  http://crashcourse.ca/dokuwiki/doku.php?id=ndo_change_carrier

so i am reduced to concluding that the drivers in question are simply
not calling correctly the very routines you mention.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Re: consequences of setting net_device_ops ndo_change_carrier()?

[Andrew Lunn]

On Sat, Aug 04, 2018 at 07:06:58AM -0400, Robert P. J. Day 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).

Hi Robert

As other have pointed out, ndo_change_carrier is not what you want
here.

You should have a PHY device of some sort. Either a traditional copper
PHY, or an SFP module. There should be a driver for this PHY. This
could be one of those in drivers/net/phy. Or it could be firmware
running, running on a little microcontroller inside your FPGA?

Assuming you are using a Linux phy driver, it will keep an eye on the
state of the Link. If the cable is unplugged, the other end downs its
interface, etc. it will notice the change in state. The core phy code,
aka. phylib, will then call netif_carrier_off() and it will call the
MAC callback which was registers when the MAC driver called one of the
phy_connect() variants. The same happens when the link goes up. If you
are using an SFP module, it is a little but more complex, and you
should look at phylink.

If you have firmware managing the PHY, (not recommended, goes against
the principals of open source, nobody can help you debug it, or fix
it, costs you more in the long run, blah, blah, blah), the MAC driver
should be informed, probably by an interrupt and a status
register. The MAC driver should then call netif_carrier_off|on as
needed.

	Andrew

Re: consequences of setting net_device_ops ndo_change_carrier()?

[Robert P. J. Day]

On Sun, 5 Aug 2018, Andrew Lunn wrote:

> On Sat, Aug 04, 2018 at 07:06:58AM -0400, Robert P. J. Day 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).
>
> Hi Robert
>
> As other have pointed out, ndo_change_carrier is not what you want
> here.

  i think i see that now ... based on the really adamant comment in
netdevice.h:

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

the impression i got was that implementing that routine for a physical
device would actually cause problems, but it seems only that it would
be a strange thing to do, but wouldn't necessarily cause problems if
you chose not to take advantage of it. which brings me back to one of
my original questions -- why *would* someone implement it? as some
sort of debugging feature? in any event, i'm convinced that that's not
where the problem lies.

> You should have a PHY device of some sort. Either a traditional
> copper PHY, or an SFP module. There should be a driver for this PHY.
> This could be one of those in drivers/net/phy. Or it could be
> firmware running, running on a little microcontroller inside your
> FPGA?

  in my case, it's properly in drivers/net/phy, so at least that part
is normal. back to investigating ...

rday

Re: consequences of setting net_device_ops ndo_change_carrier()?

[Andrew Lunn]

> > You should have a PHY device of some sort. Either a traditional
> > copper PHY, or an SFP module. There should be a driver for this PHY.
> > This could be one of those in drivers/net/phy. Or it could be
> > firmware running, running on a little microcontroller inside your
> > FPGA?
> 
>   in my case, it's properly in drivers/net/phy, so at least that part
> is normal. back to investigating ...

Hi Robert

O.K, that makes thing simpler.

PHYs are controlled via an MDIO bus. Do you have an MDIO bus driver?
You said this was an FPGA design. MDIO might be a standard cell you
can just drop in. If so, look around and see if there is an existing
driver for it. Otherwise you might have to write one. They are quite
simple, some examples are in driver/net/phy. Depending on the address
range, the MDIO bus driver can also be embedded in the MAC driver.

       Andrew