Re: [PATCH net-next] team: account for oper state

[George Wilkie <gwilkie@vyatta.att-mail.com>]

On Wed, Apr 18, 2018 at 04:33:12PM +0100, George Wilkie wrote:
> On Wed, Apr 18, 2018 at 04:58:22PM +0200, Jiri Pirko wrote:
> > Wed, Apr 18, 2018 at 03:35:49PM CEST, gwilkie@vyatta.att-mail.com wrote:
> > >On Wed, Apr 18, 2018 at 02:56:44PM +0200, Jiri Pirko wrote:
> > >> Wed, Apr 18, 2018 at 12:29:50PM CEST, gwilkie@vyatta.att-mail.com wrote:
> > >> >Account for operational state when determining port linkup state,
> > >> >as per Documentation/networking/operstates.txt.
> > >> 
> > >> Could you please point me to the exact place in the document where this
> > >> is suggested?
> > >> 
> > >
> > >Various places cover it I think.
> > >
> > >In 1. Introduction:
> > >"interface is not usable just because the admin enabled it"
> > >"userspace must be granted the possibility to
> > >influence operational state"
> > >
> > >In 4. Setting from userspace:
> > >"the userspace application can set IFLA_OPERSTATE
> > >to IF_OPER_DORMANT or IF_OPER_UP as long as the driver does not set
> > >netif_carrier_off() or netif_dormant_on()"
> > >
> > >We have a use case where we want to set the oper state of the team ports based
> > >on whether they are actually usable or not (as opposed to just admin up).
> > 
> > Are you running a supplicant there or what is the use-case?
> > 
> 
> We are using tun/tap interfaces for the team ports with the physical interfaces
> under the control of a user process.
> 
> > How is this handle in other drivers like bond, openvswitch, bridge, etc?
> 
> It looks like bridge is using it, looking at br_port_carrier_check() and
> br_add_if().
> 

commit 576eb62598f10c8c7fd75703fe89010cdcfff596
Author:     stephen hemminger <shemminger@vyatta.com>
AuthorDate: Fri Dec 28 18:15:22 2012 +0000
Commit:     David S. Miller <davem@davemloft.net>
CommitDate: Sun Dec 30 02:31:43 2012 -0800

    bridge: respect RFC2863 operational state
    
    The bridge link detection should follow the operational state
    of the lower device, rather than the carrier bit. This allows devices
    like tunnels that are controlled by userspace control plane to work
    with bridge STP link management.

> Cheers.
> 
> > 
> > >
> > >Cheers.
> > >
> > >> 
> > >> >
> > >> >Signed-off-by: George Wilkie <gwilkie@vyatta.att-mail.com>
> > >> >---
> > >> > drivers/net/team/team.c | 3 ++-
> > >> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >> >
> > >> >diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
> > >> >index a6c6ce19eeee..231264a05e55 100644
> > >> >--- a/drivers/net/team/team.c
> > >> >+++ b/drivers/net/team/team.c
> > >> >@@ -2918,7 +2918,8 @@ static int team_device_event(struct notifier_block *unused,
> > >> > 	case NETDEV_CHANGE:
> > >> > 		if (netif_running(port->dev))
> > >> > 			team_port_change_check(port,
> > >> >-					       !!netif_carrier_ok(port->dev));
> > >> >+					       !!(netif_carrier_ok(port->dev) &&
> > >> >+						  netif_oper_up(port->dev)));
> > >> > 		break;
> > >> > 	case NETDEV_UNREGISTER:
> > >> > 		team_del_slave(port->team->dev, dev);
> > >> >-- 
> > >> >2.11.0
> > >> >