All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
	netdev <netdev@vger.kernel.org>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>, Yonghong Song <yhs@fb.com>,
	Steffen Klassert <steffen.klassert@secunet.com>,
	shmulik@metanetworks.com, eyal@metanetworks.com
Subject: Re: BUG_ON in skb_segment, after bpf_skb_change_proto was applied
Date: Tue, 27 Aug 2019 14:42:18 +0300	[thread overview]
Message-ID: <20190827144218.5b098eac@pixies> (raw)
In-Reply-To: <94cd6f4d-09d4-11c0-64f4-bdc544bb3dcb@gmail.com>

On Mon, 26 Aug 2019 19:47:40 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On 8/26/19 4:07 PM, Shmulik Ladkani wrote:
> >   - ipv4 forwarding to dummy1, where eBPF nat4-to-6 program is attached
> >     at TC Egress (calls 'bpf_skb_change_proto()'), then redirect to ingress
> >     on same device.
> >     NOTE: 'bpf_skb_proto_4_to_6()' mangles 'shinfo->gso_size'  
> 
> Doing this on an skb with a frag_list is doomed, in current gso_segment() state.
> 
> A rewrite  would be needed (I believe I did so at some point, but Herbert Xu fought hard against it)

Thanks Eric,

- If a rewrite is still considered out of the question, how can one use
  eBPF's bpf_skb_change_proto() safely without disabling GRO completely?
  - e.g. is there a way to force the GROed skbs to fit into a layout that is
    tolerated by skb_segment?
  - alternatively can eBPF layer somehow enforce segmentation of the
    original GROed skb before mangling the gso_size?

- Another thing that puzzles me is that we hit the BUG_ON rather rarely
  and cannot yet reproduce synthetically. If skb_segment's handling of
  skbs with a frag_list (that have gso_size mangled) is broken, I'd expect
  to hit this more often... Any ideas?

- Suppose going for a rewrite, care to elaborate what's exactly missing
  in skb_segment's logic?
  I must admit I do not fully understand all the different code flows in
  this function, it seems to support many different input skbs - any
  assistance is highly appreciated.

Shmulik

  reply	other threads:[~2019-08-27 11:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-26 14:07 BUG_ON in skb_segment, after bpf_skb_change_proto was applied Shmulik Ladkani
2019-08-26 17:47 ` Eric Dumazet
2019-08-27 11:42   ` Shmulik Ladkani [this message]
2019-08-27 12:10     ` Daniel Borkmann
2019-08-28  5:56       ` Shmulik Ladkani
2019-08-29 12:22       ` Shmulik Ladkani
2019-09-01 20:05         ` Willem de Bruijn
2019-09-02 13:44           ` Shmulik Ladkani
2019-09-03 15:51           ` Shmulik Ladkani
2019-09-03 16:23             ` Willem de Bruijn
2019-09-03 17:03               ` Shmulik Ladkani
2019-09-03 17:24                 ` Willem de Bruijn
2019-08-27 15:09     ` Eric Dumazet

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=20190827144218.5b098eac@pixies \
    --to=shmulik.ladkani@gmail.com \
    --cc=alexander.duyck@gmail.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eric.dumazet@gmail.com \
    --cc=eyal@metanetworks.com \
    --cc=netdev@vger.kernel.org \
    --cc=shmulik@metanetworks.com \
    --cc=steffen.klassert@secunet.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 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.