bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Lorenzo Bianconi <lorenzo@kernel.org>,
	bpf@vger.kernel.org, netdev@vger.kernel.org, davem@davemloft.net,
	kuba@kernel.org, ast@kernel.org, toshiaki.makita1@gmail.com,
	lorenzo.bianconi@redhat.com, toke@redhat.com, brouer@redhat.com,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v3 bpf-next] net: veth: alloc skb in bulk for ndo_xdp_xmit
Date: Thu, 4 Feb 2021 10:05:56 +0100	[thread overview]
Message-ID: <20210204100556.59459549@carbon.lan> (raw)
In-Reply-To: <e2ae0d97-376a-07db-94fb-14f1220acca5@iogearbox.net>

On Thu, 4 Feb 2021 01:14:56 +0100
Daniel Borkmann <daniel@iogearbox.net> wrote:

> On 1/29/21 11:04 PM, Lorenzo Bianconi wrote:
> > Split ndo_xdp_xmit and ndo_start_xmit use cases in veth_xdp_rcv routine
> > in order to alloc skbs in bulk for XDP_PASS verdict.
> > Introduce xdp_alloc_skb_bulk utility routine to alloc skb bulk list.
> > The proposed approach has been tested in the following scenario:  
> [...]
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index 0d2630a35c3e..05354976c1fc 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -514,6 +514,17 @@ void xdp_warn(const char *msg, const char *func, const int line)
> >   };
> >   EXPORT_SYMBOL_GPL(xdp_warn);
> >   
> > +int xdp_alloc_skb_bulk(void **skbs, int n_skb, gfp_t gfp)
> > +{
> > +	n_skb = kmem_cache_alloc_bulk(skbuff_head_cache, gfp,
> > +				      n_skb, skbs);  
> Applied, but one question I was wondering about when reading the kmem_cache_alloc_bulk()
> code was whether it would be safer to simply test for kmem_cache_alloc_bulk() != n_skb
> given it could potentially in future also alloc less objs than requested, but I presume
> if such extension would get implemented then call-sites might need to indicate 'best
> effort' somehow via flag instead (to handle < n_skb case). Either way all current callers
> assume for != 0 that everything went well, so lgtm.

It was Andrew (AKPM) that wanted the API to either return the requested
number of objects or fail. I respected the MM-maintainers request at
that point, even-though I wanted the other API as there is a small
performance advantage (not crossing page boundary in SLUB).

At that time we discussed it on MM-list, and I see his/the point:
If API can allocate less objs than requested, then think about how this
complicated the surrounding code. E.g. in this specific code we already
have VETH_XDP_BATCH(16) xdp_frame objects, which we need to get 16 SKB
objects for.  What should the code do if it cannot get 16 SKBs(?).

Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

  reply	other threads:[~2021-02-04  9:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-29 22:04 Lorenzo Bianconi
2021-01-31 14:16 ` Toshiaki Makita
2021-02-02 14:14 ` Jesper Dangaard Brouer
2021-02-04  0:10 ` patchwork-bot+netdevbpf
2021-02-04  0:14 ` Daniel Borkmann
2021-02-04  9:05   ` Jesper Dangaard Brouer [this message]
2021-02-04 15:43     ` Daniel Borkmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210204100556.59459549@carbon.lan \
    --to=brouer@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=lorenzo.bianconi@redhat.com \
    --cc=lorenzo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=toke@redhat.com \
    --cc=toshiaki.makita1@gmail.com \
    --subject='Re: [PATCH v3 bpf-next] net: veth: alloc skb in bulk for ndo_xdp_xmit' \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).