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

Shenwei Wang
2022-09-29T12:46:32Z

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Wednesday, September 28, 2022 8:51 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Alexei
> Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>;
> Jesper Dangaard Brouer <hawk@kernel.org>; John Fastabend
> <john.fastabend@gmail.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; imx@lists.linux.dev
> Subject: [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?

> > -#define FEC_STATS_SIZE (ARRAY_SIZE(fec_stats) * sizeof(u64))
> > +static struct fec_xdp_stat {
> > + char name[ETH_GSTRING_LEN];
> > + u32 count;
> > +} fec_xdp_stats[] = {
> > + { "rx_xdp_redirect", 0 },
> > + { "rx_xdp_pass", 0 },
> > + { "rx_xdp_drop", 0 },
> > + { "rx_xdp_tx", 0 },
> > + { "rx_xdp_tx_errors", 0 },
> > + { "tx_xdp_xmit", 0 },
> > + { "tx_xdp_xmit_errors", 0 },
> > +};
> > +
> > +#define FEC_STATS_SIZE ((ARRAY_SIZE(fec_stats) + \
> > + ARRAY_SIZE(fec_xdp_stats)) * sizeof(u64))
>
> The page pool also has some stats. See page_pool_get_stats(),
> page_pool_ethtool_stats_get_strings() etc.
>

Will add those stats in the next version.

Thanks,
Shenwei

> Andrew