Messages in this thread | | | From | Shenwei Wang <> | Subject | RE: [EXT] Re: [PATCH v5 2/3] net: fec: add xdp statistics | Date | Thu, 17 Nov 2022 01:15:44 +0000 |
| |
> -----Original Message----- > From: Alexander Lobakin <alexandr.lobakin@intel.com> > Sent: Wednesday, November 16, 2022 10:02 AM > To: Shenwei Wang <shenwei.wang@nxp.com> > Cc: Alexander Lobakin <alexandr.lobakin@intel.com>; David S. Miller > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Jesper > Dangaard Brouer <hawk@kernel.org>; Ilias Apalodimas > <ilias.apalodimas@linaro.org>; Alexei Starovoitov <ast@kernel.org>; Daniel > Borkmann <daniel@iogearbox.net>; John Fastabend > <john.fastabend@gmail.com>; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; imx@lists.linux.dev; kernel test robot <lkp@intel.com> > Subject: [EXT] Re: [PATCH v5 2/3] net: fec: add xdp statistics > > Caution: EXT Email > > From: Shenwei Wang <shenwei.wang@nxp.com> > Date: Tue, 15 Nov 2022 14:49:50 -0600 > > > Add xdp statistics for ethtool stats and using u64 to record the xdp counters. > > > > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> > > Reported-by: kernel test robot <lkp@intel.com> > > Nit: would be nice if you Cc me for the next submissions as I was commenting on > the previous ones. Just to make sure reviewers won't miss anything.
Sure of course.
> > > --- > > drivers/net/ethernet/freescale/fec.h | 15 +++++ > > drivers/net/ethernet/freescale/fec_main.c | 74 > > +++++++++++++++++++++-- > > 2 files changed, 83 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/fec.h > > b/drivers/net/ethernet/freescale/fec.h > > index 61e847b18343..adbe661552be 100644 > > --- a/drivers/net/ethernet/freescale/fec.h > > +++ b/drivers/net/ethernet/freescale/fec.h > > @@ -526,6 +526,19 @@ struct fec_enet_priv_txrx_info { > > struct sk_buff *skb; > > }; > > > > +enum { > > + RX_XDP_REDIRECT = 0, > > + RX_XDP_PASS, > > + RX_XDP_DROP, > > + RX_XDP_TX, > > + RX_XDP_TX_ERRORS, > > + TX_XDP_XMIT, > > + TX_XDP_XMIT_ERRORS, > > + > > + /* The following must be the last one */ > > + XDP_STATS_TOTAL, > > +}; > > + > > struct fec_enet_priv_tx_q { > > struct bufdesc_prop bd; > > unsigned char *tx_bounce[TX_RING_SIZE]; @@ -546,6 +559,8 @@ > > struct fec_enet_priv_rx_q { > > /* page_pool */ > > struct page_pool *page_pool; > > struct xdp_rxq_info xdp_rxq; > > + struct u64_stats_sync syncp; > > + u64 stats[XDP_STATS_TOTAL]; > > Why `u64`? u64_stats infra declares `u64_stat_t` type and a bunch of helpers like > u64_stats_add(), u64_stats_read() and so on, they will be solved then by the > compiler to the most appropriate ops for the architecture. So they're more > "generic" if you prefer. > Sure, if you show some numbers where `u64_stat_t` is slower than `u64` on your > machine, then okay, but otherwise...
I will take a leave next week. Will do a compare testing when I come back. Then we can decide which way to go.
Thanks, Shenwei
> > > > > /* rx queue number, in the range 0-7 */ > > u8 id; > > [...] > > > -- > > 2.34.1 > > Thanks, > Olek
| |