Messages in this thread |  | | Date | Wed, 3 Oct 2018 12:44:36 +0200 | From | Jiri Pirko <> | Subject | Re: [PATCH] team: set IFF_SLAVE on team ports |
| |
Tue, Oct 02, 2018 at 11:20:25PM CEST, 3chas3@gmail.com wrote: > > >On 10/02/18 07:12, Jiri Pirko wrote: >> Mon, Oct 01, 2018 at 04:06:16PM CEST, 3chas3@gmail.com wrote: >> > >> > >> > On 09/30/18 05:34, Jiri Pirko wrote: >> > > Sun, Sep 30, 2018 at 11:38:05AM CEST, stephen@networkplumber.org wrote: >> > > > On Sun, 30 Sep 2018 09:14:14 +0200 >> > > > Jiri Pirko <jiri@resnulli.us> wrote: >> > > > >> > > > > Thu, Sep 27, 2018 at 04:04:26PM CEST, 3chas3@gmail.com wrote: >> > > > > > >> > > > > > >> > > > > > On 07/10/15 02:41, Jiri Pirko wrote: >> > > > > > > Thu, Jul 09, 2015 at 05:36:55PM CEST, jblunck@infradead.org wrote: >> > > > > > > > On Thu, Jul 9, 2015 at 12:07 PM, Jiri Pirko <jiri@resnulli.us> wrote: >> > > > > > > > > Thu, Jul 09, 2015 at 11:58:34AM CEST, jblunck@infradead.org wrote: >> > > > > > > > > > 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. >> > > > > > > > >> > > > > > > > > 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. > >> >> >> >> > > >> > > 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... >> >> >> > >> > 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.
> >> >> >> > 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. >> > >> > >> >
|  |