From: Lorenzo Bianconi <lorenzo@kernel.org>
To: Andrii Nakryiko <andriin@fb.com>
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, ast@fb.com,
daniel@iogearbox.net, andrii.nakryiko@gmail.com,
kernel-team@fb.com
Subject: Re: [PATCH bpf] bpf: xdp: fix XDP mode when no mode flags specified
Date: Thu, 20 Aug 2020 10:24:08 +0200 [thread overview]
Message-ID: <20200820082408.GE2282@lore-desk> (raw)
In-Reply-To: <20200820052841.1559757-1-andriin@fb.com>
[-- 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 --]
next prev parent reply other threads:[~2020-08-20 8:24 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2020-08-20 21:29 ` Alexei Starovoitov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200820082408.GE2282@lore-desk \
--to=lorenzo@kernel.org \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).