lkml.org 
[lkml]   [2022]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [EXT] Re: [PATCH v5 2/3] net: fec: add xdp statistics
Date


> -----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

\
 
 \ /
  Last update: 2022-11-17 02:16    [W:0.051 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site