[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200820082408.GE2282@lore-desk>
Date: Thu, 20 Aug 2020 10:24:08 +0200
From: Lorenzo Bianconi <lorenzo@...nel.org>
To: Andrii Nakryiko <andriin@...com>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...com,
daniel@...earbox.net, andrii.nakryiko@...il.com, kernel-team@...com
Subject: Re: [PATCH bpf] bpf: xdp: fix XDP mode when no mode flags specified
> 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@...nel.org>
> Fixes: 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
> Signed-off-by: Andrii Nakryiko <andriin@...com>
Hi Andrii,
Regarding this patch:
Tested-by: Lorenzo Bianconi <lorenzo@...nel.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
>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists