Messages in this thread | | | From | Shenwei Wang <> | Subject | RE: [EXT] Re: [PATCH v4 2/2] net: fec: add xdp and page pool statistics | Date | Tue, 15 Nov 2022 17:53:28 +0000 |
| |
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Tuesday, November 15, 2022 11:29 AM > To: Shenwei Wang <shenwei.wang@nxp.com> > Cc: 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 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
| |