From: Long Li <longli@microsoft.com>

On Hyper-V platforms, a slave VF netdev always bonds to Netvsc and remains
as Netvsc's only active slave as long as the slave device is present. This
behavior is not user-configurable.

Other kernel APIs (e.g those in "include/linux/netdevice.h") check for
IFF_MASTER, IFF_SLAVE and IFF_BONDING for determing if those are used
in a master/slave bonded setup. RDMA uses those APIs extensively when
looking for master/slave devices. Netvsc's bonding setup with its slave
device falls into this category.

Make hv_netvsc properly indicate bonding with its slave and change the
API to reflect this bonding setup.

Signed-off-by: Long Li <longli@microsoft.com>
---

Change log
v2: instead of re-using IFF_BOND, introduce permanent_bond in netdev

 drivers/net/hyperv/netvsc_drv.c | 12 ++++++++++++
 include/linux/netdevice.h       |  8 ++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index d6c4abfc3a28..7867f8e45f86 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2204,6 +2204,10 @@ static int netvsc_vf_join(struct net_device *vf_netdev,
 		goto rx_handler_failed;
 	}
 
+	vf_netdev->permanent_bond = 1;
+	ndev->permanent_bond = 1;
+	ndev->flags |= IFF_MASTER;
+
 	ret = netdev_master_upper_dev_link(vf_netdev, ndev,
 					   NULL, NULL, NULL);
 	if (ret != 0) {
@@ -2484,7 +2488,15 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
 
 	reinit_completion(&net_device_ctx->vf_add);
 	netdev_rx_handler_unregister(vf_netdev);
+
+	/* Unlink the slave device and clear flag */
+	vf_netdev->permanent_bond = 0;
+	ndev->permanent_bond = 0;
+	vf_netdev->flags &= ~IFF_SLAVE;
+	ndev->flags &= ~IFF_MASTER;
+
 	netdev_upper_dev_unlink(vf_netdev, ndev);
+
 	RCU_INIT_POINTER(net_device_ctx->vf_netdev, NULL);
 	dev_put(vf_netdev);
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ecc686409161..4531f45d3e83 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1997,6 +1997,7 @@ enum netdev_reg_state {
  *	@change_proto_down: device supports setting carrier via IFLA_PROTO_DOWN
  *	@netns_local: interface can't change network namespaces
  *	@fcoe_mtu:	device supports maximum FCoE MTU, 2158 bytes
+ *	@permanent_bond: device is permanently bonded to another device
  *
  *	@net_notifier_list:	List of per-net netdev notifier block
  *				that follow this device when it is moved
@@ -2402,6 +2403,7 @@ struct net_device {
 	unsigned long		change_proto_down:1;
 	unsigned long		netns_local:1;
 	unsigned long		fcoe_mtu:1;
+	unsigned long		permanent_bond:1;
 
 	struct list_head	net_notifier_list;
 
@@ -5150,12 +5152,14 @@ static inline bool netif_is_macvlan_port(const struct net_device *dev)
 
 static inline bool netif_is_bond_master(const struct net_device *dev)
 {
-	return dev->flags & IFF_MASTER && dev->priv_flags & IFF_BONDING;
+	return dev->flags & IFF_MASTER &&
+	       (dev->priv_flags & IFF_BONDING || dev->permanent_bond);
 }
 
 static inline bool netif_is_bond_slave(const struct net_device *dev)
 {
-	return dev->flags & IFF_SLAVE && dev->priv_flags & IFF_BONDING;
+	return dev->flags & IFF_SLAVE &&
+	       (dev->priv_flags & IFF_BONDING || dev->permanent_bond);
 }
 
 static inline bool netif_supports_nofcs(struct net_device *dev)
-- 
2.34.1

On Fri, 13 Dec 2024 12:06:01 -0800 longli@linuxonhyperv.com wrote:
> Other kernel APIs (e.g those in "include/linux/netdevice.h") check for
> IFF_MASTER, IFF_SLAVE and IFF_BONDING for determing if those are used
> in a master/slave bonded setup. RDMA uses those APIs extensively when
> looking for master/slave devices. Netvsc's bonding setup with its slave
> device falls into this category.
> 
> Make hv_netvsc properly indicate bonding with its slave and change the
> API to reflect this bonding setup.

This is severely lacking in terms of safety analysis.

> @@ -2204,6 +2204,10 @@ static int netvsc_vf_join(struct net_device *vf_netdev,
>  		goto rx_handler_failed;
>  	}
>  
> +	vf_netdev->permanent_bond = 1;
> +	ndev->permanent_bond = 1;
> +	ndev->flags |= IFF_MASTER;

> @@ -2484,7 +2488,15 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
>  
>  	reinit_completion(&net_device_ctx->vf_add);
>  	netdev_rx_handler_unregister(vf_netdev);
> +
> +	/* Unlink the slave device and clear flag */
> +	vf_netdev->permanent_bond = 0;
> +	ndev->permanent_bond = 0;

> + *	@permanent_bond: device is permanently bonded to another device

I think we have been taught a definition of the word "permanent"

On Mon, 16 Dec 2024 18:03:00 -0800 Jakub Kicinski wrote:
> > + *	@permanent_bond: device is permanently bonded to another device  
> 
> I think we have been taught a definition of the word "permanent"
                               ^ 
                               different