| Message ID | 63e09531fc47963d2e4eff376653d3db21b97058.1673980932.git.lucien.xin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4fb58ac3368c6058e61194ec047a5503059d90ae |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] Revert "net: team: use IFF_NO_ADDRCONF flag to prevent ipv6 addrconf" | expand |
On 1/17/23 11:42 AM, Xin Long wrote: > This reverts commit 0aa64df30b382fc71d4fb1827d528e0eb3eff854. > > Currently IFF_NO_ADDRCONF is used to prevent all ipv6 addrconf for the > slave ports of team, bonding and failover devices and it means no ipv6 > packets can be sent out through these slave ports. However, for team > device, "nsna_ping" link_watch requires ipv6 addrconf. Otherwise, the > link will be marked failure. This patch removes the IFF_NO_ADDRCONF > flag set for team port, and we will fix the original issue in another > patch, as Jakub suggested. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > drivers/net/team/team.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c > index fcd43d62d86b..d10606f257c4 100644 > --- a/drivers/net/team/team.c > +++ b/drivers/net/team/team.c > @@ -1044,7 +1044,6 @@ static int team_port_enter(struct team *team, struct team_port *port) > goto err_port_enter; > } > } > - port->dev->priv_flags |= IFF_NO_ADDRCONF; > > return 0; > > @@ -1058,7 +1057,6 @@ static void team_port_leave(struct team *team, struct team_port *port) > { > if (team->ops.port_leave) > team->ops.port_leave(team, port); > - port->dev->priv_flags &= ~IFF_NO_ADDRCONF; > dev_put(team->dev); > } > What about the other patches in that set - failover device and bonding?
On Tue, Jan 17, 2023 at 9:36 PM David Ahern <dsahern@gmail.com> wrote: > > On 1/17/23 11:42 AM, Xin Long wrote: > > This reverts commit 0aa64df30b382fc71d4fb1827d528e0eb3eff854. > > > > Currently IFF_NO_ADDRCONF is used to prevent all ipv6 addrconf for the > > slave ports of team, bonding and failover devices and it means no ipv6 > > packets can be sent out through these slave ports. However, for team > > device, "nsna_ping" link_watch requires ipv6 addrconf. Otherwise, the > > link will be marked failure. This patch removes the IFF_NO_ADDRCONF > > flag set for team port, and we will fix the original issue in another > > patch, as Jakub suggested. > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > > --- > > drivers/net/team/team.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c > > index fcd43d62d86b..d10606f257c4 100644 > > --- a/drivers/net/team/team.c > > +++ b/drivers/net/team/team.c > > @@ -1044,7 +1044,6 @@ static int team_port_enter(struct team *team, struct team_port *port) > > goto err_port_enter; > > } > > } > > - port->dev->priv_flags |= IFF_NO_ADDRCONF; > > > > return 0; > > > > @@ -1058,7 +1057,6 @@ static void team_port_leave(struct team *team, struct team_port *port) > > { > > if (team->ops.port_leave) > > team->ops.port_leave(team, port); > > - port->dev->priv_flags &= ~IFF_NO_ADDRCONF; > > dev_put(team->dev); > > } > > > > What about the other patches in that set - failover device and bonding? This only exists in Team, as nsna_ping link watch in userspace libteam requires IPv6 addrconf to send NS packets, see rawv6_sendmsg(): dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl6, final_p); if (IS_ERR(dst)) { err = PTR_ERR(dst); goto out; } It will break here. For bonding, nothing actually was changed as before, it's just using a new flag IFF_NO_ADDRCONF instead of IFF_SLAVE. and the late added nsna_ping link watch (trying to sync with libteam) is in kernel space, and doesn't require dst on the slave dev, see: bond_ns_send_all() -> bond_ns_send(). For failover device, it checks the link when net_failover_start_xmit() directly by: netif_running(dev) && netif_carrier_ok(dev) I don't think a "link watch" in userspace exists, not to mention a nsna_ping. Thanks.
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Tue, 17 Jan 2023 13:42:12 -0500 you wrote: > This reverts commit 0aa64df30b382fc71d4fb1827d528e0eb3eff854. > > Currently IFF_NO_ADDRCONF is used to prevent all ipv6 addrconf for the > slave ports of team, bonding and failover devices and it means no ipv6 > packets can be sent out through these slave ports. However, for team > device, "nsna_ping" link_watch requires ipv6 addrconf. Otherwise, the > link will be marked failure. This patch removes the IFF_NO_ADDRCONF > flag set for team port, and we will fix the original issue in another > patch, as Jakub suggested. > > [...] Here is the summary with links: - [net] Revert "net: team: use IFF_NO_ADDRCONF flag to prevent ipv6 addrconf" https://git.kernel.org/netdev/net/c/4fb58ac3368c You are awesome, thank you!
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c index fcd43d62d86b..d10606f257c4 100644 --- a/drivers/net/team/team.c +++ b/drivers/net/team/team.c @@ -1044,7 +1044,6 @@ static int team_port_enter(struct team *team, struct team_port *port) goto err_port_enter; } } - port->dev->priv_flags |= IFF_NO_ADDRCONF; return 0; @@ -1058,7 +1057,6 @@ static void team_port_leave(struct team *team, struct team_port *port) { if (team->ops.port_leave) team->ops.port_leave(team, port); - port->dev->priv_flags &= ~IFF_NO_ADDRCONF; dev_put(team->dev); }
This reverts commit 0aa64df30b382fc71d4fb1827d528e0eb3eff854. Currently IFF_NO_ADDRCONF is used to prevent all ipv6 addrconf for the slave ports of team, bonding and failover devices and it means no ipv6 packets can be sent out through these slave ports. However, for team device, "nsna_ping" link_watch requires ipv6 addrconf. Otherwise, the link will be marked failure. This patch removes the IFF_NO_ADDRCONF flag set for team port, and we will fix the original issue in another patch, as Jakub suggested. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- drivers/net/team/team.c | 2 -- 1 file changed, 2 deletions(-)