bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Toshiaki Makita <toshiaki.makita1@gmail.com>,
	Mao Wenan <maowenan@huawei.com>,
	"David S. Miller" <davem@davemloft.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	jwi@linux.ibm.com, jianglidong3@jd.com,
	Eric Dumazet <edumazet@google.com>,
	Network Development <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	kernel-janitors@vger.kernel.org, brouer@redhat.com
Subject: Re: [PATCH net v2] veth: xdp: use head instead of hard_start
Date: Fri, 3 Apr 2020 09:58:47 +0200	[thread overview]
Message-ID: <20200403095847.21e1e5ea@carbon> (raw)
In-Reply-To: <CAADnVQKEyv_bRhEfu1Jp=DSggj_O2xjJyd_QZ7a4LJY+dUO2rg@mail.gmail.com>

On Thu, 2 Apr 2020 08:40:23 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Thu, Apr 2, 2020 at 2:06 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> >
> > On Thu, 2 Apr 2020 09:47:03 +0900
> > Toshiaki Makita <toshiaki.makita1@gmail.com> wrote:
> >  
> > > On 2020/04/02 1:15, Jesper Dangaard Brouer wrote:
> > > ...  
> > > > [PATCH RFC net-next] veth: adjust hard_start offset on redirect XDP frames
> > > >
> > > > When native XDP redirect into a veth device, the frame arrives in the
> > > > xdp_frame structure. It is then processed in veth_xdp_rcv_one(),
> > > > which can run a new XDP bpf_prog on the packet. Doing so requires
> > > > converting xdp_frame to xdp_buff, but the tricky part is that
> > > > xdp_frame memory area is located in the top (data_hard_start) memory
> > > > area that xdp_buff will point into.
> > > >
> > > > The current code tried to protect the xdp_frame area, by assigning
> > > > xdp_buff.data_hard_start past this memory. This results in 32 bytes
> > > > less headroom to expand into via BPF-helper bpf_xdp_adjust_head().
> > > >
> > > > This protect step is actually not needed, because BPF-helper
> > > > bpf_xdp_adjust_head() already reserve this area, and don't allow
> > > > BPF-prog to expand into it. Thus, it is safe to point data_hard_start
> > > > directly at xdp_frame memory area.
> > > >
> > > > Cc: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>  
> > >
> > > FYI: This mail address is deprecated.
> > >  
> > > > Fixes: 9fc8d518d9d5 ("veth: Handle xdp_frames in xdp napi ring")
> > > > Reported-by: Mao Wenan <maowenan@huawei.com>
> > > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>  
> > >
> > > FWIW,
> > >
> > > Acked-by: Toshiaki Makita <toshiaki.makita1@gmail.com>  
> >
> > Thanks.
> >
> > I have updated your email and added your ack in my patchset.  I will
> > submit this officially once net-next opens up again[1], as part my
> > larger patchset for introducing XDP frame_sz.  
> 
> It looks like bug fix to me.
> The way I read it that behavior of bpf_xdp_adjust_head() is a bit
> buggy with veth netdev,
> so why wait ?

I want to wait to ease your life as maintainer. This is part of a
larger patchset (for XDP frame_sz) and the next patch touch same code
path and thus depend on these code adjustments.  If we apply them in
bpf vs bpf-next then you/we will have to handle merge conflicts.  The
severity of the "fix" is really low, it only means 32 bytes less
headroom (which I doubt anyone is using).

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


  reply	other threads:[~2020-04-03  7:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-30 10:26 [PATCH net] veth: xdp: use head instead of hard_start Mao Wenan
2020-03-30 11:34 ` Jesper Dangaard Brouer
2020-03-30 23:35   ` Toshiaki Makita
2020-03-31  3:56     ` maowenan
2020-03-31  5:45       ` Toshiaki Makita
2020-03-31  6:06         ` [PATCH net v2] " Mao Wenan
2020-03-31  6:16           ` Toshiaki Makita
2020-04-01 16:15             ` Jesper Dangaard Brouer
2020-04-02  0:47               ` Toshiaki Makita
2020-04-02  9:06                 ` Jesper Dangaard Brouer
2020-04-02 15:40                   ` Alexei Starovoitov
2020-04-03  7:58                     ` Jesper Dangaard Brouer [this message]
2020-04-03 21:12                       ` Alexei Starovoitov
2020-04-02  1:23               ` maowenan

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=20200403095847.21e1e5ea@carbon \
    --to=brouer@redhat.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=jianglidong3@jd.com \
    --cc=john.fastabend@gmail.com \
    --cc=jwi@linux.ibm.com \
    --cc=kafai@fb.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maowenan@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=toshiaki.makita1@gmail.com \
    --cc=yhs@fb.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 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).