* [PATCH net-next] team: account for oper state @ 2018-04-18 10:29 George Wilkie 2018-04-18 12:56 ` Jiri Pirko 0 siblings, 1 reply; 8+ messages in thread From: George Wilkie @ 2018-04-18 10:29 UTC (permalink / raw) To: Jiri Pirko, netdev Account for operational state when determining port linkup state, as per Documentation/networking/operstates.txt. 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] team: account for oper state 2018-04-18 10:29 [PATCH net-next] team: account for oper state George Wilkie @ 2018-04-18 12:56 ` Jiri Pirko 2018-04-18 13:35 ` George Wilkie 0 siblings, 1 reply; 8+ messages in thread From: Jiri Pirko @ 2018-04-18 12:56 UTC (permalink / raw) To: George Wilkie; +Cc: netdev 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? > >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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] team: account for oper state 2018-04-18 12:56 ` Jiri Pirko @ 2018-04-18 13:35 ` George Wilkie 2018-04-18 14:58 ` Jiri Pirko 0 siblings, 1 reply; 8+ messages in thread From: George Wilkie @ 2018-04-18 13:35 UTC (permalink / raw) To: Jiri Pirko; +Cc: netdev 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). 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] team: account for oper state 2018-04-18 13:35 ` George Wilkie @ 2018-04-18 14:58 ` Jiri Pirko 2018-04-18 15:33 ` George Wilkie 0 siblings, 1 reply; 8+ messages in thread From: Jiri Pirko @ 2018-04-18 14:58 UTC (permalink / raw) To: George Wilkie; +Cc: netdev 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? How is this handle in other drivers like bond, openvswitch, bridge, etc? > >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 >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] team: account for oper state 2018-04-18 14:58 ` Jiri Pirko @ 2018-04-18 15:33 ` George Wilkie 2018-04-18 15:39 ` George Wilkie 2018-04-18 19:17 ` Jiri Pirko 0 siblings, 2 replies; 8+ messages in thread From: George Wilkie @ 2018-04-18 15:33 UTC (permalink / raw) To: Jiri Pirko; +Cc: netdev 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(). 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 > >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] team: account for oper state 2018-04-18 15:33 ` George Wilkie @ 2018-04-18 15:39 ` George Wilkie 2018-04-18 19:17 ` Jiri Pirko 1 sibling, 0 replies; 8+ messages in thread From: George Wilkie @ 2018-04-18 15:39 UTC (permalink / raw) To: Jiri Pirko; +Cc: netdev 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 > > >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] team: account for oper state 2018-04-18 15:33 ` George Wilkie 2018-04-18 15:39 ` George Wilkie @ 2018-04-18 19:17 ` Jiri Pirko 2018-04-19 9:33 ` George Wilkie 1 sibling, 1 reply; 8+ messages in thread From: Jiri Pirko @ 2018-04-18 19:17 UTC (permalink / raw) To: George Wilkie; +Cc: netdev Wed, Apr 18, 2018 at 05:33:12PM CEST, gwilkie@vyatta.att-mail.com 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(). Okay, so why do you still need to check netif_carrier_ok? Looks like netif_oper_up is enough, right? > >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 >> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] team: account for oper state 2018-04-18 19:17 ` Jiri Pirko @ 2018-04-19 9:33 ` George Wilkie 0 siblings, 0 replies; 8+ messages in thread From: George Wilkie @ 2018-04-19 9:33 UTC (permalink / raw) To: Jiri Pirko; +Cc: netdev On Wed, Apr 18, 2018 at 09:17:32PM +0200, Jiri Pirko wrote: > Wed, Apr 18, 2018 at 05:33:12PM CEST, gwilkie@vyatta.att-mail.com 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(). > > Okay, so why do you still need to check netif_carrier_ok? > Looks like netif_oper_up is enough, right? Yes, I was being overly cautious. Replacing netif_carrier_ok with netif_oper_up works OK. I'll send updated patch. Cheers. > > > > > >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 > >> >> > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-19 9:34 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-18 10:29 [PATCH net-next] team: account for oper state George Wilkie 2018-04-18 12:56 ` Jiri Pirko 2018-04-18 13:35 ` George Wilkie 2018-04-18 14:58 ` Jiri Pirko 2018-04-18 15:33 ` George Wilkie 2018-04-18 15:39 ` George Wilkie 2018-04-18 19:17 ` Jiri Pirko 2018-04-19 9:33 ` George Wilkie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.