netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings
@ 2023-07-27 19:07 Jakub Kicinski
  2023-07-27 19:07 ` [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy() Jakub Kicinski
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Jakub Kicinski @ 2023-07-27 19:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, michael.chan, Jakub Kicinski

Fix a couple of build warnings.

Jakub Kicinski (2):
  eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
  eth: bnxt: fix warning for define in struct_group

 drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.41.0


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

* [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
  2023-07-27 19:07 [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings Jakub Kicinski
@ 2023-07-27 19:07 ` Jakub Kicinski
  2023-08-03 13:08   ` Gustavo A. R. Silva
  2023-07-27 19:07 ` [PATCH net-next 2/2] eth: bnxt: fix warning for define in struct_group Jakub Kicinski
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2023-07-27 19:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, michael.chan, Jakub Kicinski

Fix a W=1 warning with gcc 13.1:

In function ‘fortify_memcpy_chk’,
    inlined from ‘bnxt_hwrm_queue_cos2bw_cfg’ at drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:133:3:
include/linux/fortify-string.h:592:25: warning: call to ‘__read_overflow2_field’ declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning]
  592 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The field group is already defined and starts at queue_id:

struct bnxt_cos2bw_cfg {
	u8			pad[3];
	struct_group_attr(cfg, __packed,
		u8		queue_id;
		__le32		min_bw;

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: michael.chan@broadcom.com

Michael, the other warning looks more.. interesting.
The code does:

	data = &resp->queue_id0 + offsetof(struct bnxt_cos2bw_cfg, queue_id);

which translates to: data = &resp->queue_id0 + 3, but the HWRM struct
says:

struct hwrm_queue_cos2bw_qcfg_output {
	__le16	error_code;
	__le16	req_type;
	__le16	seq_id;
	__le16	resp_len;
	u8	queue_id0;
	u8	unused_0;
	__le16	unused_1;
	__le32	queue_id0_min_bw;

So queue_id0 is in the wrong place?
Why not just move it in the spec?
Simplest fix for the warning would be to assign the 6 fields by value.
But to get the value of queue_id we'd need to read unused_1 AFACT? :o
Could you please fix this somehow? Doing W=1 builds on bnxt is painful.
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
index caab3d626a2a..31f85f3e2364 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
@@ -130,7 +130,7 @@ static int bnxt_hwrm_queue_cos2bw_cfg(struct bnxt *bp, struct ieee_ets *ets,
 					    BW_VALUE_UNIT_PERCENT1_100);
 		}
 		data = &req->unused_0 + qidx * (sizeof(cos2bw) - 4);
-		memcpy(data, &cos2bw.queue_id, sizeof(cos2bw) - 4);
+		memcpy(data, &cos2bw.cfg, sizeof(cos2bw) - 4);
 		if (qidx == 0) {
 			req->queue_id0 = cos2bw.queue_id;
 			req->unused_0 = 0;
-- 
2.41.0


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

* [PATCH net-next 2/2] eth: bnxt: fix warning for define in struct_group
  2023-07-27 19:07 [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings Jakub Kicinski
  2023-07-27 19:07 ` [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy() Jakub Kicinski
@ 2023-07-27 19:07 ` Jakub Kicinski
  2023-07-27 20:28 ` [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings Michael Chan
  2023-07-28 21:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2023-07-27 19:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, michael.chan, Jakub Kicinski

Fix C=1 warning with sparse 0.6.4:

drivers/net/ethernet/broadcom/bnxt/bnxt.c: note: in included file:
drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h:30:1: warning: directive in macro's argument list

Don't put defines in a struct_group().

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: michael.chan@broadcom.com
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h
index 716742522161..5b2a6f678244 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h
@@ -27,11 +27,12 @@ struct bnxt_cos2bw_cfg {
 		u8		queue_id;
 		__le32		min_bw;
 		__le32		max_bw;
-#define BW_VALUE_UNIT_PERCENT1_100		(0x1UL << 29)
 		u8		tsa;
 		u8		pri_lvl;
 		u8		bw_weight;
 	);
+/* for min_bw / max_bw */
+#define BW_VALUE_UNIT_PERCENT1_100		(0x1UL << 29)
 	u8			unused;
 };
 
-- 
2.41.0


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

* Re: [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings
  2023-07-27 19:07 [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings Jakub Kicinski
  2023-07-27 19:07 ` [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy() Jakub Kicinski
  2023-07-27 19:07 ` [PATCH net-next 2/2] eth: bnxt: fix warning for define in struct_group Jakub Kicinski
@ 2023-07-27 20:28 ` Michael Chan
  2023-07-28 21:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 11+ messages in thread
From: Michael Chan @ 2023-07-27 20:28 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

On Thu, Jul 27, 2023 at 12:07 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Fix a couple of build warnings.
>
> Jakub Kicinski (2):
>   eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
>   eth: bnxt: fix warning for define in struct_group
>
>  drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 2 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
For the series,
Reviewed-by: Michael Chan <michael.chan@broadcom.com>

I will look into the other issue that you mentioned.  Thanks.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings
  2023-07-27 19:07 [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings Jakub Kicinski
                   ` (2 preceding siblings ...)
  2023-07-27 20:28 ` [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings Michael Chan
@ 2023-07-28 21:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-07-28 21:00 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni, michael.chan

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 27 Jul 2023 12:07:24 -0700 you wrote:
> Fix a couple of build warnings.
> 
> Jakub Kicinski (2):
>   eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
>   eth: bnxt: fix warning for define in struct_group
> 
>  drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 2 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Here is the summary with links:
  - [net-next,1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
    https://git.kernel.org/netdev/net-next/c/833c4a8105ac
  - [net-next,2/2] eth: bnxt: fix warning for define in struct_group
    https://git.kernel.org/netdev/net-next/c/9f49db62f58e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
  2023-07-27 19:07 ` [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy() Jakub Kicinski
@ 2023-08-03 13:08   ` Gustavo A. R. Silva
  2023-08-03 14:21     ` Jakub Kicinski
  0 siblings, 1 reply; 11+ messages in thread
From: Gustavo A. R. Silva @ 2023-08-03 13:08 UTC (permalink / raw)
  To: Jakub Kicinski, davem; +Cc: netdev, edumazet, pabeni, michael.chan



On 7/27/23 13:07, Jakub Kicinski wrote:
> Fix a W=1 warning with gcc 13.1:
> 
> In function ‘fortify_memcpy_chk’,
>      inlined from ‘bnxt_hwrm_queue_cos2bw_cfg’ at drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:133:3:
> include/linux/fortify-string.h:592:25: warning: call to ‘__read_overflow2_field’ declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning]
>    592 |                         __read_overflow2_field(q_size_field, size);
>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The field group is already defined and starts at queue_id:
> 
> struct bnxt_cos2bw_cfg {
> 	u8			pad[3];
> 	struct_group_attr(cfg, __packed,
> 		u8		queue_id;
> 		__le32		min_bw;
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: michael.chan@broadcom.com
> 
> Michael, the other warning looks more.. interesting.
> The code does:
> 
> 	data = &resp->queue_id0 + offsetof(struct bnxt_cos2bw_cfg, queue_id);
> 
> which translates to: data = &resp->queue_id0 + 3, but the HWRM struct
> says:
> 
> struct hwrm_queue_cos2bw_qcfg_output {
> 	__le16	error_code;
> 	__le16	req_type;
> 	__le16	seq_id;
> 	__le16	resp_len;
> 	u8	queue_id0;
> 	u8	unused_0;
> 	__le16	unused_1;
> 	__le32	queue_id0_min_bw;
> 
> So queue_id0 is in the wrong place?
> Why not just move it in the spec?
> Simplest fix for the warning would be to assign the 6 fields by value.
> But to get the value of queue_id we'd need to read unused_1 AFACT? :o
> Could you please fix this somehow? Doing W=1 builds on bnxt is painful.

It seems I just ran into the same issue you're talking about here. See:

In function 'fortify_memcpy_chk',
     inlined from 'bnxt_hwrm_queue_cos2bw_qcfg' at drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:165:3:
include/linux/fortify-string.h:592:25: warning: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd 
parameter); maybe use struct_group()? [-Wattribute-warning]
   592 |                         __read_overflow2_field(q_size_field, size);
       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Here is a potential fix for that:

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
index 31f85f3e2364..e2390d73b3f0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
@@ -144,7 +144,7 @@ static int bnxt_hwrm_queue_cos2bw_qcfg(struct bnxt *bp, struct ieee_ets *ets)
         struct hwrm_queue_cos2bw_qcfg_output *resp;
         struct hwrm_queue_cos2bw_qcfg_input *req;
         struct bnxt_cos2bw_cfg cos2bw;
-       void *data;
+       struct bnxt_cos2bw_cfg *data;
         int rc, i;

         rc = hwrm_req_init(bp, req, HWRM_QUEUE_COS2BW_QCFG);
@@ -158,11 +158,11 @@ static int bnxt_hwrm_queue_cos2bw_qcfg(struct bnxt *bp, struct ieee_ets *ets)
                 return rc;
         }

-       data = &resp->queue_id0 + offsetof(struct bnxt_cos2bw_cfg, queue_id);
+       data = (struct bnxt_cos2bw_cfg *)&resp->queue_id0;
         for (i = 0; i < bp->max_tc; i++, data += sizeof(cos2bw.cfg)) {
                 int tc;

-               memcpy(&cos2bw.cfg, data, sizeof(cos2bw.cfg));
+               memcpy(&cos2bw.cfg, &data->cfg, sizeof(cos2bw.cfg));
                 if (i == 0)
                         cos2bw.queue_id = resp->queue_id0;

Also, 0-day guys just reported[1] to me a -Wstringop-overflow in a similar piece of code
in the same file. See:

drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:133:17: warning: writing 12 bytes into a region of size 1 [-Wstringop-overflow=]

And I think this is a potential solution for that:

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
index 31f85f3e2364..01e0ac246e11 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
@@ -98,7 +98,7 @@ static int bnxt_hwrm_queue_cos2bw_cfg(struct bnxt *bp, struct ieee_ets *ets,
  {
         struct hwrm_queue_cos2bw_cfg_input *req;
         struct bnxt_cos2bw_cfg cos2bw;
-       void *data;
+       struct bnxt_cos2bw_cfg *data;
         int rc, i;

         rc = hwrm_req_init(bp, req, HWRM_QUEUE_COS2BW_CFG);
@@ -129,8 +129,10 @@ static int bnxt_hwrm_queue_cos2bw_cfg(struct bnxt *bp, struct ieee_ets *ets,
                                 cpu_to_le32((ets->tc_tx_bw[i] * 100) |
                                             BW_VALUE_UNIT_PERCENT1_100);
                 }
-               data = &req->unused_0 + qidx * (sizeof(cos2bw) - 4);
-               memcpy(data, &cos2bw.cfg, sizeof(cos2bw) - 4);
+               data = (struct bnxt_cos2bw_cfg *)(&req->unused_0 + qidx *
+                                                 sizeof(cos2bw.cfg) -
+                                                 offsetof(struct bnxt_cos2bw_cfg, cfg));
+               memcpy(&data->cfg, &cos2bw.cfg, sizeof(cos2bw.cfg));
                 if (qidx == 0) {
                         req->queue_id0 = cos2bw.queue_id;
                         req->unused_0 = 0;

If you agree with these changes I can send them as proper patches.

Thanks
--
Gustavo

[1] https://lore.kernel.org/lkml/202308031558.MhRIyeiu-lkp@intel.com/

> ---
>   drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> index caab3d626a2a..31f85f3e2364 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> @@ -130,7 +130,7 @@ static int bnxt_hwrm_queue_cos2bw_cfg(struct bnxt *bp, struct ieee_ets *ets,
>   					    BW_VALUE_UNIT_PERCENT1_100);
>   		}
>   		data = &req->unused_0 + qidx * (sizeof(cos2bw) - 4);
> -		memcpy(data, &cos2bw.queue_id, sizeof(cos2bw) - 4);
> +		memcpy(data, &cos2bw.cfg, sizeof(cos2bw) - 4);
>   		if (qidx == 0) {
>   			req->queue_id0 = cos2bw.queue_id;
>   			req->unused_0 = 0;

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

* Re: [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
  2023-08-03 13:08   ` Gustavo A. R. Silva
@ 2023-08-03 14:21     ` Jakub Kicinski
  2023-08-03 14:46       ` Michael Chan
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2023-08-03 14:21 UTC (permalink / raw)
  To: Gustavo A. R. Silva; +Cc: davem, netdev, edumazet, pabeni, michael.chan

On Thu, 3 Aug 2023 07:08:13 -0600 Gustavo A. R. Silva wrote:
> In function 'fortify_memcpy_chk',
>      inlined from 'bnxt_hwrm_queue_cos2bw_qcfg' at drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:165:3:
> include/linux/fortify-string.h:592:25: warning: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd 
> parameter); maybe use struct_group()? [-Wattribute-warning]
>    592 |                         __read_overflow2_field(q_size_field, size);
>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Here is a potential fix for that:
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> index 31f85f3e2364..e2390d73b3f0 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> @@ -144,7 +144,7 @@ static int bnxt_hwrm_queue_cos2bw_qcfg(struct bnxt *bp, struct ieee_ets *ets)
>          struct hwrm_queue_cos2bw_qcfg_output *resp;
>          struct hwrm_queue_cos2bw_qcfg_input *req;
>          struct bnxt_cos2bw_cfg cos2bw;
> -       void *data;
> +       struct bnxt_cos2bw_cfg *data;
>          int rc, i;
> 
>          rc = hwrm_req_init(bp, req, HWRM_QUEUE_COS2BW_QCFG);
> @@ -158,11 +158,11 @@ static int bnxt_hwrm_queue_cos2bw_qcfg(struct bnxt *bp, struct ieee_ets *ets)
>                  return rc;
>          }
> 
> -       data = &resp->queue_id0 + offsetof(struct bnxt_cos2bw_cfg, queue_id);
> +       data = (struct bnxt_cos2bw_cfg *)&resp->queue_id0;
>          for (i = 0; i < bp->max_tc; i++, data += sizeof(cos2bw.cfg)) {
>                  int tc;
> 
> -               memcpy(&cos2bw.cfg, data, sizeof(cos2bw.cfg));
> +               memcpy(&cos2bw.cfg, &data->cfg, sizeof(cos2bw.cfg));
>                  if (i == 0)
>                          cos2bw.queue_id = resp->queue_id0;

Neat trick, but seems like casting to the destination type should
really be the last resort. There's only a handful of members in this
struct, IMHO assigning member by member is cleaner.
But I'll defer to Michael.

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

* Re: [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
  2023-08-03 14:21     ` Jakub Kicinski
@ 2023-08-03 14:46       ` Michael Chan
  2023-08-03 19:59         ` Gustavo A. R. Silva
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Chan @ 2023-08-03 14:46 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Gustavo A. R. Silva, davem, netdev, edumazet, pabeni

[-- Attachment #1: Type: text/plain, Size: 2520 bytes --]

On Thu, Aug 3, 2023 at 7:21 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 3 Aug 2023 07:08:13 -0600 Gustavo A. R. Silva wrote:
> > In function 'fortify_memcpy_chk',
> >      inlined from 'bnxt_hwrm_queue_cos2bw_qcfg' at drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:165:3:
> > include/linux/fortify-string.h:592:25: warning: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd
> > parameter); maybe use struct_group()? [-Wattribute-warning]
> >    592 |                         __read_overflow2_field(q_size_field, size);
> >        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Here is a potential fix for that:
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> > index 31f85f3e2364..e2390d73b3f0 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> > @@ -144,7 +144,7 @@ static int bnxt_hwrm_queue_cos2bw_qcfg(struct bnxt *bp, struct ieee_ets *ets)
> >          struct hwrm_queue_cos2bw_qcfg_output *resp;
> >          struct hwrm_queue_cos2bw_qcfg_input *req;
> >          struct bnxt_cos2bw_cfg cos2bw;
> > -       void *data;
> > +       struct bnxt_cos2bw_cfg *data;
> >          int rc, i;
> >
> >          rc = hwrm_req_init(bp, req, HWRM_QUEUE_COS2BW_QCFG);
> > @@ -158,11 +158,11 @@ static int bnxt_hwrm_queue_cos2bw_qcfg(struct bnxt *bp, struct ieee_ets *ets)
> >                  return rc;
> >          }
> >
> > -       data = &resp->queue_id0 + offsetof(struct bnxt_cos2bw_cfg, queue_id);
> > +       data = (struct bnxt_cos2bw_cfg *)&resp->queue_id0;
> >          for (i = 0; i < bp->max_tc; i++, data += sizeof(cos2bw.cfg)) {
> >                  int tc;
> >
> > -               memcpy(&cos2bw.cfg, data, sizeof(cos2bw.cfg));
> > +               memcpy(&cos2bw.cfg, &data->cfg, sizeof(cos2bw.cfg));
> >                  if (i == 0)
> >                          cos2bw.queue_id = resp->queue_id0;
>
> Neat trick, but seems like casting to the destination type should
> really be the last resort. There's only a handful of members in this
> struct, IMHO assigning member by member is cleaner.
> But I'll defer to Michael.

The way I plan to fix this is to change the auto-generated struct
hwrm_queue_cos2bw_qcfg_output to have an array of substruct.  I think
that will look the cleanest.  I'll post it later today or tomorrow.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
  2023-08-03 14:46       ` Michael Chan
@ 2023-08-03 19:59         ` Gustavo A. R. Silva
  2023-08-03 20:04           ` Michael Chan
  0 siblings, 1 reply; 11+ messages in thread
From: Gustavo A. R. Silva @ 2023-08-03 19:59 UTC (permalink / raw)
  To: Michael Chan, Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni



On 8/3/23 08:46, Michael Chan wrote:
> 
> The way I plan to fix this is to change the auto-generated struct
> hwrm_queue_cos2bw_qcfg_output to have an array of substruct.  I think
> that will look the cleanest.  I'll post it later today or tomorrow.

Will that also fix the -Wstringop-overflow warning I mentioned in my
previous reply?

drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:133:17: warning: writing 12 bytes into a region of size 1 [-Wstringop-overflow=]

--
Gustavo


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

* Re: [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
  2023-08-03 19:59         ` Gustavo A. R. Silva
@ 2023-08-03 20:04           ` Michael Chan
  2023-08-03 20:11             ` Gustavo A. R. Silva
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Chan @ 2023-08-03 20:04 UTC (permalink / raw)
  To: Gustavo A. R. Silva; +Cc: Jakub Kicinski, davem, netdev, edumazet, pabeni

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

On Thu, Aug 3, 2023 at 12:58 PM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
>
>
> On 8/3/23 08:46, Michael Chan wrote:
> >
> > The way I plan to fix this is to change the auto-generated struct
> > hwrm_queue_cos2bw_qcfg_output to have an array of substruct.  I think
> > that will look the cleanest.  I'll post it later today or tomorrow.
>
> Will that also fix the -Wstringop-overflow warning I mentioned in my
> previous reply?
>
> drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:133:17: warning: writing 12 bytes into a region of size 1 [-Wstringop-overflow=]
>

Yes, I will fix the other similar struct hwrm_queue_cos2bw_cfg_input
in a similar way with an array of substruct.  Thanks.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy()
  2023-08-03 20:04           ` Michael Chan
@ 2023-08-03 20:11             ` Gustavo A. R. Silva
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2023-08-03 20:11 UTC (permalink / raw)
  To: Michael Chan; +Cc: Jakub Kicinski, davem, netdev, edumazet, pabeni



On 8/3/23 14:04, Michael Chan wrote:
> On Thu, Aug 3, 2023 at 12:58 PM Gustavo A. R. Silva
> <gustavo@embeddedor.com> wrote:
>>
>>
>>
>> On 8/3/23 08:46, Michael Chan wrote:
>>>
>>> The way I plan to fix this is to change the auto-generated struct
>>> hwrm_queue_cos2bw_qcfg_output to have an array of substruct.  I think
>>> that will look the cleanest.  I'll post it later today or tomorrow.
>>
>> Will that also fix the -Wstringop-overflow warning I mentioned in my
>> previous reply?
>>
>> drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c:133:17: warning: writing 12 bytes into a region of size 1 [-Wstringop-overflow=]
>>
> 
> Yes, I will fix the other similar struct hwrm_queue_cos2bw_cfg_input
> in a similar way with an array of substruct.  Thanks.

Awesome! :)

Thank you
--
Gustavo

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

end of thread, other threads:[~2023-08-03 23:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-27 19:07 [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings Jakub Kicinski
2023-07-27 19:07 ` [PATCH net-next 1/2] eth: bnxt: fix one of the W=1 warnings about fortified memcpy() Jakub Kicinski
2023-08-03 13:08   ` Gustavo A. R. Silva
2023-08-03 14:21     ` Jakub Kicinski
2023-08-03 14:46       ` Michael Chan
2023-08-03 19:59         ` Gustavo A. R. Silva
2023-08-03 20:04           ` Michael Chan
2023-08-03 20:11             ` Gustavo A. R. Silva
2023-07-27 19:07 ` [PATCH net-next 2/2] eth: bnxt: fix warning for define in struct_group Jakub Kicinski
2023-07-27 20:28 ` [PATCH net-next 0/2] eth: bnxt: fix a couple of W=1 C=1 warnings Michael Chan
2023-07-28 21:00 ` patchwork-bot+netdevbpf

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