* RE: [PATCH net] net: mana: Support holes in device list reply msg
2025-03-05 21:46 [PATCH net] net: mana: Support holes in device list reply msg Haiyang Zhang
@ 2025-03-05 23:25 ` Long Li
2025-03-07 10:29 ` Shradha Gupta
2025-03-08 3:50 ` Jakub Kicinski
2 siblings, 0 replies; 6+ messages in thread
From: Long Li @ 2025-03-05 23:25 UTC (permalink / raw)
To: Haiyang Zhang, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org
Cc: Dexuan Cui, stephen@networkplumber.org, KY Srinivasan,
Paul Rosswurm, olaf@aepfle.de, vkuznets@redhat.com,
davem@davemloft.net, wei.liu@kernel.org, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, leon@kernel.org,
ssengar@linux.microsoft.com, linux-rdma@vger.kernel.org,
daniel@iogearbox.net, john.fastabend@gmail.com,
bpf@vger.kernel.org, ast@kernel.org, hawk@kernel.org,
tglx@linutronix.de, shradhagupta@linux.microsoft.com,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
> Subject: [PATCH net] net: mana: Support holes in device list reply msg
>
> According to GDMA protocol, holes (zeros) are allowed at the beginning or middle
> of the gdma_list_devices_resp message. The existing code cannot properly
> handle this, and may miss some devices in the list.
>
> To fix, scan the entire list until the num_of_devs are found, or until the end of the
> list.
>
> Cc: stable@vger.kernel.org
> Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network
> Adapter (MANA)")
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
>
> ---
> drivers/net/ethernet/microsoft/mana/gdma_main.c | 16 ++++++++++++----
> include/net/mana/gdma.h | 11 +++++++----
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index c15a5ef4674e..df3ab31974b1 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -134,9 +134,10 @@ static int mana_gd_detect_devices(struct pci_dev
> *pdev)
> struct gdma_list_devices_resp resp = {};
> struct gdma_general_req req = {};
> struct gdma_dev_id dev;
> - u32 i, max_num_devs;
> + int found_dev = 0;
> u16 dev_type;
> int err;
> + u32 i;
>
> mana_gd_init_req_hdr(&req.hdr, GDMA_LIST_DEVICES, sizeof(req),
> sizeof(resp));
> @@ -148,12 +149,19 @@ static int mana_gd_detect_devices(struct pci_dev
> *pdev)
> return err ? err : -EPROTO;
> }
>
> - max_num_devs = min_t(u32, MAX_NUM_GDMA_DEVICES,
> resp.num_of_devs);
> -
> - for (i = 0; i < max_num_devs; i++) {
> + for (i = 0; i < GDMA_DEV_LIST_SIZE &&
> + found_dev < resp.num_of_devs; i++) {
> dev = resp.devs[i];
> dev_type = dev.type;
>
> + /* Skip empty devices */
> + if (dev.as_uint32 == 0)
> + continue;
> +
> + found_dev++;
> + dev_info(gc->dev, "Got devidx:%u, type:%u, instance:%u\n", i,
> + dev.type, dev.instance);
> +
> /* HWC is already detected in mana_hwc_create_channel(). */
> if (dev_type == GDMA_DEVICE_HWC)
> continue;
> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h index
> 90f56656b572..62e9d7673862 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -408,8 +408,6 @@ struct gdma_context {
> struct gdma_dev mana_ib;
> };
>
> -#define MAX_NUM_GDMA_DEVICES 4
> -
> static inline bool mana_gd_is_mana(struct gdma_dev *gd) {
> return gd->dev_id.type == GDMA_DEVICE_MANA; @@ -556,11 +554,15
> @@ enum { #define GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG
> BIT(3) #define
> GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT BIT(5)
>
> +/* Driver can handle holes (zeros) in the device list */ #define
> +GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP BIT(11)
> +
> #define GDMA_DRV_CAP_FLAGS1 \
> (GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
> GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
> GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG | \
> - GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT)
> + GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT |
> \
> + GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP)
>
> #define GDMA_DRV_CAP_FLAGS2 0
>
> @@ -621,11 +623,12 @@ struct gdma_query_max_resources_resp { }; /* HW
> DATA */
>
> /* GDMA_LIST_DEVICES */
> +#define GDMA_DEV_LIST_SIZE 64
> struct gdma_list_devices_resp {
> struct gdma_resp_hdr hdr;
> u32 num_of_devs;
> u32 reserved;
> - struct gdma_dev_id devs[64];
> + struct gdma_dev_id devs[GDMA_DEV_LIST_SIZE];
> }; /* HW DATA */
>
> /* GDMA_REGISTER_DEVICE */
> --
> 2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net: mana: Support holes in device list reply msg
2025-03-05 21:46 [PATCH net] net: mana: Support holes in device list reply msg Haiyang Zhang
2025-03-05 23:25 ` Long Li
@ 2025-03-07 10:29 ` Shradha Gupta
2025-03-08 3:50 ` Jakub Kicinski
2 siblings, 0 replies; 6+ messages in thread
From: Shradha Gupta @ 2025-03-07 10:29 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-hyperv, netdev, decui, stephen, kys, paulros, olaf,
vkuznets, davem, wei.liu, edumazet, kuba, pabeni, leon, longli,
ssengar, linux-rdma, daniel, john.fastabend, bpf, ast, hawk, tglx,
linux-kernel, stable
On Wed, Mar 05, 2025 at 01:46:21PM -0800, Haiyang Zhang wrote:
> According to GDMA protocol, holes (zeros) are allowed at the beginning
> or middle of the gdma_list_devices_resp message. The existing code
> cannot properly handle this, and may miss some devices in the list.
>
> To fix, scan the entire list until the num_of_devs are found, or until
> the end of the list.
>
> Cc: stable@vger.kernel.org
> Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
>
> ---
> drivers/net/ethernet/microsoft/mana/gdma_main.c | 16 ++++++++++++----
> include/net/mana/gdma.h | 11 +++++++----
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index c15a5ef4674e..df3ab31974b1 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -134,9 +134,10 @@ static int mana_gd_detect_devices(struct pci_dev *pdev)
> struct gdma_list_devices_resp resp = {};
> struct gdma_general_req req = {};
> struct gdma_dev_id dev;
> - u32 i, max_num_devs;
> + int found_dev = 0;
> u16 dev_type;
> int err;
> + u32 i;
>
> mana_gd_init_req_hdr(&req.hdr, GDMA_LIST_DEVICES, sizeof(req),
> sizeof(resp));
> @@ -148,12 +149,19 @@ static int mana_gd_detect_devices(struct pci_dev *pdev)
> return err ? err : -EPROTO;
> }
>
> - max_num_devs = min_t(u32, MAX_NUM_GDMA_DEVICES, resp.num_of_devs);
> -
> - for (i = 0; i < max_num_devs; i++) {
> + for (i = 0; i < GDMA_DEV_LIST_SIZE &&
> + found_dev < resp.num_of_devs; i++) {
> dev = resp.devs[i];
> dev_type = dev.type;
>
> + /* Skip empty devices */
> + if (dev.as_uint32 == 0)
> + continue;
> +
> + found_dev++;
> + dev_info(gc->dev, "Got devidx:%u, type:%u, instance:%u\n", i,
> + dev.type, dev.instance);
> +
> /* HWC is already detected in mana_hwc_create_channel(). */
> if (dev_type == GDMA_DEVICE_HWC)
> continue;
> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> index 90f56656b572..62e9d7673862 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -408,8 +408,6 @@ struct gdma_context {
> struct gdma_dev mana_ib;
> };
>
> -#define MAX_NUM_GDMA_DEVICES 4
> -
> static inline bool mana_gd_is_mana(struct gdma_dev *gd)
> {
> return gd->dev_id.type == GDMA_DEVICE_MANA;
> @@ -556,11 +554,15 @@ enum {
> #define GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG BIT(3)
> #define GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT BIT(5)
>
> +/* Driver can handle holes (zeros) in the device list */
> +#define GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP BIT(11)
> +
> #define GDMA_DRV_CAP_FLAGS1 \
> (GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
> GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
> GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG | \
> - GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT)
> + GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT | \
> + GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP)
>
> #define GDMA_DRV_CAP_FLAGS2 0
>
> @@ -621,11 +623,12 @@ struct gdma_query_max_resources_resp {
> }; /* HW DATA */
>
> /* GDMA_LIST_DEVICES */
> +#define GDMA_DEV_LIST_SIZE 64
> struct gdma_list_devices_resp {
> struct gdma_resp_hdr hdr;
> u32 num_of_devs;
> u32 reserved;
> - struct gdma_dev_id devs[64];
> + struct gdma_dev_id devs[GDMA_DEV_LIST_SIZE];
> }; /* HW DATA */
>
> /* GDMA_REGISTER_DEVICE */
Reviewed-by: Shradha Gupta <shradhagupta@microsoft.com>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net: mana: Support holes in device list reply msg
2025-03-05 21:46 [PATCH net] net: mana: Support holes in device list reply msg Haiyang Zhang
2025-03-05 23:25 ` Long Li
2025-03-07 10:29 ` Shradha Gupta
@ 2025-03-08 3:50 ` Jakub Kicinski
2025-03-09 22:01 ` [EXTERNAL] " Haiyang Zhang
2 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2025-03-08 3:50 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-hyperv, netdev, decui, stephen, kys, paulros, olaf,
vkuznets, davem, wei.liu, edumazet, pabeni, leon, longli, ssengar,
linux-rdma, daniel, john.fastabend, bpf, ast, hawk, tglx,
shradhagupta, linux-kernel, stable
On Wed, 5 Mar 2025 13:46:21 -0800 Haiyang Zhang wrote:
> - for (i = 0; i < max_num_devs; i++) {
> + for (i = 0; i < GDMA_DEV_LIST_SIZE &&
> + found_dev < resp.num_of_devs; i++) {
unfortunate mis-indent here, it blend with the code.
checkpatch is right that it should be aligned with opening bracket
> dev = resp.devs[i];
> dev_type = dev.type;
>
> + /* Skip empty devices */
> + if (dev.as_uint32 == 0)
> + continue;
> +
> + found_dev++;
> + dev_info(gc->dev, "Got devidx:%u, type:%u, instance:%u\n", i,
> + dev.type, dev.instance);
Are you sure you want to print this info message for each device,
each time it's probed? Seems pretty noisy. We generally recommend
printing about _unusual_ things.
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [EXTERNAL] Re: [PATCH net] net: mana: Support holes in device list reply msg
2025-03-08 3:50 ` Jakub Kicinski
@ 2025-03-09 22:01 ` Haiyang Zhang
2025-03-10 12:11 ` Shradha Gupta
0 siblings, 1 reply; 6+ messages in thread
From: Haiyang Zhang @ 2025-03-09 22:01 UTC (permalink / raw)
To: Jakub Kicinski
Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org, Dexuan Cui,
stephen@networkplumber.org, KY Srinivasan, Paul Rosswurm,
olaf@aepfle.de, vkuznets@redhat.com, davem@davemloft.net,
wei.liu@kernel.org, edumazet@google.com, pabeni@redhat.com,
leon@kernel.org, Long Li, ssengar@linux.microsoft.com,
linux-rdma@vger.kernel.org, daniel@iogearbox.net,
john.fastabend@gmail.com, bpf@vger.kernel.org, ast@kernel.org,
hawk@kernel.org, tglx@linutronix.de,
shradhagupta@linux.microsoft.com, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, March 7, 2025 10:50 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; Dexuan Cui
> <decui@microsoft.com>; stephen@networkplumber.org; KY Srinivasan
> <kys@microsoft.com>; Paul Rosswurm <paulros@microsoft.com>;
> olaf@aepfle.de; vkuznets@redhat.com; davem@davemloft.net;
> wei.liu@kernel.org; edumazet@google.com; pabeni@redhat.com;
> leon@kernel.org; Long Li <longli@microsoft.com>;
> ssengar@linux.microsoft.com; linux-rdma@vger.kernel.org;
> daniel@iogearbox.net; john.fastabend@gmail.com; bpf@vger.kernel.org;
> ast@kernel.org; hawk@kernel.org; tglx@linutronix.de;
> shradhagupta@linux.microsoft.com; linux-kernel@vger.kernel.org;
> stable@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH net] net: mana: Support holes in device
> list reply msg
>
> On Wed, 5 Mar 2025 13:46:21 -0800 Haiyang Zhang wrote:
> > - for (i = 0; i < max_num_devs; i++) {
> > + for (i = 0; i < GDMA_DEV_LIST_SIZE &&
> > + found_dev < resp.num_of_devs; i++) {
>
> unfortunate mis-indent here, it blend with the code.
> checkpatch is right that it should be aligned with opening bracket
Will fix it.
>
> > dev = resp.devs[i];
> > dev_type = dev.type;
> >
> > + /* Skip empty devices */
> > + if (dev.as_uint32 == 0)
> > + continue;
> > +
> > + found_dev++;
> > + dev_info(gc->dev, "Got devidx:%u, type:%u, instance:%u\n", i,
> > + dev.type, dev.instance);
>
> Are you sure you want to print this info message for each device,
> each time it's probed? Seems pretty noisy. We generally recommend
> printing about _unusual_ things.
Ok. I can remove it.
Thanks,
- Haiyang
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [EXTERNAL] Re: [PATCH net] net: mana: Support holes in device list reply msg
2025-03-09 22:01 ` [EXTERNAL] " Haiyang Zhang
@ 2025-03-10 12:11 ` Shradha Gupta
0 siblings, 0 replies; 6+ messages in thread
From: Shradha Gupta @ 2025-03-10 12:11 UTC (permalink / raw)
To: Haiyang Zhang
Cc: Jakub Kicinski, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, Dexuan Cui, stephen@networkplumber.org,
KY Srinivasan, Paul Rosswurm, olaf@aepfle.de, vkuznets@redhat.com,
davem@davemloft.net, wei.liu@kernel.org, edumazet@google.com,
pabeni@redhat.com, leon@kernel.org, Long Li,
ssengar@linux.microsoft.com, linux-rdma@vger.kernel.org,
daniel@iogearbox.net, john.fastabend@gmail.com,
bpf@vger.kernel.org, ast@kernel.org, hawk@kernel.org,
tglx@linutronix.de, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
On Sun, Mar 09, 2025 at 10:01:33PM +0000, Haiyang Zhang wrote:
>
>
> > -----Original Message-----
> > From: Jakub Kicinski <kuba@kernel.org>
> > Sent: Friday, March 7, 2025 10:50 PM
> > To: Haiyang Zhang <haiyangz@microsoft.com>
> > Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; Dexuan Cui
> > <decui@microsoft.com>; stephen@networkplumber.org; KY Srinivasan
> > <kys@microsoft.com>; Paul Rosswurm <paulros@microsoft.com>;
> > olaf@aepfle.de; vkuznets@redhat.com; davem@davemloft.net;
> > wei.liu@kernel.org; edumazet@google.com; pabeni@redhat.com;
> > leon@kernel.org; Long Li <longli@microsoft.com>;
> > ssengar@linux.microsoft.com; linux-rdma@vger.kernel.org;
> > daniel@iogearbox.net; john.fastabend@gmail.com; bpf@vger.kernel.org;
> > ast@kernel.org; hawk@kernel.org; tglx@linutronix.de;
> > shradhagupta@linux.microsoft.com; linux-kernel@vger.kernel.org;
> > stable@vger.kernel.org
> > Subject: [EXTERNAL] Re: [PATCH net] net: mana: Support holes in device
> > list reply msg
> >
> > On Wed, 5 Mar 2025 13:46:21 -0800 Haiyang Zhang wrote:
> > > - for (i = 0; i < max_num_devs; i++) {
> > > + for (i = 0; i < GDMA_DEV_LIST_SIZE &&
> > > + found_dev < resp.num_of_devs; i++) {
> >
> > unfortunate mis-indent here, it blend with the code.
> > checkpatch is right that it should be aligned with opening bracket
> Will fix it.
>
> >
> > > dev = resp.devs[i];
> > > dev_type = dev.type;
> > >
> > > + /* Skip empty devices */
> > > + if (dev.as_uint32 == 0)
> > > + continue;
> > > +
> > > + found_dev++;
> > > + dev_info(gc->dev, "Got devidx:%u, type:%u, instance:%u\n", i,
> > > + dev.type, dev.instance);
> >
> > Are you sure you want to print this info message for each device,
> > each time it's probed? Seems pretty noisy. We generally recommend
> > printing about _unusual_ things.
> Ok. I can remove it.
How about a dev_dbg instead?
>
> Thanks,
> - Haiyang
^ permalink raw reply [flat|nested] 6+ messages in thread