Re: [PATCH v5 3/4] tap: break out building of udp header from tap_udp6_send function

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

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

On Sun, Feb 23, 2025 at 05:59:50PM -0500, Jon Maloy wrote:
> We will need to build the UDP header at other locations than in function
> tap_udp6_send(), so we break that part out to a separate function.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

...minus one nit.

> ---
>  tap.c | 40 ++++++++++++++++++++++++++++++----------
>  tap.h |  4 ++++
>  2 files changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index 3daba28..380a7b9 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -266,7 +266,7 @@ static void *tap_push_ip6h(struct ipv6hdr *ip6h,
>  }
>  
>  /**
> - * tap_udp6_send() - Send UDP over IPv6 packet
> + * tap_push_uh6() - Build UDPv6 header with checksum
>   * @c:		Execution context
>   * @src:	IPv6 source address
>   * @sport:	UDP source port
> @@ -276,19 +276,14 @@ static void *tap_push_ip6h(struct ipv6hdr *ip6h,
>   * @in:		UDP payload contents (not including UDP header)
>   * @dlen:	UDP payload length (not including UDP header)
>   */
> -void tap_udp6_send(const struct ctx *c,
> +void *tap_push_uh6(struct udphdr *uh,
>  		   const struct in6_addr *src, in_port_t sport,
>  		   const struct in6_addr *dst, in_port_t dport,
> -		   uint32_t flow, void *in, size_t dlen)
> +		   void *in, size_t dlen)
>  {
>  	size_t l4len = dlen + sizeof(struct udphdr);
> -	char buf[USHRT_MAX];
> -	struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6);
> -	struct udphdr *uh = tap_push_ip6h(ip6h, src, dst,
> -					  l4len, IPPROTO_UDP, flow);
> -	char *data = (char *)(uh + 1);
>  	const struct iovec iov = {
> -		.iov_base = in,
> +		.iov_base = (void *)in,

'in' is already a void * so the cast is not necessary (even ignoring
the fact that you can generall coerce to and from void * without a
cast).

>  		.iov_len = dlen
>  	};
>  	struct iov_tail payload = IOV_TAIL(&iov, 1, 0);
> @@ -297,8 +292,33 @@ void tap_udp6_send(const struct ctx *c,
>  	uh->dest = htons(dport);
>  	uh->len = htons(l4len);
>  	csum_udp6(uh, src, dst, &payload);
> -	memcpy(data, in, dlen);
> +	return uh + 1;
> +}
> +
> +/**
> + * tap_udp6_send() - Send UDP over IPv6 packet
> + * @c:		Execution context
> + * @src:	IPv6 source address
> + * @sport:	UDP source port
> + * @dst:	IPv6 destination address
> + * @dport:	UDP destination port
> + * @flow:	Flow label
> + * @in:		UDP payload contents (not including UDP header)
> + * @dlen:	UDP payload length (not including UDP header)
> + */
> +void tap_udp6_send(const struct ctx *c,
> +		   const struct in6_addr *src, in_port_t sport,
> +		   const struct in6_addr *dst, in_port_t dport,
> +		   uint32_t flow, void *in, size_t dlen)
> +{
> +	size_t l4len = dlen + sizeof(struct udphdr);
> +	char buf[USHRT_MAX];
> +	struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6);
> +	struct udphdr *uh = tap_push_ip6h(ip6h, src, dst,
> +					  l4len, IPPROTO_UDP, flow);
> +	char *data = tap_push_uh6(uh, src, sport, dst, dport, in, dlen);
>  
> +	memcpy(data, in, dlen);
>  	tap_send_single(c, buf, dlen + (data - buf));
>  }
>  
> diff --git a/tap.h b/tap.h
> index 37da21d..b7b8cef 100644
> --- a/tap.h
> +++ b/tap.h
> @@ -47,6 +47,10 @@ static inline void tap_hdr_update(struct tap_hdr *thdr, size_t l2len)
>  void *tap_push_uh4(struct udphdr *uh, struct in_addr src, in_port_t sport,
>  		   struct in_addr dst, in_port_t dport,
>  		   const void *in, size_t dlen);
> +void *tap_push_uh6(struct udphdr *uh,
> +		   const struct in6_addr *src, in_port_t sport,
> +		   const struct in6_addr *dst, in_port_t dport,
> +		   void *in, size_t dlen);
>  void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src,
>  		    struct in_addr dst, size_t l4len, uint8_t proto);
>  void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport,

-- 
David Gibson (he or they)	| 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 --]