netdev - Re: [RFC PATCH v4 bpf-next 05/12] bpf: tcp: Add bpf_skops

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 20 Aug 2020 11:52:30 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     unlisted-recipients:; (no To-header on input)
CC:     <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Eric Dumazet <edumazet@...gle.com>, <kernel-team@...com>,
        Lawrence Brakmo <brakmo@...com>,
        Neal Cardwell <ncardwell@...gle.com>, <netdev@...r.kernel.org>,
        Yuchung Cheng <ycheng@...gle.com>
Subject: Re: [RFC PATCH v4 bpf-next 05/12] bpf: tcp: Add
 bpf_skops_established()

On Mon, Aug 03, 2020 at 06:28:04PM -0700, John Fastabend wrote:
> Martin KaFai Lau wrote:
> > In tcp_init_transfer(), it currently calls the bpf prog to give it a
> > chance to handle the just "ESTABLISHED" event (e.g. do setsockopt
> > on the newly established sk).  Right now, it is done by calling the
> > general purpose tcp_call_bpf().
> > 
> > In the later patch, it also needs to pass the just-received skb which
> > concludes the 3 way handshake. E.g. the SYNACK received at the active side.
> > The bpf prog can then learn some specific header options written by the
> > peer's bpf-prog and potentially do setsockopt on the newly established sk.
> > Thus, instead of reusing the general purpose tcp_call_bpf(), a new function
> > bpf_skops_established() is added to allow passing the "skb" to the bpf prog.
> > The actual skb passing from bpf_skops_established() to the bpf prog
> > will happen together in a later patch which has the necessary bpf pieces.
> > 
> > A "skb" arg is also added to tcp_init_transfer() such that
> > it can then be passed to bpf_skops_established().
> > 
> > Calling the new bpf_skops_established() instead of tcp_call_bpf()
> > should be a noop in this patch.
> 
> Yep, looks like a noop.
> 
> > 
> > Signed-off-by: Martin KaFai Lau <kafai@...com>
> 
> Acked-by: John Fastabend <john.fastabend@...il.com>
> 
> [...]
> 
> >  
> > +#ifdef CONFIG_CGROUP_BPF
> > +static void bpf_skops_established(struct sock *sk, int bpf_op,
> > +				  struct sk_buff *skb)
> 
> 
> Small nit because its an RFC anyways.
> 
> Should we call this bpf_skops_fullsock(...) instead? Just a suggestion.
I prefer to stay with the current suffix "_established" which names after the
sock_ops->op that it is calling.  I think it is understood that established
sk is a fullsock.