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

2024-11-05 Thread Stanislav Fomichev
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.



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

2024-11-04 Thread Stanislav Fomichev
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)



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

2024-11-04 Thread Jakub Kicinski
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? šŸ¤”ļø



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

2024-11-04 Thread Joe Damato
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 
> Signed-off-by: Stanislav Fomichev 
> ---
>  tools/testing/selftests/net/ncdevmem.c | 99 ++
>  1 file changed, 71 insertions(+), 28 deletions(-)

Reviewed-by: Joe Damato