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

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

[Jon Maloy]

During testing it is sometimes useful to force traffic which would
normally be forwarded by socket splicing through the tap interface.

In this commit, we add a command switch making it possible to disable
splicing for inbound local traffic.

For outbound local traffic this seems to be much trickier, so I leave
that for a possible later commit.

Suggested-by: David Gibson <dgibson@redhat.com>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 conf.c  | 5 +++++
 fwd.c   | 2 +-
 passt.h | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/conf.c b/conf.c
index eaa7d99..8d58652 100644
--- a/conf.c
+++ b/conf.c
@@ -890,6 +890,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 outbound socket splicing\n"
 		"  --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"
@@ -1319,6 +1320,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' },
@@ -1756,6 +1758,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 c038630..0271e7c 100644
--- a/passt.h
+++ b/passt.h
@@ -291,6 +291,7 @@ struct ctx {
 	int no_dhcpv6;
 	int no_ndp;
 	int no_ra;
+	int no_splice;
 	int host_lo_to_ns_lo;
 	int freebind;
 
-- 
@@ -291,6 +291,7 @@ struct ctx {
 	int no_dhcpv6;
 	int no_ndp;
 	int no_ra;
+	int no_splice;
 	int host_lo_to_ns_lo;
 	int freebind;
 
-- 
2.45.2

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

[Jon Maloy]

On 2024-11-28 19:45, Jon Maloy wrote:
> During testing it is sometimes useful to force traffic which would
> normally be forwarded by socket splicing through the tap interface.
>
> In this commit, we add a command switch making it possible to disable
> splicing for inbound local traffic.
>
> For outbound local traffic this seems to be much trickier, so I leave
> that for a possible later commit.
I am looking for more input here.

David suggested that I simply don't re-bind any sockets inwards towards
the local namespace, so that all outbound traffic would use the default
route and be forced to go via the tap interface.

I tried this, and realized it won't work. Outgoing traffic using INADDR_ANY
or loopback address will never be routed via the default route; if it 
doesn't
find the destination port in the local name space it will simply return 
with
'connection refused'. There is no nice way to force such traffic via the 
default
route, as far as I understand.

I am even questioning if it is necessary: If the port is bound on the host,
the client only needs to use some of the non-loopback addresses on the
host to reach it via the tap interface.

///jon
>
> Suggested-by: David Gibson <dgibson@redhat.com>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>   conf.c  | 5 +++++
>   fwd.c   | 2 +-
>   passt.h | 1 +
>   3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/conf.c b/conf.c
> index eaa7d99..8d58652 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -890,6 +890,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 outbound socket splicing\n"
>   		"  --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"
> @@ -1319,6 +1320,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' },
> @@ -1756,6 +1758,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 c038630..0271e7c 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -291,6 +291,7 @@ struct ctx {
>   	int no_dhcpv6;
>   	int no_ndp;
>   	int no_ra;
> +	int no_splice;
>   	int host_lo_to_ns_lo;
>   	int freebind;
>   

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

[David Gibson]

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

On Thu, Nov 28, 2024 at 07:45:32PM -0500, Jon Maloy wrote:
> During testing it is sometimes useful to force traffic which would
> normally be forwarded by socket splicing through the tap interface.
> 
> In this commit, we add a command switch making it possible to disable
> splicing for inbound local traffic.
> 
> For outbound local traffic this seems to be much trickier, so I leave
> that for a possible later commit.

See comments on your other mail.

> Suggested-by: David Gibson <dgibson@redhat.com>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

This LGTM, excepting minor details.  Arguably it's even enough, since
you can effectively disable outbound splicing by not using -T or -U.

> ---
>  conf.c  | 5 +++++
>  fwd.c   | 2 +-
>  passt.h | 1 +
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/conf.c b/conf.c
> index eaa7d99..8d58652 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -890,6 +890,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 outbound socket splicing\n"

This should be "inbound" not "outbound" yes?

For a final version this would need to be added to the manpage as
well.  I guess unless we wanted to leave it undocumented as an option
intended only for developer testing.

>  		"  --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"
> @@ -1319,6 +1320,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' },
> @@ -1756,6 +1758,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 c038630..0271e7c 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -291,6 +291,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 --]

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

[David Gibson]

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

On Thu, Nov 28, 2024 at 08:21:23PM -0500, Jon Maloy wrote:
> 
> 
> On 2024-11-28 19:45, Jon Maloy wrote:
> > During testing it is sometimes useful to force traffic which would
> > normally be forwarded by socket splicing through the tap interface.
> > 
> > In this commit, we add a command switch making it possible to disable
> > splicing for inbound local traffic.
> > 
> > For outbound local traffic this seems to be much trickier, so I leave
> > that for a possible later commit.
> I am looking for more input here.
> 
> David suggested that I simply don't re-bind any sockets inwards towards
> the local namespace, so that all outbound traffic would use the default
> route and be forced to go via the tap interface.
> 
> I tried this, and realized it won't work. Outgoing traffic using INADDR_ANY
> or loopback address will never be routed via the default route; if it
> doesn't
> find the destination port in the local name space it will simply return with
> 'connection refused'. There is no nice way to force such traffic via the
> default
> route, as far as I understand.

Right.  I think the confusion here is because splicing kind of does
two things.  First, it takes some cases that would work with tap, but
optimises them.  Second it makes some cases possible that aren't
possible with just the tap interface: specifically redirecting guest
side traffic with destination 0.0.0.0 or 127.0.0.1/8.

I've been assuming that a --no-splice option would disable both cases.
So traffic that *can* be redirected via tap instead would be, but
things that are only possible with splice would just be disallowed.
The latter has the arguable advantage that it eliminates the (small)
network behavioural differences between pasta and passt mode.

> I am even questioning if it is necessary: If the port is bound on the host,
> the client only needs to use some of the non-loopback addresses on the
> host to reach it via the tap interface.

Right.  In fact with both --no-splice as per your draft, and no -T and
-U options, I don't think any use of splice is possible.  We could
maybe put a test in fwd_nat_from_splice() to check.

> 
> ///jon
> > 
> > Suggested-by: David Gibson <dgibson@redhat.com>
> > Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> > ---
> >   conf.c  | 5 +++++
> >   fwd.c   | 2 +-
> >   passt.h | 1 +
> >   3 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index eaa7d99..8d58652 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -890,6 +890,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 outbound socket splicing\n"
> >   		"  --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"
> > @@ -1319,6 +1320,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' },
> > @@ -1756,6 +1758,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 c038630..0271e7c 100644
> > --- a/passt.h
> > +++ b/passt.h
> > @@ -291,6 +291,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 --]

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

[Stefano Brivio]

On Thu, 28 Nov 2024 20:21:23 -0500
Jon Maloy <jmaloy@redhat.com> wrote:

> I am even questioning if it is necessary: If the port is bound on the host,
> the client only needs to use some of the non-loopback addresses on the
> host to reach it via the tap interface.

The same applies to inbound traffic by the way, as I mentioned: just use
a non-loopback address in the namespace/guest to reach it via the tap
interface:

$ { sleep 1; : | nc -4 -N passt.top 9999; } & ./pasta -q --config-net -p nc.pcap -t 9999 -- nc -q1 -l 9999
[1] 2052460
[1]+  Done                    { sleep 1; : | nc -4 -N passt.top 9999; }
$ tshark -r nc.pcap ip
    8   0.968929 88.198.0.161 → 88.198.0.164 TCP 62 55898 → 9999 [SYN] Seq=0 Win=65535 Len=0 MSS=61440 WS=256
   11   0.968967 88.198.0.164 → 88.198.0.161 TCP 62 9999 → 55898 [SYN, ACK] Seq=0 Ack=1 Win=65480 Len=0 MSS=65480 WS=4096
   12   0.969000 88.198.0.161 → 88.198.0.164 TCP 54 55898 → 9999 [ACK] Seq=1 Ack=1 Win=65536 Len=0
   13   0.969017 88.198.0.161 → 88.198.0.164 TCP 54 55898 → 9999 [FIN, ACK] Seq=1 Ack=1 Win=65536 Len=0
   14   0.969063 88.198.0.164 → 88.198.0.161 TCP 54 9999 → 55898 [FIN, ACK] Seq=1 Ack=2 Win=65536 Len=0
   15   0.969118 88.198.0.161 → 88.198.0.164 TCP 54 55898 → 9999 [ACK] Seq=2 Ack=2 Win=65536 Len=0

...that is, I guess that you or David find this convenient, but strictly
speaking, it's also unnecessary (actually, I won't use it myself, because
it's more typing).

-- 
Stefano

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

[Stefano Brivio]

On Thu, 28 Nov 2024 19:45:32 -0500
Jon Maloy <jmaloy@redhat.com> wrote:

> During testing it is sometimes useful to force traffic which would
> normally be forwarded by socket splicing through the tap interface.
> 
> In this commit, we add a command switch making it possible to disable
> splicing for inbound local traffic.
> 
> For outbound local traffic this seems to be much trickier, so I leave
> that for a possible later commit.
> 
> Suggested-by: David Gibson <dgibson@redhat.com>

David's address is david@gibson.dropbear.id.au.

> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>  conf.c  | 5 +++++
>  fwd.c   | 2 +-
>  passt.h | 1 +
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/conf.c b/conf.c
> index eaa7d99..8d58652 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -890,6 +890,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 outbound socket splicing\n"
>  		"  --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"
> @@ -1319,6 +1320,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' },
> @@ -1756,6 +1758,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 c038630..0271e7c 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -291,6 +291,7 @@ struct ctx {
>  	int no_dhcpv6;
>  	int no_ndp;
>  	int no_ra;
> +	int no_splice;

This should also be documented in the comment to the struct.

-- 
Stefano