Thread (15 messages) 15 messages, 4 authors, 2018-10-06

Re: [PATCH] team: set IFF_SLAVE on team ports

From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-10-05 06:51:58
Also in: lkml

Wed, Oct 03, 2018 at 07:30:06PM CEST, 3chas3@gmail.com wrote:

On 10/03/18 06:44, Jiri Pirko wrote:
quoted
Tue, Oct 02, 2018 at 11:20:25PM CEST, 3chas3@gmail.com wrote:
quoted

On 10/02/18 07:12, Jiri Pirko wrote:
quoted
Mon, Oct 01, 2018 at 04:06:16PM CEST, 3chas3@gmail.com wrote:
quoted

On 09/30/18 05:34, Jiri Pirko wrote:
quoted
Sun, Sep 30, 2018 at 11:38:05AM CEST, stephen@networkplumber.org wrote:
quoted
On Sun, 30 Sep 2018 09:14:14 +0200
Jiri Pirko [off-list ref] wrote:
quoted
Thu, Sep 27, 2018 at 04:04:26PM CEST, 3chas3@gmail.com wrote:
quoted

On 07/10/15 02:41, Jiri Pirko wrote:
quoted
Thu, Jul 09, 2015 at 05:36:55PM CEST, jblunck@infradead.org wrote:
quoted
On Thu, Jul 9, 2015 at 12:07 PM, Jiri Pirko [off-list ref] wrote:
quoted
Thu, Jul 09, 2015 at 11:58:34AM CEST, jblunck@infradead.org wrote:
quoted
The code in net/ipv6/addrconf.c:addrconf_notify() tests for IFF_SLAVE to
decide if it should start the address configuration. Since team ports
shouldn't get link-local addresses assigned lets set IFF_SLAVE when linking
a port to the team master.
I don't want to use IFF_SLAVE in team. Other master-slave devices are
not using that as well, for example bridge, ovs, etc.
Maybe they need to get fixed too. I've used that flag because it is
documented as
a "slave of a load balancer" which describes what a team port is.
quoted
I think that this should be fixed in addrconf_notify. It should lookup
if there is a master on top and bail out in that case.
There are other virtual interfaces that have a master assigned and want to
participate in IPv6 address configuration.
Can you give me an example?
I would like to revisit this patch (yes, I know it has been a while).  I
believe the VRF implementation uses master to group the interfaces under
a single interface.

I don't see a reason not to use IFF_SLAVE since team and bonding are fairly
similar.
Again, why do you need team port to have IFF_SLAVE flag? What do you
want to achieve
Without setting this flag IPv6 will try and make a link specific address.
You are talking about addrconf_notify() right? Easy to fix to check
something more convenient. Like netif_is_lag_port() if you want to avoid
it for bond/team. netif_is_ovs_port(), netif_is_bridge_port() etc. Lot's
of helpers to cover this.
OK, IPv6 should probably be using this.
quoted

quoted
quoted
Why is it not an issue with bridge, ovs, and other master-slave devices?
It very well might be an issue for bridge and ovs.  Other master-slave
devices include the existing VRF implementation in the kernel and those slave
interfaces will certainly want to use IPv6.

However, IFF_SLAVE has a specific meaning:

./include/uapi/linux/if.h: * @IFF_SLAVE: slave of a load balancer. Volatile.
I know that some userspace apps are using this flag to determine a
"bonding slave". I don't think that they care much about eql...

quoted
The bonding driver is not the only user:

./drivers/net/eql.c:#define eql_is_slave(dev)	((dev->flags & IFF_SLAVE) ==
IFF_SLAVE)
./drivers/net/eql.c:	slave->dev->flags &= ~IFF_SLAVE;
./drivers/net/eql.c:		slave->dev->flags |= IFF_SLAVE;

The team driver would like to use this same flag since it is a load balancer
as well.  The side effect of not assigning IPv6 is a bonus. The fact that
No, please leave IFF_SLAVE as it is. Both kernel and userspace have
their clear indications right now about the master/slave relationships.
The team driver does create a master/slave relationship.  The team slaves are
literally slaves of the master device.  It's not clear to me
why you we can't mark the slaves of the team master as actually being
slave interfaces?
So? IFF_SLAVE flag serves a different purpose. That's it. Team does not
need it, bridge does not need it, macvlan does not need it, etc.
I agree.  But team *is* a load balancer.  Why can't team mark its slave
interfaces as IFF_SLAVE?  They are literally slaves of a load balancer which
is the exact meaning of the IFF_SLAVE flag.
I described that multiple times, don't want to repeat myself. Please
read the thread again.
quoted
quoted
quoted
quoted
bridges and ovs are also likely broken is a different issue.  Should there be
a another flag that says "layer 2 only"?  Very possibly, but that is
something all these interfaces should be using to include bonding, team, eql,
obs, bridge etc.  That's not a reasonable objection to labeling the team
slave as slaves since they are literally slaves of a load balancer.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help