All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: brouer@redhat.com, "Jakub Kicinski" <kuba@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Luigi Rizzo" <lrizzo@google.com>,
	"Network Development" <netdev@vger.kernel.org>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"David Miller" <davem@davemloft.net>,
	"Alexander Duyck" <alexander.duyck@gmail.com>,
	"Jubran, Samih" <sameehj@amazon.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"Alexei Starovoitov" <ast@kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH v4] netdev attribute to control xdpgeneric skb linearization
Date: Wed, 4 Mar 2020 10:18:53 +0100	[thread overview]
Message-ID: <20200304101853.760034dc@carbon> (raw)
In-Reply-To: <CA+FuTSeL_psqzpB6hxSh6f1HnO_SrpED=71Y3HcyDweG2Y3sdg@mail.gmail.com>

On Tue, 3 Mar 2020 16:10:14 -0500
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:

> On Tue, Mar 3, 2020 at 3:50 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Tue, 3 Mar 2020 20:46:55 +0100 Daniel Borkmann wrote:  
> > > Thus, when the data/data_end test fails in generic XDP, the user can
> > > call e.g. bpf_xdp_pull_data(xdp, 64) to make sure we pull in as much as
> > > is needed w/o full linearization and once done the data/data_end can be
> > > repeated to proceed. Native XDP will leave xdp->rxq->skb as NULL, but
> > > later we could perhaps reuse the same bpf_xdp_pull_data() helper for
> > > native with skb-less backing. Thoughts?  
> 
> Something akin to pskb_may_pull sounds like a great solution to me.
> 
> Another approach would be a new xdp_action XDP_NEED_LINEARIZED that
> causes the program to be restarted after linearization. But that is both
> more expensive and less elegant.
> 
> Instead of a sysctl or device option, is this an optimization that
> could be taken based on the program? Specifically, would XDP_FLAGS be
> a path to pass a SUPPORT_SG flag along with the program? I'm not
> entirely familiar with the XDP setup code, so this may be a totally
> off. But from a quick read it seems like generic_xdp_install could
> transfer such a flag to struct net_device.
> 
> > I'm curious why we consider a xdpgeneric-only addition. Is attaching
> > a cls_bpf program noticeably slower than xdpgeneric?  
> 
> This just should not be xdp*generic* only, but allow us to use any XDP
> with large MTU sizes and without having to disable GRO.

This is an important point: "should not be xdp*generic* only".

I really want to see this work for XDP-native *first*, and it seems
that with Daniel's idea, it can can also work for XDP-generic.  As Jakub
also hinted, it seems strange that people are trying to implement this
for XDP-generic, as I don't think there is any performance advantage
over cls_bpf.  We really want this to work from XDP-native.


> I'd still like a way to be able to drop or modify packets before GRO,
> or to signal that a type of packet should skip GRO.

That is a use-case, that we should remember to support.

Samih (cc'ed) is working on adding multi-frame support[1] to XDP-native.
Given the huge interest this thread shows, I think I will dedicate
some of my time to help him out on the actual coding.

For my idea to work[1], we first have storage space for the multi-buffer
references, and I propose we use the skb_shared_info area, that is
available anyhow for XDP_PASS that calls build_skb().  Thus, we first
need to standardize across all XDP drivers, how and where this memory
area is referenced/offset.


[1] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
[2] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org#storage-space-for-multi-buffer-referencessegments
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


  reply	other threads:[~2020-03-04  9:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-28 10:54 [PATCH v4] netdev attribute to control xdpgeneric skb linearization Luigi Rizzo
2020-02-28 11:20 ` Toke Høiland-Jørgensen
2020-02-28 12:12 ` Michal Kubecek
2020-02-28 12:29 ` Jesper Dangaard Brouer
2020-02-28 13:19   ` Luigi Rizzo
2020-02-28 12:30 ` Jesper Dangaard Brouer
2020-02-28 19:00 ` Jakub Kicinski
2020-02-28 23:53   ` Willem de Bruijn
2020-03-03 19:46     ` Daniel Borkmann
2020-03-03 20:50       ` Jakub Kicinski
2020-03-03 21:04         ` Daniel Borkmann
2020-03-03 21:10         ` Willem de Bruijn
2020-03-04  9:18           ` Jesper Dangaard Brouer [this message]
2020-03-04 10:06       ` Luigi Rizzo

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20200304101853.760034dc@carbon \
    --to=brouer@redhat.com \
    --cc=alexander.duyck@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lrizzo@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=sameehj@amazon.com \
    --cc=toke@redhat.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.