* [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).