From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 6997D5A0279 for ; Mon, 5 Feb 2024 07:13:51 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1707113625; bh=iLpXkKdD6SCzCG0TbnpR0Iy0jqhZ5yAhkWRZspMoOas=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=aGWQzFa3145O+7ooGevFdkCycMTnMNCjLs+SJdTFENXlBXLsbKtPbwu036hmo+ELX l2FsmwVaUpUcK6lP9fI7FWwpNFqWxwAGB8BWo3uZphhtYLgCPcVpC0DRq/3TBBaQ3S VveacJ2OCeI5woOq+kkG8JTAGfNFJUJqF1qP0tdwIoasZ6EuI2sangAdb2C9KAfuOv 7bJBdknGaDDlgP+BqF/sl4zjyD+u266AGxGtiYom6coEEHAbAL97JzytHZO1h5lFxb xMSbCZk4O4RQ/yorVupAgHZZqC+NoqXSiJO0D9kUHdE2S1ahFX3pVO7weJ9IM/Agtx 8k14EUaibU+qg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TSx092mV8z4wyj; Mon, 5 Feb 2024 17:13:45 +1100 (AEDT) Date: Mon, 5 Feb 2024 17:13:40 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 05/24] util: move IP stuff from util.[ch] to ip.[ch] Message-ID: References: <20240202141151.3762941-1-lvivier@redhat.com> <20240202141151.3762941-6-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="QkPDklIcHVWckU0+" Content-Disposition: inline In-Reply-To: <20240202141151.3762941-6-lvivier@redhat.com> Message-ID-Hash: V6MNQW2L6G4YPDZLF3UAHC7NFU3EUUSB X-Message-ID-Hash: V6MNQW2L6G4YPDZLF3UAHC7NFU3EUUSB X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --QkPDklIcHVWckU0+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 > --- > 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 >=20 > diff --git a/Makefile b/Makefile > index c1138fb91d26..acf37f5a2036 100644 > --- a/Makefile > +++ b/Makefile > @@ -47,7 +47,7 @@ FLAGS +=3D -DDUAL_STACK_SOCKETS=3D$(DUAL_STACK_SOCKETS) > PASST_SRCS =3D arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c icm= p.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 =3D qrap.c > SRCS =3D $(PASST_SRCS) $(QRAP_SRCS) > =20 > @@ -56,7 +56,7 @@ MANPAGES =3D passt.1 pasta.1 qrap.1 > PASST_HEADERS =3D 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 =3D $(PASST_HEADERS) seccomp.h > =20 > C :=3D \#include \nstruct tcp_info x =3D { .tcpi_snd_wnd = =3D 0 }; > diff --git a/conf.c b/conf.c > index 5e15b665be9c..93bfda331349 100644 > --- a/conf.c > +++ b/conf.c > @@ -35,6 +35,7 @@ > #include > =20 > #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 > =20 > #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 > =20 > #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 @@ > =20 > #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 > + */ > + > +#include > +#include "util.h" > +#include "ip.h" > + > +#define IPV6_NH_OPT(nh) \ > + ((nh) =3D=3D 0 || (nh) =3D=3D 43 || (nh) =3D=3D 44 || (nh) =3D=3D 5= 0 || \ > + (nh) =3D=3D 51 || (nh) =3D=3D 60 || (nh) =3D=3D 135 || (nh) =3D=3D 1= 39 || \ > + (nh) =3D=3D 140 || (nh) =3D=3D 253 || (nh) =3D=3D 254) > + > +/** > + * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract p= rotocol > + * @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 retu= rn > + * > + * 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 =3D packet_get(p, idx, 0, 0, NULL); > + ip6h =3D packet_get(p, idx, offset, sizeof(*ip6h), dlen); > + if (!ip6h) > + return NULL; > + > + offset +=3D sizeof(*ip6h); > + > + nh =3D ip6h->nexthdr; > + if (!IPV6_NH_OPT(nh)) > + goto found; > + > + while ((o =3D packet_get_try(p, idx, offset, sizeof(*o), dlen))) { > + nh =3D o->nexthdr; > + hdrlen =3D (o->hdrlen + 1) * 8; > + > + if (IPV6_NH_OPT(nh)) > + offset +=3D hdrlen; > + else > + goto found; > + } > + > + return NULL; > + > +found: > + if (nh =3D=3D 59) > + return NULL; > + > + *proto =3D 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 > + */ > + > +#ifndef IP_H > +#define IP_H > + > +#include > +#include > + > +#define IN4_IS_ADDR_UNSPECIFIED(a) \ > + ((a)->s_addr =3D=3D htonl_constant(INADDR_ANY)) > +#define IN4_IS_ADDR_BROADCAST(a) \ > + ((a)->s_addr =3D=3D htonl_constant(INADDR_BROADCAST)) > +#define IN4_IS_ADDR_LOOPBACK(a) \ > + (ntohl((a)->s_addr) >> IN_CLASSA_NSHIFT =3D=3D 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 =3D=3D ((struct in_addr *)b)->s_addr) > +#define IN4ADDR_LOOPBACK_INIT \ > + { .s_addr =3D htonl_constant(INADDR_LOOPBACK) } > +#define IN4ADDR_ANY_INIT \ > + { .s_addr =3D 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 =3D 4, \ > + .ihl =3D 5, \ > + .tos =3D 0, \ > + .tot_len =3D 0, \ > + .id =3D 0, \ > + .frag_off =3D 0, \ > + .ttl =3D 0xff, \ > + .protocol =3D (proto), \ > + .saddr =3D 0, \ > + .daddr =3D 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 =3D 0, \ > + .version =3D 6, \ > + .flow_lbl =3D { 0 }, \ > + .payload_len =3D 0, \ > + .nexthdr =3D (proto), \ > + .hop_limit =3D 255, \ > + .saddr =3D IN6ADDR_ANY_INIT, \ > + .daddr =3D 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 =3D=3D __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 @@ > =20 > #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 > =20 > #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 > =20 > #include "util.h" > +#include "ip.h" > #include "passt.h" > #include "arp.h" > =20 > diff --git a/tap.c b/tap.c > index 396dee7eef25..3ea03f720d6d 100644 > --- a/tap.c > +++ b/tap.c > @@ -45,6 +45,7 @@ > =20 > #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 @@ > =20 > #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 > =20 > #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 @@ > =20 > #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" > =20 > -#define IPV6_NH_OPT(nh) \ > - ((nh) =3D=3D 0 || (nh) =3D=3D 43 || (nh) =3D=3D 44 || (nh) =3D=3D 5= 0 || \ > - (nh) =3D=3D 51 || (nh) =3D=3D 60 || (nh) =3D=3D 135 || (nh) =3D=3D 1= 39 || \ > - (nh) =3D=3D 140 || (nh) =3D=3D 253 || (nh) =3D=3D 254) > - > -/** > - * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract p= rotocol > - * @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 retu= rn > - * > - * 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 =3D packet_get(p, idx, 0, 0, NULL); > - ip6h =3D packet_get(p, idx, offset, sizeof(*ip6h), dlen); > - if (!ip6h) > - return NULL; > - > - offset +=3D sizeof(*ip6h); > - > - nh =3D ip6h->nexthdr; > - if (!IPV6_NH_OPT(nh)) > - goto found; > - > - while ((o =3D packet_get_try(p, idx, offset, sizeof(*o), dlen))) { > - nh =3D o->nexthdr; > - hdrlen =3D (o->hdrlen + 1) * 8; > - > - if (IPV6_NH_OPT(nh)) > - offset +=3D hdrlen; > - else > - goto found; > - } > - > - return NULL; > - > -found: > - if (nh =3D=3D 59) > - return NULL; > - > - *proto =3D 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 > =20 > -#define IN4_IS_ADDR_UNSPECIFIED(a) \ > - ((a)->s_addr =3D=3D htonl_constant(INADDR_ANY)) > -#define IN4_IS_ADDR_BROADCAST(a) \ > - ((a)->s_addr =3D=3D htonl_constant(INADDR_BROADCAST)) > -#define IN4_IS_ADDR_LOOPBACK(a) \ > - (ntohl((a)->s_addr) >> IN_CLASSA_NSHIFT =3D=3D 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 =3D=3D ((struct in_addr *)b)->s_addr) > -#define IN4ADDR_LOOPBACK_INIT \ > - { .s_addr =3D htonl_constant(INADDR_LOOPBACK) } > -#define IN4ADDR_ANY_INIT \ > - { .s_addr =3D 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, si= ze_t stack_size, int flags, > (void *)(arg)); \ > } while (0) > =20 > -#define L2_BUF_IP4_INIT(proto) \ > - { \ > - .version =3D 4, \ > - .ihl =3D 5, \ > - .tos =3D 0, \ > - .tot_len =3D 0, \ > - .id =3D 0, \ > - .frag_off =3D 0, \ > - .ttl =3D 0xff, \ > - .protocol =3D (proto), \ > - .saddr =3D 0, \ > - .daddr =3D 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 =3D 0, \ > - .version =3D 6, \ > - .flow_lbl =3D { 0 }, \ > - .payload_len =3D 0, \ > - .nexthdr =3D (proto), \ > - .hop_limit =3D 255, \ > - .saddr =3D IN6ADDR_ANY_INIT, \ > - .daddr =3D 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, s= ize_t stack_size, int flags, > #include > #include > #include > -#include > =20 > #include "packet.h" > =20 > struct ctx; > =20 > -struct ipv6hdr { > -#pragma GCC diagnostic push > -#pragma GCC diagnostic ignored "-Wpedantic" > -#if __BYTE_ORDER =3D=3D __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); --=20 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 --QkPDklIcHVWckU0+ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXAfJMACgkQzQJF27ox 2Gewxg//ZlvTG1OvalQYiw0SkyACaENdJQma28gW+aGcElE90iTdEuEwRsKxLgmK u3KT0ldi3ijiZaBAeD+tAJoSaZNNVdFOko9lOkJ2+lmBC43d54XcZXcbkv5ZKc+E D/akNtYtNsZKGPdBWAPCGaGVGi86ZFaQ2UqVT2hJsKrlMpxz+niMBxRbH/voZdut jsWr+mkjM1VO2O/1sNopVY451ErczvSbv/mOcf2rZQUQuiIcYXoBT5+4UCMGeiqS CWrFfmCpmCRSDTF0Ogntf2gpVelgnTwztlgJnRBpIme3r7C8ow6Ards4G/gyjdv4 09gyWQpqeV0rG0TpqaXpLBG9RT2cFQUAPa+YN/amVxXtxof46Jj/7gwcPGOnLt6x YAnnvrHIMA/nMBm8qToDCXvM+64LrnPPFXQhi/8UlU1fKNJ+/Y5Rrcu3nublQ+LC jC6FbBMddvF90bmKJZYi1SoqgDVskpDeD4A77P9pjmuujX3UFcq50A7YBj99eBoX caV0bHnu20w/QHWTYhIdNguL+EzfhLRqC/hOPMGPk+9U3E6QOwrosFdl+Yp34bsL zwEweu3sMFu1M7ugv31foIeKmuYT+L1qW88hXy4+WAyuDpm1z1vidPEhCHB+Kg7I kXxOp4tj7wrS7ozNlXdWsu2r5CsWlxwIPgZgWvaI0kxz8jRbpE4= =qgZi -----END PGP SIGNATURE----- --QkPDklIcHVWckU0+--