Re: [PATCH v2 RESEND 1/1] net: fec: add xdp and page pool statistics

[Alexander Lobakin <alexandr.lobakin@intel.com>]

From: Shenwei Wang <shenwei.wang@nxp.com>
Date: Thu, 10 Nov 2022 13:29:56 +0000

> > -----Original Message-----
> > From: Paolo Abeni <pabeni@redhat.com>
> > Sent: Thursday, November 10, 2022 5:54 AM
> > To: Shenwei Wang <shenwei.wang@nxp.com>; David S. Miller
> > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> > Kicinski <kuba@kernel.org>
> > >       case ETH_SS_STATS:
> > > -             for (i = 0; i < ARRAY_SIZE(fec_stats); i++)
> > > -                     memcpy(data + i * ETH_GSTRING_LEN,
> > > -                             fec_stats[i].name, ETH_GSTRING_LEN);
> > > +             for (i = 0; i < ARRAY_SIZE(fec_stats); i++) {
> > > +                     memcpy(data, fec_stats[i].name, ETH_GSTRING_LEN);
> > > +                     data += ETH_GSTRING_LEN;
> > > +             }
> > > +             for (i = 0; i < ARRAY_SIZE(fec_xdp_stat_strs); i++) {
> > > +                     memcpy(data, fec_xdp_stat_strs[i], ETH_GSTRING_LEN);
> > > +                     data += ETH_GSTRING_LEN;
> >
> > The above triggers a warning:
> >
> > In function 'fortify_memcpy_chk',
> >     inlined from 'fec_enet_get_strings'
> > at ../drivers/net/ethernet/freescale/fec_main.c:2788:4:
> > ../include/linux/fortify-string.h:413: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]
> >   413 |                         __read_overflow2_field(q_size_field, size);
> >       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > I think you can address it changing fec_xdp_stat_strs definition to:
> >
> > static const char fec_xdp_stat_strs[XDP_STATS_TOTAL][ETH_GSTRING_LEN] =
> 
> That does a problem. How about just change the memcpy to strncpy?

Don't use a static char array, it would consume more memory than the
current code. Just replace memcpy()s with strscpy().

Why u32 for the stats tho? It will overflow sooner or later. "To
keep it simple and compatible" you can use u64_stats API :)

> 
> Regards,
> Shenwei
> 
> > { // ...
> >
> > Cheers,
> >
> > Paolo

Thanks,
Olek