On Wed, Mar 3, 2021 at 1:35 AM Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote:
>
> On Tue, 2 Mar 2021 at 18:23, Cong Wang <xiyou.wangcong@xxxxxxxxx> wrote:
> >
> > > if the function returned a struct proto * like it does at the moment.
> > > That way we keep sk->sk_prot manipulation confined to the sockmap code
> > > and don't have to copy paste it into every proto.
> >
> > Well, TCP seems too special to do this, as it could call tcp_update_ulp()
> > to update the proto.
>
> I had a quick look, tcp_bpf_update_proto is the only caller of tcp_update_ulp,
> which in turn is the only caller of icsk_ulp_ops->update, which in turn is only
> implemented as tls_update in tls_main.c. Turns out that tls_update
> has another one of these calls:
>
> } else {
> /* Pairs with lockless read in sk_clone_lock(). */
> WRITE_ONCE(sk->sk_prot, p);
> sk->sk_write_space = write_space;
> }
>
> Maybe it looks familiar? :o) I think it would be a worthwhile change.
Yeah, I am not surprised we can change tcp_update_ulp() too, but
why should I bother kTLS when I do not have to? What you suggest
could at most save us a bit of code size, not a big gain. So, I'd keep
its return value as it is, unless you see any other benefits.
BTW, I will rename it to 'psock_update_sk_prot', please let me know
if you have any better names.
Thanks.
