public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: Yumei Huang <yuhuang@redhat.com>, passt-dev@passt.top
Subject: Re: [PATCH] conf, pasta: Add --no-tap option
Date: Mon, 12 Jan 2026 15:26:14 +1100	[thread overview]
Message-ID: <aWR35hL-qDQ2phHO@zatzit> (raw)
In-Reply-To: <20260110191219.29498050@elisabeth>

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

On Sat, Jan 10, 2026 at 07:12:19PM +0100, Stefano Brivio wrote:
> On Mon, 5 Jan 2026 16:53:49 +0800
> Yumei Huang <yuhuang@redhat.com> wrote:
> 
> > On Mon, Jan 5, 2026 at 12:18 PM David Gibson
> > <david@gibson.dropbear.id.au> wrote:
> > >
> > > On Mon, Dec 29, 2025 at 05:55:58PM +0800, Yumei Huang wrote:  
> > > > This patch introduces a mode where we only forward loopback connections
> > > > and traffic between two namespaces (via the loopback interface, 'lo'),
> > > > without a tap device.
> > > >
> > > > With this, podman can support forwarding ::1 in custom networks when using
> > > > rootlesskit for forwarding ports.
> > > >
> > > > In --no-tap mode, --host-lo-to-ns-lo, --no-icmp and --no-ra is automatically
> > > > enabled. Options requiring a tap device (--ns-ifname, --ns-mac-addr,
> > > > --config-net, --outbound-if4/6) are rejected.
> > > >
> > > > Link: https://bugs.passt.top/show_bug.cgi?id=149
> > > > Signed-off-by: Yumei Huang <yuhuang@redhat.com>  
> > >
> > > Nice work.  There are some things that need polish, but overall this
> > > looks pretty good to me.  Like Stefano, I'm pleasantly surprised at
> > > how simple it turned out to be.
> > >  
> > > > ---
> > > >  conf.c  | 56 +++++++++++++++++++++++++++++++++++++++++---------------
> > > >  fwd.c   |  3 +++
> > > >  passt.1 |  5 +++++
> > > >  passt.h |  2 ++
> > > >  pasta.c |  3 +++
> > > >  tap.c   | 11 +++++++----
> > > >  6 files changed, 61 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/conf.c b/conf.c
> > > > index 84ae12b..353d0a5 100644
> > > > --- a/conf.c
> > > > +++ b/conf.c
> > > > @@ -1049,7 +1049,8 @@ pasta_opts:
> > > >               "  --no-copy-addrs      DEPRECATED:\n"
> > > >               "                       Don't copy all addresses to namespace\n"
> > > >               "  --ns-mac-addr ADDR   Set MAC address on tap interface\n"
> > > > -             "  --no-splice          Disable inbound socket splicing\n");
> > > > +             "  --no-splice          Disable inbound socket splicing\n"
> > > > +             "  --no-tap             Don't create tap device\n");  
> > >
> > > I feel like this description can be improved, but I'm not exactly sure
> > > how, yet.
> 
> A few possible alternatives:
> 
> - "Only enable loopback forwarding"

I prefer this one to the one below.

> - "Loopback only from/to namespace"
> 
> - call it --splice-only, and use one of the descriptions above
> 
> - call it --loopback-only, and use one of the descriptions above
> 
> > >  
> > > >
> > > >       passt_exit(status);
> > > >  }
> > > > @@ -1451,6 +1452,7 @@ void conf(struct ctx *c, int argc, char **argv)
> > > >               {"no-ndp",      no_argument,            &c->no_ndp,     1 },
> > > >               {"no-ra",       no_argument,            &c->no_ra,      1 },
> > > >               {"no-splice",   no_argument,            &c->no_splice,  1 },
> > > > +             {"no-tap",      no_argument,            &c->no_tap,     1 },
> > > >               {"freebind",    no_argument,            &c->freebind,   1 },
> > > >               {"no-map-gw",   no_argument,            &no_map_gw,     1 },
> > > >               {"ipv4-only",   no_argument,            NULL,           '4' },
> > > > @@ -1947,8 +1949,11 @@ void conf(struct ctx *c, int argc, char **argv)
> > > >               }
> > > >       } while (name != -1);
> > > >
> > > > -     if (c->mode != MODE_PASTA)
> > > > +     if (c->mode != MODE_PASTA) {
> > > >               c->no_splice = 1;
> > > > +             if (c->no_tap)
> > > > +                     die("--no-tap is for pasta mode only");
> > > > +     }
> > > >
> > > >       if (c->mode == MODE_PASTA && !c->pasta_conf_ns) {
> > > >               if (copy_routes_opt)
> > > > @@ -1957,6 +1962,25 @@ void conf(struct ctx *c, int argc, char **argv)
> > > >                       die("--no-copy-addrs needs --config-net");
> > > >       }
> > > >
> > > > +     if (c->mode == MODE_PASTA && c->no_tap) {
> > > > +             if (c->no_splice)
> > > > +                     die("--no-tap is incompatible with --no-splice");
> > > > +             if (*c->ip4.ifname_out || *c->ip6.ifname_out)
> > > > +                     die("--no-tap is incompatible with --outbound-if4/6");
> > > > +             if (*c->pasta_ifn)
> > > > +                     die("--no-tap is incompatible with --ns-ifname");
> > > > +             if (*c->guest_mac)
> > > > +                     die("--no-tap is incompatible with --ns-mac-addr");  
> > >
> > > These all make sense.  It might also make sense to exclude the -i
> > > option - setting a template interface also makes no sense in --no-tap
> > > mode.  
> > 
> > Sure, I can add an if condition with if4 (as if4=if6 in that case).
> > >  
> > > > +             if (c->pasta_conf_ns)
> > > > +                     die("--no-tap is incompatible with --config-net");  
> > >
> > > I don't think this is right.  We still can and should bring up 'lo' in
> > > the --no-tap case.  
> > 
> > I see your point, but seems c->pasta_conf_ns is only used for tap as
> > https://passt.top/passt/tree/pasta.c#n328, 'lo' is configured before
> > that line.
> 
> Right, and the reason is that there are basic bits of functionality
> (probing pipe sizes if I recall correctly, or anyway probing for some
> kind of capability) that need the loopback interface to be up.

Ah, right.  Drat.  In general I don't like us touching the guest
netlink at all if we don't have --config-net.  Hrm.. now what exactly
needs this.  It's not anything in sock_probe_features() - that runs in
the host ns.  Not pipe sizes, either - that also takes place in the
host ns (and netns is irrelevant to pipes, anyway).   There could well
be something, but I'm not sure what it is.

> On the other hand, checks we're adding here are kind of fragile because
> we'll add other options in the future and probably forget to check
> which ones are incompatible, so I would try a slightly different
> approach: only check the options that are *obviously* conflicting with
> --no-tap.

I agree.

> That is, the main thing "--config-net" does is to "Configure networking
> in the namespace", which we still do with "--no-tap".

Agreed.

> Now, I see that making sure c->pasta_conf_ns is false saves you checks
> elsewhere in the implementation, which is, I think, a good reason to
> have this check here.
> 
> But in general we don't need to exclude all the possible options that
> make no sense with --no-tap. We don't really confuse users if we allow
> them (or, at least, some of them).
> 
> > > > +             c->host_lo_to_ns_lo = 1;
> > > > +             c->no_icmp = 1;
> > > > +             c->no_ra = 1;
> > > > +             c->no_dns = 1;
> > > > +             c->no_dns_search = 1;  
> > >
> > > The reasoning for the last two items is a bit unclear to me.  IIUC,
> > > no_dns and no_dns_search aren't so much about "support" for DNS itself
> > > but for advertising DNS settings via DHCP.  Since DHCP will be
> > > unsupported, so are these as a consequence.  Is that right?  
> > 
> > Yeah, I think so. Actually I added c->no_dhcp, c->no_ndp here as well,
> > then removed them as they are set in later changes(conditions about
> > c->ifi4/c->ifi6), though they turn out to be not quite right :'\
> 
> Do we care about them, though? That code won't be reachable anyway,
> unless I'm missing something. Or is it to make the output of
> conf_print() nicer? In that case I guess it makes sense to go and
> disable things.

I think we want to avoid making conf_print() output misleading, and
this seems a reasonable way of doing it.

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

  reply	other threads:[~2026-01-12  4:26 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-29  9:55 Yumei Huang
2025-12-31 15:07 ` Stefano Brivio
2026-01-05  4:18 ` David Gibson
2026-01-05  8:53   ` Yumei Huang
2026-01-10 18:12     ` Stefano Brivio
2026-01-12  4:26       ` David Gibson [this message]
2026-01-13  0:12         ` Stefano Brivio
2026-01-13  2:39           ` David Gibson
2026-01-13  9:57       ` Yumei Huang
2026-01-05 13:48 ` Paul Holzinger
2026-01-05 21:10   ` Stefano Brivio
2026-01-07 15:20     ` Paul Holzinger
2026-01-10 18:12       ` Stefano Brivio
2026-01-12  8:20         ` Yumei Huang
2026-01-10 18:12 ` Stefano Brivio
2026-01-13 11:20   ` Yumei Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aWR35hL-qDQ2phHO@zatzit \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    --cc=yuhuang@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).