Re: [PATCH v6 12/18] ip: Prepare ip.[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>
> 
> Most things in ip.[ch] related purely to IP addresses and headers with
> no dependency on other passt/pasta internals.  A number of these will be
> useful to re-use in pesto.  The exception is ipv6_l4hdr() which uses
> iov_tail.
> 
> The only caller of this is in tap.c, so move the function there.  Along
> with moving the constant byteswapping functions to common.h, that lets
> ip.[ch] to be linked into pesto as well as passt/pasta.
> 
> 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>

But we can fix a pre-existing error while doing the move, see below:

> ---
>   common.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>   inany.h  |  2 ++
>   ip.c     | 56 +++-----------------------------------------------------
>   ip.h     |  4 +---
>   tap.c    | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   util.h   | 48 ------------------------------------------------
>   6 files changed, 106 insertions(+), 104 deletions(-)
> 
> diff --git a/common.h b/common.h
> index 45f66ea..4a167ae 100644
> --- a/common.h
> +++ b/common.h
> @@ -55,4 +55,52 @@ static inline const char *strerror_(int errnum)
>   
>   #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)))
> +#endif
> +
> +#ifndef __bswap_constant_32
> +#define __bswap_constant_32(x)						\
> +	((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >>  8) |	\
> +	 (((x) & 0x0000ff00) <<  8) | (((x) & 0x000000ff) << 24))
> +#endif
> +
> +#ifndef __bswap_constant_32
> +#define __bswap_constant_32(x)						\
> +	((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >>  8) |	\
> +	 (((x) & 0x0000ff00) <<  8) | (((x) & 0x000000ff) << 24))
> +#endif

__bswap_constant_32 block is written twice.

> +
> +#ifndef __bswap_constant_64
> +#define __bswap_constant_64(x) \
> +	((((x) & 0xff00000000000000ULL) >> 56) |			\
> +	 (((x) & 0x00ff000000000000ULL) >> 40) |			\
> +	 (((x) & 0x0000ff0000000000ULL) >> 24) |			\
> +	 (((x) & 0x000000ff00000000ULL) >> 8)  |			\
> +	 (((x) & 0x00000000ff000000ULL) << 8)  |			\
> +	 (((x) & 0x0000000000ff0000ULL) << 24) |			\
> +	 (((x) & 0x000000000000ff00ULL) << 40) |			\
> +	 (((x) & 0x00000000000000ffULL) << 56))
> +#endif
> +
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +#define	htons_constant(x)	(x)
> +#define	htonl_constant(x)	(x)
> +#define htonll_constant(x)	(x)
> +#define	ntohs_constant(x)	(x)
> +#define	ntohl_constant(x)	(x)
> +#define ntohll_constant(x)	(x)
> +#else
> +#define	htons_constant(x)	(__bswap_constant_16(x))
> +#define	htonl_constant(x)	(__bswap_constant_32(x))
> +#define	htonll_constant(x)	(__bswap_constant_64(x))
> +#define	ntohs_constant(x)	(__bswap_constant_16(x))
> +#define	ntohl_constant(x)	(__bswap_constant_32(x))
> +#define	ntohll_constant(x)	(__bswap_constant_64(x))
> +#endif
> +
> +#define ntohll(x)		(be64toh((x)))
> +#define htonll(x)		(htobe64((x)))
> +
>   #endif /* _COMMON_H */
> diff --git a/inany.h b/inany.h
> index 30e2416..1f7741d 100644
> --- a/inany.h
> +++ b/inany.h
> @@ -10,8 +10,10 @@
>   #define INANY_H
>   
>   #include <assert.h>
> +#include <stdbool.h>
>   #include <string.h>
>   
> +#include "util.h"
>   #include "ip.h"
>   #include "siphash.h"
>   
> diff --git a/ip.c b/ip.c
> index 25fa407..f2506bb 100644
> --- a/ip.c
> +++ b/ip.c
> @@ -6,6 +6,9 @@
>    * 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
> + *
>    * ip.c - IP related functions
>    *
>    * Copyright (c) 2020-2021 Red Hat GmbH
> @@ -16,61 +19,8 @@
>   #include <stddef.h>
>   #include <netinet/in.h>
>   
> -#include "util.h"
>   #include "ip.h"
>   
> -#define IPV6_NH_OPT(nh)							\
> -	((nh) == 0   || (nh) == 43  || (nh) == 44  || (nh) == 50  ||	\
> -	 (nh) == 51  || (nh) == 60  || (nh) == 135 || (nh) == 139 ||	\
> -	 (nh) == 140 || (nh) == 253 || (nh) == 254)
> -
> -/**
> - * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol
> - * @data:	IPv6 packet
> - * @proto:	Filled with L4 protocol number
> - * @dlen:	Data length (payload excluding header extensions), set on return
> - *
> - * Return: true if the L4 header is found and @data, @proto, @dlen are set,
> - * 	   false on error. Outputs are indeterminate on failure.
> - */
> -bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen)
> -{
> -	struct ipv6_opt_hdr o_storage;
> -	const struct ipv6_opt_hdr *o;
> -	struct ipv6hdr ip6h_storage;
> -	const struct ipv6hdr *ip6h;
> -	int hdrlen;
> -	uint8_t nh;
> -
> -	ip6h = IOV_REMOVE_HEADER(data, ip6h_storage);
> -	if (!ip6h)
> -		return false;
> -
> -	nh = ip6h->nexthdr;
> -	if (!IPV6_NH_OPT(nh))
> -		goto found;
> -
> -	while ((o = IOV_PEEK_HEADER(data, o_storage))) {
> -		nh = o->nexthdr;
> -		hdrlen = (o->hdrlen + 1) * 8;
> -
> -		if (IPV6_NH_OPT(nh))
> -			iov_drop_header(data, hdrlen);
> -		else
> -			goto found;
> -	}
> -
> -	return false;
> -
> -found:
> -	if (nh == IPPROTO_NONE)
> -		return false;
> -
> -	*dlen = iov_tail_size(data);
> -	*proto = nh;
> -	return true;
> -}
> -
>   /**
>    * ipproto_name() - Get IP protocol name from number
>    * @proto:	IP protocol number
> diff --git a/ip.h b/ip.h
> index fb4119a..aab9b86 100644
> --- a/ip.h
> +++ b/ip.h
> @@ -9,7 +9,7 @@
>   #include <netinet/ip.h>
>   #include <netinet/ip6.h>
>   
> -#include "util.h"
> +#include "common.h"
>   
>   #define IN4_IS_ADDR_UNSPECIFIED(a) \
>   	(((struct in_addr *)(a))->s_addr == htonl_constant(INADDR_ANY))
> @@ -117,8 +117,6 @@ static inline uint32_t ip6_get_flow_lbl(const struct ipv6hdr *ip6h)
>   		ip6h->flow_lbl[2];
>   }
>   
> -bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen);
> -
>   #define IPPROTO_STRLEN		(sizeof("<unknown protocol>"))
>   const char *ipproto_name(uint8_t proto);
>   
> diff --git a/tap.c b/tap.c
> index 7d06189..cfd7b64 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -874,6 +874,58 @@ append:
>   	return in->count;
>   }
>   
> +#define IPV6_NH_OPT(nh)							\
> +	((nh) == 0   || (nh) == 43  || (nh) == 44  || (nh) == 50  ||	\
> +	 (nh) == 51  || (nh) == 60  || (nh) == 135 || (nh) == 139 ||	\
> +	 (nh) == 140 || (nh) == 253 || (nh) == 254)
> +
> +/**
> + * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol
> + * @data:	IPv6 packet
> + * @proto:	Filled with L4 protocol number
> + * @dlen:	Data length (payload excluding header extensions), set on return
> + *
> + * Return: true if the L4 header is found and @data, @proto, @dlen are set,
> + * 	   false on error. Outputs are indeterminate on failure.
> + */
> +static bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen)
> +{
> +	struct ipv6_opt_hdr o_storage;
> +	const struct ipv6_opt_hdr *o;
> +	struct ipv6hdr ip6h_storage;
> +	const struct ipv6hdr *ip6h;
> +	int hdrlen;
> +	uint8_t nh;
> +
> +	ip6h = IOV_REMOVE_HEADER(data, ip6h_storage);
> +	if (!ip6h)
> +		return false;
> +
> +	nh = ip6h->nexthdr;
> +	if (!IPV6_NH_OPT(nh))
> +		goto found;
> +
> +	while ((o = IOV_PEEK_HEADER(data, o_storage))) {
> +		nh = o->nexthdr;
> +		hdrlen = (o->hdrlen + 1) * 8;
> +
> +		if (IPV6_NH_OPT(nh))
> +			iov_drop_header(data, hdrlen);
> +		else
> +			goto found;
> +	}
> +
> +	return false;
> +
> +found:
> +	if (nh == IPPROTO_NONE)
> +		return false;
> +
> +	*dlen = iov_tail_size(data);
> +	*proto = nh;
> +	return true;
> +}
> +
>   /**
>    * tap6_handler() - IPv6 packet handler for tap file descriptor
>    * @c:		Execution context
> diff --git a/util.h b/util.h
> index c788382..dc14c78 100644
> --- a/util.h
> +++ b/util.h
> @@ -101,54 +101,6 @@ void abort_with_msg(const char *fmt, ...)
>   #define MAC_UNDEF		MAC_BROADCAST
>   #define MAC_IS_UNDEF(addr)	(!memcmp((addr), MAC_UNDEF, ETH_ALEN))
>   
> -#ifndef __bswap_constant_16
> -#define __bswap_constant_16(x)						\
> -	((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)))
> -#endif
> -
> -#ifndef __bswap_constant_32
> -#define __bswap_constant_32(x)						\
> -	((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >>  8) |	\
> -	 (((x) & 0x0000ff00) <<  8) | (((x) & 0x000000ff) << 24))
> -#endif
> -
> -#ifndef __bswap_constant_32
> -#define __bswap_constant_32(x)						\
> -	((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >>  8) |	\
> -	 (((x) & 0x0000ff00) <<  8) | (((x) & 0x000000ff) << 24))
> -#endif
> -
> -#ifndef __bswap_constant_64
> -#define __bswap_constant_64(x) \
> -	((((x) & 0xff00000000000000ULL) >> 56) |			\
> -	 (((x) & 0x00ff000000000000ULL) >> 40) |			\
> -	 (((x) & 0x0000ff0000000000ULL) >> 24) |			\
> -	 (((x) & 0x000000ff00000000ULL) >> 8)  |			\
> -	 (((x) & 0x00000000ff000000ULL) << 8)  |			\
> -	 (((x) & 0x0000000000ff0000ULL) << 24) |			\
> -	 (((x) & 0x000000000000ff00ULL) << 40) |			\
> -	 (((x) & 0x00000000000000ffULL) << 56))
> -#endif
> -
> -#if __BYTE_ORDER == __BIG_ENDIAN
> -#define	htons_constant(x)	(x)
> -#define	htonl_constant(x)	(x)
> -#define htonll_constant(x)	(x)
> -#define	ntohs_constant(x)	(x)
> -#define	ntohl_constant(x)	(x)
> -#define ntohll_constant(x)	(x)
> -#else
> -#define	htons_constant(x)	(__bswap_constant_16(x))
> -#define	htonl_constant(x)	(__bswap_constant_32(x))
> -#define	htonll_constant(x)	(__bswap_constant_64(x))
> -#define	ntohs_constant(x)	(__bswap_constant_16(x))
> -#define	ntohl_constant(x)	(__bswap_constant_32(x))
> -#define	ntohll_constant(x)	(__bswap_constant_64(x))
> -#endif
> -
> -#define ntohll(x)		(be64toh((x)))
> -#define htonll(x)		(htobe64((x)))
> -
>   extern uint8_t eth_pad[ETH_ZLEN];
>   
>   /**