Subject: Re: [patch net-next 0/4] net: allow to change carrier from userspace

From: Dan Williams <[email-masked]>
Date: Thu, 13 Dec 2012 12:33:32 -0600

On Thu, 2012-12-13 at 10:20 -0800, Stephen Hemminger wrote:
> On Thu, 13 Dec 2012 16:17:33 -0200
> Flavio Leitner <[email-masked]> wrote:
> 
> > On Thu, 13 Dec 2012 10:09:33 -0800
> > Stephen Hemminger <[email-masked]> wrote:
> > 
> > > On Thu, 13 Dec 2012 15:54:23 -0200
> > > Flavio Leitner <[email-masked]> wrote:
> > > 
> > > > I am saying this because people are used to and there are scripts out
> > > > there using something like:
> > > > # ethtool <iface> | grep 'Link'
> > > > to react an interface failure.
> > > 
> > > Then the script is broken. It is asking about hardware state.
> > 
> > I was talking about the team master interface, so it makes sense
> > to check its 'hardware' state. Just think on 'bond0' interface
> > with no slaves. It should report Link detected: no.
> > 
> > See bond_release(), what happens if bond->slave_cnt == 0, for instance.
> > 
> 
> I was thinking more that ethtool operation for reporting link on
> the team device should use the proper check rather than just using netif_carrier_ok(),
> the team ethtool operation for get_link should be check IFF_RUNNING flag
> in dev->flags which is controlled by operstate transistions.

That's not entirely sufficient, because not everything uses ethtool to
deterine whether the link/carrier is active.  eg, anything listenting to
netlink for IFF_RUNNING won't know anything about this.  I may have
missed previous conversation, but IFF_RUNNING is AFAIK the sole
mechanism to indicate to userspace that the link is operational and
ready for general traffic.  Can the team drivers simply not set
IFF_RUNNING until the interface is actually ready for general traffic?

I'd just caution people to be careful when it comes to carrier/link
states, as a lot of stuff relies on it, and people keep trying to
slightly change the meaning of it.  We need it to be consistent across
net interfaces and driver implementations, not have specific meanings to
certain drivers that don't apply to others.

If teamd needs to manage the carrier state of the teamX interface,
perhaps instead of munging the actual carrier state itself, it can flip
some team-private value that is one component of determining whether
IFF_RUNNING gets set or not?

Dan