diff mbox series

[net-next,v7,06/12] selftests: ncdevmem: Switch to AF_INET6

Message ID 20241104181430.228682-7-sdf@fomichev.me
State Superseded
Headers show
Series selftests: ncdevmem: Add ncdevmem to ksft | expand

Commit Message

Stanislav Fomichev Nov. 4, 2024, 6:14 p.m. UTC 
Use dualstack socket to support both v4 and v6. v4-mapped-v6 address
can be used to do v4.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 99 ++++++++++++++++++--------
 1 file changed, 71 insertions(+), 28 deletions(-)

Comments

Joe Damato Nov. 4, 2024, 11:55 p.m. UTC | #1 
On Mon, Nov 04, 2024 at 10:14:24AM -0800, Stanislav Fomichev wrote:
> Use dualstack socket to support both v4 and v6. v4-mapped-v6 address
> can be used to do v4.
> 
> Reviewed-by: Mina Almasry <almasrymina@google.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
>  tools/testing/selftests/net/ncdevmem.c | 99 ++++++++++++++++++--------
>  1 file changed, 71 insertions(+), 28 deletions(-)

Reviewed-by: Joe Damato <jdamato@fastly.com>
Jakub Kicinski Nov. 5, 2024, 1:54 a.m. UTC | #2 
On Mon,  4 Nov 2024 10:14:24 -0800 Stanislav Fomichev wrote:
> -static int configure_flow_steering(void)
> +static int configure_flow_steering(struct sockaddr_in6 *server_sin)
>  {
> -	return run_command("sudo ethtool -N %s flow-type tcp4 %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
> +	const char *type = "tcp6";
> +	const char *server_addr;
> +	char buf[256];
> +
> +	inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf));
> +	server_addr = buf;
> +
> +	if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) {
> +		type = "tcp4";
> +		server_addr = strrchr(server_addr, ':') + 1;
> +	}
> +
> +	return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
>  			   ifname,
> +			   type,
>  			   client_ip ? "src-ip" : "",
>  			   client_ip ?: "",
> -			   server_ip,
> +			   server_addr,
>  			   client_ip ? "src-port" : "",
>  			   client_ip ? port : "",
>  			   port, start_queue);

nit: I think this generate a truncation warning, not sure if it's easy
to fix:

ncdevmem.c:259:28: warning: ā€˜%sā€™ directive output may be truncated writing up to 255 bytes into a region of size between 209 and 215 [-Wformat-truncation=]
  259 |         return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
      |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

maybe make buf smaller? šŸ¤”ļø
Stanislav Fomichev Nov. 5, 2024, 3:33 a.m. UTC | #3 
On 11/04, Jakub Kicinski wrote:
> On Mon,  4 Nov 2024 10:14:24 -0800 Stanislav Fomichev wrote:
> > -static int configure_flow_steering(void)
> > +static int configure_flow_steering(struct sockaddr_in6 *server_sin)
> >  {
> > -	return run_command("sudo ethtool -N %s flow-type tcp4 %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
> > +	const char *type = "tcp6";
> > +	const char *server_addr;
> > +	char buf[256];
> > +
> > +	inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf));
> > +	server_addr = buf;
> > +
> > +	if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) {
> > +		type = "tcp4";
> > +		server_addr = strrchr(server_addr, ':') + 1;
> > +	}
> > +
> > +	return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
> >  			   ifname,
> > +			   type,
> >  			   client_ip ? "src-ip" : "",
> >  			   client_ip ?: "",
> > -			   server_ip,
> > +			   server_addr,
> >  			   client_ip ? "src-port" : "",
> >  			   client_ip ? port : "",
> >  			   port, start_queue);
> 
> nit: I think this generate a truncation warning, not sure if it's easy
> to fix:
> 
> ncdevmem.c:259:28: warning: ā€˜%sā€™ directive output may be truncated writing up to 255 bytes into a region of size between 209 and 215 [-Wformat-truncation=]
>   259 |         return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
>       |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> maybe make buf smaller? šŸ¤”ļø

Are you having some extra -W arguments to make it trigger on the build
bot? Wonder why I don't see the warning locally.. (will try to pass
-Wformat-truncation)
Stanislav Fomichev Nov. 5, 2024, 5:46 p.m. UTC | #4 
On 11/04, Stanislav Fomichev wrote:
> On 11/04, Jakub Kicinski wrote:
> > On Mon,  4 Nov 2024 10:14:24 -0800 Stanislav Fomichev wrote:
> > > -static int configure_flow_steering(void)
> > > +static int configure_flow_steering(struct sockaddr_in6 *server_sin)
> > >  {
> > > -	return run_command("sudo ethtool -N %s flow-type tcp4 %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
> > > +	const char *type = "tcp6";
> > > +	const char *server_addr;
> > > +	char buf[256];
> > > +
> > > +	inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf));
> > > +	server_addr = buf;
> > > +
> > > +	if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) {
> > > +		type = "tcp4";
> > > +		server_addr = strrchr(server_addr, ':') + 1;
> > > +	}
> > > +
> > > +	return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
> > >  			   ifname,
> > > +			   type,
> > >  			   client_ip ? "src-ip" : "",
> > >  			   client_ip ?: "",
> > > -			   server_ip,
> > > +			   server_addr,
> > >  			   client_ip ? "src-port" : "",
> > >  			   client_ip ? port : "",
> > >  			   port, start_queue);
> > 
> > nit: I think this generate a truncation warning, not sure if it's easy
> > to fix:
> > 
> > ncdevmem.c:259:28: warning: ā€˜%sā€™ directive output may be truncated writing up to 255 bytes into a region of size between 209 and 215 [-Wformat-truncation=]
> >   259 |         return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
> >       |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > maybe make buf smaller? šŸ¤”ļø
> 
> Are you having some extra -W arguments to make it trigger on the build
> bot? Wonder why I don't see the warning locally.. (will try to pass
> -Wformat-truncation)

It is indeed -Wformat-truncation but only gcc complains about it, not clang.
Setting buf size to 40 calms it down.
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index 0feeca56c049..c4897b2bdc7c 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -242,13 +242,26 @@  static int configure_channels(unsigned int rx, unsigned int tx)
 	return run_command("sudo ethtool -L %s rx %u tx %u", ifname, rx, tx);
 }
 
-static int configure_flow_steering(void)
+static int configure_flow_steering(struct sockaddr_in6 *server_sin)
 {
-	return run_command("sudo ethtool -N %s flow-type tcp4 %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
+	const char *type = "tcp6";
+	const char *server_addr;
+	char buf[256];
+
+	inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf));
+	server_addr = buf;
+
+	if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) {
+		type = "tcp4";
+		server_addr = strrchr(server_addr, ':') + 1;
+	}
+
+	return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
 			   ifname,
+			   type,
 			   client_ip ? "src-ip" : "",
 			   client_ip ?: "",
-			   server_ip,
+			   server_addr,
 			   client_ip ? "src-port" : "",
 			   client_ip ? port : "",
 			   port, start_queue);
@@ -299,13 +312,53 @@  static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd,
 	return -1;
 }
 
+static int enable_reuseaddr(int fd)
+{
+	int opt = 1;
+	int ret;
+
+	ret = setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &opt, sizeof(opt));
+	if (ret)
+		return -errno;
+
+	ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
+	if (ret)
+		return -errno;
+
+	return 0;
+}
+
+static int parse_address(const char *str, int port, struct sockaddr_in6 *sin6)
+{
+	int ret;
+
+	sin6->sin6_family = AF_INET6;
+	sin6->sin6_port = htons(port);
+
+	ret = inet_pton(sin6->sin6_family, str, &sin6->sin6_addr);
+	if (ret != 1) {
+		/* fallback to plain IPv4 */
+		ret = inet_pton(AF_INET, str, &sin6->sin6_addr.s6_addr32[3]);
+		if (ret != 1)
+			return -1;
+
+		/* add ::ffff prefix */
+		sin6->sin6_addr.s6_addr32[0] = 0;
+		sin6->sin6_addr.s6_addr32[1] = 0;
+		sin6->sin6_addr.s6_addr16[4] = 0;
+		sin6->sin6_addr.s6_addr16[5] = 0xffff;
+	}
+
+	return 0;
+}
+
 int do_server(struct memory_buffer *mem)
 {
 	char ctrl_data[sizeof(int) * 20000];
 	struct netdev_queue_id *queues;
 	size_t non_page_aligned_frags = 0;
-	struct sockaddr_in client_addr;
-	struct sockaddr_in server_sin;
+	struct sockaddr_in6 client_addr;
+	struct sockaddr_in6 server_sin;
 	size_t page_aligned_frags = 0;
 	size_t total_received = 0;
 	socklen_t client_addr_len;
@@ -317,9 +370,12 @@  int do_server(struct memory_buffer *mem)
 	int socket_fd;
 	int client_fd;
 	size_t i = 0;
-	int opt = 1;
 	int ret;
 
+	ret = parse_address(server_ip, atoi(port), &server_sin);
+	if (ret < 0)
+		error(1, 0, "parse server address");
+
 	if (reset_flow_steering())
 		error(1, 0, "Failed to reset flow steering\n");
 
@@ -328,7 +384,7 @@  int do_server(struct memory_buffer *mem)
 		error(1, 0, "Failed to configure rss\n");
 
 	/* Flow steer our devmem flows to start_queue */
-	if (configure_flow_steering())
+	if (configure_flow_steering(&server_sin))
 		error(1, 0, "Failed to configure flow steering\n");
 
 	sleep(1);
@@ -349,29 +405,16 @@  int do_server(struct memory_buffer *mem)
 	if (!tmp_mem)
 		error(1, ENOMEM, "malloc failed");
 
-	server_sin.sin_family = AF_INET;
-	server_sin.sin_port = htons(atoi(port));
-
-	ret = inet_pton(server_sin.sin_family, server_ip, &server_sin.sin_addr);
-	if (ret < 0)
-		error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX);
-
-	socket_fd = socket(server_sin.sin_family, SOCK_STREAM, 0);
+	socket_fd = socket(AF_INET6, SOCK_STREAM, 0);
 	if (socket_fd < 0)
 		error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX);
 
-	ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEPORT, &opt,
-			 sizeof(opt));
-	if (ret)
-		error(1, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
-
-	ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEADDR, &opt,
-			 sizeof(opt));
+	ret = enable_reuseaddr(socket_fd);
 	if (ret)
-		error(1, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
+		error(1, errno, "%s: [FAIL, reuseaddr]\n", TEST_PREFIX);
 
 	fprintf(stderr, "binding to address %s:%d\n", server_ip,
-		ntohs(server_sin.sin_port));
+		ntohs(server_sin.sin6_port));
 
 	ret = bind(socket_fd, &server_sin, sizeof(server_sin));
 	if (ret)
@@ -383,16 +426,16 @@  int do_server(struct memory_buffer *mem)
 
 	client_addr_len = sizeof(client_addr);
 
-	inet_ntop(server_sin.sin_family, &server_sin.sin_addr, buffer,
+	inet_ntop(AF_INET6, &server_sin.sin6_addr, buffer,
 		  sizeof(buffer));
 	fprintf(stderr, "Waiting or connection on %s:%d\n", buffer,
-		ntohs(server_sin.sin_port));
+		ntohs(server_sin.sin6_port));
 	client_fd = accept(socket_fd, &client_addr, &client_addr_len);
 
-	inet_ntop(client_addr.sin_family, &client_addr.sin_addr, buffer,
+	inet_ntop(AF_INET6, &client_addr.sin6_addr, buffer,
 		  sizeof(buffer));
 	fprintf(stderr, "Got connection from %s:%d\n", buffer,
-		ntohs(client_addr.sin_port));
+		ntohs(client_addr.sin6_port));
 
 	while (1) {
 		struct iovec iov = { .iov_base = iobuf,