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