From nobody Sun Jul 10 12:48:32 2022 X-Original-To: dev-commits-src-all@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 A5AE91D01535; Sun, 10 Jul 2022 12:48:32 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Lgmz441LMz3yYv; Sun, 10 Jul 2022 12:48:32 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1657457312; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=zKx8GTwBd0tUW9M/trWDddS0b3/W1ei4ZPrgIxuPuO8=; b=AWOMcYxMV/IzayhdAFlQMvyrBw4/yStEAg6u5vA9fZJoZNqyv+qeR+MvfgxxTzr+pvLGMo Z40gs7+B6GVPoPYmYOk4mrDU1AKdMEvO0SyF8DJ4/wTA8yHFv6x4lCYKOrztw/fk10Nniz OFBjvcJklI3DW7UVTlpOEKkybnQkER4Mtg9U9+hyj3efD1hvqRTz8wivteUzV4glKkMdtw o8t02KTxn5CMQmYMhLm4SfpCIpvK/pY57P+c40h1EnwbNYbV0OC71i2Ih5lOJe1Y/VLqC5 hV1hBRSWJcSHtXXxUM8/OyREdrCqYlll9ld8adJ9nuw9z0k8bus+iUzE7YnmFg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Lgmz432bPzPF6; Sun, 10 Jul 2022 12:48:32 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 26ACmWAW069734; Sun, 10 Jul 2022 12:48:32 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 26ACmWT1069733; Sun, 10 Jul 2022 12:48:32 GMT (envelope-from git) Date: Sun, 10 Jul 2022 12:48:32 GMT Message-Id: <202207101248.26ACmWT1069733@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Alexander V. Chernikov" Subject: git: 50fa27e795ea - main - netinet6: fix interface handling for loopback traffic List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: melifaro X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 50fa27e795eaae97dae87ac4532799f7aea87e9f Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1657457312; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=zKx8GTwBd0tUW9M/trWDddS0b3/W1ei4ZPrgIxuPuO8=; b=jXbrtwFv1Nj4cByGMbLNDCXTHP75Wqu1x9wTidafQL4phc73Qdu1dFYesVY3EMC9wZkWBT 35f+ykG2dg7bNS6+yj0YfSdSOqhdaU5xCGxso1QeD8BzU410j67IaqxmnVlLopOUPU4ajn kAuHa645/mR/t/plECaNB0r6ariVILZemUtfd9dOnv1fo3iUWdK9ppsW4vQwsIeCJ0/Wr/ raE66ipWiTXlW0x+K6og4a4h6lAYHfoZvX/TFPvQ645HSfpmZSB8/dfXrmEl8TICY+4B9N cLatM1tLIkbpMAQRNtWWfw7bFuXwaOpS7VoKm0P07KbQS8Fh6Il7HjNoAVw7Sg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1657457312; a=rsa-sha256; cv=none; b=reeRAMALJ5OL8Z5GcZjibR0leMVxTs+GZX4Z4HjeQx2EYnMLsNqHnlWiDs2zVIixLqK0za 4WHUJeMYfHWJM6ui//J5/TQDtgLo7CNC04h1jA61FAgL8aNw4D0BBZive9k9ayLchT1+q6 N7ErxWHTINhD4xEp7wZKJ32J7TPHnvj/LS3r2VmF2WliAhUGyHGDx9MlWsAxQLCkrn9HkE GiWKRFx6U5eDC8rzo/JZjG+T68bReA2J2iJ7+dJVC49nfTmgQXNiRzyzw9wc/PAtYi/OEg 2aQKWNfiW3RE3fL2e8Wzorr3ZJMhxVMwkEiq/tKuwWOyswjL0rvXW3BfSeTpDg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by melifaro: URL: https://cgit.FreeBSD.org/src/commit/?id=50fa27e795eaae97dae87ac4532799f7aea87e9f commit 50fa27e795eaae97dae87ac4532799f7aea87e9f Author: Alexander V. Chernikov AuthorDate: 2022-07-10 12:27:23 +0000 Commit: Alexander V. Chernikov CommitDate: 2022-07-10 12:47:47 +0000 netinet6: fix interface handling for loopback traffic Currently, processing of IPv6 local traffic is partially broken: link-local connection fails and global unicast connect() takes 3 seconds to complete. This happens due to the combination of multiple factors. IPv6 code passes original interface "origifp" when passing traffic via loopack to retain the scope that is mandatory for the correct hadling of link-local traffic. First problem is that the logic of passing source interface is not working correcly for TCP connections, resulting in passing "origifp" on the first 2 connection attempts and lo0 on the subsequent ones. Second problem is that source address validation logic skips its checks iff the source interface is loopback, which doesn't cover "origifp" case. More detailed description is available at https://reviews.freebsd.org/D35732 Fix the first problem by untangling&simplifying ifp/origifp logic. Fix the second problem by switching source address validation check to using M_LOOP mbuf flag instead of interface type. PR: 265089 Reviewed by: ae, bz(previous version) Differential Revision: https://reviews.freebsd.org/D35732 MFC after: 2 weeks --- sys/netinet6/ip6_input.c | 2 +- sys/netinet6/ip6_output.c | 61 ++++++++++++++++++----------------- tests/sys/netinet6/test_ip6_output.py | 4 +-- 3 files changed, 35 insertions(+), 32 deletions(-) diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c index 6394475d7df8..d6a81109e60b 100644 --- a/sys/netinet6/ip6_input.c +++ b/sys/netinet6/ip6_input.c @@ -824,7 +824,7 @@ passin: ip6_sprintf(ip6bufd, &ip6->ip6_dst))); goto bad; } - if (V_ip6_sav && !(rcvif->if_flags & IFF_LOOPBACK) && + if (V_ip6_sav && !(m->m_flags & M_LOOP) && __predict_false(in6_localip_fib(&ip6->ip6_src, rcvif->if_fib))) { IP6STAT_INC(ip6s_badscope); /* XXX */ diff --git a/sys/netinet6/ip6_output.c b/sys/netinet6/ip6_output.c index 818b08c3ab40..fc330290d6a3 100644 --- a/sys/netinet6/ip6_output.c +++ b/sys/netinet6/ip6_output.c @@ -688,8 +688,8 @@ again: if (ro->ro_nh != NULL && fwd_tag == NULL && ro->ro_dst.sin6_family == AF_INET6 && IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, &ip6->ip6_dst)) { + /* Nexthop is valid and contains valid ifp */ nh = ro->ro_nh; - ifp = nh->nh_ifp; } else { if (ro->ro_lle) LLE_FREE(ro->ro_lle); /* zeros ro_lle */ @@ -708,8 +708,11 @@ again: in6_ifstat_inc(ifp, ifs6_out_discard); goto bad; } - if (ifp != NULL) - mtu = ifp->if_mtu; + /* + * At this point at least @ifp is not NULL + * Can be the case when dst is multicast, link-local or + * interface is explicitly specificed by the caller. + */ } if (nh == NULL) { /* @@ -717,9 +720,11 @@ again: * dst may not have been updated. */ *dst = dst_sa; /* XXX */ + origifp = ifp; + mtu = ifp->if_mtu; } else { - if (nh->nh_flags & NHF_HOST) - mtu = nh->nh_mtu; + ifp = nh->nh_ifp; + origifp = nh->nh_aifp; ia = (struct in6_ifaddr *)(nh->nh_ifa); counter_u64_add(nh->nh_pksent, 1); } @@ -740,6 +745,7 @@ again: (ifp = im6o->im6o_multicast_ifp) != NULL) { /* We do not need a route lookup. */ *dst = dst_sa; /* XXX */ + origifp = ifp; goto nonh6lookup; } @@ -754,6 +760,7 @@ again: goto bad; } *dst = dst_sa; /* XXX */ + origifp = ifp; goto nonh6lookup; } } @@ -768,7 +775,7 @@ again: } ifp = nh->nh_ifp; - mtu = nh->nh_mtu; + origifp = nh->nh_aifp; ia = ifatoia6(nh->nh_ifa); if (nh->nh_flags & NHF_GATEWAY) dst->sin6_addr = nh->gw6_sa.sin6_addr; @@ -777,8 +784,21 @@ again: nonh6lookup: ; } + /* + * At this point ifp MUST be pointing to the valid transmit ifp. + * origifp MUST be valid and pointing to either the same ifp or, + * in case of loopback output, to the interface which ip6_src + * belongs to. + * Examples: + * fe80::1%em0 -> fe80::2%em0 -> ifp=em0, origifp=em0 + * fe80::1%em0 -> fe80::1%em0 -> ifp=lo0, origifp=em0 + * ::1 -> ::1 -> ifp=lo0, origifp=lo0 + * + * mtu can be 0 and will be refined later. + */ + KASSERT((ifp != NULL), ("output interface must not be NULL")); + KASSERT((origifp != NULL), ("output address interface must not be NULL")); - /* Then nh (for unicast) and ifp must be non-NULL valid values. */ if ((flags & IPV6_FORWARDING) == 0) { /* XXX: the FORWARDING flag can be set for mrouting. */ in6_ifstat_inc(ifp, ifs6_out_request); @@ -799,39 +819,22 @@ nonh6lookup: dst_sa.sin6_addr = ip6->ip6_dst; /* Check for valid scope ID. */ - if (in6_setscope(&src0, ifp, &zone) == 0 && + if (in6_setscope(&src0, origifp, &zone) == 0 && sa6_recoverscope(&src_sa) == 0 && zone == src_sa.sin6_scope_id && - in6_setscope(&dst0, ifp, &zone) == 0 && + in6_setscope(&dst0, origifp, &zone) == 0 && sa6_recoverscope(&dst_sa) == 0 && zone == dst_sa.sin6_scope_id) { /* * The outgoing interface is in the zone of the source * and destination addresses. * - * Because the loopback interface cannot receive - * packets with a different scope ID than its own, - * there is a trick to pretend the outgoing packet - * was received by the real network interface, by - * setting "origifp" different from "ifp". This is - * only allowed when "ifp" is a loopback network - * interface. Refer to code in nd6_output_ifp() for - * more details. */ - origifp = ifp; - - /* - * We should use ia_ifp to support the case of sending - * packets to an address of our own. - */ - if (ia != NULL && ia->ia_ifp) - ifp = ia->ia_ifp; - - } else if ((ifp->if_flags & IFF_LOOPBACK) == 0 || + } else if ((origifp->if_flags & IFF_LOOPBACK) == 0 || sa6_recoverscope(&src_sa) != 0 || sa6_recoverscope(&dst_sa) != 0 || dst_sa.sin6_scope_id == 0 || (src_sa.sin6_scope_id != 0 && src_sa.sin6_scope_id != dst_sa.sin6_scope_id) || - (origifp = ifnet_byindex(dst_sa.sin6_scope_id)) == NULL) { + ifnet_byindex(dst_sa.sin6_scope_id) == NULL) { /* * If the destination network interface is not a * loopback interface, or the destination network @@ -842,7 +845,7 @@ nonh6lookup: * pair is considered invalid. */ IP6STAT_INC(ip6s_badscope); - in6_ifstat_inc(ifp, ifs6_out_discard); + in6_ifstat_inc(origifp, ifs6_out_discard); if (error == 0) error = EHOSTUNREACH; /* XXX */ goto bad; diff --git a/tests/sys/netinet6/test_ip6_output.py b/tests/sys/netinet6/test_ip6_output.py index 112423698cdf..35adb6a7137a 100644 --- a/tests/sys/netinet6/test_ip6_output.py +++ b/tests/sys/netinet6/test_ip6_output.py @@ -447,7 +447,7 @@ class TestIP6OutputLoopback(SingleVnetTestTemplate): "source_validation", [ pytest.param(0, id="no_sav"), - pytest.param(1, id="sav", marks=pytest.mark.skip(reason="fails")), + pytest.param(1, id="sav"), ], ) @pytest.mark.parametrize("scope", ["gu", "ll", "lo"]) @@ -495,7 +495,7 @@ class TestIP6OutputLoopback(SingleVnetTestTemplate): "source_validation", [ pytest.param(0, id="no_sav"), - pytest.param(1, id="sav", marks=pytest.mark.skip(reason="fails")), + pytest.param(1, id="sav"), ], ) @pytest.mark.parametrize("scope", ["gu", "ll", "lo"])