From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 2B1133858C30 for ; Mon, 4 Sep 2023 15:53:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2B1133858C30 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1693842805; 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: in-reply-to:in-reply-to:references:references; bh=6CH0IDXB7xPdaO5ZWuOjFToymGv1zlv6L971dmGlCMM=; b=VjGyThPZJUT/z2qaMEfoOcyeAsR7XdyeiTj9n5e4ftkN2PFLXunExcIPN70yINNbEpe9eB 6+ydByBCwbCJayrwfHM1Lo3E962ZBSB37rWr4+Eguur75841bhyfH0S3JS3SziEHPK6xaV BWtCYXjD0dGISX9MAHrp4p9bzBu0LS8= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-150-eOwop8PbMi-Ll3JMS_Tl7g-1; Mon, 04 Sep 2023 11:53:24 -0400 X-MC-Unique: eOwop8PbMi-Ll3JMS_Tl7g-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E48051C05B0F; Mon, 4 Sep 2023 15:53:23 +0000 (UTC) Received: from oldenburg3.str.redhat.com (unknown [10.39.194.68]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5AB7440C6CCC; Mon, 4 Sep 2023 15:53:23 +0000 (UTC) From: Florian Weimer To: Andreas Schwab via Libc-alpha Cc: Andreas Schwab Subject: Re: [PATCH] Update getaddrinfo to RFC 6724 (bug 29496) References: Date: Mon, 04 Sep 2023 17:53:22 +0200 In-Reply-To: (Andreas Schwab via Libc-alpha's message of "Tue, 06 Sep 2022 11:28:09 +0200") Message-ID: <87v8cqsznx.fsf@oldenburg3.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: * Andreas Schwab via Libc-alpha: > diff --git a/posix/gai.conf b/posix/gai.conf > index 4616ed005b..de76e2c26f 100644 > --- a/posix/gai.conf > +++ b/posix/gai.conf > @@ -16,40 +16,34 @@ > # used. There are possible runtime problems. The default is no. > # > # label > +# Add another rule to the RFC 6724 label table. See section 2.1 in > +# RFC 6724. The default is: > # > #label ::1/128 0 > #label ::/0 1 > #label 2002::/16 2 > #label ::/96 3 > #label ::ffff:0:0/96 4 > +#label 2001::/32 5 > +#label fec0::/10 11 > +#label 3ffe::/16 12 > +#label fc00::/7 13 Change matches new table in section 2.1. > -# For sites which prefer IPv4 connections change the last line to > +# Add another rule to the RFC 6724 precedence table. See section 2.1 > +# and 10.3 in RFC 6724. The default is: > +# > +#precedence ::1/128 50 > +#precedence ::/0 40 > +#precedence ::ffff:0:0/96 35 > +#precedence 2002::/16 30 > +#precedence 2001::/32 5 > +#precedence fc00::/7 3 > +#precedence ::/96 1 > +#precedence fec0::/10 1 > +#precedence 3ffe::/16 1 Likewise. > diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c > index bcff909b2f..3da296816a 100644 > --- a/sysdeps/posix/getaddrinfo.c > +++ b/sysdeps/posix/getaddrinfo.c > @@ -1304,7 +1304,7 @@ static const struct prefixentry *labels; > /* Default labels. */ > static const struct prefixentry default_labels[] = > { > - /* See RFC 3484 for the details. */ > + /* See RFC 6724 for the details. */ > { { .__in6_u > = { .__u6_addr8 = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 } } > @@ -1354,23 +1350,39 @@ static const struct prefixentry *precedence; > /* The default precedences. */ > static const struct prefixentry default_precedence[] = Table updates match the RFC. > @@ -1453,7 +1465,9 @@ fls (uint32_t a) > { > uint32_t mask; > int n; > - for (n = 0, mask = 1 << 31; n < 32; mask >>= 1, ++n) > + if (a == 0) > + return 0; > + for (n = 0, mask = 1u << 31; n < 32; mask >>= 1, ++n) > if ((a & mask) != 0) > break; > return n; > @@ -1461,7 +1475,7 @@ fls (uint32_t a) > > > static int > -rfc3484_sort (const void *p1, const void *p2, void *arg) > +rfc6724_sort (const void *p1, const void *p2, void *arg) > { > const size_t idx1 = *(const size_t *) p1; > const size_t idx2 = *(const size_t *) p2; > @@ -1642,7 +1656,7 @@ rfc3484_sort (const void *p1, const void *p2, void *arg) > in_addr_t netmask1 = 0xffffffffu << (32 - a1->prefixlen); > > if ((in1_src_addr & netmask1) == (in1_dst_addr & netmask1)) > - bit1 = fls (in1_dst_addr ^ in1_src_addr); > + bit1 = a1->prefixlen; > > struct sockaddr_in *in2_dst > = (struct sockaddr_in *) a2->dest_addr->ai_addr; > @@ -1653,7 +1667,7 @@ rfc3484_sort (const void *p1, const void *p2, void *arg) > in_addr_t netmask2 = 0xffffffffu << (32 - a2->prefixlen); > > if ((in2_src_addr & netmask2) == (in2_dst_addr & netmask2)) > - bit2 = fls (in2_dst_addr ^ in2_src_addr); > + bit2 = a2->prefixlen; > } This is an undocumented change that I can't find in RFC 6724. I have a general distaste for Rule 9, so anything that treats more addresses as equivalent is fine with me. However, it is not clear to me why we would prefer subnets with longer prefixes. Setting bit1 and bit2 to 1 if the source and destination address are on the same subnet should achieve the same thing. This does not conform to RFC 6724, so it needs a comment, and probably a NEWS entry as well. > else if (a1->dest_addr->ai_family == PF_INET6) > { > @@ -1672,18 +1686,33 @@ rfc3484_sort (const void *p1, const void *p2, void *arg) > > int i; > for (i = 0; i < 4; ++i) > { > + uint32_t mask1, mask2; > + if (i * 32 >= a1->prefixlen) > + mask1 = 0; > + else if ((i + 1) * 32 > a1->prefixlen) > + mask1 = 0xffffffffu << (32 - (a1->prefixlen & 31)); > + else > + mask1 = 0xffffffffu; > + if (i * 32 >= a2->prefixlen) > + mask2 = 0; > + else if ((i + 1) * 32 > a2->prefixlen) > + mask2 = 0xffffffffu << (32 - (a2->prefixlen & 31)); > + else > + mask2 = 0xffffffffu; > + if (((in1_dst->sin6_addr.s6_addr32[i] > + ^ in1_src->sin6_addr.s6_addr32[i]) & htonl (mask1)) != 0 > + || ((in2_dst->sin6_addr.s6_addr32[i] > + ^ in2_src->sin6_addr.s6_addr32[i]) & htonl (mask2)) != 0) > + { > + bit1 = fls (ntohl (in1_dst->sin6_addr.s6_addr32[i] > + ^ in1_src->sin6_addr.s6_addr32[i]) > + & mask1); > + bit2 = fls (ntohl (in2_dst->sin6_addr.s6_addr32[i] > + ^ in2_src->sin6_addr.s6_addr32[i]) > + & mask2); > + break; > + } Why use the common prefix length for IPv6 addresses? RFC 6724 pretends that IPv6 assignment is strictly hierarchical, but in reality, it's not. Shouldn't we do the same thing as for IPv4 addresses? It's not clear to me why you changed the implementation. Does it fix a bug? Thanks, Florian