Re: [PATCHv3 bpf-next 0/5] Add bpf_sk_assign eBPF helper

[Joe Stringer <joe@wand.net.nz>]

On Fri, Mar 27, 2020 at 7:14 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2020-03-27 12:25 a.m., Joe Stringer wrote:
> > Introduce a new helper that allows assigning a previously-found socket
> > to the skb as the packet is received towards the stack, to cause the
> > stack to guide the packet towards that socket subject to local routing
> > configuration. The intention is to support TProxy use cases more
> > directly from eBPF programs attached at TC ingress, to simplify and
> > streamline Linux stack configuration in scale environments with Cilium.
> >
> > Normally in ip{,6}_rcv_core(), the skb will be orphaned, dropping any
> > existing socket reference associated with the skb. Existing tproxy
> > implementations in netfilter get around this restriction by running the
> > tproxy logic after ip_rcv_core() in the PREROUTING table. However, this
> > is not an option for TC-based logic (including eBPF programs attached at
> > TC ingress).
> >
> > This series introduces the BPF helper bpf_sk_assign() to associate the
> > socket with the skb on the ingress path as the packet is passed up the
> > stack. The initial patch in the series simply takes a reference on the
> > socket to ensure safety, but later patches relax this for listen
> > sockets.
> >
> > To ensure delivery to the relevant socket, we still consult the routing
> > table, for full examples of how to configure see the tests in patch #5;
> > the simplest form of the route would look like this:
> >
> >    $ ip route add local default dev lo
> >
>
> Trying to understand so if we can port our tc action (and upstream),
> we would need to replicate:
>
>   bpf_sk_assign() - invoked everytime we succeed finding the sk
>   bpf_sk_release() - invoked everytime we are done processing the sk

The skb->destructor = sock_pfree() is the balanced other half of
bpf_sk_assign(), so you shouldn't need to explicitly call
bpf_sk_release() to handle the refcounting of the assigned socket.

The `bpf_sk_release()` pairs with BPF socket lookup, so if you already
have other socket lookup code handling the core tproxy logic (looking
up established, then looking up listen sockets with different tuple)
then you're presumably already handling that to avoid leaking
references.

I think that looking at the test_sk_assign.c BPF program in patch 4/5
should give you a good sense for what you'd need in the TC action
logic.