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

[Shenwei Wang]

> -----Original Message-----
> From: Alexander Lobakin <alexandr.lobakin@xxxxxxxxx>
> Sent: Thursday, November 10, 2022 10:43 AM
> To: Shenwei Wang <shenwei.wang@xxxxxxx>
> Cc: Alexander Lobakin <alexandr.lobakin@xxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Alexei
> > > 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 :)

The reason to use u32 here is : 1. It is simple to implement. 2. To follow the same
behavior as the other MAC hardware statistic counters which are all 32bit. 3. I did
investigate the u64_stats API, and think it is still a little expensive here.

Thanks,
Shenwei

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