linux-kernel - RE: [EXT] Re: [PATCH v5 2/3] net: fec: add xdp statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <PAXPR04MB9185C6B616C16CF8D1DEB71789069@PAXPR04MB9185.eurprd04.prod.outlook.com>
Date:   Thu, 17 Nov 2022 01:15:44 +0000
From:   Shenwei Wang <shenwei.wang@....com>
To:     Alexander Lobakin <alexandr.lobakin@...el.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
        kernel test robot <lkp@...el.com>
Subject: RE: [EXT] Re: [PATCH v5 2/3] net: fec: add xdp statistics



> -----Original Message-----
> From: Alexander Lobakin <alexandr.lobakin@...el.com>
> Sent: Wednesday, November 16, 2022 10:02 AM
> To: Shenwei Wang <shenwei.wang@....com>
> Cc: Alexander Lobakin <alexandr.lobakin@...el.com>; David S. Miller
> <davem@...emloft.net>; Eric Dumazet <edumazet@...gle.com>; Jakub
> Kicinski <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>; Jesper
> Dangaard Brouer <hawk@...nel.org>; Ilias Apalodimas
> <ilias.apalodimas@...aro.org>; Alexei Starovoitov <ast@...nel.org>; Daniel
> Borkmann <daniel@...earbox.net>; John Fastabend
> <john.fastabend@...il.com>; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org; imx@...ts.linux.dev; kernel test robot <lkp@...el.com>
> Subject: [EXT] Re: [PATCH v5 2/3] net: fec: add xdp statistics
> 
> Caution: EXT Email
> 
> From: Shenwei Wang <shenwei.wang@....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@....com>
> > Reported-by: kernel test robot <lkp@...el.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