mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf: Pass extack to bpf_xdp_link_attach
@ 2026-10-21 15:24 Leon Hwang
  2026-10-21 16:36 ` bot+bpf-ci
  2026-10-21 16:43 ` Alexei Starovoitov
  0 siblings, 2 replies; 4+ messages in thread
From: Leon Hwang @ 2026-10-21 15:24 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Jesper Dangaard Brouer, Stanislav Fomichev, Kuniyuki Iwashima,
	Hangbin Liu, Martin Karsten, linux-kernel, netdev, Leon Hwang

In order to drop the dependency of bpf_verifier.h in net/core/dev.c, pass
extack to bpf_xdp_link_attach() in link_create(). Afterwards, transfer
the error message from extack to log buffer in link_create().

Drop the stub bpf_log(), because it is not used in net/core/dev.c
anymore when CONFIG_BPF_SYSCALL is not set.

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
Address Jakub's review in the series
"bpf: Add syscall common attrs support for xdp link_create" [1].

Links:
[1] https://lore.kernel.org/bpf/20261020152804.39863-1-leon.hwang@linux.dev/
---
 include/linux/bpf_verifier.h |  5 -----
 include/linux/netdevice.h    |  2 +-
 kernel/bpf/syscall.c         |  8 +++++++-
 net/core/dev.c               | 10 +++-------
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 1d34ece19bcc..5926f13ff43c 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1269,13 +1269,8 @@ __printf(2, 0) void bpf_verifier_vlog(struct bpf_verifier_log *log,
 				      const char *fmt, va_list args);
 __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env,
 					   const char *fmt, ...);
-#ifdef CONFIG_BPF_SYSCALL
 __printf(2, 3) void bpf_log(struct bpf_verifier_log *log,
 			    const char *fmt, ...);
-#else
-__printf(2, 3) static inline void bpf_log(struct bpf_verifier_log *log,
-					  const char *fmt, ...) {}
-#endif
 int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level,
 		  char __user *log_buf, u32 log_size);
 void bpf_vlog_reset(struct bpf_verifier_log *log, u64 new_pos);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4548b0fd824d..f5d6438c4b8f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4418,7 +4418,7 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 				    struct netdev_queue *txq, int *ret);

 int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
-			struct bpf_verifier_log *log);
+			struct netlink_ext_ack *extack);
 u8 dev_xdp_prog_count(struct net_device *dev);
 int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf);
 int dev_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 87cd122d1024..26917a1ecbb2 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -5944,8 +5944,14 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_a
 		break;
 #ifdef CONFIG_NET
 	case BPF_PROG_TYPE_XDP:
-		ret = bpf_xdp_link_attach(attr, prog, log);
+{
+		struct netlink_ext_ack extack = {};
+
+		ret = bpf_xdp_link_attach(attr, prog, &extack);
+		if (extack._msg)
+			bpf_log(log, "%s\n", extack._msg);
 		break;
+}
 	case BPF_PROG_TYPE_SCHED_CLS:
 		if (attr->link_create.attach_type == BPF_TCX_INGRESS ||
 		    attr->link_create.attach_type == BPF_TCX_EGRESS)
diff --git a/net/core/dev.c b/net/core/dev.c
index a142613536b0..d79a589206e1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -97,7 +97,6 @@
 #include <linux/kthread.h>
 #include <linux/bpf.h>
 #include <linux/bpf_trace.h>
-#include <linux/bpf_verifier.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <net/busy_poll.h>
@@ -10773,11 +10772,10 @@ static const struct bpf_link_ops bpf_xdp_link_lops = {
 };

 int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
-			struct bpf_verifier_log *log)
+			struct netlink_ext_ack *extack)
 {
 	struct net *net = current->nsproxy->net_ns;
 	struct bpf_link_primer link_primer;
-	struct netlink_ext_ack extack = {};
 	struct bpf_xdp_link *link;
 	struct net_device *dev;
 	int err, fd;
@@ -10786,7 +10784,7 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
 	dev = dev_get_by_index(net, attr->link_create.target_ifindex);
 	if (!dev) {
 		rtnl_unlock();
-		bpf_log(log, "Invalid target_ifindex.\n");
+		NL_SET_ERR_MSG(extack, "Invalid target_ifindex.");
 		return -EINVAL;
 	}

@@ -10808,15 +10806,13 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
 	}

 	netdev_lock_ops(dev);
-	err = dev_xdp_attach_link(dev, &extack, link);
+	err = dev_xdp_attach_link(dev, extack, link);
 	netdev_unlock_ops(dev);
 	rtnl_unlock();

 	if (err) {
 		link->dev = NULL;
 		bpf_link_cleanup(&link_primer);
-		if (extack._msg)
-			bpf_log(log, "%s\n", extack._msg);
 		goto out_put_dev;
 	}

--
2.55.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf: Pass extack to bpf_xdp_link_attach
  2026-10-21 15:24 [PATCH bpf-next] bpf: Pass extack to bpf_xdp_link_attach Leon Hwang
@ 2026-10-21 16:36 ` bot+bpf-ci
  2026-10-21 16:43 ` Alexei Starovoitov
  1 sibling, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-10-21 16:36 UTC (permalink / raw)
  To: leon.hwang, bpf
  Cc: ast, daniel, john.fastabend, andrii, eddyz87, memxor, martin.lau,
	song, yonghong.song, jolsa, emil, ihor.solodrai, andrew+netdev,
	davem, edumazet, kuba, pabeni, horms, hawk, sdf, kuniyu,
	liuhangbin, mkarsten, linux-kernel, netdev, leon.hwang, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, mason,
	ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 3403 bytes --]

> bpf: Pass extack to bpf_xdp_link_attach

TL;DR:
- link_create() now logs extack message even when XDP attach succeeds (e.g.
  vmxnet3 LRO), changing behaviour; success path doesn't finalize the log
- Nit: braces in column 0 in the new case block

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 87cd122d1024..26917a1ecbb2 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -5944,8 +5944,14 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_a
>  		break;
>  #ifdef CONFIG_NET
>  	case BPF_PROG_TYPE_XDP:
> -		ret = bpf_xdp_link_attach(attr, prog, log);
> +{

This isn't a bug, but the opening and closing braces of this new block scope
sit in column 0 on lines of their own inside the function body.  Could they
follow the usual 'case X: {' form instead?

	case BPF_PROG_TYPE_XDP: {
		struct netlink_ext_ack extack = {};

		ret = bpf_xdp_link_attach(attr, prog, &extack);
		if (extack._msg)
			bpf_log(log, "%s\n", extack._msg);
		break;
	}

A '{' or '}' in column 0 inside a function body can also confuse diff hunk
headers, ctags and vim's [[ motions.

> +		struct netlink_ext_ack extack = {};
> +
> +		ret = bpf_xdp_link_attach(attr, prog, &extack);
> +		if (extack._msg)
> +			bpf_log(log, "%s\n", extack._msg);

Is it intended that this now logs extack._msg regardless of the value of
ret?

Before this patch the copy sat inside the failure branch of
bpf_xdp_link_attach():

	if (err) {
		link->dev = NULL;
		bpf_link_cleanup(&link_primer);
		if (extack._msg)
			bpf_log(log, "%s\n", extack._msg);
		goto out_put_dev;
	}

Some ndo_bpf handlers set extack on a successful install.  For example,
vmxnet3_xdp_set() does:

	if (adapter->netdev->features & NETIF_F_LRO) {
		NL_SET_ERR_MSG_MOD(extack, "LRO is not supported with XDP");
		adapter->netdev->features &= ~NETIF_F_LRO;
	}

and then installs the program and returns 0.  The path is:

__sys_bpf()
  link_create()
    bpf_xdp_link_attach()
      dev_xdp_attach_link()
        dev_xdp_attach()
          dev_xdp_install()
            vmxnet3_xdp()
              vmxnet3_xdp_set()

On that path BPF_LINK_CREATE succeeds and returns a link fd, yet
"vmxnet3: LRO is not supported with XDP\n" is now copied into the user's
log_buf by bpf_verifier_vlog() through copy_to_user().  Before this patch
nothing was written on success.

The success path is also only partly handled.  link_create() calls
bpf_log_attr_finalize() only when ret < 0:

	out:
		if (ret < 0) {
			int err = bpf_log_attr_finalize(&attr_log, log);

so on success the text lands in the user buffer, but log_true_size is never
reported and the rotating-mode buffer is never linearized by
bpf_vlog_finalize().

The commit message only talks about transferring "the error message".  If
the old behaviour is wanted, should the condition be 'if (ret < 0 &&
extack._msg)'?  If logging warnings on success is intended, should the
commit message say so, and should the success path finalize the log as well?

>  		break;
> +}
>  	case BPF_PROG_TYPE_SCHED_CLS:
>  		if (attr->link_create.attach_type == BPF_TCX_INGRESS ||
>  		    attr->link_create.attach_type == BPF_TCX_EGRESS)


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf: Pass extack to bpf_xdp_link_attach
  2026-10-21 15:24 [PATCH bpf-next] bpf: Pass extack to bpf_xdp_link_attach Leon Hwang
  2026-10-21 16:36 ` bot+bpf-ci
@ 2026-10-21 16:43 ` Alexei Starovoitov
  2026-10-21 17:53   ` Alexei Starovoitov
  1 sibling, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2026-10-21 16:43 UTC (permalink / raw)
  To: Leon Hwang, bpf
  Cc: Daniel Borkmann, John Fastabend, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman,
	Jesper Dangaard Brouer, Stanislav Fomichev, Kuniyuki Iwashima,
	Hangbin Liu, Martin Karsten, linux-kernel, netdev

On Wed, Oct 21, 2026 at 08:24 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>  	case BPF_PROG_TYPE_XDP:
> -		ret = bpf_xdp_link_attach(attr, prog, log);
> +{
> +		struct netlink_ext_ack extack = {};
> +
> +		ret = bpf_xdp_link_attach(attr, prog, &extack);
> +		if (extack._msg)
> +			bpf_log(log, "%s\n", extack._msg);
>  		break;
> +}

CI is right. This looks odd,
but instead of CI's suggestion:
        case BPF_PROG_TYPE_XDP: {
                struct netlink_ext_ack extack = {};

do something like below:

sock_map, netkit and nf links will probably need the same when you
get to them. They can take &extack too.

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 1d34ece..5926f13 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1269,13 +1269,8 @@ __printf(2, 0) void bpf_verifier_vlog(struct bpf_verifier_log *log,
 				      const char *fmt, va_list args);
 __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env,
 					   const char *fmt, ...);
-#ifdef CONFIG_BPF_SYSCALL
 __printf(2, 3) void bpf_log(struct bpf_verifier_log *log,
 			    const char *fmt, ...);
-#else
-__printf(2, 3) static inline void bpf_log(struct bpf_verifier_log *log,
-					  const char *fmt, ...) {}
-#endif
 int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level,
 		  char __user *log_buf, u32 log_size);
 void bpf_vlog_reset(struct bpf_verifier_log *log, u64 new_pos);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4548b0f..f5d6438 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4418,7 +4418,7 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 				    struct netdev_queue *txq, int *ret);

 int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
-			struct bpf_verifier_log *log);
+			struct netlink_ext_ack *extack);
 u8 dev_xdp_prog_count(struct net_device *dev);
 int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf);
 int dev_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 87cd122..0ce75ee 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -5865,6 +5865,7 @@ err_put:
 static int link_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_attr *attr_common,
 		       bpfptr_t uattr_common, u32 size_common)
 {
+	struct netlink_ext_ack extack = {};
 	struct bpf_verifier_log *log;
 	struct bpf_log_attr attr_log;
 	struct bpf_prog *prog;
@@ -5944,7 +5945,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_a
 		break;
 #ifdef CONFIG_NET
 	case BPF_PROG_TYPE_XDP:
-		ret = bpf_xdp_link_attach(attr, prog, log);
+		ret = bpf_xdp_link_attach(attr, prog, &extack);
 		break;
 	case BPF_PROG_TYPE_SCHED_CLS:
 		if (attr->link_create.attach_type == BPF_TCX_INGRESS ||
@@ -5978,8 +5979,11 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_a

 out:
 	if (ret < 0) {
-		int err = bpf_log_attr_finalize(&attr_log, log);
+		int err;

+		if (extack._msg)
+			bpf_log(log, "%s\n", extack._msg);
+		err = bpf_log_attr_finalize(&attr_log, log);
 		ret = err ? err : ret;
 		if (prog)
 			bpf_prog_put(prog);
diff --git a/net/core/dev.c b/net/core/dev.c
index a142613..d79a589 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -97,7 +97,6 @@
 #include <linux/kthread.h>
 #include <linux/bpf.h>
 #include <linux/bpf_trace.h>
-#include <linux/bpf_verifier.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <net/busy_poll.h>
@@ -10773,11 +10772,10 @@ static const struct bpf_link_ops bpf_xdp_link_lops = {
 };

 int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
-			struct bpf_verifier_log *log)
+			struct netlink_ext_ack *extack)
 {
 	struct net *net = current->nsproxy->net_ns;
 	struct bpf_link_primer link_primer;
-	struct netlink_ext_ack extack = {};
 	struct bpf_xdp_link *link;
 	struct net_device *dev;
 	int err, fd;
@@ -10786,7 +10784,7 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
 	dev = dev_get_by_index(net, attr->link_create.target_ifindex);
 	if (!dev) {
 		rtnl_unlock();
-		bpf_log(log, "Invalid target_ifindex.\n");
+		NL_SET_ERR_MSG(extack, "Invalid target_ifindex.");
 		return -EINVAL;
 	}

@@ -10808,15 +10806,13 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
 	}

 	netdev_lock_ops(dev);
-	err = dev_xdp_attach_link(dev, &extack, link);
+	err = dev_xdp_attach_link(dev, extack, link);
 	netdev_unlock_ops(dev);
 	rtnl_unlock();

 	if (err) {
 		link->dev = NULL;
 		bpf_link_cleanup(&link_primer);
-		if (extack._msg)
-			bpf_log(log, "%s\n", extack._msg);
 		goto out_put_dev;
 	}


pw-bot: cr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf: Pass extack to bpf_xdp_link_attach
  2026-10-21 16:43 ` Alexei Starovoitov
@ 2026-10-21 17:53   ` Alexei Starovoitov
  0 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2026-10-21 17:53 UTC (permalink / raw)
  To: Leon Hwang, bpf
  Cc: Daniel Borkmann, John Fastabend, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman,
	Jesper Dangaard Brouer, Stanislav Fomichev, Kuniyuki Iwashima,
	Hangbin Liu, Martin Karsten, LKML, Network Development

On Wed, Oct 21, 2026 at 9:43 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
>
> CI is right. This looks odd,
> but instead of CI's suggestion:
>         case BPF_PROG_TYPE_XDP: {
>                 struct netlink_ext_ack extack = {};
>
> do something like below:
>
> sock_map, netkit and nf links will probably need the same when you
> get to them. They can take &extack too.

We don't have time to do this back and forth.
So I dropped the series from bpf-next.
Pls repost the whole set.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-21 17:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-21 15:24 [PATCH bpf-next] bpf: Pass extack to bpf_xdp_link_attach Leon Hwang
2026-10-21 16:36 ` bot+bpf-ci
2026-10-21 16:43 ` Alexei Starovoitov
2026-10-21 17:53   ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®