linux-hyperv.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hv_netvsc: set device master/slave flags on bonding
@ 2025-02-28 22:25 longli
  2025-03-03 11:40 ` Jiri Pirko
  0 siblings, 1 reply; 4+ messages in thread
From: longli @ 2025-02-28 22:25 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shradha Gupta, Simon Horman, Konstantin Taranov,
	Souradeep Chakrabarti, Erick Archer, linux-hyperv, netdev,
	linux-kernel, linux-rdma
  Cc: Long Li

From: Long Li <longli@microsoft.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.

Signed-off-by: Long Li <longli@microsoft.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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] hv_netvsc: set device master/slave flags on bonding
  2025-02-28 22:25 [PATCH] hv_netvsc: set device master/slave flags on bonding longli
@ 2025-03-03 11:40 ` Jiri Pirko
  2025-03-03 18:47   ` [EXTERNAL] " Long Li
  0 siblings, 1 reply; 4+ messages in thread
From: Jiri Pirko @ 2025-03-03 11:40 UTC (permalink / raw)
  To: longli
  Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shradha Gupta, Simon Horman, Konstantin Taranov,
	Souradeep Chakrabarti, Erick Archer, linux-hyperv, netdev,
	linux-kernel, linux-rdma, Long Li

Fri, Feb 28, 2025 at 11:25:13PM +0100, longli@linuxonhyperv.com wrote:
>From: Long Li <longli@microsoft.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?


>
>Signed-off-by: Long Li <longli@microsoft.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
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [EXTERNAL] Re: [PATCH] hv_netvsc: set device master/slave flags on bonding
  2025-03-03 11:40 ` Jiri Pirko
@ 2025-03-03 18:47   ` Long Li
  2025-03-05 11:34     ` Jiri Pirko
  0 siblings, 1 reply; 4+ messages in thread
From: Long Li @ 2025-03-03 18:47 UTC (permalink / raw)
  To: Jiri Pirko, longli@linuxonhyperv.com
  Cc: KY Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shradha Gupta, Simon Horman, Konstantin Taranov,
	Souradeep Chakrabarti, Erick Archer, linux-hyperv@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-rdma@vger.kernel.org

> Subject: [EXTERNAL] Re: [PATCH] hv_netvsc: set device master/slave flags on
> bonding
> 
> Fri, Feb 28, 2025 at 11:25:13PM +0100, longli@linuxonhyperv.com wrote:
> >From: Long Li <longli@microsoft.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
 and is also checked in some user-mode programs.

There is no code checking for MASTER currently. It is added for completeness. SLAVE doesn't make sense without a MASTER.

> 
> 
> >
> >Signed-off-by: Long Li <longli@microsoft.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
> >
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [EXTERNAL] Re: [PATCH] hv_netvsc: set device master/slave flags on bonding
  2025-03-03 18:47   ` [EXTERNAL] " Long Li
@ 2025-03-05 11:34     ` Jiri Pirko
  0 siblings, 0 replies; 4+ messages in thread
From: Jiri Pirko @ 2025-03-05 11:34 UTC (permalink / raw)
  To: Long Li
  Cc: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shradha Gupta, Simon Horman, Konstantin Taranov,
	Souradeep Chakrabarti, Erick Archer, linux-hyperv@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-rdma@vger.kernel.org

Mon, Mar 03, 2025 at 07:47:37PM +0100, longli@microsoft.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@linuxonhyperv.com wrote:
>> >From: Long Li <longli@microsoft.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@microsoft.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
>> >
>> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-03-05 11:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-28 22:25 [PATCH] hv_netvsc: set device master/slave flags on bonding longli
2025-03-03 11:40 ` Jiri Pirko
2025-03-03 18:47   ` [EXTERNAL] " Long Li
2025-03-05 11:34     ` Jiri Pirko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).