Chas Williams <3ch...@gmail.com> wrote:
>netif_is_lag_port should be used to identify link aggregation ports.
>For this to work, we need to reorganize the bonding and team drivers
>so that the necessary flags are set before dev_open is called.
>
>commit 31e77c93e432 ("sched/fair: Update blocked load when newly idle")
>made this decision originally based on the IFF_SLAVE flag which isn't
>used by the team driver. Note, we do need to retain the IFF_SLAVE
>check for the eql driver.
Is 31e77c93e432 the correct commit reference? I don't see
anything in there about IFF_SLAVE or bonding; it's a patch to the
process scheduler.
And, as Jiri said, the subject doesn't mention bonding.
>Signed-off-by: Chas Williams <3ch...@gmail.com>
>---
> drivers/net/bonding/bond_main.c | 4 ++--
> drivers/net/team/team.c | 7 +++++--
> net/ipv6/addrconf.c | 2 +-
> 3 files changed, 8 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index ffa37adb7681..5cdad164332b 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1536,6 +1536,7 @@ int bond_enslave(struct net_device *bond_dev, struct
>net_device *slave_dev,
>
> /* set slave flag before open to prevent IPv6 addrconf */
> slave_dev->flags |= IFF_SLAVE;
>+ slave_dev->priv_flags |= IFF_BONDING;
>
> /* open the slave since the application closed it */
> res = dev_open(slave_dev);
>@@ -1544,7 +1545,6 @@ int bond_enslave(struct net_device *bond_dev, struct
>net_device *slave_dev,
> goto err_restore_mac;
> }
>
>- slave_dev->priv_flags |= IFF_BONDING;
> /* initialize slave stats */
> dev_get_stats(new_slave->dev, &new_slave->slave_stats);
>
>@@ -1804,10 +1804,10 @@ int bond_enslave(struct net_device *bond_dev, struct
>net_device *slave_dev,
> slave_disable_netpoll(new_slave);
>
> err_close:
>- slave_dev->priv_flags &= ~IFF_BONDING;
> dev_close(slave_dev);
>
> err_restore_mac:
>+ slave_dev->priv_flags &= ~IFF_BONDING;
> slave_dev->flags &= ~IFF_SLAVE;
> if (!bond->params.fail_over_mac ||
> BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) {
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index db633ae9f784..8fc7d57e9f6d 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -1128,14 +1128,12 @@ static int team_upper_dev_link(struct team *team,
>struct team_port *port,
> &lag_upper_info, extack);
> if (err)
> return err;
>- port->dev->priv_flags |= IFF_TEAM_PORT;
> return 0;
> }
>
> 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;
> }
>
> static void __team_port_change_port_added(struct team_port *port, bool
> linkup);
>@@ -1214,6 +1212,9 @@ static int team_port_add(struct team *team, struct
>net_device *port_dev,
> goto err_port_enter;
> }
>
>+ /* set slave flag before open to prevent IPv6 addrconf */
>+ port->dev->priv_flags |= IFF_TEAM_PORT;
>+
> err = dev_open(port_dev);
> if (err) {
> netdev_dbg(dev, "Device %s opening failed\n",
>@@ -1292,6 +1293,7 @@ static int team_port_add(struct team *team, struct
>net_device *port_dev,
> dev_close(port_dev);
>
> err_dev_open:
>+ port->dev->priv_flags &= ~IFF_TEAM_PORT;
> team_port_leave(team, port);
> team_port_set_orig_dev_addr(port);
>
>@@ -1328,6 +1330,7 @@ static int team_port_del(struct team *team, struct
>net_device *port_dev)
> dev_uc_unsync(port_dev, dev);
> dev_mc_unsync(port_dev, dev);
> dev_close(port_dev);
>+ port->dev->priv_flags &= ~IFF_TEAM_PORT;
> team_port_leave(team, port);
>
> __team_option_inst_mark_removed_port(team, port);
>diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>index 45b84dd5c4eb..121f863022ed 100644
>--- a/net/ipv6/addrconf.c
>+++ b/net/ipv6/addrconf.c
>@@ -3482,7 +3482,7 @@ static int addrconf_notify(struct notifier_block *this,
>unsigned long event,
>
> case NETDEV_UP:
> case NETDEV_CHANGE:
>- if (dev->flags & IFF_SLAVE)
>+ if (netif_is_lag_port(dev) || dev->flags & IFF_SLAVE)
Note that netvsc_vf_join() also uses IFF_SLAVE in order skip
IPv6 addrconf for netvsc devices; I don't believe its usage will pass
netif_is_lag_port(). It looks like the above will work, but your commit
message mentions eql as the reason for retaining the IFF_SLAVE test, and
eql isn't the only user of IFF_SLAVE in this manner.
-J
> break;
>
> if (idev && idev->cnf.disable_ipv6)
>--
>2.14.4
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
