Daniel Borkmann wrote: > On 9/26/20 6:27 AM, 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. > > > > 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); > > small nit: u32
Sure. > > > + bool shrink = len_diff < 0; > > + int ret = 0; > > + > > + if (unlikely(flags)) > > + return -EINVAL; > > Parameter 'mode' is not used here, I guess we need to reject anything > non-zero? Probably its not used. > > Similarly, any interaction wrt bpf_csum_level() that was needed back then for > the > bpf_skb_adjust_room()? I don't believe so because we are above csum checks at this point. Either we will put the skb data in the receive_queue for the socket or redirect it into sendpage. > > > + if (unlikely(len_diff_abs > 0xfffU)) > > + return -EFAULT; > > + > > + if (!shrink) { > > + unsigned int grow = len_diff; > > nit: u32 or just directly len_diff? Just use len_diff missed when I cleaned this up. > > > + 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. > > + */ > > + 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; > > If skb_cow() failed, we still adjust rxm->full_len? Thanks. Will just return above on error like in the else branch. I'll send a v2 shortly.