netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/6] Enhancing selftests/xsk Framework: Maximum and Minimum Ring Configurations
@ 2024-03-15 14:07 Tushar Vyavahare
  2024-03-15 14:07 ` [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam definition to UAPI header Tushar Vyavahare
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Tushar Vyavahare @ 2024-03-15 14:07 UTC (permalink / raw)
  To: bpf
  Cc: netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar, tushar.vyavahare

Please find enclosed a patch set that introduces enhancements and new test
cases to the selftests/xsk framework. These test the robustness and
reliability of AF_XDP across both minimal and maximal ring size
configurations.

While running these tests, a bug [1] was identified when the batch size is
roughly the same as the NIC ring size. This has now been addressed by
Maciej's fix.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=913eda2b08cc49d31f382579e2be34c2709eb789

Patch Summary:

1: Adds the definition of ethtool_ringparam to the UAPI header, a
   necessary step for the subsequent patches to run tests with various
   ring sizes.

2: Modifies the BATCH_SIZE from a constant to a variable, batch_size, to
   support dynamic modification at runtime for testing different hardware
   ring sizes.

3: Implements a function, get_hw_ring_size, to retrieve the current
   maximum interface size and store this information in the hw_ring
   structure.

4: Implements a new function, set_hw_ring_size, which allows for the
   dynamic configuration of the ring size within an interface.

5: Adds a new test case that puts the AF_XDP driver under stress by
   configuring minimal hardware and software ring sizes, verifying its
   functionality under constrained conditions.

6: Enhances the framework by adding a new test case that evaluates the
   maximum ring sizes for AF_XDP, ensuring its reliability under maximum
   ring utilization.

Testing Strategy:

Check the system in extreme scenarios, such as maximum and minimum
configurations. This helps identify and fix any bugs that may occur.

Tushar Vyavahare (6):
  tools/include: add ethtool_ringparam definition to UAPI header
  selftests/xsk: make batch size variable
  selftests/xsk: implement get_hw_ring_size function to retrieve current
    and max interface size
  selftests/xsk: implement set_hw_ring_size function to configure
    interface ring size
  selftests/xsk: test AF_XDP functionality under minimal ring
    configurations
  selftests/xsk: enhance framework with a new test case for AF_XDP under
    max ring sizes

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>

---
 tools/include/uapi/linux/ethtool.h       |  41 ++++++
 tools/testing/selftests/bpf/xskxceiver.c | 155 +++++++++++++++++++++--
 tools/testing/selftests/bpf/xskxceiver.h |  14 +-
 3 files changed, 200 insertions(+), 10 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam definition to UAPI header
  2024-03-15 14:07 [PATCH bpf-next 0/6] Enhancing selftests/xsk Framework: Maximum and Minimum Ring Configurations Tushar Vyavahare
@ 2024-03-15 14:07 ` Tushar Vyavahare
  2024-03-15 17:33   ` Stanislav Fomichev
  2024-03-15 14:07 ` [PATCH bpf-next 2/6] selftests/xsk: make batch size variable Tushar Vyavahare
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Tushar Vyavahare @ 2024-03-15 14:07 UTC (permalink / raw)
  To: bpf
  Cc: netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar, tushar.vyavahare

Introduce the definition for ethtool_ringparam in the UAPI header located
in the include directory. This is needed by the next patches as they run
tests with various ring sizes.

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 tools/include/uapi/linux/ethtool.h | 41 ++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/include/uapi/linux/ethtool.h b/tools/include/uapi/linux/ethtool.h
index 47afae3895ec..570afcd15bca 100644
--- a/tools/include/uapi/linux/ethtool.h
+++ b/tools/include/uapi/linux/ethtool.h
@@ -101,4 +101,45 @@ struct ethtool_drvinfo {
 
 #define ETHTOOL_GDRVINFO	0x00000003
 
+/**
+ * struct ethtool_ringparam - RX/TX ring parameters
+ * @cmd: Command number = %ETHTOOL_GRINGPARAM or %ETHTOOL_SRINGPARAM
+ * @rx_max_pending: Maximum supported number of pending entries per
+ *      RX ring.  Read-only.
+ * @rx_mini_max_pending: Maximum supported number of pending entries
+ *      per RX mini ring.  Read-only.
+ * @rx_jumbo_max_pending: Maximum supported number of pending entries
+ *      per RX jumbo ring.  Read-only.
+ * @tx_max_pending: Maximum supported number of pending entries per
+ *      TX ring.  Read-only.
+ * @rx_pending: Current maximum number of pending entries per RX ring
+ * @rx_mini_pending: Current maximum number of pending entries per RX
+ *      mini ring
+ * @rx_jumbo_pending: Current maximum number of pending entries per RX
+ *      jumbo ring
+ * @tx_pending: Current maximum supported number of pending entries
+ *      per TX ring
+ *
+ * If the interface does not have separate RX mini and/or jumbo rings,
+ * @rx_mini_max_pending and/or @rx_jumbo_max_pending will be 0.
+ *
+ * There may also be driver-dependent minimum values for the number
+ * of entries per ring.
+ */
+
+struct ethtool_ringparam {
+	__u32   cmd;
+	__u32   rx_max_pending;
+	__u32   rx_mini_max_pending;
+	__u32   rx_jumbo_max_pending;
+	__u32   tx_max_pending;
+	__u32   rx_pending;
+	__u32   rx_mini_pending;
+	__u32   rx_jumbo_pending;
+	__u32   tx_pending;
+};
+
+#define ETHTOOL_GRINGPARAM      0x00000010 /* Get ring parameters. */
+#define ETHTOOL_SRINGPARAM      0x00000011 /* Set ring parameters. */
+
 #endif /* _UAPI_LINUX_ETHTOOL_H */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH bpf-next 2/6] selftests/xsk: make batch size variable
  2024-03-15 14:07 [PATCH bpf-next 0/6] Enhancing selftests/xsk Framework: Maximum and Minimum Ring Configurations Tushar Vyavahare
  2024-03-15 14:07 ` [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam definition to UAPI header Tushar Vyavahare
@ 2024-03-15 14:07 ` Tushar Vyavahare
  2024-03-15 14:07 ` [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size function to retrieve current and max interface size Tushar Vyavahare
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Tushar Vyavahare @ 2024-03-15 14:07 UTC (permalink / raw)
  To: bpf
  Cc: netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar, tushar.vyavahare

Convert the constant BATCH_SIZE into a variable named batch_size to allow
dynamic modification at runtime. This is required for the forthcoming
changes to support testing different hardware ring sizes.

While running these tests, a bug [1] was identified when the batch size is
roughly the same as the NIC ring size. This has now been addressed by
Maciej's fix.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=913eda2b08cc49d31f382579e2be34c2709eb789

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 20 +++++++++++---------
 tools/testing/selftests/bpf/xskxceiver.h |  3 ++-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index b1102ee13faa..eaa102c8098b 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -239,7 +239,7 @@ static void enable_busy_poll(struct xsk_socket_info *xsk)
 		       (void *)&sock_opt, sizeof(sock_opt)) < 0)
 		exit_with_error(errno);
 
-	sock_opt = BATCH_SIZE;
+	sock_opt = xsk->batch_size;
 	if (setsockopt(xsk_socket__fd(xsk->xsk), SOL_SOCKET, SO_BUSY_POLL_BUDGET,
 		       (void *)&sock_opt, sizeof(sock_opt)) < 0)
 		exit_with_error(errno);
@@ -439,6 +439,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		for (j = 0; j < MAX_SOCKETS; j++) {
 			memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
 			ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
+			ifobj->xsk_arr[j].batch_size = DEFAULT_BATCH_SIZE;
 			if (i == 0)
 				ifobj->xsk_arr[j].pkt_stream = test->tx_pkt_stream_default;
 			else
@@ -1087,7 +1088,7 @@ static int __receive_pkts(struct test_spec *test, struct xsk_socket_info *xsk)
 			return TEST_CONTINUE;
 	}
 
-	rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
+	rcvd = xsk_ring_cons__peek(&xsk->rx, xsk->batch_size, &idx_rx);
 	if (!rcvd)
 		return TEST_CONTINUE;
 
@@ -1239,7 +1240,8 @@ static int __send_pkts(struct ifobject *ifobject, struct xsk_socket_info *xsk, b
 
 	buffer_len = pkt_get_buffer_len(umem, pkt_stream->max_pkt_len);
 	/* pkts_in_flight might be negative if many invalid packets are sent */
-	if (pkts_in_flight >= (int)((umem_size(umem) - BATCH_SIZE * buffer_len) / buffer_len)) {
+	if (pkts_in_flight >= (int)((umem_size(umem) - xsk->batch_size * buffer_len) /
+	    buffer_len)) {
 		ret = kick_tx(xsk);
 		if (ret)
 			return TEST_FAILURE;
@@ -1249,7 +1251,7 @@ static int __send_pkts(struct ifobject *ifobject, struct xsk_socket_info *xsk, b
 	fds.fd = xsk_socket__fd(xsk->xsk);
 	fds.events = POLLOUT;
 
-	while (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) < BATCH_SIZE) {
+	while (xsk_ring_prod__reserve(&xsk->tx, xsk->batch_size, &idx) < xsk->batch_size) {
 		if (use_poll) {
 			ret = poll(&fds, 1, POLL_TMOUT);
 			if (timeout) {
@@ -1269,10 +1271,10 @@ static int __send_pkts(struct ifobject *ifobject, struct xsk_socket_info *xsk, b
 			}
 		}
 
-		complete_pkts(xsk, BATCH_SIZE);
+		complete_pkts(xsk, xsk->batch_size);
 	}
 
-	for (i = 0; i < BATCH_SIZE; i++) {
+	for (i = 0; i < xsk->batch_size; i++) {
 		struct pkt *pkt = pkt_stream_get_next_tx_pkt(pkt_stream);
 		u32 nb_frags_left, nb_frags, bytes_written = 0;
 
@@ -1280,9 +1282,9 @@ static int __send_pkts(struct ifobject *ifobject, struct xsk_socket_info *xsk, b
 			break;
 
 		nb_frags = pkt_nb_frags(umem->frame_size, pkt_stream, pkt);
-		if (nb_frags > BATCH_SIZE - i) {
+		if (nb_frags > xsk->batch_size - i) {
 			pkt_stream_cancel(pkt_stream);
-			xsk_ring_prod__cancel(&xsk->tx, BATCH_SIZE - i);
+			xsk_ring_prod__cancel(&xsk->tx, xsk->batch_size - i);
 			break;
 		}
 		nb_frags_left = nb_frags;
@@ -1370,7 +1372,7 @@ static int wait_for_tx_completion(struct xsk_socket_info *xsk)
 			return TEST_FAILURE;
 		}
 
-		complete_pkts(xsk, BATCH_SIZE);
+		complete_pkts(xsk, xsk->batch_size);
 	}
 
 	return TEST_PASS;
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index f174df2d693f..425304e52f35 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -44,7 +44,7 @@
 #define MAX_ETH_JUMBO_SIZE 9000
 #define USLEEP_MAX 10000
 #define SOCK_RECONF_CTR 10
-#define BATCH_SIZE 64
+#define DEFAULT_BATCH_SIZE 64
 #define POLL_TMOUT 1000
 #define THREAD_TMOUT 3
 #define DEFAULT_PKT_CNT (4 * 1024)
@@ -91,6 +91,7 @@ struct xsk_socket_info {
 	struct pkt_stream *pkt_stream;
 	u32 outstanding_tx;
 	u32 rxqsize;
+	u32 batch_size;
 	u8 dst_mac[ETH_ALEN];
 	u8 src_mac[ETH_ALEN];
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size function to retrieve current and max interface size
  2024-03-15 14:07 [PATCH bpf-next 0/6] Enhancing selftests/xsk Framework: Maximum and Minimum Ring Configurations Tushar Vyavahare
  2024-03-15 14:07 ` [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam definition to UAPI header Tushar Vyavahare
  2024-03-15 14:07 ` [PATCH bpf-next 2/6] selftests/xsk: make batch size variable Tushar Vyavahare
@ 2024-03-15 14:07 ` Tushar Vyavahare
  2024-03-15 17:40   ` Stanislav Fomichev
  2024-03-15 14:07 ` [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size Tushar Vyavahare
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Tushar Vyavahare @ 2024-03-15 14:07 UTC (permalink / raw)
  To: bpf
  Cc: netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar, tushar.vyavahare

Introduce a new function called get_hw_size that retrieves both the
current and maximum size of the interface and stores this information in
the 'hw_ring' structure.

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 32 ++++++++++++++++++++++++
 tools/testing/selftests/bpf/xskxceiver.h |  8 ++++++
 2 files changed, 40 insertions(+)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index eaa102c8098b..32005bfb9c9f 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -81,6 +81,8 @@
 #include <linux/mman.h>
 #include <linux/netdev.h>
 #include <linux/bitmap.h>
+#include <linux/sockios.h>
+#include <linux/ethtool.h>
 #include <arpa/inet.h>
 #include <net/if.h>
 #include <locale.h>
@@ -95,6 +97,7 @@
 #include <sys/socket.h>
 #include <sys/time.h>
 #include <sys/types.h>
+#include <sys/ioctl.h>
 #include <unistd.h>
 
 #include "xsk_xdp_progs.skel.h"
@@ -409,6 +412,35 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 	}
 }
 
+static int get_hw_ring_size(struct ifobject *ifobj)
+{
+	struct ethtool_ringparam ring_param = {0};
+	struct ifreq ifr = {0};
+	int sockfd;
+
+	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
+	if (sockfd < 0)
+		return errno;
+
+	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
+
+	ring_param.cmd = ETHTOOL_GRINGPARAM;
+	ifr.ifr_data = (char *)&ring_param;
+
+	if (ioctl(sockfd, SIOCETHTOOL, &ifr) < 0) {
+		close(sockfd);
+		return errno;
+	}
+
+	ifobj->ring.default_tx = ring_param.tx_pending;
+	ifobj->ring.default_rx = ring_param.rx_pending;
+	ifobj->ring.max_tx = ring_param.tx_max_pending;
+	ifobj->ring.max_rx = ring_param.rx_max_pending;
+
+	close(sockfd);
+	return 0;
+}
+
 static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			     struct ifobject *ifobj_rx)
 {
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index 425304e52f35..4f58b70fa781 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -114,6 +114,13 @@ struct pkt_stream {
 	bool verbatim;
 };
 
+struct hw_ring {
+	u32 default_tx;
+	u32 default_rx;
+	u32 max_tx;
+	u32 max_rx;
+};
+
 struct ifobject;
 struct test_spec;
 typedef int (*validation_func_t)(struct ifobject *ifobj);
@@ -130,6 +137,7 @@ struct ifobject {
 	struct xsk_xdp_progs *xdp_progs;
 	struct bpf_map *xskmap;
 	struct bpf_program *xdp_prog;
+	struct hw_ring ring;
 	enum test_mode mode;
 	int ifindex;
 	int mtu;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size
  2024-03-15 14:07 [PATCH bpf-next 0/6] Enhancing selftests/xsk Framework: Maximum and Minimum Ring Configurations Tushar Vyavahare
                   ` (2 preceding siblings ...)
  2024-03-15 14:07 ` [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size function to retrieve current and max interface size Tushar Vyavahare
@ 2024-03-15 14:07 ` Tushar Vyavahare
  2024-03-15 15:47   ` Alexei Starovoitov
  2024-03-15 17:44   ` Stanislav Fomichev
  2024-03-15 14:07 ` [PATCH bpf-next 5/6] selftests/xsk: test AF_XDP functionality under minimal ring configurations Tushar Vyavahare
  2024-03-15 14:07 ` [PATCH bpf-next 6/6] selftests/xsk: enhance framework with a new test case for AF_XDP under max ring sizes Tushar Vyavahare
  5 siblings, 2 replies; 15+ messages in thread
From: Tushar Vyavahare @ 2024-03-15 14:07 UTC (permalink / raw)
  To: bpf
  Cc: netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar, tushar.vyavahare

Introduce a new function called set_hw_ring_size that allows for the
dynamic configuration of the ring size within the interface.

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 35 ++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 32005bfb9c9f..aafa78307586 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -441,6 +441,41 @@ static int get_hw_ring_size(struct ifobject *ifobj)
 	return 0;
 }
 
+static int set_hw_ring_size(struct ifobject *ifobj, u32 tx, u32 rx)
+{
+	struct ethtool_ringparam ring_param = {0};
+	struct ifreq ifr = {0};
+	int sockfd, ret;
+	u32 ctr = 0;
+
+	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
+	if (sockfd < 0)
+		return errno;
+
+	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
+
+	ring_param.tx_pending = tx;
+	ring_param.rx_pending = rx;
+
+	ring_param.cmd = ETHTOOL_SRINGPARAM;
+	ifr.ifr_data = (char *)&ring_param;
+
+	while (ctr++ < SOCK_RECONF_CTR) {
+		ret = ioctl(sockfd, SIOCETHTOOL, &ifr);
+		if (!ret)
+			break;
+		/* Retry if it fails */
+		if (ctr >= SOCK_RECONF_CTR) {
+			close(sockfd);
+			return errno;
+		}
+		usleep(USLEEP_MAX);
+	}
+
+	close(sockfd);
+	return 0;
+}
+
 static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			     struct ifobject *ifobj_rx)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH bpf-next 5/6] selftests/xsk: test AF_XDP functionality under minimal ring configurations
  2024-03-15 14:07 [PATCH bpf-next 0/6] Enhancing selftests/xsk Framework: Maximum and Minimum Ring Configurations Tushar Vyavahare
                   ` (3 preceding siblings ...)
  2024-03-15 14:07 ` [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size Tushar Vyavahare
@ 2024-03-15 14:07 ` Tushar Vyavahare
  2024-03-15 14:07 ` [PATCH bpf-next 6/6] selftests/xsk: enhance framework with a new test case for AF_XDP under max ring sizes Tushar Vyavahare
  5 siblings, 0 replies; 15+ messages in thread
From: Tushar Vyavahare @ 2024-03-15 14:07 UTC (permalink / raw)
  To: bpf
  Cc: netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar, tushar.vyavahare

Add a new test case that stresses AF_XDP and the driver by configuring
small hardware and software ring sizes. This verifies that AF_XDP continues
to function properly even with insufficient ring space that could lead to
frequent producer/consumer throttling. The test procedure involves:

1. Set the minimum possible ring configuration(tx 64 and rx 64).
2. Run tests with various batch sizes(1 and 63) to validate the system's
   behavior under different configurations.

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 47 ++++++++++++++++++++++++
 tools/testing/selftests/bpf/xskxceiver.h |  3 ++
 2 files changed, 50 insertions(+)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index aafa78307586..5326ca5c458c 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -476,6 +476,12 @@ static int set_hw_ring_size(struct ifobject *ifobj, u32 tx, u32 rx)
 	return 0;
 }
 
+static int hw_ring_size_reset(struct ifobject *ifobj)
+{
+	return set_hw_ring_size(ifobj, ifobj->ring.default_tx,
+				ifobj->ring.default_rx);
+}
+
 static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			     struct ifobject *ifobj_rx)
 {
@@ -519,6 +525,9 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		}
 	}
 
+	if (ifobj_tx->hw_ring_size_supp)
+		hw_ring_size_reset(ifobj_tx);
+
 	test->ifobj_tx = ifobj_tx;
 	test->ifobj_rx = ifobj_rx;
 	test->current_step = 0;
@@ -530,6 +539,8 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 	test->xskmap_rx = ifobj_rx->xdp_progs->maps.xsk;
 	test->xdp_prog_tx = ifobj_tx->xdp_progs->progs.xsk_def_prog;
 	test->xskmap_tx = ifobj_tx->xdp_progs->maps.xsk;
+	test->ifobj_tx->ring.set_tx = 0;
+	test->ifobj_tx->ring.set_rx = 0;
 }
 
 static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
@@ -1929,6 +1940,16 @@ static int testapp_validate_traffic(struct test_spec *test)
 		return TEST_SKIP;
 	}
 
+	if (ifobj_tx->ring.set_tx) {
+		if (ifobj_tx->hw_ring_size_supp) {
+			return set_hw_ring_size(ifobj_tx, ifobj_tx->ring.set_tx,
+						ifobj_tx->ring.set_rx);
+		} else {
+			ksft_test_result_skip("Changing HW ring size not supported.\n");
+			return TEST_SKIP;
+		}
+	}
+
 	xsk_attach_xdp_progs(test, ifobj_rx, ifobj_tx);
 	return __testapp_validate_traffic(test, ifobj_rx, ifobj_tx);
 }
@@ -2442,6 +2463,23 @@ static int testapp_xdp_metadata_mb(struct test_spec *test)
 	return testapp_xdp_metadata_copy(test);
 }
 
+static int testapp_hw_sw_min_ring_size(struct test_spec *test)
+{
+	int ret;
+
+	test->total_steps = 2;
+	test->ifobj_tx->ring.set_tx = DEFAULT_BATCH_SIZE;
+	test->ifobj_tx->ring.set_rx = DEFAULT_BATCH_SIZE;
+	test->ifobj_rx->xsk->batch_size = 1;
+	ret = testapp_validate_traffic(test);
+	if (ret)
+		return ret;
+
+	/* Set batch size to hw_ring_size - 1 */
+	test->ifobj_rx->xsk->batch_size = DEFAULT_BATCH_SIZE - 1;
+	return testapp_validate_traffic(test);
+}
+
 static void run_pkt_test(struct test_spec *test)
 {
 	int ret;
@@ -2546,6 +2584,7 @@ static const struct test_spec tests[] = {
 	{.name = "ALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_aligned_inv_desc_mb},
 	{.name = "UNALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_unaligned_inv_desc_mb},
 	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
+	{.name = "HW_SW_MIN_RING_SIZE", .test_func = testapp_hw_sw_min_ring_size},
 };
 
 static void print_tests(void)
@@ -2566,6 +2605,7 @@ int main(int argc, char **argv)
 	int modes = TEST_MODE_SKB + 1;
 	struct test_spec test;
 	bool shared_netdev;
+	int ret;
 
 	/* Use libbpf 1.0 API mode */
 	libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
@@ -2603,6 +2643,10 @@ int main(int argc, char **argv)
 			modes++;
 	}
 
+	ret = get_hw_ring_size(ifobj_tx);
+	if (!ret)
+		ifobj_tx->hw_ring_size_supp = true;
+
 	init_iface(ifobj_rx, worker_testapp_validate_rx);
 	init_iface(ifobj_tx, worker_testapp_validate_tx);
 
@@ -2650,6 +2694,9 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (ifobj_tx->hw_ring_size_supp)
+		hw_ring_size_reset(ifobj_tx);
+
 	pkt_stream_delete(tx_pkt_stream_default);
 	pkt_stream_delete(rx_pkt_stream_default);
 	xsk_unload_xdp_programs(ifobj_tx);
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index 4f58b70fa781..21926672f877 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -119,6 +119,8 @@ struct hw_ring {
 	u32 default_rx;
 	u32 max_tx;
 	u32 max_rx;
+	u32 set_tx;
+	u32 set_rx;
 };
 
 struct ifobject;
@@ -154,6 +156,7 @@ struct ifobject {
 	bool unaligned_supp;
 	bool multi_buff_supp;
 	bool multi_buff_zc_supp;
+	bool hw_ring_size_supp;
 };
 
 struct test_spec {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH bpf-next 6/6] selftests/xsk: enhance framework with a new test case for AF_XDP under max ring sizes
  2024-03-15 14:07 [PATCH bpf-next 0/6] Enhancing selftests/xsk Framework: Maximum and Minimum Ring Configurations Tushar Vyavahare
                   ` (4 preceding siblings ...)
  2024-03-15 14:07 ` [PATCH bpf-next 5/6] selftests/xsk: test AF_XDP functionality under minimal ring configurations Tushar Vyavahare
@ 2024-03-15 14:07 ` Tushar Vyavahare
  5 siblings, 0 replies; 15+ messages in thread
From: Tushar Vyavahare @ 2024-03-15 14:07 UTC (permalink / raw)
  To: bpf
  Cc: netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar, tushar.vyavahare

Introduce a test case to evaluate AF_XDP's robustness by pushing hardware
and software ring sizes to their limits. This test ensures AF_XDP's
reliability amidst potential producer/consumer throttling due to maximum
ring utilization. The testing strategy includes:

1. Configuring rings to their maximum allowable sizes.
2. Executing a series of tests across diverse batch sizes to assess system
   performance under varying load conditions.

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 5326ca5c458c..f545b529e404 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -2480,6 +2480,26 @@ static int testapp_hw_sw_min_ring_size(struct test_spec *test)
 	return testapp_validate_traffic(test);
 }
 
+static int testapp_hw_sw_max_ring_size(struct test_spec *test)
+{
+	u32 max_descs = XSK_RING_PROD__DEFAULT_NUM_DESCS * 2;
+	int ret;
+
+	test->total_steps = 2;
+	test->ifobj_tx->ring.set_tx = test->ifobj_tx->ring.max_tx;
+	test->ifobj_tx->ring.set_rx = test->ifobj_tx->ring.max_rx;
+	test->ifobj_rx->umem->num_frames = max_descs;
+	test->ifobj_rx->xsk->rxqsize = max_descs;
+	test->ifobj_rx->xsk->batch_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
+	ret = testapp_validate_traffic(test);
+	if (ret)
+		return ret;
+
+	/* Set batch_size to 4095 */
+	test->ifobj_rx->xsk->batch_size = max_descs - 1;
+	return testapp_validate_traffic(test);
+}
+
 static void run_pkt_test(struct test_spec *test)
 {
 	int ret;
@@ -2585,6 +2605,7 @@ static const struct test_spec tests[] = {
 	{.name = "UNALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_unaligned_inv_desc_mb},
 	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
 	{.name = "HW_SW_MIN_RING_SIZE", .test_func = testapp_hw_sw_min_ring_size},
+	{.name = "HW_SW_MAX_RING_SIZE", .test_func = testapp_hw_sw_max_ring_size},
 };
 
 static void print_tests(void)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size
  2024-03-15 14:07 ` [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size Tushar Vyavahare
@ 2024-03-15 15:47   ` Alexei Starovoitov
  2024-03-19  6:54     ` Vyavahare, Tushar
  2024-03-15 17:44   ` Stanislav Fomichev
  1 sibling, 1 reply; 15+ messages in thread
From: Alexei Starovoitov @ 2024-03-15 15:47 UTC (permalink / raw)
  To: Tushar Vyavahare
  Cc: bpf, Network Development, Björn Töpel, Karlsson,
	Magnus, Fijalkowski, Maciej, Jonathan Lemon, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	Sarkar, Tirthendu

On Fri, Mar 15, 2024 at 7:23 AM Tushar Vyavahare
<tushar.vyavahare@intel.com> wrote:
>
> Introduce a new function called set_hw_ring_size that allows for the
> dynamic configuration of the ring size within the interface.
>
> Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> ---
>  tools/testing/selftests/bpf/xskxceiver.c | 35 ++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 32005bfb9c9f..aafa78307586 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -441,6 +441,41 @@ static int get_hw_ring_size(struct ifobject *ifobj)
>         return 0;
>  }
>
> +static int set_hw_ring_size(struct ifobject *ifobj, u32 tx, u32 rx)
> +{
> +       struct ethtool_ringparam ring_param = {0};
> +       struct ifreq ifr = {0};
> +       int sockfd, ret;
> +       u32 ctr = 0;
> +
> +       sockfd = socket(AF_INET, SOCK_DGRAM, 0);
> +       if (sockfd < 0)
> +               return errno;
> +
> +       memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
> +
> +       ring_param.tx_pending = tx;
> +       ring_param.rx_pending = rx;
> +
> +       ring_param.cmd = ETHTOOL_SRINGPARAM;
> +       ifr.ifr_data = (char *)&ring_param;
> +
> +       while (ctr++ < SOCK_RECONF_CTR) {
> +               ret = ioctl(sockfd, SIOCETHTOOL, &ifr);
> +               if (!ret)
> +                       break;
> +               /* Retry if it fails */
> +               if (ctr >= SOCK_RECONF_CTR) {
> +                       close(sockfd);
> +                       return errno;
> +               }
> +               usleep(USLEEP_MAX);

Does it really have to sleep or copy paste from other places?
This ioctl() is supposed to do synchronous config, no?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam definition to UAPI header
  2024-03-15 14:07 ` [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam definition to UAPI header Tushar Vyavahare
@ 2024-03-15 17:33   ` Stanislav Fomichev
  2024-03-19  6:42     ` Vyavahare, Tushar
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Fomichev @ 2024-03-15 17:33 UTC (permalink / raw)
  To: Tushar Vyavahare
  Cc: bpf, netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar

On 03/15, Tushar Vyavahare wrote:
> Introduce the definition for ethtool_ringparam in the UAPI header located
> in the include directory. This is needed by the next patches as they run
> tests with various ring sizes.

Any reason not to 'cp {,tools/}include/uapi/linux/ethtool.h' instead?
Less divergence should be easier to support/understand.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size function to retrieve current and max interface size
  2024-03-15 14:07 ` [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size function to retrieve current and max interface size Tushar Vyavahare
@ 2024-03-15 17:40   ` Stanislav Fomichev
  2024-03-19  6:47     ` Vyavahare, Tushar
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Fomichev @ 2024-03-15 17:40 UTC (permalink / raw)
  To: Tushar Vyavahare
  Cc: bpf, netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar

On 03/15, Tushar Vyavahare wrote:
> Introduce a new function called get_hw_size that retrieves both the
> current and maximum size of the interface and stores this information in
> the 'hw_ring' structure.
> 
> Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> ---
>  tools/testing/selftests/bpf/xskxceiver.c | 32 ++++++++++++++++++++++++
>  tools/testing/selftests/bpf/xskxceiver.h |  8 ++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index eaa102c8098b..32005bfb9c9f 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -81,6 +81,8 @@
>  #include <linux/mman.h>
>  #include <linux/netdev.h>
>  #include <linux/bitmap.h>
> +#include <linux/sockios.h>
> +#include <linux/ethtool.h>
>  #include <arpa/inet.h>
>  #include <net/if.h>
>  #include <locale.h>
> @@ -95,6 +97,7 @@
>  #include <sys/socket.h>
>  #include <sys/time.h>
>  #include <sys/types.h>
> +#include <sys/ioctl.h>
>  #include <unistd.h>
>  
>  #include "xsk_xdp_progs.skel.h"
> @@ -409,6 +412,35 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  	}
>  }
>  
> +static int get_hw_ring_size(struct ifobject *ifobj)
> +{
> +	struct ethtool_ringparam ring_param = {0};
> +	struct ifreq ifr = {0};
> +	int sockfd;
> +
> +	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
> +	if (sockfd < 0)
> +		return errno;
> +
> +	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
> +
> +	ring_param.cmd = ETHTOOL_GRINGPARAM;
> +	ifr.ifr_data = (char *)&ring_param;
> +
> +	if (ioctl(sockfd, SIOCETHTOOL, &ifr) < 0) {
> +		close(sockfd);
> +		return errno;

close(sockfd) can potentially override the errno. Also, return -errno to
match the other cases where errors are < 0.

> +	}
> +
> +	ifobj->ring.default_tx = ring_param.tx_pending;
> +	ifobj->ring.default_rx = ring_param.rx_pending;
> +	ifobj->ring.max_tx = ring_param.tx_max_pending;
> +	ifobj->ring.max_rx = ring_param.rx_max_pending;
> +
> +	close(sockfd);
> +	return 0;
> +}
> +
>  static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  			     struct ifobject *ifobj_rx)
>  {
> diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> index 425304e52f35..4f58b70fa781 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -114,6 +114,13 @@ struct pkt_stream {
>  	bool verbatim;
>  };
>  
> +struct hw_ring {
> +	u32 default_tx;
> +	u32 default_rx;
> +	u32 max_tx;
> +	u32 max_rx;
> +};
> +
>  struct ifobject;
>  struct test_spec;
>  typedef int (*validation_func_t)(struct ifobject *ifobj);
> @@ -130,6 +137,7 @@ struct ifobject {
>  	struct xsk_xdp_progs *xdp_progs;
>  	struct bpf_map *xskmap;
>  	struct bpf_program *xdp_prog;
> +	struct hw_ring ring;

Any reason not to store ethtool_ringparam directly here? No need to
introduce new hw_ring.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size
  2024-03-15 14:07 ` [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size Tushar Vyavahare
  2024-03-15 15:47   ` Alexei Starovoitov
@ 2024-03-15 17:44   ` Stanislav Fomichev
  2024-03-19  6:52     ` Vyavahare, Tushar
  1 sibling, 1 reply; 15+ messages in thread
From: Stanislav Fomichev @ 2024-03-15 17:44 UTC (permalink / raw)
  To: Tushar Vyavahare
  Cc: bpf, netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel,
	tirthendu.sarkar

On 03/15, Tushar Vyavahare wrote:
> Introduce a new function called set_hw_ring_size that allows for the
> dynamic configuration of the ring size within the interface.
> 
> Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> ---
>  tools/testing/selftests/bpf/xskxceiver.c | 35 ++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 32005bfb9c9f..aafa78307586 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -441,6 +441,41 @@ static int get_hw_ring_size(struct ifobject *ifobj)
>  	return 0;
>  }
>  
> +static int set_hw_ring_size(struct ifobject *ifobj, u32 tx, u32 rx)

Hmm, now that we have set/get, should we put them into
network_helpers.c? These seem pretty generic if you accept
iface_name + ethtool_ringparam in the api.

> +{
> +	struct ethtool_ringparam ring_param = {0};
> +	struct ifreq ifr = {0};
> +	int sockfd, ret;
> +	u32 ctr = 0;
> +
> +	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
> +	if (sockfd < 0)
> +		return errno;
> +
> +	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
> +
> +	ring_param.tx_pending = tx;
> +	ring_param.rx_pending = rx;
> +
> +	ring_param.cmd = ETHTOOL_SRINGPARAM;
> +	ifr.ifr_data = (char *)&ring_param;
> +

[..]

> +	while (ctr++ < SOCK_RECONF_CTR) {

Is it to retry EINTR? Retrying something else doesn't make sense
probably? So maybe do only errno==EINTR cases? Will make it more
generic and not dependent on SOCK_RECONF_CTR.


> +		ret = ioctl(sockfd, SIOCETHTOOL, &ifr);
> +		if (!ret)
> +			break;
> +		/* Retry if it fails */
> +		if (ctr >= SOCK_RECONF_CTR) {
> +			close(sockfd);
> +			return errno;
> +		}

[..]

> +		usleep(USLEEP_MAX);

Same here. Not sure what's the purpose of sleep? Alternatively, maybe
clarify in the commit description what particular error case we want
to retry.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam definition to UAPI header
  2024-03-15 17:33   ` Stanislav Fomichev
@ 2024-03-19  6:42     ` Vyavahare, Tushar
  0 siblings, 0 replies; 15+ messages in thread
From: Vyavahare, Tushar @ 2024-03-19  6:42 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, netdev, bjorn, Karlsson, Magnus, Fijalkowski, Maciej,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel, Sarkar,
	Tirthendu



> -----Original Message-----
> From: Stanislav Fomichev <sdf@google.com>
> Sent: Friday, March 15, 2024 11:04 PM
> To: Vyavahare, Tushar <tushar.vyavahare@intel.com>
> Cc: bpf@vger.kernel.org; netdev@vger.kernel.org; bjorn@kernel.org;
> Karlsson, Magnus <magnus.karlsson@intel.com>; Fijalkowski, Maciej
> <maciej.fijalkowski@intel.com>; jonathan.lemon@gmail.com;
> davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com;
> ast@kernel.org; daniel@iogearbox.net; Sarkar, Tirthendu
> <tirthendu.sarkar@intel.com>
> Subject: Re: [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam
> definition to UAPI header
> 
> On 03/15, Tushar Vyavahare wrote:
> > Introduce the definition for ethtool_ringparam in the UAPI header
> > located in the include directory. This is needed by the next patches
> > as they run tests with various ring sizes.
> 
> Any reason not to 'cp {,tools/}include/uapi/linux/ethtool.h' instead?
> Less divergence should be easier to support/understand.

Sure, I will do it.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size function to retrieve current and max interface size
  2024-03-15 17:40   ` Stanislav Fomichev
@ 2024-03-19  6:47     ` Vyavahare, Tushar
  0 siblings, 0 replies; 15+ messages in thread
From: Vyavahare, Tushar @ 2024-03-19  6:47 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, netdev, bjorn, Karlsson, Magnus, Fijalkowski, Maciej,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel, Sarkar,
	Tirthendu



> -----Original Message-----
> From: Stanislav Fomichev <sdf@google.com>
> Sent: Friday, March 15, 2024 11:11 PM
> To: Vyavahare, Tushar <tushar.vyavahare@intel.com>
> Cc: bpf@vger.kernel.org; netdev@vger.kernel.org; bjorn@kernel.org; Karlsson,
> Magnus <magnus.karlsson@intel.com>; Fijalkowski, Maciej
> <maciej.fijalkowski@intel.com>; jonathan.lemon@gmail.com;
> davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com;
> ast@kernel.org; daniel@iogearbox.net; Sarkar, Tirthendu
> <tirthendu.sarkar@intel.com>
> Subject: Re: [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size
> function to retrieve current and max interface size
> 
> On 03/15, Tushar Vyavahare wrote:
> > Introduce a new function called get_hw_size that retrieves both the
> > current and maximum size of the interface and stores this information
> > in the 'hw_ring' structure.
> >
> > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> > ---
> >  tools/testing/selftests/bpf/xskxceiver.c | 32
> > ++++++++++++++++++++++++  tools/testing/selftests/bpf/xskxceiver.h |
> > 8 ++++++
> >  2 files changed, 40 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c
> > b/tools/testing/selftests/bpf/xskxceiver.c
> > index eaa102c8098b..32005bfb9c9f 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -81,6 +81,8 @@
> >  #include <linux/mman.h>
> >  #include <linux/netdev.h>
> >  #include <linux/bitmap.h>
> > +#include <linux/sockios.h>
> > +#include <linux/ethtool.h>
> >  #include <arpa/inet.h>
> >  #include <net/if.h>
> >  #include <locale.h>
> > @@ -95,6 +97,7 @@
> >  #include <sys/socket.h>
> >  #include <sys/time.h>
> >  #include <sys/types.h>
> > +#include <sys/ioctl.h>
> >  #include <unistd.h>
> >
> >  #include "xsk_xdp_progs.skel.h"
> > @@ -409,6 +412,35 @@ static void parse_command_line(struct ifobject
> *ifobj_tx, struct ifobject *ifobj
> >  	}
> >  }
> >
> > +static int get_hw_ring_size(struct ifobject *ifobj) {
> > +	struct ethtool_ringparam ring_param = {0};
> > +	struct ifreq ifr = {0};
> > +	int sockfd;
> > +
> > +	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
> > +	if (sockfd < 0)
> > +		return errno;
> > +
> > +	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
> > +
> > +	ring_param.cmd = ETHTOOL_GRINGPARAM;
> > +	ifr.ifr_data = (char *)&ring_param;
> > +
> > +	if (ioctl(sockfd, SIOCETHTOOL, &ifr) < 0) {
> > +		close(sockfd);
> > +		return errno;
> 
> close(sockfd) can potentially override the errno. Also, return -errno to match
> the other cases where errors are < 0.
> 

I will do it.

> > +	}
> > +
> > +	ifobj->ring.default_tx = ring_param.tx_pending;
> > +	ifobj->ring.default_rx = ring_param.rx_pending;
> > +	ifobj->ring.max_tx = ring_param.tx_max_pending;
> > +	ifobj->ring.max_rx = ring_param.rx_max_pending;
> > +
> > +	close(sockfd);
> > +	return 0;
> > +}
> > +
> >  static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >  			     struct ifobject *ifobj_rx)
> >  {
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.h
> > b/tools/testing/selftests/bpf/xskxceiver.h
> > index 425304e52f35..4f58b70fa781 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -114,6 +114,13 @@ struct pkt_stream {
> >  	bool verbatim;
> >  };
> >
> > +struct hw_ring {
> > +	u32 default_tx;
> > +	u32 default_rx;
> > +	u32 max_tx;
> > +	u32 max_rx;
> > +};
> > +
> >  struct ifobject;
> >  struct test_spec;
> >  typedef int (*validation_func_t)(struct ifobject *ifobj); @@ -130,6
> > +137,7 @@ struct ifobject {
> >  	struct xsk_xdp_progs *xdp_progs;
> >  	struct bpf_map *xskmap;
> >  	struct bpf_program *xdp_prog;
> > +	struct hw_ring ring;
> 
> Any reason not to store ethtool_ringparam directly here? No need to
> introduce new hw_ring.

I will use ethtool_ringparam directly for get_hw_ring_size().

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size
  2024-03-15 17:44   ` Stanislav Fomichev
@ 2024-03-19  6:52     ` Vyavahare, Tushar
  0 siblings, 0 replies; 15+ messages in thread
From: Vyavahare, Tushar @ 2024-03-19  6:52 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, netdev, bjorn, Karlsson, Magnus, Fijalkowski, Maciej,
	jonathan.lemon, davem, kuba, pabeni, ast, daniel, Sarkar,
	Tirthendu



> -----Original Message-----
> From: Stanislav Fomichev <sdf@google.com>
> Sent: Friday, March 15, 2024 11:14 PM
> To: Vyavahare, Tushar <tushar.vyavahare@intel.com>
> Cc: bpf@vger.kernel.org; netdev@vger.kernel.org; bjorn@kernel.org; Karlsson,
> Magnus <magnus.karlsson@intel.com>; Fijalkowski, Maciej
> <maciej.fijalkowski@intel.com>; jonathan.lemon@gmail.com;
> davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com;
> ast@kernel.org; daniel@iogearbox.net; Sarkar, Tirthendu
> <tirthendu.sarkar@intel.com>
> Subject: Re: [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size
> function to configure interface ring size
> 
> On 03/15, Tushar Vyavahare wrote:
> > Introduce a new function called set_hw_ring_size that allows for the
> > dynamic configuration of the ring size within the interface.
> >
> > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> > ---
> >  tools/testing/selftests/bpf/xskxceiver.c | 35
> > ++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c
> > b/tools/testing/selftests/bpf/xskxceiver.c
> > index 32005bfb9c9f..aafa78307586 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -441,6 +441,41 @@ static int get_hw_ring_size(struct ifobject *ifobj)
> >  	return 0;
> >  }
> >
> > +static int set_hw_ring_size(struct ifobject *ifobj, u32 tx, u32 rx)
> 
> Hmm, now that we have set/get, should we put them into network_helpers.c?
> These seem pretty generic if you accept iface_name + ethtool_ringparam in
> the api.
> 

Clean version of get_hw_ring_size() and set_hw_ring_size() both are moved to network_helpers.c

> > +{
> > +	struct ethtool_ringparam ring_param = {0};
> > +	struct ifreq ifr = {0};
> > +	int sockfd, ret;
> > +	u32 ctr = 0;
> > +
> > +	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
> > +	if (sockfd < 0)
> > +		return errno;
> > +
> > +	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
> > +
> > +	ring_param.tx_pending = tx;
> > +	ring_param.rx_pending = rx;
> > +
> > +	ring_param.cmd = ETHTOOL_SRINGPARAM;
> > +	ifr.ifr_data = (char *)&ring_param;
> > +
> 
> [..]
> 
> > +	while (ctr++ < SOCK_RECONF_CTR) {
> 
> Is it to retry EINTR? Retrying something else doesn't make sense probably? So
> maybe do only errno==EINTR cases? Will make it more generic and not
> dependent on SOCK_RECONF_CTR.
> 
> 

The close of an AF_XDP socket is an asynchronous operation. When an AF_XDP socket is active, changing the ring size is forbidden. Therefore, when we call set_hw_ring_size(), a previous AF_XDP socket might still be in the process of being closed and the ioct() will then fail, as it is forbidden to change the ring size when there is an active AF_XDP socket.

When the AF_XDP socket is active, we are getting an EBUSY error. I will handle the retry logic for this in a separate patch for xskxceiver.c.

> > +		ret = ioctl(sockfd, SIOCETHTOOL, &ifr);
> > +		if (!ret)
> > +			break;
> > +		/* Retry if it fails */
> > +		if (ctr >= SOCK_RECONF_CTR) {
> > +			close(sockfd);
> > +			return errno;
> > +		}
> 
> [..]
> 
> > +		usleep(USLEEP_MAX);
> 
> Same here. Not sure what's the purpose of sleep? Alternatively, maybe clarify
> in the commit description what particular error case we want to retry.

Removed this retry logic from set_hw_ring_size() , which moved to networks_helpers.c.
I will handle the retry logic with sleep for this in a separate patch for xskxceiver.c and I will put this in the commit message.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size
  2024-03-15 15:47   ` Alexei Starovoitov
@ 2024-03-19  6:54     ` Vyavahare, Tushar
  0 siblings, 0 replies; 15+ messages in thread
From: Vyavahare, Tushar @ 2024-03-19  6:54 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: bpf, Network Development, Björn Töpel, Karlsson,
	Magnus, Fijalkowski, Maciej, Jonathan Lemon, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	Sarkar, Tirthendu



> -----Original Message-----
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Sent: Friday, March 15, 2024 9:18 PM
> To: Vyavahare, Tushar <tushar.vyavahare@intel.com>
> Cc: bpf <bpf@vger.kernel.org>; Network Development
> <netdev@vger.kernel.org>; Björn Töpel <bjorn@kernel.org>; Karlsson, Magnus
> <magnus.karlsson@intel.com>; Fijalkowski, Maciej
> <maciej.fijalkowski@intel.com>; Jonathan Lemon
> <jonathan.lemon@gmail.com>; David S. Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Alexei
> Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>;
> Sarkar, Tirthendu <tirthendu.sarkar@intel.com>
> Subject: Re: [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size
> function to configure interface ring size
> 
> On Fri, Mar 15, 2024 at 7:23 AM Tushar Vyavahare
> <tushar.vyavahare@intel.com> wrote:
> >
> > Introduce a new function called set_hw_ring_size that allows for the
> > dynamic configuration of the ring size within the interface.
> >
> > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> > ---
> >  tools/testing/selftests/bpf/xskxceiver.c | 35
> > ++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c
> > b/tools/testing/selftests/bpf/xskxceiver.c
> > index 32005bfb9c9f..aafa78307586 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -441,6 +441,41 @@ static int get_hw_ring_size(struct ifobject *ifobj)
> >         return 0;
> >  }
> >
> > +static int set_hw_ring_size(struct ifobject *ifobj, u32 tx, u32 rx) {
> > +       struct ethtool_ringparam ring_param = {0};
> > +       struct ifreq ifr = {0};
> > +       int sockfd, ret;
> > +       u32 ctr = 0;
> > +
> > +       sockfd = socket(AF_INET, SOCK_DGRAM, 0);
> > +       if (sockfd < 0)
> > +               return errno;
> > +
> > +       memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
> > +
> > +       ring_param.tx_pending = tx;
> > +       ring_param.rx_pending = rx;
> > +
> > +       ring_param.cmd = ETHTOOL_SRINGPARAM;
> > +       ifr.ifr_data = (char *)&ring_param;
> > +
> > +       while (ctr++ < SOCK_RECONF_CTR) {
> > +               ret = ioctl(sockfd, SIOCETHTOOL, &ifr);
> > +               if (!ret)
> > +                       break;
> > +               /* Retry if it fails */
> > +               if (ctr >= SOCK_RECONF_CTR) {
> > +                       close(sockfd);
> > +                       return errno;
> > +               }
> > +               usleep(USLEEP_MAX);
> 
> Does it really have to sleep or copy paste from other places?
> This ioctl() is supposed to do synchronous config, no?

Response in the previous mail  to Stanislav Fomichev <sdf@google.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-03-19  6:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-15 14:07 [PATCH bpf-next 0/6] Enhancing selftests/xsk Framework: Maximum and Minimum Ring Configurations Tushar Vyavahare
2024-03-15 14:07 ` [PATCH bpf-next 1/6] tools/include: add ethtool_ringparam definition to UAPI header Tushar Vyavahare
2024-03-15 17:33   ` Stanislav Fomichev
2024-03-19  6:42     ` Vyavahare, Tushar
2024-03-15 14:07 ` [PATCH bpf-next 2/6] selftests/xsk: make batch size variable Tushar Vyavahare
2024-03-15 14:07 ` [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size function to retrieve current and max interface size Tushar Vyavahare
2024-03-15 17:40   ` Stanislav Fomichev
2024-03-19  6:47     ` Vyavahare, Tushar
2024-03-15 14:07 ` [PATCH bpf-next 4/6] selftests/xsk: implement set_hw_ring_size function to configure interface ring size Tushar Vyavahare
2024-03-15 15:47   ` Alexei Starovoitov
2024-03-19  6:54     ` Vyavahare, Tushar
2024-03-15 17:44   ` Stanislav Fomichev
2024-03-19  6:52     ` Vyavahare, Tushar
2024-03-15 14:07 ` [PATCH bpf-next 5/6] selftests/xsk: test AF_XDP functionality under minimal ring configurations Tushar Vyavahare
2024-03-15 14:07 ` [PATCH bpf-next 6/6] selftests/xsk: enhance framework with a new test case for AF_XDP under max ring sizes Tushar Vyavahare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).