| Message ID | 20220712181456.3398-1-olek2@wp.pl (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: lantiq_xrx200: use skb cache | expand |
On 7/12/22 20:14, Aleksander Jan Bajkowski wrote: > napi_build_skb() reuses NAPI skbuff_head cache in order to save some > cycles on freeing/allocating skbuff_heads on every new Rx or completed > Tx. > Use napi_consume_skb() to feed the cache with skbuff_heads of completed > Tx. The budget parameter is added to indicate NAPI context, as a value > of zero can be passed in the case of netpoll. > > NAT performance results on BT Home Hub 5A (kernel 5.15.45, mtu 1500): > > Fast path (Software Flow Offload): > Up Down > Before 702.4 719.3 > After 707.3 739.9 > > Slow path: > Up Down > Before 91.8 184.1 > After 92.0 185.7 > > Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl> Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
On Tue, Jul 12, 2022 at 8:15 PM Aleksander Jan Bajkowski <olek2@wp.pl> wrote: > > napi_build_skb() reuses NAPI skbuff_head cache in order to save some > cycles on freeing/allocating skbuff_heads on every new Rx or completed > Tx. > Use napi_consume_skb() to feed the cache with skbuff_heads of completed > Tx. The budget parameter is added to indicate NAPI context, as a value > of zero can be passed in the case of netpoll. > > NAT performance results on BT Home Hub 5A (kernel 5.15.45, mtu 1500): > > Fast path (Software Flow Offload): > Up Down > Before 702.4 719.3 > After 707.3 739.9 > > Slow path: > Up Down > Before 91.8 184.1 > After 92.0 185.7 > > Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl> > --- > drivers/net/ethernet/lantiq_xrx200.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c > index 5edb68a8aab1..83e07404803f 100644 > --- a/drivers/net/ethernet/lantiq_xrx200.c > +++ b/drivers/net/ethernet/lantiq_xrx200.c > @@ -238,7 +238,7 @@ static int xrx200_hw_receive(struct xrx200_chan *ch) > return ret; > } > > - skb = build_skb(buf, priv->rx_skb_size); > + skb = napi_build_skb(buf, priv->rx_skb_size); If you are changing this code path, what about adding proper error recovery ? skb can be NULL at this point :/ > skb_reserve(skb, NET_SKB_PAD); > skb_put(skb, len); > > @@ -321,7 +321,7 @@ static int xrx200_tx_housekeeping(struct napi_struct *napi, int budget) > pkts++; > bytes += skb->len; > ch->skb[ch->tx_free] = NULL; > - consume_skb(skb); > + napi_consume_skb(skb, budget); > memset(&ch->dma.desc_base[ch->tx_free], 0, > sizeof(struct ltq_dma_desc)); > ch->tx_free++; > -- > 2.30.2 >
Hi Eric, On 7/13/22 14:50, Eric Dumazet wrote: > On Tue, Jul 12, 2022 at 8:15 PM Aleksander Jan Bajkowski <olek2@wp.pl> wrote: >> >> napi_build_skb() reuses NAPI skbuff_head cache in order to save some >> cycles on freeing/allocating skbuff_heads on every new Rx or completed >> Tx. >> Use napi_consume_skb() to feed the cache with skbuff_heads of completed >> Tx. The budget parameter is added to indicate NAPI context, as a value >> of zero can be passed in the case of netpoll. >> >> NAT performance results on BT Home Hub 5A (kernel 5.15.45, mtu 1500): >> >> Fast path (Software Flow Offload): >> Up Down >> Before 702.4 719.3 >> After 707.3 739.9 >> >> Slow path: >> Up Down >> Before 91.8 184.1 >> After 92.0 185.7 >> >> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl> >> --- >> drivers/net/ethernet/lantiq_xrx200.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c >> index 5edb68a8aab1..83e07404803f 100644 >> --- a/drivers/net/ethernet/lantiq_xrx200.c >> +++ b/drivers/net/ethernet/lantiq_xrx200.c >> @@ -238,7 +238,7 @@ static int xrx200_hw_receive(struct xrx200_chan *ch) >> return ret; >> } >> >> - skb = build_skb(buf, priv->rx_skb_size); >> + skb = napi_build_skb(buf, priv->rx_skb_size); > > If you are changing this code path, what about adding proper error recovery ? > > skb can be NULL at this point :/ > Good catch. I will try to test the fix on the device tomorrow and send the patch. >> skb_reserve(skb, NET_SKB_PAD); >> skb_put(skb, len); >> >> @@ -321,7 +321,7 @@ static int xrx200_tx_housekeeping(struct napi_struct *napi, int budget) >> pkts++; >> bytes += skb->len; >> ch->skb[ch->tx_free] = NULL; >> - consume_skb(skb); >> + napi_consume_skb(skb, budget); >> memset(&ch->dma.desc_base[ch->tx_free], 0, >> sizeof(struct ltq_dma_desc)); >> ch->tx_free++; >> -- >> 2.30.2 >> Best regards, Aleksander
On Wed, 13 Jul 2022 22:38:37 +0200 Aleksander Bajkowski wrote: > > If you are changing this code path, what about adding proper error recovery ? > > > > skb can be NULL at this point :/ > > Good catch. I will try to test the fix on the device tomorrow and send the patch. Let's defer this patch until we merge the fix, otherwise we'll get a conflict. Not a big deal but easily avoidable here.
diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c index 5edb68a8aab1..83e07404803f 100644 --- a/drivers/net/ethernet/lantiq_xrx200.c +++ b/drivers/net/ethernet/lantiq_xrx200.c @@ -238,7 +238,7 @@ static int xrx200_hw_receive(struct xrx200_chan *ch) return ret; } - skb = build_skb(buf, priv->rx_skb_size); + skb = napi_build_skb(buf, priv->rx_skb_size); skb_reserve(skb, NET_SKB_PAD); skb_put(skb, len); @@ -321,7 +321,7 @@ static int xrx200_tx_housekeeping(struct napi_struct *napi, int budget) pkts++; bytes += skb->len; ch->skb[ch->tx_free] = NULL; - consume_skb(skb); + napi_consume_skb(skb, budget); memset(&ch->dma.desc_base[ch->tx_free], 0, sizeof(struct ltq_dma_desc)); ch->tx_free++;
napi_build_skb() reuses NAPI skbuff_head cache in order to save some cycles on freeing/allocating skbuff_heads on every new Rx or completed Tx. Use napi_consume_skb() to feed the cache with skbuff_heads of completed Tx. The budget parameter is added to indicate NAPI context, as a value of zero can be passed in the case of netpoll. NAT performance results on BT Home Hub 5A (kernel 5.15.45, mtu 1500): Fast path (Software Flow Offload): Up Down Before 702.4 719.3 After 707.3 739.9 Slow path: Up Down Before 91.8 184.1 After 92.0 185.7 Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl> --- drivers/net/ethernet/lantiq_xrx200.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)