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: <20190201134403.44131386@cakuba.hsd1.ca.comcast.net> Date: Fri, 1 Feb 2019 13:44:03 -0800 From: Jakub Kicinski <jakub.kicinski@...ronome.com> To: Daniel Borkmann <daniel@...earbox.net> Cc: Jesper Dangaard Brouer <brouer@...hat.com>, ast@...nel.org, David Miller <davem@...emloft.net>, Maciej Fijalkowski <maciejromanfijalkowski@...il.com>, netdev@...r.kernel.org, john.fastabend@...il.com, David Ahern <dsahern@...il.com>, Saeed Mahameed <saeedm@...lanox.com> Subject: Re: Co-existing XDP generic and native mode? (Re: [PATCH bpf-next v5 5/8] xdp: Provide extack messages when prog attachment failed) On Fri, 1 Feb 2019 22:33:22 +0100, Daniel Borkmann wrote: > On 02/01/2019 07:47 PM, Jakub Kicinski wrote: > >> These are only refactor ideas, so if you can argue why your internal > >> feature request for simultaneous generic and native make more sense, > >> then I'm open for allowing this ? > > > > The request was actually to enable xdpoffload and xdpgeneric at the > > same time. I'm happy to have that as another HW offload exclusive > > for now :) > > The latter is probably fine, though what's the concrete use case? :) I think it was more of a "I expected this to work, since driver+offload worked" than a feature request. Looking back at it it was filed as bug I converted it to feature. > Reason we kept native vs generic separate is mainly so that native XDP > drivers are discouraged to punt missing features to generic hook instead > of properly implementing them in native mode. Agreed, although one could make a counter argument that the performance should be a strong enough incentive and we shouldn't stop people for experimenting and prototyping :) The code change looks simple enough: diff --git a/net/core/dev.c b/net/core/dev.c index 8e276e0192a1..ce4880e5e95d 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -7976,11 +7976,13 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack, enum bpf_netdev_command query; struct bpf_prog *prog = NULL; bpf_op_t bpf_op, bpf_chk; + bool offload; int err; ASSERT_RTNL(); - query = flags & XDP_FLAGS_HW_MODE ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG; + offload = flags & XDP_FLAGS_HW_MODE; + query = offload ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG; bpf_op = bpf_chk = ops->ndo_bpf; if (!bpf_op && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE))) @@ -7991,8 +7993,7 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack, bpf_chk = generic_xdp_install; if (fd >= 0) { - if (__dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG) || - __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG_HW)) + if (!offload && __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) return -EEXIST; if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) && __dev_xdp_query(dev, bpf_op, query)) @@ -8003,8 +8004,7 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack, if (IS_ERR(prog)) return PTR_ERR(prog); - if (!(flags & XDP_FLAGS_HW_MODE) && - bpf_prog_is_dev_bound(prog->aux)) { + if (!offload && bpf_prog_is_dev_bound(prog->aux)) { NL_SET_ERR_MSG(extack, "using device-bound program without HW_MODE flag is not supported"); bpf_prog_put(prog); return -EINVAL; Do you think we shouldn't do it?
Powered by blists - more mailing lists