2022-09-29 13:27:37

by Andrew Lunn

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH 1/1] net: fec: add initial XDP support

> > > +struct fec_enet_xdp_stats {
> > > + u64 xdp_pass;
> > > + u64 xdp_drop;
> > > + u64 xdp_xmit;
> > > + u64 xdp_redirect;
> > > + u64 xdp_xmit_err;
> > > + u64 xdp_tx;
> > > + u64 xdp_tx_err;
> > > +};
> > > +
> > > + switch (act) {
> > > + case XDP_PASS:
> > > + rxq->stats.xdp_pass++;
> >
> > Since the stats are u64, and most machines using the FEC are 32 bit, you cannot
> > just do an increment. Took a look at u64_stats_sync.h.
> >
>

> As this increment is only executed under the NAPI kthread context,
> is the protection still required?

Are the statistics values read by ethtool under NAPI kthread context?

Andrew


2022-09-29 13:47:21

by Shenwei Wang

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH 1/1] net: fec: add initial XDP support



> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: Thursday, September 29, 2022 8:24 AM
> To: Shenwei Wang <[email protected]>
> Cc: Joakim Zhang <[email protected]>; David S. Miller
> <[email protected]>; Eric Dumazet <[email protected]>; Jakub
> Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Alexei
> Starovoitov <[email protected]>; Daniel Borkmann <[email protected]>;
> Jesper Dangaard Brouer <[email protected]>; John Fastabend
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [EXT] Re: [PATCH 1/1] net: fec: add initial XDP support
>
> Caution: EXT Email
>
> > > > +struct fec_enet_xdp_stats {
> > > > + u64 xdp_pass;
> > > > + u64 xdp_drop;
> > > > + u64 xdp_xmit;
> > > > + u64 xdp_redirect;
> > > > + u64 xdp_xmit_err;
> > > > + u64 xdp_tx;
> > > > + u64 xdp_tx_err;
> > > > +};
> > > > +
> > > > + switch (act) {
> > > > + case XDP_PASS:
> > > > + rxq->stats.xdp_pass++;
> > >
> > > Since the stats are u64, and most machines using the FEC are 32 bit,
> > > you cannot just do an increment. Took a look at u64_stats_sync.h.
> > >
> >
>
> > As this increment is only executed under the NAPI kthread context, is
> > the protection still required?
>
> Are the statistics values read by ethtool under NAPI kthread context?
>

You are right. The read is not under NAPI context.

Thanks,
Shenwei

> Andrew