Re: [PATCH v2] pasta: make it possible to disable socket splicing

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

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

On Fri, Nov 29, 2024 at 05:49:46PM -0500, Jon Maloy wrote:
> During testing it is sometimes useful to force traffic which would
> normally be forwared by socket splicing through the tap interface.
> 
> In this commit, we add a command switch enabling such funtionality
> for inbound local traffic.
> 
> For outbound local traffic this is much trickier, if even possible,
> so leave that for a later commit.
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

One nit, and the man page needs updating as Stefano mentions,
otherwise LGTM.

> ---
> v2: Some minor changes based on feedback from PASST team
> ---
>  conf.c  | 5 +++++
>  fwd.c   | 2 +-
>  passt.h | 2 ++
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/conf.c b/conf.c
> index 5107549..fe6fa04 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -846,6 +846,7 @@ static void usage(const char *name, FILE *f, int status)
>  		"  --no-ndp		Disable NDP responses\n"
>  		"  --no-dhcpv6		Disable DHCPv6 server\n"
>  		"  --no-ra		Disable router advertisements\n"
> +		"  --no-splice		Disable inbound socket splicing\n"

This is currently listed in the common options, but has no effect for
passt mode.  Is that what we want?  Or should we move this to the
pasta specific options?

>  		"  --freebind		Bind to any address for forwarding\n"
>  		"  --no-map-gw		Don't map gateway address to host\n"
>  		"  -4, --ipv4-only	Enable IPv4 operation only\n"
> @@ -1274,6 +1275,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  		{"no-dhcpv6",	no_argument,		&c->no_dhcpv6,	1 },
>  		{"no-ndp",	no_argument,		&c->no_ndp,	1 },
>  		{"no-ra",	no_argument,		&c->no_ra,	1 },
> +		{"no-splice",	no_argument,		&c->no_splice,	1 },
>  		{"freebind",	no_argument,		&c->freebind,	1 },
>  		{"no-map-gw",	no_argument,		&no_map_gw,	1 },
>  		{"ipv4-only",	no_argument,		NULL,		'4' },
> @@ -1701,6 +1703,9 @@ void conf(struct ctx *c, int argc, char **argv)
>  		}
>  	} while (name != -1);
>  
> +	if (c->mode == MODE_PASST)
> +		c->no_splice = 1;
> +
>  	if (c->mode == MODE_PASTA && !c->pasta_conf_ns) {
>  		if (copy_routes_opt)
>  			die("--no-copy-routes needs --config-net");
> diff --git a/fwd.c b/fwd.c
> index 0b7f8b1..2829cd2 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -443,7 +443,7 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
>  	else if (proto == IPPROTO_UDP)
>  		tgt->eport += c->udp.fwd_in.delta[tgt->eport];
>  
> -	if (c->mode == MODE_PASTA && inany_is_loopback(&ini->eaddr) &&
> +	if (!c->no_splice && inany_is_loopback(&ini->eaddr) &&
>  	    (proto == IPPROTO_TCP || proto == IPPROTO_UDP)) {
>  		/* spliceable */
>  
> diff --git a/passt.h b/passt.h
> index 72c7f72..45ad26a 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -225,6 +225,7 @@ struct ip6_ctx {
>   * @no_dhcpv6:		Disable DHCPv6 server
>   * @no_ndp:		Disable NDP handler altogether
>   * @no_ra:		Disable router advertisements
> + * @no_splice:		Disable socket splicing for inbound traffic
>   * @host_lo_to_ns_lo:	Map host loopback addresses to ns loopback addresses
>   * @freebind:		Allow binding of non-local addresses for forwarding
>   * @low_wmem:		Low probed net.core.wmem_max
> @@ -286,6 +287,7 @@ struct ctx {
>  	int no_dhcpv6;
>  	int no_ndp;
>  	int no_ra;
> +	int no_splice;
>  	int host_lo_to_ns_lo;
>  	int freebind;
>  

-- 
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 --]