[PATCH net] team: prevent ipv6 link local address on port devices

[PATCH net] team: prevent ipv6 link local address on port devices

[Xin Long]

The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
for slaves separately from master") is also needed in Team. Otherwise,
DAD and RS packets to be sent from the slaves in turn can confuse the
switches and cause them to incorrectly update their forwarding tables
as Liang noticed in the test with activebackup mode.

Note that the patch also sets IFF_MASTER flag for Team dev accordingly
while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
not really used in Team, it's good to show in 'ip link':

  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>

Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
Reported-by: LiLiang <liali@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 drivers/net/team/team.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 62ade69295a9..5b187913cfec 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1127,6 +1127,7 @@ static void team_upper_dev_unlink(struct team *team, struct team_port *port)
 {
 	netdev_upper_dev_unlink(port->dev, team->dev);
 	port->dev->priv_flags &= ~IFF_TEAM_PORT;
+	port->dev->flags &= ~IFF_SLAVE;
 }
 
 static void __team_port_change_port_added(struct team_port *port, bool linkup);
@@ -1212,6 +1213,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
 		goto err_port_enter;
 	}
 
+	port_dev->flags |= IFF_SLAVE;
 	err = dev_open(port_dev, extack);
 	if (err) {
 		netdev_dbg(dev, "Device %s opening failed\n",
@@ -1312,6 +1314,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
 	dev_close(port_dev);
 
 err_dev_open:
+	port_dev->flags &= ~IFF_SLAVE;
 	team_port_leave(team, port);
 	team_port_set_orig_dev_addr(port);
 
@@ -2171,6 +2174,7 @@ static void team_setup(struct net_device *dev)
 	dev->ethtool_ops = &team_ethtool_ops;
 	dev->needs_free_netdev = true;
 	dev->priv_destructor = team_destructor;
+	dev->flags |= IFF_MASTER;
 	dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
 	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->priv_flags |= IFF_TEAM;
-- 
2.31.1

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Stephen Hemminger]

On Mon,  5 Dec 2022 12:46:05 -0500
Xin Long <lucien.xin@gmail.com> wrote:

> The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
> for slaves separately from master") is also needed in Team. Otherwise,
> DAD and RS packets to be sent from the slaves in turn can confuse the
> switches and cause them to incorrectly update their forwarding tables
> as Liang noticed in the test with activebackup mode.
> 
> Note that the patch also sets IFF_MASTER flag for Team dev accordingly
> while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
> not really used in Team, it's good to show in 'ip link':
> 
>   eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
>   team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
> 
> Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
> Reported-by: LiLiang <liali@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

The failover device probably needs the same changes.
Does anyone use the failover network device? Looks like KVM never got support for it.

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Jiri Pirko]

Mon, Dec 05, 2022 at 06:46:05PM CET, lucien.xin@gmail.com wrote:
>The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
>for slaves separately from master") is also needed in Team. Otherwise,
>DAD and RS packets to be sent from the slaves in turn can confuse the
>switches and cause them to incorrectly update their forwarding tables
>as Liang noticed in the test with activebackup mode.
>
>Note that the patch also sets IFF_MASTER flag for Team dev accordingly
>while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
>not really used in Team, it's good to show in 'ip link':
>
>  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
>  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
>
>Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
>Reported-by: LiLiang <liali@redhat.com>
>Signed-off-by: Xin Long <lucien.xin@gmail.com>

Nack. Please don't do this. IFF_MASTER and IFF_SLAVE are historical
flags used by bonding and eql. Should not be used for other devices.

addrconf_addr_gen() should not check IFF_SLAVE. It should use:
netif_is_lag_port() and netif_is_failover_slave() helpers.

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Xin Long]

On Tue, Dec 6, 2022 at 3:05 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Mon, Dec 05, 2022 at 06:46:05PM CET, lucien.xin@gmail.com wrote:
> >The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
> >for slaves separately from master") is also needed in Team. Otherwise,
> >DAD and RS packets to be sent from the slaves in turn can confuse the
> >switches and cause them to incorrectly update their forwarding tables
> >as Liang noticed in the test with activebackup mode.
> >
> >Note that the patch also sets IFF_MASTER flag for Team dev accordingly
> >while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
> >not really used in Team, it's good to show in 'ip link':
> >
> >  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
> >  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
> >
> >Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
> >Reported-by: LiLiang <liali@redhat.com>
> >Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> Nack. Please don't do this. IFF_MASTER and IFF_SLAVE are historical
> flags used by bonding and eql. Should not be used for other devices.
I see. I was wondering why it was not used in Team at the beginning. :)

>
> addrconf_addr_gen() should not check IFF_SLAVE. It should use:
> netif_is_lag_port() and netif_is_failover_slave() helpers.
netif_is_failover_slave() should be able to address Stephen's concern
on failover network device.

Will repost, Thanks.

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Xin Long]

On Tue, Dec 6, 2022 at 8:32 AM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Tue, Dec 6, 2022 at 3:05 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >
> > Mon, Dec 05, 2022 at 06:46:05PM CET, lucien.xin@gmail.com wrote:
> > >The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
> > >for slaves separately from master") is also needed in Team. Otherwise,
> > >DAD and RS packets to be sent from the slaves in turn can confuse the
> > >switches and cause them to incorrectly update their forwarding tables
> > >as Liang noticed in the test with activebackup mode.
> > >
> > >Note that the patch also sets IFF_MASTER flag for Team dev accordingly
> > >while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
> > >not really used in Team, it's good to show in 'ip link':
> > >
> > >  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
> > >  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
> > >
> > >Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
> > >Reported-by: LiLiang <liali@redhat.com>
> > >Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >
> > Nack. Please don't do this. IFF_MASTER and IFF_SLAVE are historical
> > flags used by bonding and eql. Should not be used for other devices.
> I see. I was wondering why it was not used in Team at the beginning. :)
>
> >
> > addrconf_addr_gen() should not check IFF_SLAVE. It should use:
> > netif_is_lag_port() and netif_is_failover_slave() helpers.
Hi Jiri,

Sorry, it seems not to work with this.

As addrconf_addr_gen() is also called in NETDEV_UP event where
IFF_TEAM_PORT and IFF_BONDING haven't yet been set before
dev_open() when adding the port.

If we move IFF_TEAM_PORT setting ahead of dev_open(), it will revert
the fix in:

commit d7d3c05135f37d8fdf73f9966d27155cada36e56
Author: Jiri Pirko <jiri@resnulli.us>
Date:   Mon Aug 25 21:38:27 2014 +0200

    team: set IFF_TEAM_PORT priv_flag after rx_handler is registered

Can we keep IFF_SLAVE here only for no ipv6 addrconf?
or do you see a better solution?

Thanks.

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Jiri Pirko]

Tue, Dec 06, 2022 at 10:52:33PM CET, lucien.xin@gmail.com wrote:
>On Tue, Dec 6, 2022 at 8:32 AM Xin Long <lucien.xin@gmail.com> wrote:
>>
>> On Tue, Dec 6, 2022 at 3:05 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >
>> > Mon, Dec 05, 2022 at 06:46:05PM CET, lucien.xin@gmail.com wrote:
>> > >The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
>> > >for slaves separately from master") is also needed in Team. Otherwise,
>> > >DAD and RS packets to be sent from the slaves in turn can confuse the
>> > >switches and cause them to incorrectly update their forwarding tables
>> > >as Liang noticed in the test with activebackup mode.
>> > >
>> > >Note that the patch also sets IFF_MASTER flag for Team dev accordingly
>> > >while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
>> > >not really used in Team, it's good to show in 'ip link':
>> > >
>> > >  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
>> > >  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
>> > >
>> > >Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
>> > >Reported-by: LiLiang <liali@redhat.com>
>> > >Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >
>> > Nack. Please don't do this. IFF_MASTER and IFF_SLAVE are historical
>> > flags used by bonding and eql. Should not be used for other devices.
>> I see. I was wondering why it was not used in Team at the beginning. :)
>>
>> >
>> > addrconf_addr_gen() should not check IFF_SLAVE. It should use:
>> > netif_is_lag_port() and netif_is_failover_slave() helpers.
>Hi Jiri,
>
>Sorry, it seems not to work with this.
>
>As addrconf_addr_gen() is also called in NETDEV_UP event where
>IFF_TEAM_PORT and IFF_BONDING haven't yet been set before
>dev_open() when adding the port.
>
>If we move IFF_TEAM_PORT setting ahead of dev_open(), it will revert
>the fix in:
>
>commit d7d3c05135f37d8fdf73f9966d27155cada36e56
>Author: Jiri Pirko <jiri@resnulli.us>
>Date:   Mon Aug 25 21:38:27 2014 +0200
>
>    team: set IFF_TEAM_PORT priv_flag after rx_handler is registered
>
>Can we keep IFF_SLAVE here only for no ipv6 addrconf?

So, shouldn't it be rather a new flag specifically for this purpose?


>or do you see a better solution?
>
>Thanks.

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Xin Long]

On Wed, Dec 7, 2022 at 8:31 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, Dec 06, 2022 at 10:52:33PM CET, lucien.xin@gmail.com wrote:
> >On Tue, Dec 6, 2022 at 8:32 AM Xin Long <lucien.xin@gmail.com> wrote:
> >>
> >> On Tue, Dec 6, 2022 at 3:05 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >> >
> >> > Mon, Dec 05, 2022 at 06:46:05PM CET, lucien.xin@gmail.com wrote:
> >> > >The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
> >> > >for slaves separately from master") is also needed in Team. Otherwise,
> >> > >DAD and RS packets to be sent from the slaves in turn can confuse the
> >> > >switches and cause them to incorrectly update their forwarding tables
> >> > >as Liang noticed in the test with activebackup mode.
> >> > >
> >> > >Note that the patch also sets IFF_MASTER flag for Team dev accordingly
> >> > >while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
> >> > >not really used in Team, it's good to show in 'ip link':
> >> > >
> >> > >  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
> >> > >  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
> >> > >
> >> > >Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
> >> > >Reported-by: LiLiang <liali@redhat.com>
> >> > >Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> >
> >> > Nack. Please don't do this. IFF_MASTER and IFF_SLAVE are historical
> >> > flags used by bonding and eql. Should not be used for other devices.
> >> I see. I was wondering why it was not used in Team at the beginning. :)
> >>
> >> >
> >> > addrconf_addr_gen() should not check IFF_SLAVE. It should use:
> >> > netif_is_lag_port() and netif_is_failover_slave() helpers.
> >Hi Jiri,
> >
> >Sorry, it seems not to work with this.
> >
> >As addrconf_addr_gen() is also called in NETDEV_UP event where
> >IFF_TEAM_PORT and IFF_BONDING haven't yet been set before
> >dev_open() when adding the port.
> >
> >If we move IFF_TEAM_PORT setting ahead of dev_open(), it will revert
> >the fix in:
> >
> >commit d7d3c05135f37d8fdf73f9966d27155cada36e56
> >Author: Jiri Pirko <jiri@resnulli.us>
> >Date:   Mon Aug 25 21:38:27 2014 +0200
> >
> >    team: set IFF_TEAM_PORT priv_flag after rx_handler is registered
> >
> >Can we keep IFF_SLAVE here only for no ipv6 addrconf?
>
> So, shouldn't it be rather a new flag specifically for this purpose?
Maybe IFF_NO_ADDRCONF in dev->priv_flags?

I will give it a try.

Thanks.

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Jiri Pirko]

Thu, Dec 08, 2022 at 12:35:48AM CET, lucien.xin@gmail.com wrote:
>On Wed, Dec 7, 2022 at 8:31 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Tue, Dec 06, 2022 at 10:52:33PM CET, lucien.xin@gmail.com wrote:
>> >On Tue, Dec 6, 2022 at 8:32 AM Xin Long <lucien.xin@gmail.com> wrote:
>> >>
>> >> On Tue, Dec 6, 2022 at 3:05 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >> >
>> >> > Mon, Dec 05, 2022 at 06:46:05PM CET, lucien.xin@gmail.com wrote:
>> >> > >The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
>> >> > >for slaves separately from master") is also needed in Team. Otherwise,
>> >> > >DAD and RS packets to be sent from the slaves in turn can confuse the
>> >> > >switches and cause them to incorrectly update their forwarding tables
>> >> > >as Liang noticed in the test with activebackup mode.
>> >> > >
>> >> > >Note that the patch also sets IFF_MASTER flag for Team dev accordingly
>> >> > >while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
>> >> > >not really used in Team, it's good to show in 'ip link':
>> >> > >
>> >> > >  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
>> >> > >  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
>> >> > >
>> >> > >Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
>> >> > >Reported-by: LiLiang <liali@redhat.com>
>> >> > >Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >> >
>> >> > Nack. Please don't do this. IFF_MASTER and IFF_SLAVE are historical
>> >> > flags used by bonding and eql. Should not be used for other devices.
>> >> I see. I was wondering why it was not used in Team at the beginning. :)
>> >>
>> >> >
>> >> > addrconf_addr_gen() should not check IFF_SLAVE. It should use:
>> >> > netif_is_lag_port() and netif_is_failover_slave() helpers.
>> >Hi Jiri,
>> >
>> >Sorry, it seems not to work with this.
>> >
>> >As addrconf_addr_gen() is also called in NETDEV_UP event where
>> >IFF_TEAM_PORT and IFF_BONDING haven't yet been set before
>> >dev_open() when adding the port.
>> >
>> >If we move IFF_TEAM_PORT setting ahead of dev_open(), it will revert
>> >the fix in:
>> >
>> >commit d7d3c05135f37d8fdf73f9966d27155cada36e56
>> >Author: Jiri Pirko <jiri@resnulli.us>
>> >Date:   Mon Aug 25 21:38:27 2014 +0200
>> >
>> >    team: set IFF_TEAM_PORT priv_flag after rx_handler is registered
>> >
>> >Can we keep IFF_SLAVE here only for no ipv6 addrconf?
>>
>> So, shouldn't it be rather a new flag specifically for this purpose?
>Maybe IFF_NO_ADDRCONF in dev->priv_flags?

Sounds fine to me.


>
>I will give it a try.
>
>Thanks.

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Xin Long]

On Thu, Dec 8, 2022 at 6:19 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Thu, Dec 08, 2022 at 12:35:48AM CET, lucien.xin@gmail.com wrote:
> >On Wed, Dec 7, 2022 at 8:31 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> Tue, Dec 06, 2022 at 10:52:33PM CET, lucien.xin@gmail.com wrote:
> >> >On Tue, Dec 6, 2022 at 8:32 AM Xin Long <lucien.xin@gmail.com> wrote:
> >> >>
> >> >> On Tue, Dec 6, 2022 at 3:05 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >> >> >
> >> >> > Mon, Dec 05, 2022 at 06:46:05PM CET, lucien.xin@gmail.com wrote:
> >> >> > >The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
> >> >> > >for slaves separately from master") is also needed in Team. Otherwise,
> >> >> > >DAD and RS packets to be sent from the slaves in turn can confuse the
> >> >> > >switches and cause them to incorrectly update their forwarding tables
> >> >> > >as Liang noticed in the test with activebackup mode.
> >> >> > >
> >> >> > >Note that the patch also sets IFF_MASTER flag for Team dev accordingly
> >> >> > >while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
> >> >> > >not really used in Team, it's good to show in 'ip link':
> >> >> > >
> >> >> > >  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
> >> >> > >  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
> >> >> > >
> >> >> > >Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
> >> >> > >Reported-by: LiLiang <liali@redhat.com>
> >> >> > >Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> >> >
> >> >> > Nack. Please don't do this. IFF_MASTER and IFF_SLAVE are historical
> >> >> > flags used by bonding and eql. Should not be used for other devices.
> >> >> I see. I was wondering why it was not used in Team at the beginning. :)
> >> >>
> >> >> >
> >> >> > addrconf_addr_gen() should not check IFF_SLAVE. It should use:
> >> >> > netif_is_lag_port() and netif_is_failover_slave() helpers.
> >> >Hi Jiri,
> >> >
> >> >Sorry, it seems not to work with this.
> >> >
> >> >As addrconf_addr_gen() is also called in NETDEV_UP event where
> >> >IFF_TEAM_PORT and IFF_BONDING haven't yet been set before
> >> >dev_open() when adding the port.
> >> >
> >> >If we move IFF_TEAM_PORT setting ahead of dev_open(), it will revert
> >> >the fix in:
> >> >
> >> >commit d7d3c05135f37d8fdf73f9966d27155cada36e56
> >> >Author: Jiri Pirko <jiri@resnulli.us>
> >> >Date:   Mon Aug 25 21:38:27 2014 +0200
> >> >
> >> >    team: set IFF_TEAM_PORT priv_flag after rx_handler is registered
> >> >
> >> >Can we keep IFF_SLAVE here only for no ipv6 addrconf?
> >>
> >> So, shouldn't it be rather a new flag specifically for this purpose?
> >Maybe IFF_NO_ADDRCONF in dev->priv_flags?
>
> Sounds fine to me.
BTW, IFF_LIVE_RENAME_OK flag was just deleted in net-next.git by:

commit bd039b5ea2a91ea707ee8539df26456bd5be80af
Author: Andy Ren <andy.ren@getcruise.com>
Date:   Mon Nov 7 09:42:42 2022 -0800

    net/core: Allow live renaming when an interface is up

do you think it is okay to use that vacance and define:

IFF_NO_ADDRCONF = BIT_ULL(30)

in netdev_priv_flags ?

Thanks.

>
>
> >
> >I will give it a try.
> >
> >Thanks.

Re: [PATCH net] team: prevent ipv6 link local address on port devices

[Jiri Pirko]

Thu, Dec 08, 2022 at 06:07:17PM CET, lucien.xin@gmail.com wrote:
>On Thu, Dec 8, 2022 at 6:19 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Thu, Dec 08, 2022 at 12:35:48AM CET, lucien.xin@gmail.com wrote:
>> >On Wed, Dec 7, 2022 at 8:31 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >>
>> >> Tue, Dec 06, 2022 at 10:52:33PM CET, lucien.xin@gmail.com wrote:
>> >> >On Tue, Dec 6, 2022 at 8:32 AM Xin Long <lucien.xin@gmail.com> wrote:
>> >> >>
>> >> >> On Tue, Dec 6, 2022 at 3:05 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >> >> >
>> >> >> > Mon, Dec 05, 2022 at 06:46:05PM CET, lucien.xin@gmail.com wrote:
>> >> >> > >The similar fix from commit c2edacf80e15 ("bonding / ipv6: no addrconf
>> >> >> > >for slaves separately from master") is also needed in Team. Otherwise,
>> >> >> > >DAD and RS packets to be sent from the slaves in turn can confuse the
>> >> >> > >switches and cause them to incorrectly update their forwarding tables
>> >> >> > >as Liang noticed in the test with activebackup mode.
>> >> >> > >
>> >> >> > >Note that the patch also sets IFF_MASTER flag for Team dev accordingly
>> >> >> > >while IFF_SLAVE flag is set for port devs. Although IFF_MASTER flag is
>> >> >> > >not really used in Team, it's good to show in 'ip link':
>> >> >> > >
>> >> >> > >  eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP>
>> >> >> > >  team0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP>
>> >> >> > >
>> >> >> > >Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
>> >> >> > >Reported-by: LiLiang <liali@redhat.com>
>> >> >> > >Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >> >> >
>> >> >> > Nack. Please don't do this. IFF_MASTER and IFF_SLAVE are historical
>> >> >> > flags used by bonding and eql. Should not be used for other devices.
>> >> >> I see. I was wondering why it was not used in Team at the beginning. :)
>> >> >>
>> >> >> >
>> >> >> > addrconf_addr_gen() should not check IFF_SLAVE. It should use:
>> >> >> > netif_is_lag_port() and netif_is_failover_slave() helpers.
>> >> >Hi Jiri,
>> >> >
>> >> >Sorry, it seems not to work with this.
>> >> >
>> >> >As addrconf_addr_gen() is also called in NETDEV_UP event where
>> >> >IFF_TEAM_PORT and IFF_BONDING haven't yet been set before
>> >> >dev_open() when adding the port.
>> >> >
>> >> >If we move IFF_TEAM_PORT setting ahead of dev_open(), it will revert
>> >> >the fix in:
>> >> >
>> >> >commit d7d3c05135f37d8fdf73f9966d27155cada36e56
>> >> >Author: Jiri Pirko <jiri@resnulli.us>
>> >> >Date:   Mon Aug 25 21:38:27 2014 +0200
>> >> >
>> >> >    team: set IFF_TEAM_PORT priv_flag after rx_handler is registered
>> >> >
>> >> >Can we keep IFF_SLAVE here only for no ipv6 addrconf?
>> >>
>> >> So, shouldn't it be rather a new flag specifically for this purpose?
>> >Maybe IFF_NO_ADDRCONF in dev->priv_flags?
>>
>> Sounds fine to me.
>BTW, IFF_LIVE_RENAME_OK flag was just deleted in net-next.git by:
>
>commit bd039b5ea2a91ea707ee8539df26456bd5be80af
>Author: Andy Ren <andy.ren@getcruise.com>
>Date:   Mon Nov 7 09:42:42 2022 -0800
>
>    net/core: Allow live renaming when an interface is up
>
>do you think it is okay to use that vacance and define:
>
>IFF_NO_ADDRCONF = BIT_ULL(30)
>
>in netdev_priv_flags ?

It's a private define, no UAPI, I don't see why not. Let's make the
backporter live a bit harder :)

>
>Thanks.
>
>>
>>
>> >
>> >I will give it a try.
>> >
>> >Thanks.