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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ