On 01/04/24 22:24, Jakub Kicinski wrote:
On Thu, 28 Mar 2024 19:17:10 -0600 Gustavo A. R. Silva wrote:
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -34,8 +34,11 @@ enum nfp_dumpspec_type {
/* generic type plus length */
struct nfp_dump_tl {
- __be32 type;
- __be32 length; /* chunk length to follow, aligned to 8 bytes */
+ /* New members must be added within the struct_group() macro below. */
+ struct_group_tagged(nfp_dump_tl_hdr, hdr,
+ __be32 type;
+ __be32 length; /* chunk length to follow, aligned to 8 bytes */
+ );
char data[];
};
I counted 9 references to nfp_dump_tl->data.
Better to add:
static void *nfp_dump_tl_data(struct nfp_dump_tl *spec)
{
return &spec[1];
}
Unfortunately, that's out-of-bounds for the compiler, and well, basically
the reason why flex-array members were created in the first place.
I was looking into implementing two separate structs:
struct nfp_dump_tl_hdr {
__be32 type;
__be32 length; /* chunk length to follow, aligned to 8 bytes */
};
struct nfp_dump_tl {
__be32 type;
__be32 length; /* chunk length to follow, aligned to 8 bytes */
char data[];
};
and at least for structs nfp_dumpspec_csr, nfp_dumpspec_rtsym, nfp_dump_csr, and
nfp_dump_rtsym it'd be a clean change (no need for container_of()), but not for
structs nfp_dumpspec_csr and nfp_dumpspec_rtsym because of some casts from
the flex struct:
nfp_add_tlv_size():
case NFP_DUMPSPEC_TYPE_ME_CSR:
spec_csr = (struct nfp_dumpspec_csr *)tl;
if (!nfp_csr_spec_valid(spec_csr))
...
case NFP_DUMPSPEC_TYPE_INDIRECT_ME_CSR:
spec_csr = (struct nfp_dumpspec_csr *)tl;
if (!nfp_csr_spec_valid(spec_csr))
...
case NFP_DUMPSPEC_TYPE_RTSYM:
spec_rtsym = (struct nfp_dumpspec_rtsym *)tl;
err = nfp_dump_single_rtsym(pf, spec_rtsym, dump);
nfp_calc_rtsym_dump_sz():
spec_rtsym = (struct nfp_dumpspec_rtsym *)spec;
At least for those two structs, it's probably more straightforward to use
struct_group_tagged() and container_of().
--
Gustavo
