linux-kernel - Re: [EXTERNAL] Re: [PATCH] hv_netvsc: set device master/slave flags on bonding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7z2vlhnu2seaujnuvtpjllbj4pnc5aqn5pwfujvzsrzc7emllb@ce2v22r3ombt>
Date: Wed, 5 Mar 2025 12:34:27 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Long Li <longli@...rosoft.com>
Cc: "longli@...uxonhyperv.com" <longli@...uxonhyperv.com>, 
	KY Srinivasan <kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, 
	Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Shradha Gupta <shradhagupta@...ux.microsoft.com>, Simon Horman <horms@...nel.org>, 
	Konstantin Taranov <kotaranov@...rosoft.com>, Souradeep Chakrabarti <schakrabarti@...ux.microsoft.com>, 
	Erick Archer <erick.archer@...look.com>, "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
Subject: Re: [EXTERNAL] Re: [PATCH] hv_netvsc: set device master/slave flags
 on bonding

Mon, Mar 03, 2025 at 07:47:37PM +0100, longli@...rosoft.com wrote:
>> Subject: [EXTERNAL] Re: [PATCH] hv_netvsc: set device master/slave flags on
>> bonding
>> 
>> Fri, Feb 28, 2025 at 11:25:13PM +0100, longli@...uxonhyperv.com wrote:
>> >From: Long Li <longli@...rosoft.com>
>> >
>> >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@...rosoft.com>
>> >---
>> > 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
>> >
>> >