Re: [PATCH bpf-next v5 1/5] net: Add connect_socket() helper

On Fri, Aug 07, 2026 at 06:11:50PM +0000, [email protected] wrote:
> > diff --git a/include/linux/socket.h b/include/linux/socket.h
> > index 2a8d7b14f1d1..5a5eb1250103 100644
> > --- a/include/linux/socket.h
> > +++ b/include/linux/socket.h
> > @@ -461,6 +461,8 @@ extern struct file *__sys_socket_file(int family, int type, int protocol);
> >  extern int __sys_bind(int fd, struct sockaddr __user *umyaddr, int addrlen);
> >  extern int __sys_bind_socket(struct socket *sock, struct sockaddr_storage *address,
> >  			     int addrlen);
> > +int connect_socket(struct socket *sock, struct sockaddr_storage *addr,
> > +		   int addrlen, int flags);
> >  extern int __sys_connect_file(struct file *file, struct sockaddr_storage *addr,
> >  			      int addrlen, int file_flags);
> >  extern int __sys_connect(int fd, struct sockaddr __user *uservaddr,
> > diff --git a/net/socket.c b/net/socket.c
> > index 63c69a0fa74e..8f18f124d92a 100644
> > --- a/net/socket.c
> > +++ b/net/socket.c
> > @@ -2103,6 +2103,20 @@ SYSCALL_DEFINE3(accept, int, fd, struct sockaddr __user *, upeer_sockaddr,
> >  	return __sys_accept4(fd, upeer_sockaddr, upeer_addrlen, 0);
> >  }
> >
> > +int connect_socket(struct socket *sock, struct sockaddr_storage *address,
> > +		   int addrlen, int flags)
> > +{
> 
> This isn't a bug, but would a name closer to the neighbouring helpers,
> such as __sys_connect_socket(), read better here? The equivalent
> socket-level helpers in the same file follow the __sys_<op>_socket() or
> __sys_<op>_sock() shape (__sys_bind_socket(), __sys_listen_socket(),
> __sys_shutdown_sock()), and the declaration is added in the middle of
> that same helpers block in include/linux/socket.h. Also, net/socket.c

The __sys prefix was removed from this comment from Song[^1] which I
think makes sense instead of following the convention around.

[^1]: https://lore.kernel.org/bpf/CAPhsuW5ZTqfzpxNf=S6JbmGxWf2m9qPZGberdzRdLuvv2-Xwgw@mail.gmail.com/

> already has kernel_connect(), which performs the same
> READ_ONCE(sock->ops)->connect() dispatch and differs only in not calling
> the LSM hook, so the two names don't signal which one a caller should
> pick. Could a short comment note how it differs from kernel_connect()?

This is the whole point of this new function indeed.

> > +	int err;
> > +
> > +	err = security_socket_connect(sock, (struct sockaddr *)address, addrlen);
> > +	if (err)
> > +		return err;
> > +
> > +	return READ_ONCE(sock->ops)->connect(sock,
> > +				    (struct sockaddr_unsized *)address,
> > +				    addrlen, flags);
> > +}
> > +
> >  /*
> >   *	Attempt to connect to a socket with the server address.  The address
> >   *	is in user space so we verify it is OK and move it to kernel space.
> 
> [ ... ]
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31202489240