|SUCCESS| [v2] net/ark: use standard IPv4 address parser

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164962

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-04

This patch replaces a hand-rolled IPv4 address parser with the standard
`inet_pton()` function in the ARK driver's pktgen and pktchkr modules.

The previous parser used `sscanf()` with "%u" format, which could accept
values above 255 and produce incorrect register values. The new
implementation properly validates IPv4 addresses.

However, there are a few concerns:

> diff --git a/drivers/net/ark/ark_pktchkr.c b/drivers/net/ark/ark_pktchkr.c
> index e1f336c73c..67405ac18a 100644
> --- a/drivers/net/ark/ark_pktchkr.c
> +++ b/drivers/net/ark/ark_pktchkr.c
> @@ -4,9 +4,11 @@
>  
>  #include <stdlib.h>
>  #include <unistd.h>
> +#include <arpa/inet.h>

Can this code compile on Windows? The header arpa/inet.h is POSIX-specific
and not available on Windows platforms. DPDK needs to support cross-platform
builds.

>  
>  #include <rte_string_fns.h>
>  #include <rte_malloc.h>
> +#include <rte_byteorder.h>
>  
>  #include "ark_pktchkr.h"
>  #include "ark_logs.h"
> @@ -374,12 +376,11 @@ static int32_t parse_ipv4_string(char const *ip_address);
>  static int32_t
>  parse_ipv4_string(char const *ip_address)
>  {
> -	unsigned int ip[4];
> +	struct in_addr addr;
>  
> -	if (sscanf(ip_address, "%u.%u.%u.%u",
> -		   &ip[0], &ip[1], &ip[2], &ip[3]) != 4)
> +	if (inet_pton(AF_INET, ip_address, &addr) != 1)
>  		return 0;

Does this error handling properly report the failure? When inet_pton()
returns 0 (invalid format) or -1 (system error), this function returns 0,
which may be indistinguishable from a valid address like "0.0.0.0". Should
this return a negative error code instead to clearly indicate failure?

Additionally, when inet_pton() returns -1 due to a system error (not just
invalid format), should errno be checked or logged?

> -	return ip[3] + ip[2] * 0x100 + ip[1] * 0x10000ul + ip[0] * 0x1000000ul;
> +	return rte_be_to_cpu_32(addr.s_addr);
>  }

[ ... ]

> diff --git a/drivers/net/ark/ark_pktgen.c b/drivers/net/ark/ark_pktgen.c
> index 69ff7072b2..05d5773d44 100644
> --- a/drivers/net/ark/ark_pktgen.c
> +++ b/drivers/net/ark/ark_pktgen.c
> @@ -4,9 +4,11 @@
>  
>  #include <stdlib.h>
>  #include <unistd.h>
> +#include <arpa/inet.h>

Same portability concern here - can this compile on Windows?

>  
>  #include <rte_string_fns.h>
>  #include <rte_malloc.h>
> +#include <rte_byteorder.h>
>  #include <rte_thread.h>
>  
>  #include "ark_pktgen.h"
> @@ -355,12 +357,11 @@ static int32_t parse_ipv4_string(char const *ip_address);
>  static int32_t
>  parse_ipv4_string(char const *ip_address)
>  {
> -	unsigned int ip[4];
> +	struct in_addr addr;
>  
> -	if (sscanf(ip_address, "%u.%u.%u.%u",
> -		   &ip[0], &ip[1], &ip[2], &ip[3]) != 4)
> +	if (inet_pton(AF_INET, ip_address, &addr) != 1)
>  		return 0;

Same error handling question - should this return a negative value to
distinguish errors from valid address "0.0.0.0"?

> -	return ip[3] + ip[2] * 0x100 + ip[1] * 0x10000ul + ip[0] * 0x1000000ul;
> +	return rte_be_to_cpu_32(addr.s_addr);
>  }