bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: xdp: fix XDP mode when no mode flags specified
@ 2020-08-20  5:28 Andrii Nakryiko
  2020-08-20  8:24 ` Lorenzo Bianconi
  0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2020-08-20  5:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Lorenzo Bianconi

7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
inadvertently changed which XDP mode is assumed when no mode flags are
specified explicitly. Previously, driver mode was preferred, if driver
supported it. If not, generic SKB mode was chosen. That commit changed default
to SKB mode always. This patch fixes the issue and restores the original
logic.

Reported-by: Lorenzo Bianconi <lorenzo@kernel.org>
Fixes: 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 net/core/dev.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index b5d1129d8310..d42c9ea0c3c0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8742,13 +8742,15 @@ struct bpf_xdp_link {
 	int flags;
 };
 
-static enum bpf_xdp_mode dev_xdp_mode(u32 flags)
+static enum bpf_xdp_mode dev_xdp_mode(struct net_device *dev, u32 flags)
 {
 	if (flags & XDP_FLAGS_HW_MODE)
 		return XDP_MODE_HW;
 	if (flags & XDP_FLAGS_DRV_MODE)
 		return XDP_MODE_DRV;
-	return XDP_MODE_SKB;
+	if (flags & XDP_FLAGS_SKB_MODE)
+		return XDP_MODE_SKB;
+	return dev->netdev_ops->ndo_bpf ? XDP_MODE_DRV : XDP_MODE_SKB;
 }
 
 static bpf_op_t dev_xdp_bpf_op(struct net_device *dev, enum bpf_xdp_mode mode)
@@ -8896,7 +8898,7 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
 		return -EINVAL;
 	}
 
-	mode = dev_xdp_mode(flags);
+	mode = dev_xdp_mode(dev, flags);
 	/* can't replace attached link */
 	if (dev_xdp_link(dev, mode)) {
 		NL_SET_ERR_MSG(extack, "Can't replace active BPF XDP link");
@@ -8984,7 +8986,7 @@ static int dev_xdp_detach_link(struct net_device *dev,
 
 	ASSERT_RTNL();
 
-	mode = dev_xdp_mode(link->flags);
+	mode = dev_xdp_mode(dev, link->flags);
 	if (dev_xdp_link(dev, mode) != link)
 		return -EINVAL;
 
@@ -9080,7 +9082,7 @@ static int bpf_xdp_link_update(struct bpf_link *link, struct bpf_prog *new_prog,
 		goto out_unlock;
 	}
 
-	mode = dev_xdp_mode(xdp_link->flags);
+	mode = dev_xdp_mode(xdp_link->dev, xdp_link->flags);
 	bpf_op = dev_xdp_bpf_op(xdp_link->dev, mode);
 	err = dev_xdp_install(xdp_link->dev, mode, bpf_op, NULL,
 			      xdp_link->flags, new_prog);
@@ -9164,7 +9166,7 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		      int fd, int expected_fd, u32 flags)
 {
-	enum bpf_xdp_mode mode = dev_xdp_mode(flags);
+	enum bpf_xdp_mode mode = dev_xdp_mode(dev, flags);
 	struct bpf_prog *new_prog = NULL, *old_prog = NULL;
 	int err;
 
-- 
2.24.1


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

* Re: [PATCH bpf] bpf: xdp: fix XDP mode when no mode flags specified
  2020-08-20  5:28 [PATCH bpf] bpf: xdp: fix XDP mode when no mode flags specified Andrii Nakryiko
@ 2020-08-20  8:24 ` Lorenzo Bianconi
  2020-08-20 21:29   ` Alexei Starovoitov
  0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2020-08-20  8:24 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, andrii.nakryiko, kernel-team

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

> 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
> inadvertently changed which XDP mode is assumed when no mode flags are
> specified explicitly. Previously, driver mode was preferred, if driver
> supported it. If not, generic SKB mode was chosen. That commit changed default
> to SKB mode always. This patch fixes the issue and restores the original
> logic.
> 
> Reported-by: Lorenzo Bianconi <lorenzo@kernel.org>
> Fixes: 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Hi Andrii,

Regarding this patch:

Tested-by: Lorenzo Bianconi <lorenzo@kernel.org>

I found another similar issue (not sure if it is related yet), the program removal is failing:

$ip link show dev eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq state UP mode DEFAULT group default qlen 1024
    link/ether f0:ad:4e:09:6b:57 brd ff:ff:ff:ff:ff:ff
    prog/xdp id 1 tag 3b185187f1855c4c jited 

$ip link set dev eth0 xdp off                                                                                                                                                                                            
Error: XDP program already attached.

Regards,
Lorenzo

> ---
>  net/core/dev.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b5d1129d8310..d42c9ea0c3c0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -8742,13 +8742,15 @@ struct bpf_xdp_link {
>  	int flags;
>  };
>  
> -static enum bpf_xdp_mode dev_xdp_mode(u32 flags)
> +static enum bpf_xdp_mode dev_xdp_mode(struct net_device *dev, u32 flags)
>  {
>  	if (flags & XDP_FLAGS_HW_MODE)
>  		return XDP_MODE_HW;
>  	if (flags & XDP_FLAGS_DRV_MODE)
>  		return XDP_MODE_DRV;
> -	return XDP_MODE_SKB;
> +	if (flags & XDP_FLAGS_SKB_MODE)
> +		return XDP_MODE_SKB;
> +	return dev->netdev_ops->ndo_bpf ? XDP_MODE_DRV : XDP_MODE_SKB;
>  }
>  
>  static bpf_op_t dev_xdp_bpf_op(struct net_device *dev, enum bpf_xdp_mode mode)
> @@ -8896,7 +8898,7 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
>  		return -EINVAL;
>  	}
>  
> -	mode = dev_xdp_mode(flags);
> +	mode = dev_xdp_mode(dev, flags);
>  	/* can't replace attached link */
>  	if (dev_xdp_link(dev, mode)) {
>  		NL_SET_ERR_MSG(extack, "Can't replace active BPF XDP link");
> @@ -8984,7 +8986,7 @@ static int dev_xdp_detach_link(struct net_device *dev,
>  
>  	ASSERT_RTNL();
>  
> -	mode = dev_xdp_mode(link->flags);
> +	mode = dev_xdp_mode(dev, link->flags);
>  	if (dev_xdp_link(dev, mode) != link)
>  		return -EINVAL;
>  
> @@ -9080,7 +9082,7 @@ static int bpf_xdp_link_update(struct bpf_link *link, struct bpf_prog *new_prog,
>  		goto out_unlock;
>  	}
>  
> -	mode = dev_xdp_mode(xdp_link->flags);
> +	mode = dev_xdp_mode(xdp_link->dev, xdp_link->flags);
>  	bpf_op = dev_xdp_bpf_op(xdp_link->dev, mode);
>  	err = dev_xdp_install(xdp_link->dev, mode, bpf_op, NULL,
>  			      xdp_link->flags, new_prog);
> @@ -9164,7 +9166,7 @@ int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>  int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
>  		      int fd, int expected_fd, u32 flags)
>  {
> -	enum bpf_xdp_mode mode = dev_xdp_mode(flags);
> +	enum bpf_xdp_mode mode = dev_xdp_mode(dev, flags);
>  	struct bpf_prog *new_prog = NULL, *old_prog = NULL;
>  	int err;
>  
> -- 
> 2.24.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH bpf] bpf: xdp: fix XDP mode when no mode flags specified
  2020-08-20  8:24 ` Lorenzo Bianconi
@ 2020-08-20 21:29   ` Alexei Starovoitov
  0 siblings, 0 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2020-08-20 21:29 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Andrii Nakryiko, bpf, Network Development, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Kernel Team

On Thu, Aug 20, 2020 at 1:24 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> > 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
> > inadvertently changed which XDP mode is assumed when no mode flags are
> > specified explicitly. Previously, driver mode was preferred, if driver
> > supported it. If not, generic SKB mode was chosen. That commit changed default
> > to SKB mode always. This patch fixes the issue and restores the original
> > logic.
> >
> > Reported-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > Fixes: 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Hi Andrii,
>
> Regarding this patch:
>
> Tested-by: Lorenzo Bianconi <lorenzo@kernel.org>

Applied to bpf tree. Thanks

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

end of thread, other threads:[~2020-08-20 21:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20  5:28 [PATCH bpf] bpf: xdp: fix XDP mode when no mode flags specified Andrii Nakryiko
2020-08-20  8:24 ` Lorenzo Bianconi
2020-08-20 21:29   ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).