On Sat, Sep 26, 2020 at 06:27 AM CEST, John Fastabend wrote: > This implements a new helper skb_adjust_room() so users can push/pop > extra bytes from a BPF_SK_SKB_STREAM_VERDICT program. > > Some protocols may include headers and other information that we may > not want to include when doing a redirect from a BPF_SK_SKB_STREAM_VERDICT > program. One use case is to redirect TLS packets into a receive socket > that doesn't expect TLS data. In TLS case the first 13B or so contain the > protocol header. With KTLS the payload is decrypted so we should be able > to redirect this to a receiving socket, but the receiving socket may not > be expecting to receive a TLS header and discard the data. Using the > above helper we can pop the header off and put an appropriate header on > the payload. This allows for creating a proxy between protocols without > extra hops through the stack or userspace.
This is useful stuff. Apart from the TLS use-case, you might want to pop off proxy headers like PROXY v1/v2 (CC Marek): https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt > > So in order to fix this case add skb_adjust_room() so users can strip the > header. After this the user can strip the header and an unmodified receiver > thread will work correctly when data is redirected into the ingress path > of a sock. > > Signed-off-by: John Fastabend <john.fastab...@gmail.com> > --- > net/core/filter.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/net/core/filter.c b/net/core/filter.c > index 4d8dc7a31a78..d232358f1dcd 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -76,6 +76,7 @@ > #include <net/bpf_sk_storage.h> > #include <net/transp_v6.h> > #include <linux/btf_ids.h> > +#include <net/tls.h> > > static const struct bpf_func_proto * > bpf_sk_base_func_proto(enum bpf_func_id func_id); > @@ -3218,6 +3219,53 @@ static u32 __bpf_skb_max_len(const struct sk_buff *skb) > SKB_MAX_ALLOC; > } > > +BPF_CALL_4(sk_skb_adjust_room, struct sk_buff *, skb, s32, len_diff, > + u32, mode, u64, flags) > +{ > + unsigned int len_diff_abs = abs(len_diff); > + bool shrink = len_diff < 0; > + int ret = 0; > + > + if (unlikely(flags)) > + return -EINVAL; > + if (unlikely(len_diff_abs > 0xfffU)) > + return -EFAULT; > + > + if (!shrink) { > + unsigned int grow = len_diff; > + > + ret = skb_cow(skb, grow); > + if (likely(!ret)) { > + __skb_push(skb, len_diff_abs); > + memset(skb->data, 0, len_diff_abs); > + } > + } else { > + /* skb_ensure_writable() is not needed here, as we're > + * already working on an uncloned skb. > + */ I'm trying to digest the above comment. What if: static int __strp_recv(…) { … while (eaten < orig_len) { /* Always clone since we will consume something */ skb = skb_clone(orig_skb, GFP_ATOMIC); … head = strp->skb_head; if (!head) { head = skb; … } else { … } … /* Give skb to upper layer */ strp->cb.rcv_msg(strp, head); // → sk_psock_init_strp … } … } That looks like a code path where we pass a cloned SKB. > + if (unlikely(!pskb_may_pull(skb, len_diff_abs))) > + return -ENOMEM; > + __skb_pull(skb, len_diff_abs); > + } > + bpf_compute_data_end_sk_skb(skb); > + if (tls_sw_has_ctx_rx(skb->sk)) { > + struct strp_msg *rxm = strp_msg(skb); > + > + rxm->full_len += len_diff; > + } > + return ret; > +} [...]
