Re: [PATCH v6 13/18] inany: Prepare inany.[ch] for sharing with pesto tool

[Laurent Vivier <lvivier@redhat.com>]

On 5/3/26 23:55, Stefano Brivio wrote:
> From: David Gibson <david@gibson.dropbear.id.au>
> 
> inany contains a number of helpful functions for dealing with addresses
> which might be IPv4 or IPv6.  We're going to want to use that in pesto.
> For the most part inany doesn't depend on other passt/pasta internals,
> however it does depend on siphash.h, which pesto doesn't need.
> 
> Move the single dependent function, inany_siphash_feed() to siphash.h,
> renaming to match.  Use that include inany.[ch] into pesto as well as
> passt/pasta.  While we're there reformat pesto.c's header comment to match
> the convention used in most other files.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   common.h   | 20 ++++++++++++++++++--
>   flow.c     |  4 ++--
>   fwd.c      |  2 +-
>   fwd.h      |  1 +
>   fwd_rule.h |  1 +
>   inany.c    | 19 ++++++++++++++-----
>   inany.h    | 17 ++---------------
>   pif.h      |  1 +
>   siphash.h  | 13 +++++++++++++
>   util.h     | 16 ----------------
>   10 files changed, 53 insertions(+), 41 deletions(-)
> 
> diff --git a/common.h b/common.h
> index 4a167ae..370cfa1 100644
> --- a/common.h
> +++ b/common.h
> @@ -18,9 +18,27 @@
>   	"This is free software: you are free to change and redistribute it.\n" \
>   	"There is NO WARRANTY, to the extent permitted by law.\n\n"
>   
> +#ifndef MIN
> +#define MIN(x, y)		(((x) < (y)) ? (x) : (y))
> +#endif
> +#ifndef MAX
> +#define MAX(x, y)		(((x) > (y)) ? (x) : (y))
> +#endif
> +
> +#define MAX_FROM_BITS(n)	(((1U << (n)) - 1))
> +
>   /* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */
>   #define FPRINTF(f, ...)	(void)fprintf(f, __VA_ARGS__)
>   
> +#define ARRAY_SIZE(a)		((int)(sizeof(a) / sizeof((a)[0])))
> +
> +#define DIV_ROUND_UP(n, d)	(((n) + (d) - 1) / (d))
> +#define DIV_ROUND_CLOSEST(n, d)	(((n) + (d) / 2) / (d))
> +#define ROUND_DOWN(x, y)	((x) & ~((y) - 1))
> +#define ROUND_UP(x, y)		(((x) + (y) - 1) & ~((y) - 1))
> +
> +#define UINT16_STRLEN		(sizeof("65535"))
> +
>   /*
>    * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror,
>    * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs
> @@ -53,8 +71,6 @@ static inline const char *strerror_(int errnum)
>   
>   #define strerror(x) @ "Don't call strerror() directly, use strerror_() instead"
>   
> -#define ARRAY_SIZE(a)		((int)(sizeof(a) / sizeof((a)[0])))
> -
>   #ifndef __bswap_constant_16
>   #define __bswap_constant_16(x)						\
>   	((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
> diff --git a/flow.c b/flow.c
> index 56a6c6d..91f2b81 100644
> --- a/flow.c
> +++ b/flow.c
> @@ -680,8 +680,8 @@ static uint64_t flow_hash(const struct ctx *c, uint8_t proto, uint8_t pif,
>   {
>   	struct siphash_state state = SIPHASH_INIT(c->hash_secret);
>   
> -	inany_siphash_feed(&state, &side->oaddr);
> -	inany_siphash_feed(&state, &side->eaddr);
> +	siphash_feed_inany(&state, &side->oaddr);
> +	siphash_feed_inany(&state, &side->eaddr);
>   
>   	return siphash_final(&state, 38, (uint64_t)proto << 40 |
>   			     (uint64_t)pif << 32 |
> diff --git a/fwd.c b/fwd.c
> index 728a783..8849cfc 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -80,7 +80,7 @@ static size_t neigh_table_slot(const struct ctx *c,
>   	struct siphash_state st = SIPHASH_INIT(c->hash_secret);
>   	uint32_t i;
>   
> -	inany_siphash_feed(&st, key);
> +	siphash_feed_inany(&st, key);
>   	i = siphash_final(&st, sizeof(*key), 0);
>   
>   	return ((size_t)i) & (NEIGH_TABLE_SIZE - 1);
> diff --git a/fwd.h b/fwd.h
> index 8f845d0..ac24782 100644
> --- a/fwd.h
> +++ b/fwd.h
> @@ -19,6 +19,7 @@
>   #include "fwd_rule.h"
>   
>   struct flowside;
> +struct ctx;
>   
>   #define FWD_NO_HINT	(-1)
>   
> diff --git a/fwd_rule.h b/fwd_rule.h
> index 5855138..f51f1b4 100644
> --- a/fwd_rule.h
> +++ b/fwd_rule.h
> @@ -13,6 +13,7 @@
>   #include <net/if.h>
>   #include <netinet/in.h>
>   
> +#include "common.h"
>   #include "ip.h"
>   #include "inany.h"
>   #include "bitmap.h"
> diff --git a/inany.c b/inany.c
> index 2a586ed..23faf3f 100644
> --- a/inany.c
> +++ b/inany.c
> @@ -1,9 +1,19 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later
> - * Copyright Red Hat
> - * Author: David Gibson <david@gibson.dropbear.id.au>
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/* PASST - Plug A Simple Socket Transport
> + *  for qemu/UNIX domain socket mode
> + *
> + * PASTA - Pack A Subtle Tap Abstraction
> + *  for network namespace/tap device mode
> + *
> + * PESTO - Programmable Extensible Socket Translation Orchestrator
> + *  front-end for passt(1) and pasta(1) forwarding configuration
>    *
>    * inany.c - Types and helpers for handling addresses which could be
>    *           IPv6 or IPv4 (encoded as IPv4-mapped IPv6 addresses)
> + *
> + * Copyright Red Hat
> + * Author: David Gibson <david@gibson.dropbear.id.au>
>    */
>   
>   #include <stdlib.h>
> @@ -13,9 +23,8 @@
>   #include <arpa/inet.h>
>   #include <errno.h>
>   
> -#include "util.h"
> +#include "common.h"
>   #include "ip.h"
> -#include "siphash.h"
>   #include "inany.h"
>   #include "fwd.h"
>   
> diff --git a/inany.h b/inany.h
> index 1f7741d..73385b9 100644
> --- a/inany.h
> +++ b/inany.h
> @@ -11,13 +11,11 @@
>   
>   #include <assert.h>
>   #include <stdbool.h>
> +#include <stddef.h>
>   #include <string.h>
>   
> -#include "util.h"
> +#include "common.h"
>   #include "ip.h"
> -#include "siphash.h"
> -
> -struct siphash_state;
>   
>   /** union inany_addr - Represents either an IPv4 or IPv6 address
>    * @a6:			Address as an IPv6 address, may be IPv4-mapped
> @@ -301,17 +299,6 @@ static inline int inany_from_sockaddr(union inany_addr *dst, in_port_t *port,
>   	return -1;
>   }
>   
> -/** inany_siphash_feed- Fold IPv[46] address into an in-progress siphash
> - * @state:	siphash state
> - * @aa:		inany to hash
> - */
> -static inline void inany_siphash_feed(struct siphash_state *state,
> -				      const union inany_addr *aa)
> -{
> -	siphash_feed(state, (uint64_t)aa->u32[0] << 32 | aa->u32[1]);
> -	siphash_feed(state, (uint64_t)aa->u32[2] << 32 | aa->u32[3]);
> -}
> -
>   #define INANY_ADDRSTRLEN	MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN)
>   
>   bool inany_matches(const union inany_addr *a, const union inany_addr *b);
> diff --git a/pif.h b/pif.h
> index d770860..37409d5 100644
> --- a/pif.h
> +++ b/pif.h
> @@ -16,6 +16,7 @@
>   
>   union inany_addr;
>   union sockaddr_inany;
> +struct ctx;
>   
>   /**
>    * enum pif_type - Type of passt/pasta interface ("pif")
> diff --git a/siphash.h b/siphash.h
> index bbddcac..313b894 100644
> --- a/siphash.h
> +++ b/siphash.h
> @@ -47,6 +47,8 @@
>   #include <stddef.h>
>   #include <stdint.h>
>   
> +#include "inany.h"
> +
>   /**
>    * struct siphash_state - Internal state of siphash calculation
>    */
> @@ -101,6 +103,17 @@ static inline void siphash_feed(struct siphash_state *state, uint64_t in)
>   	state->v[0] ^= in;
>   }
>   
> +/** siphash_feed_inany() - Fold IPv[46] address into an in-progress siphash
> + * @state:	siphash state
> + * @aa:		inany to hash
> + */
> +static inline void siphash_feed_inany(struct siphash_state *state,
> +				      const union inany_addr *aa)
> +{
> +	siphash_feed(state, (uint64_t)aa->u32[0] << 32 | aa->u32[1]);
> +	siphash_feed(state, (uint64_t)aa->u32[2] << 32 | aa->u32[3]);
> +}
> +
>   /**
>    * siphash_final() - Finalize SipHash calculations
>    * @v:		siphash state (4 x 64-bit integers)
> diff --git a/util.h b/util.h
> index dc14c78..70aadeb 100644
> --- a/util.h
> +++ b/util.h
> @@ -29,20 +29,6 @@
>   #define IP_MAX_MTU			USHRT_MAX
>   #endif
>   
> -#ifndef MIN
> -#define MIN(x, y)		(((x) < (y)) ? (x) : (y))
> -#endif
> -#ifndef MAX
> -#define MAX(x, y)		(((x) > (y)) ? (x) : (y))
> -#endif
> -
> -#define DIV_ROUND_UP(n, d)	(((n) + (d) - 1) / (d))
> -#define DIV_ROUND_CLOSEST(n, d)	(((n) + (d) / 2) / (d))
> -#define ROUND_DOWN(x, y)	((x) & ~((y) - 1))
> -#define ROUND_UP(x, y)		(((x) + (y) - 1) & ~((y) - 1))
> -
> -#define MAX_FROM_BITS(n)	(((1U << (n)) - 1))
> -
>   #define SWAP(a, b)							\
>   	do {								\
>   		__typeof__(a) __x = (a); (a) = (b); (b) = __x;		\
> @@ -202,8 +188,6 @@ static inline const char *af_name(sa_family_t af)
>   	}
>   }
>   
> -#define UINT16_STRLEN		(sizeof("65535"))
> -
>   /* inet address (- '\0') + port (u16) (- '\0') + ':' + '\0' */
>   #define SOCKADDR_INET_STRLEN					\
>   	(INET_ADDRSTRLEN-1 + UINT16_STRLEN-1 + sizeof(":"))