Mon, Mar 03, 2025 at 07:47:37PM +0100, longli@xxxxxxxxxxxxx wrote: >> Subject: [EXTERNAL] Re: [PATCH] hv_netvsc: set device master/slave flags on >> bonding >> >> Fri, Feb 28, 2025 at 11:25:13PM +0100, longli@xxxxxxxxxxxxxxxxx wrote: >> >From: Long Li <longli@xxxxxxxxxxxxx> >> > >> >Currently netvsc only sets the SLAVE flag on VF netdev when it's >> >bonded. It should also set the MASTER flag on itself and clear all >> >those flags when the VF is unbonded. >> >> I don't understand why you need this. Who looks at these flags? > >The SLAVE flag is checked here: >https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/microsoft/mana/mana_en.c?h=v6.14-rc5#n3144 In kernel. We have other means. The check is incorrect. The code should use netif_is_*_port() herlper. Does not exist, add it. > and is also checked in some user-mode programs. As currently it is not exposed, it can't be checked. Don't add it. > >There is no code checking for MASTER currently. It is added for completeness. SLAVE doesn't make sense without a MASTER. Does not make sense. Either you need it or not. If not, don't add it. NAK. Please let IFF_MASTER and IFF_SLAVE rot. > >> >> >> > >> >Signed-off-by: Long Li <longli@xxxxxxxxxxxxx> >> >--- >> > drivers/net/hyperv/netvsc_drv.c | 6 ++++++ >> > 1 file changed, 6 insertions(+) >> > >> >diff --git a/drivers/net/hyperv/netvsc_drv.c >> >b/drivers/net/hyperv/netvsc_drv.c index d6c4abfc3a28..7ac18fede2f3 >> >100644 >> >--- a/drivers/net/hyperv/netvsc_drv.c >> >+++ b/drivers/net/hyperv/netvsc_drv.c >> >@@ -2204,6 +2204,7 @@ static int netvsc_vf_join(struct net_device >> *vf_netdev, >> > goto rx_handler_failed; >> > } >> > >> >+ ndev->flags |= IFF_MASTER; >> > ret = netdev_master_upper_dev_link(vf_netdev, ndev, >> > NULL, NULL, NULL); >> > if (ret != 0) { >> >@@ -2484,7 +2485,12 @@ 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->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); >> > >> >-- >> >2.34.1 >> > >> >
