From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=D0My7k4o; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 603345A0275 for ; Thu, 02 Apr 2026 23:55:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1775166952; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kBqHQXv6yRLUn9xvxQRnUui1zSqlGJDEPhyM/mA0TsY=; b=D0My7k4o87VhwOXSxBjn9MCFatyD0nW2Icr2Lg0bfvKItSpu8PWbq2BNFBuiHyOJXVOYgd L8u454AOeiOgzWBI71+hS1LLXsSAlIgAcSxYPM9SS0O7QJJC5fahODZPV8p0DikoTt5peA wANCfOVOsNkdoEn6rMgrzMdDWtW6QJk= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-687-hm6IXB7wPam5p4TSZoxSNA-1; Thu, 02 Apr 2026 17:55:50 -0400 X-MC-Unique: hm6IXB7wPam5p4TSZoxSNA-1 X-Mimecast-MFC-AGG-ID: hm6IXB7wPam5p4TSZoxSNA_1775166950 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-486fe36cf73so8448595e9.1 for ; Thu, 02 Apr 2026 14:55:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775166949; x=1775771749; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kBqHQXv6yRLUn9xvxQRnUui1zSqlGJDEPhyM/mA0TsY=; b=m+Dle7zpzcjdTZcH9fnjHAlLtL9CqUgSoR3jdQHHxYCsghmHMkisJvUkLOxQaK/0Xx bKEn2YpBJDp02ZFtQn/TaSu5M+Llz17azSPW8hGr8nc1EVRdSiDDogxLBAnfLaizdaMx e+zXY96cloEaMo01GRXm6cLW5/nG0SGToaLrdrLLfMTpuIVolWmGJ14U45DASAS8l/6Z fXmE6+6J3WWmD2K7pk2WhDxRyt/tsALYla/2BB855rf08ZxAPRYtaX0d+TJMHRP7Yi1O M8XS1fhHxU0hjBKCnI5k7PzB11CdqQ4Y2rbxKiwKvieIBtiEyi/T651EA/xBsmkngwCJ 6Lng== X-Forwarded-Encrypted: i=1; AJvYcCUlB+wHkbSwDoySdIQJaZF+v8i80m3yyOX8UC7pm/UEWLX4FMJTKR78riGeBHH83Hubbr7lln1cSZc=@passt.top X-Gm-Message-State: AOJu0YwrmOANkCK88ONmBJ5Y0NpXwh4xbuxKibgbFmO2qWSlR7aISaBc hJj6Vlh8Hsi6v8Z9wszDG3pgYHiTBwQcgtlFMvZaWKtQv67GvvclP4qPNmIFp+7bLOum1IAJCv+ XRHRB1j/8GZEzL/kk8k9tELF+XDZD/VBz49tgoykAyUOo6YWrfDHcn85y2OdPhQ== X-Gm-Gg: ATEYQzwGKExknWM8CDiGou11w2xzVWwHjgSic0pQ47oo/dSAxMo5+rofZD+2KVhN2+m qjCrS2IKIqChCMqvHlXqr8bL4iIJ25Myv0WPB0sB/w91uND8EnT2BjViM+VNI71t+v9a0F+g42s PZwYCUQ8caWZT0/Gd23EA1KCE9NbgZBEBoQBercvrIHWO8L4rOHn3yeiz/92bEtxKqIjw9vmhKI yfE91MGbROcLCxPJSWk3ozDmUKIfkaVNur8fVwQccuFxwY7LYzDPHAQyQkUE+kr5zUFbLSmmF8r 5v/0hDGEqP22gBi/1N3IyQ/yQOQj3VRpRSBl7xRGwI2g9Ww62iNcq9fuhSoUZY4s2zrHVCI/KFY TMck3kYODTXQk53f1DbeIsVRiyPQnJYz0 X-Received: by 2002:a05:600c:4ec7:b0:485:3a03:ceca with SMTP id 5b1f17b1804b1-488997ee016mr9011315e9.23.1775166949118; Thu, 02 Apr 2026 14:55:49 -0700 (PDT) X-Received: by 2002:a05:600c:4ec7:b0:485:3a03:ceca with SMTP id 5b1f17b1804b1-488997ee016mr9010865e9.23.1775166948552; Thu, 02 Apr 2026 14:55:48 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4887e80a63esm284199995e9.3.2026.04.02.14.55.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Apr 2026 14:55:48 -0700 (PDT) From: Stefano Brivio To: Jon Maloy Subject: Re: [PATCH v6 13/13] ndp: Support advertising multiple prefixes in Router Advertisements Message-ID: <20260402235547.132f660c@elisabeth> In-Reply-To: <20260322004333.365713-14-jmaloy@redhat.com> References: <20260322004333.365713-1-jmaloy@redhat.com> <20260322004333.365713-14-jmaloy@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Thu, 02 Apr 2026 23:55:47 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 3iYHPrNQQzAIJ-ceQ3F4MB0kz32VYzq0qUNPBqCYLnM_1775166950 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: NA35NNAH2TCL4LGETZURYRNBX2QUCGXL X-Message-ID-Hash: NA35NNAH2TCL4LGETZURYRNBX2QUCGXL X-MailFrom: sbrivio@redhat.com 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: david@gibson.dropbear.id.au, 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: On Sat, 21 Mar 2026 20:43:33 -0400 Jon Maloy wrote: > We extend NDP to advertise all suitable IPv6 prefixes in Router > Advertisements, per RFC 4861. Observed and link-local addresses, > plus addresses with a prefix length != 64, are excluded. > > Signed-off-by: Jon Maloy > > --- > v6: Adapted to previous changes in series > --- > conf.c | 19 ++++++--- > fwd.c | 3 ++ > migrate.c | 5 +++ > ndp.c | 121 ++++++++++++++++++++++++++++++++++++------------------ > passt.h | 1 + > 5 files changed, 103 insertions(+), 46 deletions(-) > > diff --git a/conf.c b/conf.c > index de2fb7c..b8dd40a 100644 > --- a/conf.c > +++ b/conf.c > @@ -1213,7 +1213,7 @@ static void conf_print(const struct ctx *c) > } > > if (c->ifi6) { > - bool has_dhcpv6 = false; > + bool has_ndp = false, has_dhcpv6 = false; > const char *head; > > if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback)) > @@ -1223,18 +1223,25 @@ static void conf_print(const struct ctx *c) > > /* Check what we have to advertise */ > for_each_addr(a, c, AF_INET6) { > + if (a->flags & CONF_ADDR_NDP) > + has_ndp = true; > if (a->flags & CONF_ADDR_DHCPV6) > has_dhcpv6 = true; > } > > - if (c->no_ndp && !has_dhcpv6) > + if (!has_ndp && !has_dhcpv6) > goto dns6; > > - a = fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); > - if (!c->no_ndp && a) { > + if (has_ndp) { > info("NDP:"); > - inany_ntop(&a->addr, buf, sizeof(buf)); > - info(" assign: %s", buf); > + head = "assign"; Same comment as for 12/13. > + for_each_addr(a, c, AF_INET6) { > + if (!(a->flags & CONF_ADDR_NDP)) > + continue; > + inany_ntop(&a->addr, buf, sizeof(buf)); > + info(" %s: %s/%d", head, buf, a->prefix_len); It's a bit redundant to have the prefix length here as a variable given that it's always /64, but it might make the code clearer, so I don't really have a preference. > + head = " "; > + } > } > > if (has_dhcpv6) { > diff --git a/fwd.c b/fwd.c > index f867398..fe6e9d4 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -305,6 +305,9 @@ void fwd_set_addr(struct ctx *c, const union inany_addr *addr, > /* DHCPv6 for IPv6 */ > if (!c->no_dhcpv6) > flags |= CONF_ADDR_DHCPV6; > + /* NDP/RA only if /64 prefix, as required for SLAAC */ It's not "only" in the sense of "NDP only", rather in the "only if" sense, this could be less ambiguous. Regardless of that, I don't think it's a requirement, because we can derive a prefix (a subnet prefix, that is, /64) from an address with a longer or shorter prefix length. If we don't, we're actually going to break functionality as we can't assume that the guest will always have a DHCPv6 client running (but the host might, and that would typically mean a /128 address). In the case of prefix delegation, you'll have a shorter prefix length on the host, but that doesn't mean we should totally ignore that prefix while configuring the guest. > + if (!c->no_ndp && prefix_len == 64) > + flags |= CONF_ADDR_NDP; > } > } > > diff --git a/migrate.c b/migrate.c > index 105f624..98c2d7e 100644 > --- a/migrate.c > +++ b/migrate.c > @@ -54,6 +54,7 @@ struct migrate_seen_addrs_v2 { > #define MIGRATE_ADDR_OBSERVED BIT(3) > #define MIGRATE_ADDR_DHCP BIT(4) > #define MIGRATE_ADDR_DHCPV6 BIT(5) > +#define MIGRATE_ADDR_NDP BIT(6) Nit: maybe we should call this SLAAC as it's the specific address assignment mechanism (saying NDP is a bit like saying "UDP" to imply DHCP). NDP is perfectly clear though. > > /** > * struct migrate_addr_v3 - Wire format for a single address entry > @@ -89,6 +90,8 @@ static uint8_t flags_to_wire(uint8_t flags) > wire |= MIGRATE_ADDR_DHCP; > if (flags & CONF_ADDR_DHCPV6) > wire |= MIGRATE_ADDR_DHCPV6; > + if (flags & CONF_ADDR_NDP) > + wire |= MIGRATE_ADDR_NDP; > > return wire; > } > @@ -115,6 +118,8 @@ static uint8_t flags_from_wire(uint8_t wire) > flags |= CONF_ADDR_DHCP; > if (wire & MIGRATE_ADDR_DHCPV6) > flags |= CONF_ADDR_DHCPV6; > + if (wire & MIGRATE_ADDR_NDP) > + flags |= CONF_ADDR_NDP; > > return flags; > } > diff --git a/ndp.c b/ndp.c > index 3750fc5..f47286e 100644 > --- a/ndp.c > +++ b/ndp.c > @@ -32,6 +32,8 @@ > #include "passt.h" > #include "tap.h" > #include "log.h" > +#include "fwd.h" > +#include "conf.h" > > #define RT_LIFETIME 65535 > > @@ -82,7 +84,7 @@ struct ndp_na { > } __attribute__((packed)); > > /** > - * struct opt_prefix_info - Prefix Information option > + * struct opt_prefix_info - Prefix Information option header This looks unrelated. > * @header: Option header > * @prefix_len: The number of leading bits in the Prefix that are valid > * @prefix_flags: Flags associated with the prefix > @@ -99,6 +101,16 @@ struct opt_prefix_info { > uint32_t reserved; > } __attribute__((packed)); > > +/** > + * struct ndp_prefix - Complete Prefix Information option with prefix If it's complete, it'd better have a prefix... > + * @info: Prefix Information option header > + * @prefix: IPv6 prefix > + */ > +struct ndp_prefix { > + struct opt_prefix_info info; > + struct in6_addr prefix; > +} __attribute__((__packed__)); > + > /** > * struct opt_mtu - Maximum transmission unit (MTU) option > * @header: Option header > @@ -140,27 +152,23 @@ struct opt_dnssl { > } __attribute__((packed)); > > /** > - * struct ndp_ra - NDP Router Advertisement (RA) message > + * struct ndp_ra_hdr - NDP Router Advertisement fixed header > * @ih: ICMPv6 header > * @reachable: Reachability time, after confirmation (ms) > * @retrans: Time between retransmitted NS messages (ms) > - * @prefix_info: Prefix Information option > - * @prefix: IPv6 prefix > - * @mtu: MTU option > - * @source_ll: Target link-layer address > - * @var: Variable fields > */ > -struct ndp_ra { > +struct ndp_ra_hdr { > struct icmp6hdr ih; > uint32_t reachable; > uint32_t retrans; > - struct opt_prefix_info prefix_info; > - struct in6_addr prefix; > - struct opt_l2_addr source_ll; > +} __attribute__((__packed__)); > > - unsigned char var[sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) + > - sizeof(struct opt_dnssl)]; > -} __attribute__((packed, aligned(__alignof__(struct in6_addr)))); > +/* Maximum RA message size: hdr + prefixes + source_ll + mtu + rdnss + dnssl */ > +#define NDP_RA_MAX_SIZE (sizeof(struct ndp_ra_hdr) + \ > + MAX_GUEST_ADDRS * sizeof(struct ndp_prefix) + \ > + sizeof(struct opt_l2_addr) + \ > + sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) + \ > + sizeof(struct opt_dnssl)) > > /** > * struct ndp_ns - NDP Neighbor Solicitation (NS) message > @@ -231,6 +239,42 @@ void ndp_unsolicited_na(const struct ctx *c, const struct in6_addr *addr) > ndp_na(c, &in6addr_ll_all_nodes, addr); > } > > +/** > + * ndp_prefix_fill() - Fill prefix options for all suitable addresses > + * @c: Execution context > + * @buf: Buffer to write prefix options into > + * > + * Fills buffer with Prefix Information options for all non-linklocal, > + * non-observed addresses with prefix_len == 64 (required for SLAAC). Not really... the only requirement is that we _offer_ a subnet prefix, not that we have one. > + * > + * Return: number of bytes written > + */ > +static size_t ndp_prefix_fill(const struct ctx *c, unsigned char *buf) > +{ > + const struct guest_addr *a; > + struct ndp_prefix *p; > + size_t offset = 0; > + > + for_each_addr(a, c, AF_INET6) { > + if (!(a->flags & CONF_ADDR_NDP)) > + continue; > + > + p = (struct ndp_prefix *)(buf + offset); > + p->info.header.type = OPT_PREFIX_INFO; > + p->info.header.len = 4; /* 4 * 8 = 32 bytes */ > + p->info.prefix_len = 64; > + p->info.prefix_flags = 0xc0; /* L, A flags */ > + p->info.valid_lifetime = ~0U; > + p->info.pref_lifetime = ~0U; > + p->info.reserved = 0; > + p->prefix = a->addr.a6; > + > + offset += sizeof(struct ndp_prefix); > + } > + > + return offset; > +} > + > /** > * ndp_ra() - Send an NDP Router Advertisement (RA) message > * @c: Execution context > @@ -238,7 +282,15 @@ void ndp_unsolicited_na(const struct ctx *c, const struct in6_addr *addr) > */ > static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) > { > - struct ndp_ra ra = { > + unsigned char buf[NDP_RA_MAX_SIZE] > + __attribute__((__aligned__(__alignof__(struct in6_addr)))); > + struct ndp_ra_hdr *hdr = (struct ndp_ra_hdr *)buf; > + struct opt_l2_addr *source_ll; > + unsigned char *ptr; > + size_t prefix_len; > + > + /* Build RA header */ > + *hdr = (struct ndp_ra_hdr){ > .ih = { > .icmp6_type = RA, > .icmp6_code = 0, > @@ -247,31 +299,22 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) > .icmp6_rt_lifetime = htons_constant(RT_LIFETIME), > .icmp6_addrconf_managed = 1, > }, > - .prefix_info = { > - .header = { > - .type = OPT_PREFIX_INFO, > - .len = 4, > - }, > - .prefix_len = 64, > - .prefix_flags = 0xc0, /* prefix flags: L, A */ > - .valid_lifetime = ~0U, > - .pref_lifetime = ~0U, > - }, > - .source_ll = { > - .header = { > - .type = OPT_SRC_L2_ADDR, > - .len = 1, > - }, > - }, > }; > - const struct guest_addr *a = fwd_get_addr(c, AF_INET6, 0, 0); > - unsigned char *ptr = NULL; > - > - ASSERT(a); > > - ra.prefix = a->addr.a6; > + /* Fill prefix options */ > + prefix_len = ndp_prefix_fill(c, (unsigned char *)(hdr + 1)); > + if (prefix_len == 0) { > + /* No suitable prefixes to advertise */ > + return; > + } > > - ptr = &ra.var[0]; > + /* Add source link-layer address option */ Commit c16141eda5e8 ("ndp.c: Turn NDP responder into more declarative implementation") finally turned this into something more readable, and I think we could keep that by assigning this to a struct and then copying the whole thing into the message. By the way nothing should prevent us from moving this as first option and keeping the fields assigned in the initialiser. > + ptr = (unsigned char *)(hdr + 1) + prefix_len; > + source_ll = (struct opt_l2_addr *)ptr; > + source_ll->header.type = OPT_SRC_L2_ADDR; > + source_ll->header.len = 1; > + memcpy(source_ll->mac, c->our_tap_mac, ETH_ALEN); > + ptr += sizeof(struct opt_l2_addr); > > if (c->mtu) { > struct opt_mtu *mtu = (struct opt_mtu *)ptr; > @@ -345,10 +388,8 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) > } > } > > - memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN); > - > /* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */ > - ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra); > + ndp_send(c, dst, buf, ptr - buf); > } > > /** > diff --git a/passt.h b/passt.h > index c4c1f04..c6f4406 100644 > --- a/passt.h > +++ b/passt.h > @@ -77,6 +77,7 @@ enum passt_modes { > #define CONF_ADDR_OBSERVED BIT(3) /* Seen in guest traffic */ > #define CONF_ADDR_DHCP BIT(4) /* Advertise via DHCP (IPv4) */ > #define CONF_ADDR_DHCPV6 BIT(5) /* Advertise via DHCPv6 (IPv6) */ > +#define CONF_ADDR_NDP BIT(6) /* Advertise via NDP/RA (IPv6, /64) */ Same as above, I think "SLAAC" is more fitting here (even though "NDP" is perfectly clear). > > /** > * struct guest_addr - Unified IPv4/IPv6 address entry -- Stefano