| Message ID | 1553521344-31094-1-git-send-email-ioana.ciornei@nxp.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] dpaa2-eth: use netif_receive_skb_list | expand |
On Mon, 25 Mar 2019 13:42:39 +0000 Ioana Ciornei <ioana.ciornei@nxp.com> wrote: > Take advantage of the software Rx batching by using > netif_receive_skb_list instead of napi_gro_receive. > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> > --- Nice to see more people/drivers using: netif_receive_skb_list() We should likely add a similar napi_gro_receive_list() function. > drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 8 +++++++- > drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h | 1 + > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > index 2ba49e9..e923e5c 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > @@ -435,7 +435,7 @@ static void dpaa2_eth_rx(struct dpaa2_eth_priv *priv, > percpu_stats->rx_packets++; > percpu_stats->rx_bytes += dpaa2_fd_get_len(fd); > > - napi_gro_receive(&ch->napi, skb); > + list_add_tail(&skb->list, ch->rx_list); > > return; > > @@ -1108,12 +1108,16 @@ static int dpaa2_eth_poll(struct napi_struct *napi, int budget) > struct dpaa2_eth_fq *fq, *txc_fq = NULL; > struct netdev_queue *nq; > int store_cleaned, work_done; > + struct list_head rx_list; > int err; > > ch = container_of(napi, struct dpaa2_eth_channel, napi); > ch->xdp.res = 0; > priv = ch->priv; > > + INIT_LIST_HEAD(&rx_list); > + ch->rx_list = &rx_list; > + > do { > err = pull_channel(ch); > if (unlikely(err)) > @@ -1157,6 +1161,8 @@ static int dpaa2_eth_poll(struct napi_struct *napi, int budget) > work_done = max(rx_cleaned, 1); > > out: > + netif_receive_skb_list(ch->rx_list); > + > if (txc_fq && txc_fq->dq_frames) { > nq = netdev_get_tx_queue(priv->net_dev, txc_fq->flowid); > netdev_tx_completed_queue(nq, txc_fq->dq_frames, > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h > index 7879622..a11ebfd 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h > @@ -334,6 +334,7 @@ struct dpaa2_eth_channel { > struct dpaa2_eth_ch_stats stats; > struct dpaa2_eth_ch_xdp xdp; > struct xdp_rxq_info xdp_rxq; > + struct list_head *rx_list; > }; > > struct dpaa2_eth_dist_fields {
On 03/26/2019 07:43 AM, Jesper Dangaard Brouer wrote: > On Mon, 25 Mar 2019 13:42:39 +0000 > Ioana Ciornei <ioana.ciornei@nxp.com> wrote: > >> Take advantage of the software Rx batching by using >> netif_receive_skb_list instead of napi_gro_receive. >> >> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> >> --- > > Nice to see more people/drivers using: netif_receive_skb_list() > > We should likely add a similar napi_gro_receive_list() function. GRO is essentially processing one frame at a time, and recycling one sk_buff. (one sk_buff per aggregated train) Having to allocate one sk_buff per MSS just to provide a list would likely be slower, as far as GRO is concerned.
From: Ioana Ciornei <ioana.ciornei@nxp.com> Date: Mon, 25 Mar 2019 13:42:39 +0000 > Take advantage of the software Rx batching by using > netif_receive_skb_list instead of napi_gro_receive. > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> Applied, thanks.
On 26/03/2019 14:43, Jesper Dangaard Brouer wrote: > On Mon, 25 Mar 2019 13:42:39 +0000 > Ioana Ciornei <ioana.ciornei@nxp.com> wrote: > >> Take advantage of the software Rx batching by using >> netif_receive_skb_list instead of napi_gro_receive. >> >> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> >> --- > Nice to see more people/drivers using: netif_receive_skb_list() > > We should likely add a similar napi_gro_receive_list() function. I had a patch series that did that; last posting was v3 back in November: https://marc.info/?l=linux-netdev&m=154221888012410&w=2 However, Eric raised some issues, also some Mellanox folks privately reported that using it in their driver regressed performance, and I've been too busy since to make progress with it. Since you seem to be much better than me at perf investigations, Jesper, maybe you could take over the series? -Ed
On Wed, 27 Mar 2019 12:02:13 +0000 Edward Cree <ecree@solarflare.com> wrote: > On 26/03/2019 14:43, Jesper Dangaard Brouer wrote: > > On Mon, 25 Mar 2019 13:42:39 +0000 > > Ioana Ciornei <ioana.ciornei@nxp.com> wrote: > > > >> Take advantage of the software Rx batching by using > >> netif_receive_skb_list instead of napi_gro_receive. > >> > >> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> > >> --- > > Nice to see more people/drivers using: netif_receive_skb_list() > > > > We should likely add a similar napi_gro_receive_list() function. > > I had a patch series that did that; last posting was v3 back in > November: https://marc.info/?l=linux-netdev&m=154221888012410&w=2 > However, Eric raised some issues, also some Mellanox folks privately > reported that using it in their driver regressed performance, and > I've been too busy since to make progress with it. Since you seem > to be much better than me at perf investigations, Jesper, maybe you > could take over the series? I'm hoping Florian Westphal might also have some cycles for this? (We talked about doing this during NetDevConf-0x13, because if we can make more driver use these SKB-lists, then it makes sense to let iptables/nftables build a SKB-list of packets to drop, instead of doing it individually, and then we leverage Felix'es work on bulk free in kfree_skb_list). I'm currently coding up use of netif_receive_skb_list() in CPUMAP redirect. As this makes is easier for e.g. Florian (and others) to play with this API, as we no-longer depend on a device driver having this (although we do depend on XDP_REDIRECT in a driver). And the trick to get this as faster than GRO (that basically recycle the same SKB) is to use the slub/kmem_cache bulk API for SKBs.
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c index 2ba49e9..e923e5c 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c @@ -435,7 +435,7 @@ static void dpaa2_eth_rx(struct dpaa2_eth_priv *priv, percpu_stats->rx_packets++; percpu_stats->rx_bytes += dpaa2_fd_get_len(fd); - napi_gro_receive(&ch->napi, skb); + list_add_tail(&skb->list, ch->rx_list); return; @@ -1108,12 +1108,16 @@ static int dpaa2_eth_poll(struct napi_struct *napi, int budget) struct dpaa2_eth_fq *fq, *txc_fq = NULL; struct netdev_queue *nq; int store_cleaned, work_done; + struct list_head rx_list; int err; ch = container_of(napi, struct dpaa2_eth_channel, napi); ch->xdp.res = 0; priv = ch->priv; + INIT_LIST_HEAD(&rx_list); + ch->rx_list = &rx_list; + do { err = pull_channel(ch); if (unlikely(err)) @@ -1157,6 +1161,8 @@ static int dpaa2_eth_poll(struct napi_struct *napi, int budget) work_done = max(rx_cleaned, 1); out: + netif_receive_skb_list(ch->rx_list); + if (txc_fq && txc_fq->dq_frames) { nq = netdev_get_tx_queue(priv->net_dev, txc_fq->flowid); netdev_tx_completed_queue(nq, txc_fq->dq_frames, diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h index 7879622..a11ebfd 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h @@ -334,6 +334,7 @@ struct dpaa2_eth_channel { struct dpaa2_eth_ch_stats stats; struct dpaa2_eth_ch_xdp xdp; struct xdp_rxq_info xdp_rxq; + struct list_head *rx_list; }; struct dpaa2_eth_dist_fields {
Take advantage of the software Rx batching by using netif_receive_skb_list instead of napi_gro_receive. Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> --- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 8 +++++++- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h | 1 + 2 files changed, 8 insertions(+), 1 deletion(-)