Re: [PATCH bpf-next v2 0/8] Support defragmenting IPv(4|6) packets in BPF

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/28/23 5:56 AM, Alexei Starovoitov wrote:
On Mon, Feb 27, 2023 at 5:57 PM Daniel Xu <dxu@xxxxxxxxx> wrote:
On Mon, Feb 27, 2023 at 03:03:38PM -0800, Alexei Starovoitov wrote:
On Mon, Feb 27, 2023 at 12:51:02PM -0700, Daniel Xu wrote:
=== Context ===

In the context of a middlebox, fragmented packets are tricky to handle.
The full 5-tuple of a packet is often only available in the first
fragment which makes enforcing consistent policy difficult. There are
really only two stateless options, neither of which are very nice:

1. Enforce policy on first fragment and accept all subsequent fragments.
    This works but may let in certain attacks or allow data exfiltration.

2. Enforce policy on first fragment and drop all subsequent fragments.
    This does not really work b/c some protocols may rely on
    fragmentation. For example, DNS may rely on oversized UDP packets for
    large responses.

So stateful tracking is the only sane option. RFC 8900 [0] calls this
out as well in section 6.3:

     Middleboxes [...] should process IP fragments in a manner that is
     consistent with [RFC0791] and [RFC8200]. In many cases, middleboxes
     must maintain state in order to achieve this goal.

=== BPF related bits ===

However, when policy is enforced through BPF, the prog is run before the
kernel reassembles fragmented packets. This leaves BPF developers in a
awkward place: implement reassembly (possibly poorly) or use a stateless
method as described above.

Fortunately, the kernel has robust support for fragmented IP packets.
This patchset wraps the existing defragmentation facilities in kfuncs so
that BPF progs running on middleboxes can reassemble fragmented packets
before applying policy.

=== Patchset details ===

This patchset is (hopefully) relatively straightforward from BPF perspective.
One thing I'd like to call out is the skb_copy()ing of the prog skb. I
did this to maintain the invariant that the ctx remains valid after prog
has run. This is relevant b/c ip_defrag() and ip_check_defrag() may
consume the skb if the skb is a fragment.

Instead of doing all that with extra skb copy can you hook bpf prog after
the networking stack already handled ip defrag?
What kind of middle box are you doing? Why does it have to run at TC layer?

Unless I'm missing something, the only other relevant hooks would be
socket hooks, right?

Unfortunately I don't think my use case can do that. We are running the
kernel as a router, so no sockets are involved.

Are you using bpf_fib_lookup and populating kernel routing
table and doing everything on your own including neigh ?

Have you considered to skb redirect to another netdev that does ip defrag?
Like macvlan does it under some conditions. This can be generalized.

Recently Florian proposed to allow calling bpf progs from all existing
netfilter hooks.
You can pretend to local deliver and hook in NF_INET_LOCAL_IN ?
I feel it would be so much cleaner if stack does ip_defrag normally.
The general issue of skb ownership between bpf prog and defrag logic
isn't really solved with skb_copy. It's still an issue.

I do like this series and we would also use it for Cilium case, so +1 on the
tc BPF integration. Today we have in Cilium what Ed [0] hinted in his earlier
mail where we extract information from first fragment and store the meta data
in a BPF map for subsequent packets based on ipid [1], but limitations apply
e.g. service load-balancing won't work. Redirecting to a different device
or moving higher up the stack is cumbersome since we then need to go and
recirculate back into tc BPF layer where all the business logic is located and
handling the regular (non-fragmented) path, too. Wrt skb ownership, can you
elaborate what is a concrete issue exactly? Anything that comes to mind with
this approach that could crash the kernel?

  [0] https://lore.kernel.org/bpf/cf49a091-9b14-05b8-6a79-00e56f3019e1@xxxxxxxxx/
  [1] https://github.com/cilium/cilium/pull/10264



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux