Re: [PATCH bpf-next v6 2/5] bpf: Add ksock kfuncs

[Mahe Tardy <mahe.tardy@gmail.com>]

On Wed, Aug 12, 2026 at 05:39:05PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/net/core/Makefile b/net/core/Makefile
> > index b3fdcb4e355fa..a9295b7859010 100644
> > --- a/net/core/Makefile
> > +++ b/net/core/Makefile
> > @@ -44,6 +44,9 @@ obj-$(CONFIG_FAILOVER) += failover.o
> >  obj-$(CONFIG_NET_SOCK_MSG) += skmsg.o
> >  obj-$(CONFIG_BPF_SYSCALL) += sock_map.o
> >  obj-$(CONFIG_BPF_SYSCALL) += bpf_sk_storage.o
> > +ifneq ($(CONFIG_INET),)
> > +obj-$(CONFIG_BPF_SYSCALL) += bpf_ksock.o
> > +endif
> >  obj-$(CONFIG_OF)	+= of_net.o
> >  obj-$(CONFIG_NET_TEST) += net_test.o
> >  obj-$(CONFIG_NET_DEVMEM) += devmem.o
> 
> This isn't a bug, but would 'ifdef CONFIG_INET' be preferable here to
> match the rest of net/? There are zero occurrences of 'ifneq
> ($(CONFIG_...),)' and fourteen occurrences of 'ifdef CONFIG_...' across
> net/**/Makefile, and this is the only conditional block in
> net/core/Makefile.

whatever is fine by me.

> 
> > diff --git a/net/core/bpf_ksock.c b/net/core/bpf_ksock.c
> > new file mode 100644
> > index 0000000000000..d65200679b65d
> > --- /dev/null
> > +++ b/net/core/bpf_ksock.c
> 
> [ ... ]
> 
> > +/**
> > + * bpf_ksock_connect() - Connect a BPF kernel socket to a remote address.
> > + * @ks:		The BPF kernel socket context.
> > + * @addr:	Pointer to an IPv4 or IPv6 socket address.
> > + * @addr__sz:	Size of the address union.
> > + *
> > + * Connects the socket to the specified remote address and port.
> > + *
> > + * This function may sleep while connecting the socket, so it can only be used
> > + * in sleepable BPF programs (SYSCALL).
> > + *
> > + * Return: 0 on success, negative errno on error.
> > + */
> > +__bpf_kfunc int bpf_ksock_connect(struct bpf_ksock *ks,
> > +				  const union bpf_ksock_addr *addr,
> > +				  u32 addr__sz)
> > +{
> > +	struct sockaddr_storage sa;
> > +	int addrlen;
> > +
> > +	if (!bpf_ksock_has_user_task_context())
> > +		return -EOPNOTSUPP;
> > +
> > +	if (!addr || addr__sz != sizeof(*addr))
> > +		return -EINVAL;
> > +
> > +	/* Kfunc memory arguments may be unaligned. */
> > +	memcpy(&sa, addr, sizeof(*addr));
> > +
> > +	switch (sa.ss_family) {
> > +	case AF_INET:
> > +		addrlen = sizeof(struct sockaddr_in);
> > +		break;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +	case AF_INET6:
> > +		addrlen = sizeof(struct sockaddr_in6);
> > +		break;
> > +#endif
> > +	default:
> > +		return -EAFNOSUPPORT;
> > +	}
> > +
> > +	return connect_socket(ks->sock, &sa, addrlen, 0);
> > +}
> 
> This isn't a bug, but could the #if IS_ENABLED(CONFIG_IPV6) guard be
> dropped? struct sockaddr_in6 is defined unconditionally in
> include/uapi/linux/in6.h, and union bpf_ksock_addr in the new header
> already references it without a guard. bpf_ksock_create() accepts
> opts_copy.family == AF_INET6 unconditionally and relies on sock_create()
> to return -EAFNOSUPPORT when IPv6 is not built. With CONFIG_IPV6=n, the
> net effect is identical (-EAFNOSUPPORT either way), so dropping the guard
> would make bpf_ksock_connect() and bpf_ksock_create() validate families
> the same way.

Yep let's drop this if unecessary.

> 
> [ ... ]
> 
> > +/**
> > + * bpf_ksock_send() - Send data through a BPF kernel socket.
> > + * @ks:		The BPF kernel socket context. Must be an acquired reference.
> > + * @data:	Pointer to the data to send.
> > + * @data__sz:	Size of the data to send (max 65535 bytes).
> > + *
> > + * Sends data on a connected socket, best-effort and nonblocking. This may sleep
> > + * (kernel_sendmsg), so it can only be called from sleepable BPF programs.
> > + *
> > + * Return: Number of bytes sent on success, negative errno on error.
> > + */
> > +__bpf_kfunc int bpf_ksock_send(struct bpf_ksock *ks, const void *data,
> > +			       u32 data__sz)
> > +{
> > +	struct msghdr msg = {
> > +		.msg_flags = MSG_DONTWAIT,
> > +	};
> > +	struct kvec iov = {
> > +		.iov_base = (void *)data,
> > +		.iov_len = data__sz,
> > +	};
> > +	int ret;
> > +
> > +	if (!bpf_ksock_has_user_task_context())
> > +		return -EOPNOTSUPP;
> > +
> > +	/* Early check for UDP. Exact limits enforced by kernel_sendmsg(). */
> > +	if (data__sz > IP_MAX_MTU)
> > +		return -EMSGSIZE;
> > +
> > +	ret = kernel_sendmsg(ks->sock, &msg, &iov, 1, data__sz);
> > +
> > +	return ret;
> > +}
> 
> This isn't a bug, but is the 64K cap intentional for AF_INET6 sockets?
> The early size check uses IP_MAX_MTU, an IPv4-specific constant (0xFFFF),
> even though bpf_ksock_create() also accepts AF_INET6. udp_sendmsg()
> rejects len > 0xFFFF so the bound matches exactly for IPv4, but
> udpv6_sendmsg() allows up to INT_MAX - sizeof(struct udphdr), so AF_INET6
> sockets are silently capped at 64K by this kfunc rather than by the
> protocol. Also, should the comment say udp_sendmsg()/udpv6_sendmsg()
> instead of kernel_sendmsg()? kernel_sendmsg() performs no length
> validation - the limits come from udp_sendmsg()/udpv6_sendmsg() and
> ip_append_data().

I think we can just remove this check for now.

> 
> 
> ---
> 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/31618864886