netdev - BPF redirect API design issue for BPF-prog MTU feedback?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200917143846.37ce43a0@carbon>
Date:   Thu, 17 Sep 2020 14:38:46 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        BPF-dev-list <bpf@...r.kernel.org>
Cc:     brouer@...hat.com,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Maciej Zenczykowski <maze@...gle.com>,
        Lorenzo Bianconi <lorenzo.bianconi@...hat.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        John Fastabend <john.fastabend@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Shaun Crampton <shaun@...era.io>,
        David Miller <davem@...emloft.net>,
        Marek Majkowski <marek@...udflare.com>
Subject: BPF redirect API design issue for BPF-prog MTU feedback?

As you likely know[1] I'm looking into moving the MTU check (for TC-BPF)
in __bpf_skb_max_len() when e.g. called by bpf_skb_adjust_room(),
because when redirecting packets to another netdev it is not correct to
limit the MTU based on the incoming netdev.

I was looking at doing the MTU check in bpf_redirect() helper, because
at this point we know the redirect to netdev, and returning an
indication/error that MTU was exceed, would allow the BPF-prog logic to
react, e.g. sending ICMP (instead of packet getting silently dropped).
BUT this is not possible because bpf_redirect(index, flags) helper
don't provide the packet context-object (so I cannot lookup the packet
length).

Seeking input:

Should/can we change the bpf_redirect API or create a new helper with
packet-context?

 Note: We have the same need for the packet context for XDP when
 redirecting the new multi-buffer packets, as not all destination netdev
 will support these new multi-buffer packets.

I can of-cause do the MTU checks on kernel-side in skb_do_redirect, but
then how do people debug this? as packet will basically be silently dropped.

(Looking at how does BPF-prog logic handle MTU today)

How do bpf_skb_adjust_room() report that the MTU was exceeded?
Unfortunately it uses a common return code -ENOTSUPP which used for
multiple cases (include MTU exceeded). Thus, the BPF-prog logic cannot
use this reliably to know if this is a MTU exceeded event. (Looked
BPF-prog code and they all simply exit with TC_ACT_SHOT for all error
codes, cloudflare have the most advanced handling with
metrics->errors_total_encap_adjust_failed++).

[1] https://lore.kernel.org/bpf/159921182827.1260200.9699352760916903781.stgit@firesoul/
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer