| 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: <ZJx9WkB/dfB5EFjE@boxer>
Date: Wed, 28 Jun 2023 20:35:06 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>
CC: Magnus Karlsson <magnus.karlsson@...il.com>,
Jakub Kicinski <kuba@...nel.org>, <bpf@...r.kernel.org>,
<ast@...nel.org>, <daniel@...earbox.net>, <andrii@...nel.org>,
<netdev@...r.kernel.org>, <magnus.karlsson@...el.com>,
<bjorn@...nel.org>, <tirthendu.sarkar@...el.com>,
<simon.horman@...igine.com>
Subject: Re: [PATCH v4 bpf-next 15/22] xsk: add multi-buffer documentation
On Thu, Jun 22, 2023 at 12:56:31PM +0200, Toke Høiland-Jørgensen wrote:
> Magnus Karlsson <magnus.karlsson@...il.com> writes:
>
> > On Wed, 21 Jun 2023 at 22:34, Jakub Kicinski <kuba@...nel.org> wrote:
> >>
> >> On Wed, 21 Jun 2023 16:15:32 +0200 Magnus Karlsson wrote:
> >> > > Hmm, okay, that sounds pretty tedious :P
> >> >
> >> > Indeed if you had to do it manually ;-). Do not think this max is
> >> > important though, see next answer.
> >>
> >> Can't we add max segs to Lorenzo's XDP info?
> >> include/uapi/linux/netdev.h
> >
> > That should be straight forward. I am just reluctant to add a user
> > interface that might not be necessary.
>
> Yeah, that was why I was asking what the expectations were before
> suggesting adding this to the feature bits :)
>
> However, given that the answer seems to be "it varies"...
>
> > Maciej, how about changing your patch #13 so that we do not add a flag
> > for zc_mb supported or not, but instead we add a flag that gives the
> > user the max number of frags supported in zc mode? A 1 returned would
> > mean that max 1 frag is supported, i.e. mb is not supported. Any
> > number >1 would mean that mb is supported in zc mode for this device
> > and the returned number is the max number of frags supported. This way
> > we would not have to add one more user interface solely for getting
> > the max number of frags supported. What do you think?
>
> ...I think it's a good idea to add the field, and this sounds like a
> reasonable way of dealing with it (although it may need a bit more
> plumbing on the netlink side?)
Okay, here's what I came up with, PTAL, it's on top of the current set but
that should not matter a lot, you'll get the idea of it. I think it's
better to post a diff here and if you guys find it alright then I'll
include this in v5.
diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index ba69c3196980..e41015310a6e 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -42,11 +42,6 @@ definitions:
doc:
This feature informs if netdev implements non-linear XDP buffer
support in ndo_xdp_xmit callback.
- -
- name: zc-sg
- doc:
- This feature informs if netdev implements non-linear XDP buffer
- support in zero-copy mode.
attribute-sets:
-
@@ -67,6 +62,12 @@ attribute-sets:
type: u64
enum: xdp-act
enum-as-flags: true
+ -
+ name: xdp_zc_max_segs
+ doc: max fragment count supported by ZC driver
+ type: u32
+ checks:
+ min: 1
operations:
list:
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index cd562856f23a..f3283e16fae5 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3377,7 +3377,8 @@ static void ice_set_ops(struct ice_vsi *vsi)
netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
NETDEV_XDP_ACT_XSK_ZEROCOPY |
- NETDEV_XDP_ACT_RX_SG | NETDEV_XDP_ACT_ZC_SG;
+ NETDEV_XDP_ACT_RX_SG;
+ netdev->xdp_zc_max_segs = ICE_MAX_BUF_TXD;
}
/**
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 08fbd4622ccf..3078a0879309 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2247,6 +2247,7 @@ struct net_device {
#define GRO_MAX_SIZE (8 * 65535u)
unsigned int gro_max_size;
unsigned int gro_ipv4_max_size;
+ unsigned int xdp_zc_max_segs;
rx_handler_func_t __rcu *rx_handler;
void __rcu *rx_handler_data;
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 1f0bf76dade6..bf71698a1e82 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -24,8 +24,6 @@
* XDP buffer support in the driver napi callback.
* @NETDEV_XDP_ACT_NDO_XMIT_SG: This feature informs if netdev implements
* non-linear XDP buffer support in ndo_xdp_xmit callback.
- * @NETDEV_XDP_ACT_ZC_SG: This feature informs if netdev implements non-linear
- * XDP buffer support in zero-copy mode.
*/
enum netdev_xdp_act {
NETDEV_XDP_ACT_BASIC = 1,
@@ -35,15 +33,15 @@ enum netdev_xdp_act {
NETDEV_XDP_ACT_HW_OFFLOAD = 16,
NETDEV_XDP_ACT_RX_SG = 32,
NETDEV_XDP_ACT_NDO_XMIT_SG = 64,
- NETDEV_XDP_ACT_ZC_SG = 128,
- NETDEV_XDP_ACT_MASK = 255,
+ NETDEV_XDP_ACT_MASK = 127,
};
enum {
NETDEV_A_DEV_IFINDEX = 1,
NETDEV_A_DEV_PAD,
NETDEV_A_DEV_XDP_FEATURES,
+ NETDEV_A_DEV_XDP_ZC_MAX_SEGS,
__NETDEV_A_DEV_MAX,
NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1)
diff --git a/net/core/dev.c b/net/core/dev.c
index 3393c2f3dbe8..ef46e0c622e7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10652,6 +10652,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
dev_net_set(dev, &init_net);
dev->gso_max_size = GSO_LEGACY_MAX_SIZE;
+ dev->xdp_zc_max_segs = 1;
dev->gso_max_segs = GSO_MAX_SEGS;
dev->gro_max_size = GRO_LEGACY_MAX_SIZE;
dev->gso_ipv4_max_size = GSO_LEGACY_MAX_SIZE;
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index a4270fafdf11..b24244f768e3 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -19,6 +19,8 @@ netdev_nl_dev_fill(struct net_device *netdev, struct sk_buff *rsp,
return -EMSGSIZE;
if (nla_put_u32(rsp, NETDEV_A_DEV_IFINDEX, netdev->ifindex) ||
+ nla_put_u32(rsp, NETDEV_A_DEV_XDP_ZC_MAX_SEGS,
+ netdev->xdp_zc_max_segs) ||
nla_put_u64_64bit(rsp, NETDEV_A_DEV_XDP_FEATURES,
netdev->xdp_features, NETDEV_A_DEV_PAD)) {
genlmsg_cancel(rsp, hdr);
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 826709270077..bb035722000e 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -193,8 +193,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
goto err_unreg_pool;
}
- if (!(netdev->xdp_features & NETDEV_XDP_ACT_ZC_SG) &&
- flags & XDP_USE_SG) {
+ if ((netdev->xdp_zc_max_segs == 1) && (flags & XDP_USE_SG)) {
err = -EOPNOTSUPP;
goto err_unreg_pool;
}
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index 1f0bf76dade6..bf71698a1e82 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -24,8 +24,6 @@
* XDP buffer support in the driver napi callback.
* @NETDEV_XDP_ACT_NDO_XMIT_SG: This feature informs if netdev implements
* non-linear XDP buffer support in ndo_xdp_xmit callback.
- * @NETDEV_XDP_ACT_ZC_SG: This feature informs if netdev implements non-linear
- * XDP buffer support in zero-copy mode.
*/
enum netdev_xdp_act {
NETDEV_XDP_ACT_BASIC = 1,
@@ -35,15 +33,15 @@ enum netdev_xdp_act {
NETDEV_XDP_ACT_HW_OFFLOAD = 16,
NETDEV_XDP_ACT_RX_SG = 32,
NETDEV_XDP_ACT_NDO_XMIT_SG = 64,
- NETDEV_XDP_ACT_ZC_SG = 128,
- NETDEV_XDP_ACT_MASK = 255,
+ NETDEV_XDP_ACT_MASK = 127,
};
enum {
NETDEV_A_DEV_IFINDEX = 1,
NETDEV_A_DEV_PAD,
NETDEV_A_DEV_XDP_FEATURES,
+ NETDEV_A_DEV_XDP_ZC_MAX_SEGS,
__NETDEV_A_DEV_MAX,
NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 754da73c643b..fcd36435af36 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1090,6 +1090,7 @@ struct bpf_xdp_query_opts {
__u32 skb_prog_id; /* output */
__u8 attach_mode; /* output */
__u64 feature_flags; /* output */
+ __u32 xdp_zc_max_segs; /* output */
size_t :0;
};
#define bpf_xdp_query_opts__last_field feature_flags
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 84dd5fa14905..c473375d6b7f 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -45,6 +45,7 @@ struct xdp_id_md {
struct xdp_features_md {
int ifindex;
+ __u32 xdp_zc_max_segs;
__u64 flags;
};
@@ -421,6 +422,8 @@ static int parse_xdp_features(struct nlmsghdr *nh, libbpf_dump_nlmsg_t fn,
return NL_CONT;
md->flags = libbpf_nla_getattr_u64(tb[NETDEV_A_DEV_XDP_FEATURES]);
+ md->xdp_zc_max_segs =
+ libbpf_nla_getattr_u32(tb[NETDEV_A_DEV_XDP_ZC_MAX_SEGS]);
return NL_DONE;
}
@@ -493,6 +496,7 @@ int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts)
return libbpf_err(err);
opts->feature_flags = md.flags;
+ opts->xdp_zc_max_segs = md.xdp_zc_max_segs;
skip_feature_flags:
return 0;
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index f5eed27759df..20f5c002e97a 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -2076,7 +2076,7 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
}
if (query_opts.feature_flags & NETDEV_XDP_ACT_RX_SG)
ifobj->multi_buff_supp = true;
- if (query_opts.feature_flags & NETDEV_XDP_ACT_ZC_SG)
+ if (query_opts.xdp_zc_max_segs > 1)
ifobj->multi_buff_zc_supp = true;
}
>
> -Toke
>
Powered by blists - more mailing lists