From nobody Sat Sep 6 04:03:07 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4cJfhj0M4rz66tZf for ; Sat, 06 Sep 2025 04:03:33 +0000 (UTC) (envelope-from rlinnemann@netgate.com) Received: from mail-ed1-x534.google.com (mail-ed1-x534.google.com [IPv6:2a00:1450:4864:20::534]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4cJfhh4Dyqz3YHV for ; Sat, 06 Sep 2025 04:03:27 +0000 (UTC) (envelope-from rlinnemann@netgate.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ed1-x534.google.com with SMTP id 4fb4d7f45d1cf-61d3d622a2bso5102954a12.0 for ; Fri, 05 Sep 2025 21:03:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netgate.com; s=google; t=1757131400; x=1757736200; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=An/IToYiMg9ak1M0gmgaX22CnA1ZzzvKDxp9uTcUFtU=; b=A14EjK+Ax0XLRLVjhsxpgJoZ0gQmvLRjqHcBP4SS2W+cFQjXU45OPv4uabXJKpoL6U qXwdxwZYf6ReQvx+Xpx8qeNszwj+UMlKdyTunA61YxeoKatsBl9ZCiY7EuGRfSNBcfpH dxDYoJB91tpZy46JluFLKRpbQLIGknjgEdWgw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757131400; x=1757736200; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=An/IToYiMg9ak1M0gmgaX22CnA1ZzzvKDxp9uTcUFtU=; b=tT9a/C1b3uByMGSLzCGSkjZRBzXkQrqh463L7S59I8gLL3AjQDc5GGu2ZYQwopNavv /KEpcN8+UCHtqDkzDGUJHpjUtb5HkRGwplMoNg6FqJ/Kh3y4Da1veSx1WR/mvpH3lTqm AhjPiWXSXsuht71d8ti6/GQ9w7RVkD0LUNhy8aSjgrc/5sw+wvE4WDprmXk1tzwL9hhH thW2DK0ItpZR5a4WqqAU/1NgISB/brCzETlZ1MsDfFVqS6Y8qMRPbAtjJhOJ+KQT538L c0EorYWeWMezWB7qMJggT9CQB2L9AtDsMhdT8n7n8rK8g9AkrgsLrurYTbY2cXIuGyMZ bNGg== X-Forwarded-Encrypted: i=1; AJvYcCWByteftW6Eg9XB6sv/SC1z9fUjwQIt2IQ7hv9ai5BUZXLJyclIPC669V1EZk7K73WFO6I6tKR9mU8wL5ZJV8GvB9NL1g==@freebsd.org X-Gm-Message-State: AOJu0YwHdGQr5klYGLjBtGmhlBGszYNqt2zWLxCqmoO4k58JTZQ2z1lH xD2Lf5aOSRqbyslUJD2nShhgRTV1gFmj5e7ZKWYvpk8Gy5R582/YjInm3r4YD0rN5KMXjV+TVV2 7aMWbXb/ISRWmSPXkFFJPO8L2nReec3/sP23ATn+d X-Gm-Gg: ASbGnctYgSLFN46Qld7HlI6iaI2cCJnm7B9SHj/tOxqugccAsAUWKU//Wn0KcTzH2eQ PyKewzixbQCF6cpT0WYpNQ67NEXGn4/W/vhPbVVdwrqEGUDBVpsesVyTUdR1dtakqclyYQp09Ds nV9ofKz9eh22wDRZt7vGgMfz63wFcuWaobL/p6S1O6ikZjzooX64oxolvi2M+y3r6OsJZt4BpzP WxhTMeswZuYDtI= X-Google-Smtp-Source: AGHT+IH0KiJnxPiI4A45XdC5HeZsFtIwTIZ1fCfgkFpmNMpRlX/NxGVvhesIbmYPAzRzQZAYBRNfal0/NlyEl+mAUow= X-Received: by 2002:a05:6402:5110:b0:61c:6e16:4d73 with SMTP id 4fb4d7f45d1cf-623db077218mr726893a12.2.1757131399628; Fri, 05 Sep 2025 21:03:19 -0700 (PDT) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 References: <202509052153.585LrUjo056473@gitrepo.freebsd.org> <8rq4s287-pq47-58no-9r1o-p3o7659s0n60@SerrOFQ.bet> In-Reply-To: <8rq4s287-pq47-58no-9r1o-p3o7659s0n60@SerrOFQ.bet> From: Reid Linnemann Date: Fri, 5 Sep 2025 22:03:07 -0600 X-Gm-Features: Ac12FXzItGWa95LCsek9Lsp1SyuNeCYWfeyYTrRNCIw3ozf-UxlMVYFBz43plTg Message-ID: Subject: Re: git: 9e792f7ef729 - main - sys/netinet6: Fix SLAAC for interfaces with no /64 LL address To: "Bjoern A. Zeeb" Cc: Kristof Provost , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="000000000000e62010063e1a089a" X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Rspamd-Queue-Id: 4cJfhh4Dyqz3YHV --000000000000e62010063e1a089a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Bjoern, See responses inline below: On Fri, Sep 5, 2025 at 5:17=E2=80=AFPM Bjoern A. Zeeb wrot= e: > On Fri, 5 Sep 2025, Kristof Provost wrote: > > Hi, > > > The branch main has been updated by kp: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3D9e792f7ef7298080c058fbc2d36a4e6= 0e596dae9 > > > > commit 9e792f7ef7298080c058fbc2d36a4e60e596dae9 > > Author: Reid Linnemann > > AuthorDate: 2025-09-05 19:57:44 +0000 > > Commit: Kristof Provost > > CommitDate: 2025-09-05 21:48:48 +0000 > > > > sys/netinet6: Fix SLAAC for interfaces with no /64 LL address > > > > in6_ifadd() asserts that an interface has an existing LL address wit= h > a /64 > > prefix from which to extract the ifid for SLAAC address selection > (even though > > the comments suggest that an ifid will be generated if one does not > exist). This > > is adequate for most generic cases, however to support PPP links wit= h > /128 LL > > addresses we must be able to fall back on another source for the ifi= d > since we > > cannot assume the /128 LL has a unique ifid in the lower 64 bits. > > Excuse my ignorance but where do you have a IPv6 LL/128 on a PPP link? > > I am totally suprised given this hasn't been a problem in about 25 > years and I wonder what software this is or what got broken where? > > Okay, I scrolled through the review. I see I am the third person asking > the same question as the intial intuitive reply. > > I was intiially tempted to say 'please back this out' but see the end. > > While the impetus for this change was PPPoE, you can effectively ignore it. The IID selection is equally valid for any other scenario where the LLA prefix and RA prefix do not match, in which case *in6_ifadd()* originally would just throw its hands up and return an error. > For (1) do not start applying IPv4 to IPv6 just because. It's not just > a different number, it is also different concepts. People said this 20 > years ago and still do. > > I'm not comparing IPv4 to IPv6, the specific PPP(oE) problem I'm addressing that prompted this change is at the generic level of the struct ifaddr, in which any p2p address has a source and destination address. PPP(oE) implementations simply have to work around some of the behavior in the v6 implementation right now, primarily that only addresses with a /128 prefix are permitted to have a dstaddr (see *in6_validate_ifra()*), RA processing previously gave up if it could not get a IID from a /64 LLA, and an existing LLA prefix cannot be changed from /64 to /128, requiring the PPP(oE) client software to first remove an existing LLA (for instance if net.inet6.ip6.auto_linklocal is enabled) once address negotiation is complete in order to add a proper ifaddr to the address list. Either /64 dstaddrs needed to be permitted (which is not sensible as a p2p address is by definition composed solely of two endpoints), or RA processing should work in the case where the RA prefix doesn't match the LLA prefix (which I would argue should have been done since the original KAME implementation). > (2) Both of you talk about PPPoE in the review which tells me something > got wrong as that is a totally different protocol and has nothing to do > with IIDs. > > Selecting an appropriate LL for IPV6CP can be done entirely in user > space for protocol negotiations. And to my memory we have no IPV6CP > implementation in the kernel to require anything different. > > I don't know what the mentioned if_pppoe.ko is but if this is a convolute= d > implenentation in-kernel incl. protocol negotiations then I am tempted to > say you are off on your own with that, especially if you want to do thing= s > differently. Looking what Linux is doing is not always the best source o= f > truth. > > Please follow what the IPV6CP RFC (got updated since I last implemented i= t > 20-something years ago I have to admit) says and you end up with a /64 LL > on the interface and your problem is solved. > > I did refer to RFC5072, however I don't see any mention at all of prefix selection for the negotiated address. Regardless, let's ignore PPPoE for the purpose of this conversation because IID selection for SLAAC is not specific to this case. With regard to address selection for SLAAC, RFC2462 section 5.5.3 states: "d) *If the prefix advertised does not match the prefix of an address* * already in the list*, and the Valid Lifetime is not 0, form an address (and add it to the list) by combining the advertised prefix with the link's interface identifier as follows:" This is perfectly applicable to the situation that prompted the change, as the LLA prefix does not match the RA prefix. In such a case, rather than giving up and going home when the LLA prefix doesn't match the advertised prefix, we should take the bare minimum effort and try to get it from the horse's mouth before returning an error. Your negotiated LOCAL_LL (fe80::LOCAL_IID/64) address goes onto the IF: > > IF: flags=3D1008051 metric 0 m= tu > 1492 > options=3D80000 > inet ... > inet6 LOCAL_LL%IF prefixlen 64 scopeid 0x > inet6 ... > nd6 options=3D122 > > You get an entry in the routing table: > fe80::%IF/64 link#11 US IF > > and possibly a > default REMOTE_LL%IF UGS IF > > if you decide to install a default route. > > Other routes I would assume are installed on the link but I assume you > could > chose the REMOTE_LL%IF as well if you wanted to complicate things. > > So I crystal-ball that your real problem could have been stated that you > may need to get the IID for IPV6CP negotiations in-kernel? If that is no= t > the case, then I am still lost as-to what you are trying to accomplish. > But then you'd still never need the /128. So maybe I am just lsot. > > Again, we can forget PPPoE. PPPoE, whether in-kernel or in-userland handles IID selection itself for IPV6CP negotiation. The issue I'm addressing is SLAAC on interfaces with LLA prefixes that do not match the prefix in the RA. As PPP or PPPoE are both point-to-point links the ifaddr for the link should consist of a srcaddr and dstaddr, regardless of the specific IP protocol, and I think it follows that for either protocol the host address should be fully masked for the same reason. > .. > > > To do this, the static function get_ifid() in in6_ifattach.c is > renamed to > > non-static in6_get_ifid(), and this is used in lieu of a proper /64 > LL address > > to attempt to obtain a valid ifid. > > .. > > Now we can start arguing if we forget most of the explanation about > PPP[not oE], if it makes sense to keep this? But then we should fix > style, avoid unneeded initializations, etc. which should have been caught > in proper review in first place. > I assume the entire thing up there and in review distracted from the actu= al > case more than it was good for. > Sadly the commit message also kept carrying that forward. > > That's a fair criticism, I can submit another diff to correct style. Aside from the verbose initializations, was there anything else I missed? I hear what you are saying about the commit message, I intended to change it but I neglected to do so, that's my fault and I apologize for the confusion. > So the change is probably right, the description is not? > > If I am guessing right, are you going to fix style, etc.? Might want a > test > case for the extra condition added which I have not cross-checked yet > either > for all possible cases? ... > > > > Reviewed by kp > > Sponsored by: Rubicon Communications, LLC ("Netgate") > > Differential Revision: https://reviews.freebsd.org/D51778 > > --- > > sys/netinet6/in6_ifattach.c | 7 +++--- > > sys/netinet6/in6_ifattach.h | 1 + > > sys/netinet6/nd6_rtr.c | 54 > ++++++++++++++++++++++++++++++++------------- > > 3 files changed, 43 insertions(+), 19 deletions(-) > > -- > Bjoern A. Zeeb r15:7 > Thanks, -Reid --=20 Reid Linnemann Senior Software Engineer rlinnemann@netgate.com --000000000000e62010063e1a089a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Bjoern,

See responses inl= ine below:

On Fri, Sep 5, 2025 at 5:17=E2=80=AFPM Bjoe= rn A. Zeeb <bz@freebsd.org> wro= te:
On Fri, 5 Se= p 2025, Kristof Provost wrote:

Hi,

> The branch main has been updated by kp:
>
> URL: https://= cgit.FreeBSD.org/src/commit/?id=3D9e792f7ef7298080c058fbc2d36a4e60e596dae9<= /a>
>
> commit 9e792f7ef7298080c058fbc2d36a4e60e596dae9
> Author:=C2=A0 =C2=A0 =C2=A0Reid Linnemann <rlinnemann@netgate.com>
> AuthorDate: 2025-09-05 19:57:44 +0000
> Commit:=C2=A0 =C2=A0 =C2=A0Kristof Provost <kp@FreeBSD.org>
> CommitDate: 2025-09-05 21:48:48 +0000
>
>=C2=A0 =C2=A0 sys/netinet6: Fix SLAAC for interfaces with no /64 LL add= ress
>
>=C2=A0 =C2=A0 in6_ifadd() asserts that an interface has an existing LL = address with a /64
>=C2=A0 =C2=A0 prefix from which to extract the ifid for SLAAC address s= election (even though
>=C2=A0 =C2=A0 the comments suggest that an ifid will be generated if on= e does not exist). This
>=C2=A0 =C2=A0 is adequate for most generic cases, however to support PP= P links with /128 LL
>=C2=A0 =C2=A0 addresses we must be able to fall back on another source = for the ifid since we
>=C2=A0 =C2=A0 cannot assume the /128 LL has a unique ifid in the lower = 64 bits.

Excuse my ignorance but where do you have a IPv6 LL/128 on a PPP link?

I am totally suprised given this hasn't been a problem in about 25
years and I wonder what software this is or what got broken where?

Okay, I scrolled through the review.=C2=A0 I see I am the third person aski= ng
the same question as the intial intuitive reply.

I was intiially tempted to say 'please back this out' but see the e= nd.


While the impetus for this change was = PPPoE, you can effectively ignore it. The IID selection is equally valid fo= r any other scenario where the LLA prefix and RA prefix do not match,=C2=A0= in which case in6_ifadd()=C2=A0originally would just throw its=C2=A0= hands up and return an error.
=C2=A0
For (1) do not start applying IPv4 to IPv6 just because.=C2=A0 It's not= just
a different number, it is also different concepts. People said this 20
years ago and still do.


I'm not comparing IPv4 to IPv6, th= e specific PPP(oE) problem I'm addressing that prompted this change is = at the generic level of the struct ifaddr, in which any p2p address has a s= ource and destination address.

PPP(oE) implementat= ions simply have to work around some of the behavior in the v6 implementati= on right now, primarily that only addresses with a /128 prefix are permitte= d to have a dstaddr (see in6_validate_ifra()), RA processing previou= sly gave up if it could not get a IID from a /64 LLA, and an existing LLA p= refix cannot be changed from /64 to /128, requiring the PPP(oE) client soft= ware to first remove an existing LLA (for instance if net.inet6.ip6.auto_li= nklocal is enabled) once address negotiation is complete in order to add a = proper ifaddr to the address list.

Either /64 dsta= ddrs needed to be permitted (which is not sensible as a p2p address is by d= efinition composed solely of two endpoints), or RA processing should work i= n the case where the RA prefix doesn't match the LLA prefix (which I wo= uld argue should have been done since the original KAME implementation).
=C2=A0
(2) Both of you talk about PPPoE in the review which tells me something
got wrong as that is a totally different protocol and has nothing to do
with IIDs.

Selecting an appropriate LL for IPV6CP can be done entirely in user
space for protocol negotiations.=C2=A0 And to my memory we have no IPV6CP implementation in the kernel to require anything different.

I don't know what the mentioned if_pppoe.ko is but if this is a convolu= ted
implenentation in-kernel incl. protocol negotiations then I am tempted to say you are off on your own with that, especially if you want to do things<= br> differently.=C2=A0 Looking what Linux is doing is not always the best sourc= e of
truth.

Please follow what the IPV6CP RFC (got updated since I last implemented it<= br> 20-something years ago I have to admit) says and you end up with a /64 LL on the interface and your problem is solved.


I did refer to RFC5072, however I don&= #39;t see any mention at all of prefix selection for the negotiated address= . Regardless, let's ignore PPPoE for the purpose of this conversation b= ecause IID selection for SLAAC is not specific to this case.

=
With regard to address selection for SLAAC, RFC2462 section 5.5.= 3 states:
=C2=A0 "d) If the prefix advertised does not ma= tch the prefix of an address
=C2=A0 =C2=A0 =C2=A0 =C2=A0already= in the list, and the Valid Lifetime is not 0, form an
=C2=A0 =C2=A0= =C2=A0 =C2=A0address (and add it to the list) by combining the advertised<= br>=C2=A0 =C2=A0 =C2=A0 =C2=A0prefix with the link's interface identifi= er as follows:"
=
This is perfectl= y applicable to the situation that prompted the change, as the LLA prefix d= oes not match the RA prefix. In such a case, rather than giving up and goin= g home when the LLA prefix=C2=A0doesn't match the=C2=A0advertised prefi= x, we should take the bare minimum effort and try to get it from the horse&= #39;s mouth before returning an error.

Your negotiated LOCAL_LL (fe80::LOCAL_IID/64) address goes onto the IF:

IF: flags=3D1008051<UP,POINTOPOINT,RUNNING,MULTICAST,LOWER_UP> metric= 0 mtu 1492
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0options=3D80000<LINKSTATE>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 inet ...
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0inet6 LOCAL_LL%IF prefixlen 64 scopeid 0x= <n>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 inet6 ...
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0nd6 options=3D122<ACCEPT_RTADV,AUTO_LI= NKLOCAL,NO_DAD>

You get an entry in the routing table:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 fe80::%IF/64=C2=A0 link#11=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0US=C2=A0 IF

and possibly a
=C2=A0 =C2=A0 =C2=A0 =C2=A0 default=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0REMOTE_LL%IF=C2=A0 =C2=A0 =C2=A0 UGS IF

if you decide to install a default route.

Other routes I would assume are installed on the link but I assume you coul= d
chose the REMOTE_LL%IF as well if you wanted to complicate things.

So I crystal-ball that your real problem could have been stated that you may need to get the IID for IPV6CP negotiations in-kernel?=C2=A0 If that is= not
the case, then I am still lost as-to what you are trying to accomplish.
But then you'd still never need the /128.=C2=A0 So maybe I am just lsot= .


Again, we can forget PPPoE. PPPoE, whe= ther in-kernel or in-userland handles IID selection itself for IPV6CP negot= iation. The issue I'm addressing is SLAAC on interfaces with LLA prefix= es that do not match the prefix in the RA.

As PPP = or PPPoE are both point-to-point links the ifaddr for the link should consi= st of a srcaddr and dstaddr, regardless of the specific IP protocol, and I = think it follows that for either protocol the host address should be fully = masked for the same reason.
=C2=A0
..

>=C2=A0 =C2=A0 To do this, the static function get_ifid() in in6_ifattac= h.c is renamed to
>=C2=A0 =C2=A0 non-static in6_get_ifid(), and this is used in lieu of a = proper /64 LL address
>=C2=A0 =C2=A0 to attempt to obtain a valid ifid.

..

Now we can start arguing if we forget most of the explanation about
PPP[not oE], if it makes sense to keep this?=C2=A0 But then we should fix style, avoid unneeded initializations, etc. which should have been caught in proper review in first place.
I assume the entire thing up there and in review distracted from the actual=
case more than it was good for.
Sadly the commit message also kept carrying that forward.


That's a fair=C2=A0criticism,=C2= =A0I can submit another diff to=C2=A0correct style. Aside from the verbose = initializations, was there anything else I missed? I hear what you are sayi= ng about the commit message, I intended to change it but I neglected to do = so, that's my fault and I apologize for the confusion.
=C2=A0=
So the change is probably right, the description is not?

If I am guessing right, are you going to fix style, etc.?=C2=A0 Might want = a test
case for the extra condition added which I have not cross-checked yet eithe= r
for all possible cases? ...


>=C2=A0 =C2=A0 Reviewed by=C2=A0 =C2=A0 =C2=A0kp
>=C2=A0 =C2=A0 Sponsored by:=C2=A0 =C2=A0Rubicon Communications, LLC (&q= uot;Netgate")
>=C2=A0 =C2=A0 Differential Revision:=C2=A0 https://reviews.fre= ebsd.org/D51778
> ---
> sys/netinet6/in6_ifattach.c |=C2=A0 7 +++---
> sys/netinet6/in6_ifattach.h |=C2=A0 1 +
> sys/netinet6/nd6_rtr.c=C2=A0 =C2=A0 =C2=A0 | 54 ++++++++++++++++++++++= ++++++++++-------------
> 3 files changed, 43 insertions(+), 19 deletions(-)

--
Bjoern A. Zeeb=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0r15:7


Thanks,=

-Reid

--
Reid Linnemann
Senior Software Engineer
3D""<= br>
--000000000000e62010063e1a089a--