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

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:	Thu, 13 Dec 2012 11:09:03 -0800
From:	John Fastabend <john.r.fastabend@...el.com>
To:	Dan Williams <dcbw@...hat.com>
CC:	Stephen Hemminger <shemminger@...tta.com>,
	Flavio Leitner <fbl@...hat.com>,
	John Fastabend <john.fastabend@...il.com>,
	Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org,
	davem@...emloft.net, edumazet@...gle.com,
	bhutchings@...arflare.com, mirqus@...il.com,
	greearb@...delatech.com
Subject: Re: [patch net-next 0/4] net: allow to change carrier from userspace

On 12/13/2012 10:33 AM, Dan Williams wrote:
> On Thu, 2012-12-13 at 10:20 -0800, Stephen Hemminger wrote:
>> On Thu, 13 Dec 2012 16:17:33 -0200
>> Flavio Leitner <fbl@...hat.com> wrote:
>>
>>> On Thu, 13 Dec 2012 10:09:33 -0800
>>> Stephen Hemminger <shemminger@...tta.com> wrote:
>>>
>>>> On Thu, 13 Dec 2012 15:54:23 -0200
>>>> Flavio Leitner <fbl@...hat.com> 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

Assuming the operstate documentation is correct IFF_RUNNING should only
be set if the operstate is up or unknown. Quoting the text again,

  32	ifinfomsg::if_flags & IFF_RUNNING:
  33	 Interface is in RFC2863 operational state UP or UNKNOWN. This is for
  34	 backward compatibility, routing daemons, dhcp clients can use this
  35	 flag to determine whether they should use the interface.

So my guess is having a team ethtool get_link handler AND managing the
operstate from teamd would satisfy the requirement. But maybe I missed
something.

.John
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html