Re: [PATCH 05/24] util: move IP stuff from util.[ch] to ip.[ch]

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 15371 bytes --]

On Fri, Feb 02, 2024 at 03:11:32PM +0100, Laurent Vivier wrote:

As with previous patches, could really do with a rationale in the
commit message.

> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  Makefile     |  4 +--
>  conf.c       |  1 +
>  dhcp.c       |  1 +
>  flow.c       |  1 +
>  icmp.c       |  1 +
>  ip.c         | 72 +++++++++++++++++++++++++++++++++++++++++++
>  ip.h         | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ndp.c        |  1 +
>  port_fwd.c   |  1 +
>  qrap.c       |  1 +
>  tap.c        |  1 +
>  tcp.c        |  1 +
>  tcp_splice.c |  1 +
>  udp.c        |  1 +
>  util.c       | 55 ---------------------------------
>  util.h       | 76 ----------------------------------------------
>  16 files changed, 171 insertions(+), 133 deletions(-)
>  create mode 100644 ip.c
>  create mode 100644 ip.h
> 
> diff --git a/Makefile b/Makefile
> index c1138fb91d26..acf37f5a2036 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -47,7 +47,7 @@ FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
>  PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c icmp.c \
>  	igmp.c isolation.c lineread.c log.c mld.c ndp.c netlink.c packet.c \
>  	passt.c pasta.c pcap.c pif.c port_fwd.c tap.c tcp.c tcp_splice.c udp.c \
> -	util.c iov.c
> +	util.c iov.c ip.c
>  QRAP_SRCS = qrap.c
>  SRCS = $(PASST_SRCS) $(QRAP_SRCS)
>  
> @@ -56,7 +56,7 @@ MANPAGES = passt.1 pasta.1 qrap.1
>  PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h flow.h \
>  	flow_table.h icmp.h inany.h isolation.h lineread.h log.h ndp.h \
>  	netlink.h packet.h passt.h pasta.h pcap.h pif.h port_fwd.h siphash.h \
> -	tap.h tcp.h tcp_conn.h tcp_splice.h udp.h util.h iov.h
> +	tap.h tcp.h tcp_conn.h tcp_splice.h udp.h util.h iov.h ip.h
>  HEADERS = $(PASST_HEADERS) seccomp.h
>  
>  C := \#include <linux/tcp.h>\nstruct tcp_info x = { .tcpi_snd_wnd = 0 };
> diff --git a/conf.c b/conf.c
> index 5e15b665be9c..93bfda331349 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -35,6 +35,7 @@
>  #include <netinet/if_ether.h>
>  
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "netlink.h"
>  #include "udp.h"
> diff --git a/dhcp.c b/dhcp.c
> index 110772867632..ff4834a3dce9 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -25,6 +25,7 @@
>  #include <limits.h>
>  
>  #include "util.h"
> +#include "ip.h"
>  #include "checksum.h"
>  #include "packet.h"
>  #include "passt.h"
> diff --git a/flow.c b/flow.c
> index 5e94a7a949e5..73d52bda8774 100644
> --- a/flow.c
> +++ b/flow.c
> @@ -11,6 +11,7 @@
>  #include <string.h>
>  
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "siphash.h"
>  #include "inany.h"
> diff --git a/icmp.c b/icmp.c
> index 9434fc5a7490..3b85a8578316 100644
> --- a/icmp.c
> +++ b/icmp.c
> @@ -33,6 +33,7 @@
>  
>  #include "packet.h"
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "tap.h"
>  #include "log.h"
> diff --git a/ip.c b/ip.c
> new file mode 100644
> index 000000000000..64e336139819
> --- /dev/null
> +++ b/ip.c
> @@ -0,0 +1,72 @@
> +// 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
> + *
> + * util.c - Convenience helpers

Needs an update for the move.

> + *
> + * Copyright (c) 2020-2021 Red Hat GmbH
> + * Author: Stefano Brivio <sbrivio@redhat.com>
> + */
> +
> +#include <stddef.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
> + * @p:		Packet pool, packet number @idx has IPv6 header at @offset
> + * @idx:	Index of packet in pool
> + * @offset:	Pre-calculated IPv6 header offset
> + * @proto:	Filled with L4 protocol number
> + * @dlen:	Data length (payload excluding header extensions), set on return
> + *
> + * Return: pointer to L4 header, NULL if not found
> + */
> +char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
> +		 size_t *dlen)
> +{
> +	const struct ipv6_opt_hdr *o;
> +	const struct ipv6hdr *ip6h;
> +	char *base;
> +	int hdrlen;
> +	uint8_t nh;
> +
> +	base = packet_get(p, idx, 0, 0, NULL);
> +	ip6h = packet_get(p, idx, offset, sizeof(*ip6h), dlen);
> +	if (!ip6h)
> +		return NULL;
> +
> +	offset += sizeof(*ip6h);
> +
> +	nh = ip6h->nexthdr;
> +	if (!IPV6_NH_OPT(nh))
> +		goto found;
> +
> +	while ((o = packet_get_try(p, idx, offset, sizeof(*o), dlen))) {
> +		nh = o->nexthdr;
> +		hdrlen = (o->hdrlen + 1) * 8;
> +
> +		if (IPV6_NH_OPT(nh))
> +			offset += hdrlen;
> +		else
> +			goto found;
> +	}
> +
> +	return NULL;
> +
> +found:
> +	if (nh == 59)
> +		return NULL;
> +
> +	*proto = nh;
> +	return base + offset;
> +}
> diff --git a/ip.h b/ip.h
> new file mode 100644
> index 000000000000..b2e08bc049f3
> --- /dev/null
> +++ b/ip.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (c) 2021 Red Hat GmbH
> + * Author: Stefano Brivio <sbrivio@redhat.com>
> + */
> +
> +#ifndef IP_H
> +#define IP_H
> +
> +#include <netinet/ip.h>
> +#include <netinet/ip6.h>
> +
> +#define IN4_IS_ADDR_UNSPECIFIED(a) \
> +	((a)->s_addr == htonl_constant(INADDR_ANY))
> +#define IN4_IS_ADDR_BROADCAST(a) \
> +	((a)->s_addr == htonl_constant(INADDR_BROADCAST))
> +#define IN4_IS_ADDR_LOOPBACK(a) \
> +	(ntohl((a)->s_addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET)
> +#define IN4_IS_ADDR_MULTICAST(a) \
> +	(IN_MULTICAST(ntohl((a)->s_addr)))
> +#define IN4_ARE_ADDR_EQUAL(a, b) \
> +	(((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr)
> +#define IN4ADDR_LOOPBACK_INIT \
> +	{ .s_addr	= htonl_constant(INADDR_LOOPBACK) }
> +#define IN4ADDR_ANY_INIT \
> +	{ .s_addr	= htonl_constant(INADDR_ANY) }

I'm hoping to eliminate some of these with increased use of inany.h,
but that's not really relevant to you.

> +#define L2_BUF_IP4_INIT(proto)						\
> +	{								\
> +		.version	= 4,					\
> +		.ihl		= 5,					\
> +		.tos		= 0,					\
> +		.tot_len	= 0,					\
> +		.id		= 0,					\
> +		.frag_off	= 0,					\
> +		.ttl		= 0xff,					\
> +		.protocol	= (proto),				\
> +		.saddr		= 0,					\
> +		.daddr		= 0,					\
> +	}
> +#define L2_BUF_IP4_PSUM(proto)	((uint32_t)htons_constant(0x4500) +	\
> +				 (uint32_t)htons_constant(0xff00 | (proto)))
> +
> +#define L2_BUF_IP6_INIT(proto)						\
> +	{								\
> +		.priority	= 0,					\
> +		.version	= 6,					\
> +		.flow_lbl	= { 0 },				\
> +		.payload_len	= 0,					\
> +		.nexthdr	= (proto),				\
> +		.hop_limit	= 255,					\
> +		.saddr		= IN6ADDR_ANY_INIT,			\
> +		.daddr		= IN6ADDR_ANY_INIT,			\
> +	}
> +
> +struct ipv6hdr {

Not really in scope for this patch, but I have wondered if we should
try to use struct ip6_hdr from netinet/ip6.h instead of our own
version (derived, I think, from the kernel one).

> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wpedantic"
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +	uint8_t			version:4,
> +				priority:4;
> +#else
> +	uint8_t			priority:4,
> +				version:4;
> +#endif
> +#pragma GCC diagnostic pop
> +	uint8_t			flow_lbl[3];
> +
> +	uint16_t		payload_len;
> +	uint8_t			nexthdr;
> +	uint8_t			hop_limit;
> +
> +	struct in6_addr		saddr;
> +	struct in6_addr		daddr;
> +};
> +
> +struct ipv6_opt_hdr {
> +	uint8_t			nexthdr;
> +	uint8_t			hdrlen;
> +	/*
> +	 * TLV encoded option data follows.
> +	 */
> +} __attribute__((packed));	/* required for some archs */
> +
> +char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
> +		 size_t *dlen);
> +#endif /* IP_H */
> diff --git a/ndp.c b/ndp.c
> index 4c85ab8bcaee..c58f4b222b76 100644
> --- a/ndp.c
> +++ b/ndp.c
> @@ -28,6 +28,7 @@
>  
>  #include "checksum.h"
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "tap.h"
>  #include "log.h"
> diff --git a/port_fwd.c b/port_fwd.c
> index 6f6c836c57ad..e1ec31e2232c 100644
> --- a/port_fwd.c
> +++ b/port_fwd.c
> @@ -21,6 +21,7 @@
>  #include <stdio.h>
>  
>  #include "util.h"
> +#include "ip.h"
>  #include "port_fwd.h"
>  #include "passt.h"
>  #include "lineread.h"
> diff --git a/qrap.c b/qrap.c
> index 97f350a4bf0b..d59670621731 100644
> --- a/qrap.c
> +++ b/qrap.c
> @@ -32,6 +32,7 @@
>  #include <linux/icmpv6.h>
>  
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "arp.h"
>  
> diff --git a/tap.c b/tap.c
> index 396dee7eef25..3ea03f720d6d 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -45,6 +45,7 @@
>  
>  #include "checksum.h"
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "arp.h"
>  #include "dhcp.h"
> diff --git a/tcp.c b/tcp.c
> index 905d26f6c656..4c9c5fb51c60 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -289,6 +289,7 @@
>  
>  #include "checksum.h"
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "tap.h"
>  #include "siphash.h"
> diff --git a/tcp_splice.c b/tcp_splice.c
> index 26d32065cd47..66575ca95a1e 100644
> --- a/tcp_splice.c
> +++ b/tcp_splice.c
> @@ -49,6 +49,7 @@
>  #include <sys/socket.h>
>  
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "log.h"
>  #include "tcp_splice.h"
> diff --git a/udp.c b/udp.c
> index b5b8f8a7cd5b..d514c864ab5b 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -112,6 +112,7 @@
>  
>  #include "checksum.h"
>  #include "util.h"
> +#include "ip.h"
>  #include "passt.h"
>  #include "tap.h"
>  #include "pcap.h"
> diff --git a/util.c b/util.c
> index 21b35ff94db1..f73ea1d98a09 100644
> --- a/util.c
> +++ b/util.c
> @@ -30,61 +30,6 @@
>  #include "packet.h"
>  #include "log.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
> - * @p:		Packet pool, packet number @idx has IPv6 header at @offset
> - * @idx:	Index of packet in pool
> - * @offset:	Pre-calculated IPv6 header offset
> - * @proto:	Filled with L4 protocol number
> - * @dlen:	Data length (payload excluding header extensions), set on return
> - *
> - * Return: pointer to L4 header, NULL if not found
> - */
> -char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
> -		 size_t *dlen)
> -{
> -	const struct ipv6_opt_hdr *o;
> -	const struct ipv6hdr *ip6h;
> -	char *base;
> -	int hdrlen;
> -	uint8_t nh;
> -
> -	base = packet_get(p, idx, 0, 0, NULL);
> -	ip6h = packet_get(p, idx, offset, sizeof(*ip6h), dlen);
> -	if (!ip6h)
> -		return NULL;
> -
> -	offset += sizeof(*ip6h);
> -
> -	nh = ip6h->nexthdr;
> -	if (!IPV6_NH_OPT(nh))
> -		goto found;
> -
> -	while ((o = packet_get_try(p, idx, offset, sizeof(*o), dlen))) {
> -		nh = o->nexthdr;
> -		hdrlen = (o->hdrlen + 1) * 8;
> -
> -		if (IPV6_NH_OPT(nh))
> -			offset += hdrlen;
> -		else
> -			goto found;
> -	}
> -
> -	return NULL;
> -
> -found:
> -	if (nh == 59)
> -		return NULL;
> -
> -	*proto = nh;
> -	return base + offset;
> -}
> -
>  /**
>   * sock_l4() - Create and bind socket for given L4, add to epoll list
>   * @c:		Execution context
> diff --git a/util.h b/util.h
> index d2320f8cc99a..f7c3dfee9972 100644
> --- a/util.h
> +++ b/util.h
> @@ -110,22 +110,6 @@
>  #define	htonl_constant(x)	(__bswap_constant_32(x))
>  #endif
>  
> -#define IN4_IS_ADDR_UNSPECIFIED(a) \
> -	((a)->s_addr == htonl_constant(INADDR_ANY))
> -#define IN4_IS_ADDR_BROADCAST(a) \
> -	((a)->s_addr == htonl_constant(INADDR_BROADCAST))
> -#define IN4_IS_ADDR_LOOPBACK(a) \
> -	(ntohl((a)->s_addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET)
> -#define IN4_IS_ADDR_MULTICAST(a) \
> -	(IN_MULTICAST(ntohl((a)->s_addr)))
> -#define IN4_ARE_ADDR_EQUAL(a, b) \
> -	(((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr)
> -#define IN4ADDR_LOOPBACK_INIT \
> -	{ .s_addr	= htonl_constant(INADDR_LOOPBACK) }
> -#define IN4ADDR_ANY_INIT \
> -	{ .s_addr	= htonl_constant(INADDR_ANY) }
> -
> -
>  #define NS_FN_STACK_SIZE	(RLIMIT_STACK_VAL * 1024 / 8)
>  int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
>  	     void *arg);
> @@ -138,34 +122,6 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
>  			 (void *)(arg));				\
>  	} while (0)
>  
> -#define L2_BUF_IP4_INIT(proto)						\
> -	{								\
> -		.version	= 4,					\
> -		.ihl		= 5,					\
> -		.tos		= 0,					\
> -		.tot_len	= 0,					\
> -		.id		= 0,					\
> -		.frag_off	= 0,					\
> -		.ttl		= 0xff,					\
> -		.protocol	= (proto),				\
> -		.saddr		= 0,					\
> -		.daddr		= 0,					\
> -	}
> -#define L2_BUF_IP4_PSUM(proto)	((uint32_t)htons_constant(0x4500) +	\
> -				 (uint32_t)htons_constant(0xff00 | (proto)))
> -
> -#define L2_BUF_IP6_INIT(proto)						\
> -	{								\
> -		.priority	= 0,					\
> -		.version	= 6,					\
> -		.flow_lbl	= { 0 },				\
> -		.payload_len	= 0,					\
> -		.nexthdr	= (proto),				\
> -		.hop_limit	= 255,					\
> -		.saddr		= IN6ADDR_ANY_INIT,			\
> -		.daddr		= IN6ADDR_ANY_INIT,			\
> -	}
> -
>  #define RCVBUF_BIG		(2UL * 1024 * 1024)
>  #define SNDBUF_BIG		(4UL * 1024 * 1024)
>  #define SNDBUF_SMALL		(128UL * 1024)
> @@ -173,45 +129,13 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
>  #include <net/if.h>
>  #include <limits.h>
>  #include <stdint.h>
> -#include <netinet/ip6.h>
>  
>  #include "packet.h"
>  
>  struct ctx;
>  
> -struct ipv6hdr {
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wpedantic"
> -#if __BYTE_ORDER == __BIG_ENDIAN
> -	uint8_t			version:4,
> -				priority:4;
> -#else
> -	uint8_t			priority:4,
> -				version:4;
> -#endif
> -#pragma GCC diagnostic pop
> -	uint8_t			flow_lbl[3];
> -
> -	uint16_t		payload_len;
> -	uint8_t			nexthdr;
> -	uint8_t			hop_limit;
> -
> -	struct in6_addr		saddr;
> -	struct in6_addr		daddr;
> -};
> -
> -struct ipv6_opt_hdr {
> -	uint8_t			nexthdr;
> -	uint8_t			hdrlen;
> -	/*
> -	 * TLV encoded option data follows.
> -	 */
> -} __attribute__((packed));	/* required for some archs */
> -
>  /* cppcheck-suppress funcArgNamesDifferent */
>  __attribute__ ((weak)) int ffsl(long int i) { return __builtin_ffsl(i); }
> -char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
> -		 size_t *dlen);
>  int sock_l4(const struct ctx *c, int af, uint8_t proto,
>  	    const void *bind_addr, const char *ifname, uint16_t port,
>  	    uint32_t data);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]