linux-kernel - RE: [EXT] Re: [PATCH v4 2/2] net: fec: add xdp and page pool 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <PAXPR04MB9185DC399BA10036C0C7930D89049@PAXPR04MB9185.eurprd04.prod.outlook.com>
Date:   Tue, 15 Nov 2022 17:53:28 +0000
From:   Shenwei Wang <shenwei.wang@....com>
To:     Andrew Lunn <andrew@...n.ch>
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 v4 2/2] net: fec: add xdp and page pool
 statistics



> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Tuesday, November 15, 2022 11:29 AM
> To: Shenwei Wang <shenwei.wang@....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; linux-
> kernel@...r.kernel.org; imx@...ts.linux.dev; kernel test robot <lkp@...el.com>
> Subject: [EXT] Re: [PATCH v4 2/2] net: fec: add xdp and page pool statistics
> 
> Caution: EXT Email
> 
> > @@ -1582,6 +1586,7 @@ fec_enet_rx_queue(struct net_device *ndev, int
> budget, u16 queue_id)
> >       struct bpf_prog *xdp_prog = READ_ONCE(fep->xdp_prog);
> >       u32 ret, xdp_result = FEC_ENET_XDP_PASS;
> >       u32 data_start = FEC_ENET_XDP_HEADROOM;
> > +     u32 xdp_stats[XDP_STATS_TOTAL];
> >       struct xdp_buff xdp;
> >       struct page *page;
> >       u32 sub_len = 4;
> > @@ -1656,11 +1661,13 @@ fec_enet_rx_queue(struct net_device *ndev, int
> budget, u16 queue_id)
> >               fec_enet_update_cbd(rxq, bdp, index);
> >
> >               if (xdp_prog) {
> > +                     memset(xdp_stats, 0, sizeof(xdp_stats));
> >                       xdp_buff_clear_frags_flag(&xdp);
> >                       /* subtract 16bit shift and FCS */
> >                       xdp_prepare_buff(&xdp, page_address(page),
> >                                        data_start, pkt_len - sub_len, false);
> > -                     ret = fec_enet_run_xdp(fep, xdp_prog, &xdp, rxq, index);
> > +                     ret = fec_enet_run_xdp(fep, xdp_prog, &xdp, rxq,
> > +                                            xdp_stats, index);
> >                       xdp_result |= ret;
> >                       if (ret != FEC_ENET_XDP_PASS)
> >                               goto rx_processing_done; @@ -1762,6
> > +1769,15 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16
> queue_id)
> >       if (xdp_result & FEC_ENET_XDP_REDIR)
> >               xdp_do_flush_map();
> >
> > +     if (xdp_prog) {
> > +             int i;
> > +
> > +             u64_stats_update_begin(&rxq->syncp);
> > +             for (i = 0; i < XDP_STATS_TOTAL; i++)
> > +                     rxq->stats[i] += xdp_stats[i];
> > +             u64_stats_update_end(&rxq->syncp);
> > +     }
> > +
> 
> This looks wrong. You are processing upto the napi budget, 64 frames, in a loop.
> The memset to 0 happens inside the loop, but you do the accumulation outside
> the loop?
> 

My bad. That should be moved outside the loop.

Thanks,
Shenwei

> This patch is getting pretty big. Please break it up, at least into one patch for XDP
> stats and one for page pool stats.
> 
>     Andrew