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?
