| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <pj41zla6zl88le.fsf@ua97a68a4e7db56.ant.amazon.com>
Date: Mon, 27 Jul 2020 15:07:25 +0300
From: Shay Agroskin <shayagr@...zon.com>
To: Andrii Nakryiko <andriin@...com>
CC: <bpf@...r.kernel.org>, <netdev@...r.kernel.org>, <ast@...com>,
<daniel@...earbox.net>, <dsahern@...il.com>,
<andrii.nakryiko@...il.com>, <kernel-team@...com>
Subject: Re: [PATCH v4 bpf-next 3/9] bpf, xdp: extract common XDP program attachment logic
Andrii Nakryiko <andriin@...com> writes:
> Further refactor XDP attachment code. dev_change_xdp_fd() is
> split into two
> parts: getting bpf_progs from FDs and attachment logic, working
> with
> bpf_progs. This makes attachment logic a bit more
> straightforward and
> prepares code for bpf_xdp_link inclusion, which will share the
> common logic.
>
> Signed-off-by: Andrii Nakryiko <andriin@...com>
> ---
> net/core/dev.c | 165
> +++++++++++++++++++++++++++----------------------
> 1 file changed, 91 insertions(+), 74 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 7e753e248cef..abf573b2dcf4 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -8815,111 +8815,128 @@ static void dev_xdp_uninstall(struct
> net_device *dev)
> }
> }
>
> -/**
> - * dev_change_xdp_fd - set or clear a bpf program for a
> device rx path
> - * @dev: device
> - * @extack: netlink extended ack
> - * @fd: new program fd or negative value to clear
> - * @expected_fd: old program fd that userspace expects to
> replace or clear
> - * @flags: xdp-related flags
> - *
> - * Set or clear a bpf program for a device
> - */
> -int dev_change_xdp_fd(struct net_device *dev, struct
> netlink_ext_ack *extack,
> - int fd, int expected_fd, u32 flags)
> +static int dev_xdp_attach(struct net_device *dev, struct
> netlink_ext_ack *extack,
> + struct bpf_prog *new_prog, struct
> bpf_prog *old_prog,
> + u32 flags)
> {
> - const struct net_device_ops *ops = dev->netdev_ops;
> - enum bpf_xdp_mode mode = dev_xdp_mode(flags);
> - bool offload = mode == XDP_MODE_HW;
> - u32 prog_id, expected_id = 0;
> - struct bpf_prog *prog;
> + struct bpf_prog *cur_prog;
> + enum bpf_xdp_mode mode;
> bpf_op_t bpf_op;
> int err;
>
> ASSERT_RTNL();
>
> - bpf_op = dev_xdp_bpf_op(dev, mode);
> - if (!bpf_op) {
> - NL_SET_ERR_MSG(extack, "underlying driver does not
> support XDP in native mode");
> - return -EOPNOTSUPP;
> + /* just one XDP mode bit should be set, zero defaults to
> SKB mode */
> + if (hweight32(flags & XDP_FLAGS_MODES) > 1) {
Not sure if it's more efficient but running
if ((flags & XDP) & ((flags & XDP) - 1) != 0)
returns whether a number is a multiple of 2.
Should be equivalent to what you checked with hweight32. It is
less readable though. Just thought I'd throw that in.
Taken from
https://graphics.stanford.edu/~seander/bithacks.html#DetermineIfPowerOf2
> + NL_SET_ERR_MSG(extack, "Only one XDP mode flag can
> be set");
> + return -EINVAL;
> + }
> + /* old_prog != NULL implies XDP_FLAGS_REPLACE is set */
> + if (old_prog && !(flags & XDP_FLAGS_REPLACE)) {
> + NL_SET_ERR_MSG(extack, "XDP_FLAGS_REPLACE is not
> specified");
> + return -EINVAL;
> }
>
> - prog_id = dev_xdp_prog_id(dev, mode);
> - if (flags & XDP_FLAGS_REPLACE) {
> - if (expected_fd >= 0) {
> - prog = bpf_prog_get_type_dev(expected_fd,
> -
> BPF_PROG_TYPE_XDP,
> - bpf_op ==
> ops->ndo_bpf);
> - if (IS_ERR(prog))
> - return PTR_ERR(prog);
> - expected_id = prog->aux->id;
> - bpf_prog_put(prog);
> - }
> -
> - if (prog_id != expected_id) {
> - NL_SET_ERR_MSG(extack, "Active program
> does not match expected");
> - return -EEXIST;
> - }
> + mode = dev_xdp_mode(flags);
> + cur_prog = dev_xdp_prog(dev, mode);
> + if ((flags & XDP_FLAGS_REPLACE) && cur_prog != old_prog) {
> + NL_SET_ERR_MSG(extack, "Active program does not
> match expected");
> + return -EEXIST;
> }
> - if (fd >= 0) {
> + if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) && cur_prog) {
> + NL_SET_ERR_MSG(extack, "XDP program already
> attached");
> + return -EBUSY;
> + }
> +
> + if (new_prog) {
> + bool offload = mode == XDP_MODE_HW;
> enum bpf_xdp_mode other_mode = mode ==
> XDP_MODE_SKB
> ? XDP_MODE_DRV :
> XDP_MODE_SKB;
>
> - if (!offload && dev_xdp_prog_id(dev, other_mode))
> {
> + if (!offload && dev_xdp_prog(dev, other_mode)) {
> NL_SET_ERR_MSG(extack, "Native and generic
> XDP can't be active at the same time");
> return -EEXIST;
> }
> -
> - if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) &&
> prog_id) {
> - NL_SET_ERR_MSG(extack, "XDP program
> already attached");
> - return -EBUSY;
> - }
> -
> - prog = bpf_prog_get_type_dev(fd,
> BPF_PROG_TYPE_XDP,
> - bpf_op ==
> ops->ndo_bpf);
> - if (IS_ERR(prog))
> - return PTR_ERR(prog);
> -
> - if (!offload && bpf_prog_is_dev_bound(prog->aux))
> {
> + if (!offload &&
> bpf_prog_is_dev_bound(new_prog->aux)) {
> NL_SET_ERR_MSG(extack, "Using device-bound
> program without HW_MODE flag is not supported");
> - bpf_prog_put(prog);
> return -EINVAL;
> }
> -
> - if (prog->expected_attach_type == BPF_XDP_DEVMAP)
> {
> + if (new_prog->expected_attach_type ==
> BPF_XDP_DEVMAP) {
> NL_SET_ERR_MSG(extack, "BPF_XDP_DEVMAP
> programs can not be attached to a device");
> - bpf_prog_put(prog);
> return -EINVAL;
> }
> -
> - if (prog->expected_attach_type == BPF_XDP_CPUMAP)
> {
> - NL_SET_ERR_MSG(extack,
> - "BPF_XDP_CPUMAP programs
> can not be attached to a device");
> - bpf_prog_put(prog);
> + if (new_prog->expected_attach_type ==
> BPF_XDP_CPUMAP) {
> + NL_SET_ERR_MSG(extack, "BPF_XDP_CPUMAP
> programs can not be attached to a device");
> return -EINVAL;
> }
> + }
>
> - /* prog->aux->id may be 0 for orphaned
> device-bound progs */
> - if (prog->aux->id && prog->aux->id == prog_id) {
> - bpf_prog_put(prog);
> - return 0;
> + /* don't call drivers if the effective program didn't
> change */
> + if (new_prog != cur_prog) {
> + bpf_op = dev_xdp_bpf_op(dev, mode);
> + if (!bpf_op) {
> + NL_SET_ERR_MSG(extack, "Underlying driver
> does not support XDP in native mode");
> + return -EOPNOTSUPP;
> }
> - } else {
> - if (!prog_id)
> - return 0;
> - prog = NULL;
> - }
>
> - err = dev_xdp_install(dev, mode, bpf_op, extack, flags,
> prog);
> - if (err < 0 && prog) {
> - bpf_prog_put(prog);
> - return err;
> + err = dev_xdp_install(dev, mode, bpf_op, extack,
> flags, new_prog);
> + if (err)
> + return err;
> }
> - dev_xdp_set_prog(dev, mode, prog);
> +
> + dev_xdp_set_prog(dev, mode, new_prog);
> + if (cur_prog)
> + bpf_prog_put(cur_prog);
>
> return 0;
> }
>
> +/**
> + * dev_change_xdp_fd - set or clear a bpf program for a
> device rx path
> + * @dev: device
> + * @extack: netlink extended ack
> + * @fd: new program fd or negative value to clear
> + * @expected_fd: old program fd that userspace expects to
> replace or clear
> + * @flags: xdp-related flags
> + *
> + * Set or clear a bpf program for a device
> + */
> +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);
> + struct bpf_prog *new_prog = NULL, *old_prog = NULL;
> + int err;
> +
> + ASSERT_RTNL();
> +
> + if (fd >= 0) {
> + new_prog = bpf_prog_get_type_dev(fd,
> BPF_PROG_TYPE_XDP,
> + mode !=
> XDP_MODE_SKB);
> + if (IS_ERR(new_prog))
> + return PTR_ERR(new_prog);
> + }
> +
> + if (expected_fd >= 0) {
> + old_prog = bpf_prog_get_type_dev(expected_fd,
> BPF_PROG_TYPE_XDP,
> + mode !=
> XDP_MODE_SKB);
> + if (IS_ERR(old_prog)) {
> + err = PTR_ERR(old_prog);
> + old_prog = NULL;
> + goto err_out;
> + }
> + }
> +
> + err = dev_xdp_attach(dev, extack, new_prog, old_prog,
> flags);
> +
> +err_out:
> + if (err && new_prog)
> + bpf_prog_put(new_prog);
> + if (old_prog)
> + bpf_prog_put(old_prog);
> + return err;
> +}
> +
> /**
> * dev_new_index - allocate an ifindex
> * @net: the applicable net namespace
Powered by blists - more mailing lists