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=QOlfYHj5; 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 6014E5A026F for ; Fri, 17 Oct 2025 20:49:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1760726976; 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=+QbKCUAEvC52LpftkO9mD32naXujpLTMHdKdvXa4wQo=; b=QOlfYHj5O19v3cPyaci0oufoqP7WQRPuNVpHToFu6RwEMYIlRQK6Ry3yjPp1UMFXyfKWz/ Ejw7jeNRXm5CdCQ3ws0I4/xyIa7NKP56jLYdK/WpaP2Efhix7E0YdShvHYU0QCrAIBgtx+ vPNs2pjFFsWFXEdqnJQcpFf8V0EcBQ8= Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-57-bWBZnouHNCOhifvaj5ukUQ-1; Fri, 17 Oct 2025 14:49:35 -0400 X-MC-Unique: bWBZnouHNCOhifvaj5ukUQ-1 X-Mimecast-MFC-AGG-ID: bWBZnouHNCOhifvaj5ukUQ_1760726974 Received: by mail-qk1-f197.google.com with SMTP id af79cd13be357-88ea6169a96so457553885a.2 for ; Fri, 17 Oct 2025 11:49:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1760726974; x=1761331774; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=+QbKCUAEvC52LpftkO9mD32naXujpLTMHdKdvXa4wQo=; b=RCV7x1wpo9nxKuRZWW94dZvt5D2GwkCzv9HzbjTp74qBGQUNdUNiKjm962oftYA3jI koOzJGB8H3iSqu7UBbCyDH6MAKXlyFiSlPWuS9mziU6lWlC9/FKEVyLqihZO1LN44+ec ZhSUuKf4A3VkRjK3yBEpPTq703Sl3khKjzXqrt6IDzRUmYVFzRMJUJR3sEuPPL/p09EB dajptGmfT676xWcJL6VxwmsoeCQMiV44QPPL3BGb3VtwyrJuQH481pZ/1xi5tgMKVk18 pXjxCKmf0IhcpLZRfm8I7EIxMBmTjZUtUm6D085aTTVkYv+3wfDxfqdjoQE+rOxftUBk ZejA== X-Forwarded-Encrypted: i=1; AJvYcCXOzsgAUpczu/5Z2RcIYt9kT65ZvYuLvPzpGeApdFdsVZQDr/O/Oia1nIHztuI1/sf+6XxjgRPysIs=@passt.top X-Gm-Message-State: AOJu0YzSVrCvtwhOz37V6MGNAJs8I3iui/sVwBOotwivHOUySNiGq/C5 qdMvi4q9yoty5fN+WAYsdb7RGGcFEyoAwtWSnwNdlceuW0cJtvo8er6QVex6NiALZHHDRgQinFZ wdAeeEN+OwyRrzJq89JtzMI5N0v3riDihU91qCN7GBtDTmqKS76ovMw== X-Gm-Gg: ASbGnctOpK/8+TWXitrubrAfbbQDbwIkSsRvOtq6SZqpZZF+njUJF9VNx8SloGd8kiG X179NFpuLnbJTTXsjrAB4wSiHY2habdh47Y8Nyp857YbPt/scFn9CCgkIVwEXOj7v7/sSWR8jPw Ln7+NKK6Vdp+DIK4dl7eS3boqd/zKguvIFMLVU9Sl4uIKA39+yTodUeLHfSizu3r+mzYnJDNZm4 lPyMlFwzEfnSG/fqBkhdztQYseKik2aMq383ejhHOOrN/8gxDC4IbwHmhaulrnHjuOYhrhZt9Zw MMzn0JxKWRTkf4RVse1JdTzeU0U49UcE3qjxGHyD52ja8Y75YQh5CIdhRPMjVOtGutqBhWBZaOt WvNQ0vyRdoiWwb1lp7lcZQr9yAzytnQS4ZZx7lZtFaPGF X-Received: by 2002:a05:620a:4621:b0:864:48eb:368 with SMTP id af79cd13be357-8906e9a4945mr554436585a.29.1760726974393; Fri, 17 Oct 2025 11:49:34 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGJ0bnGP4bfSg4GLgvbrFB9QkjG+9d5mgRcf7feDuBhGXEr4Sbv6mjvOMtu9ZYP9xNRsFBoUg== X-Received: by 2002:a05:620a:4621:b0:864:48eb:368 with SMTP id af79cd13be357-8906e9a4945mr554433385a.29.1760726973895; Fri, 17 Oct 2025 11:49:33 -0700 (PDT) Received: from ?IPV6:2001:4958:2193:9901:6217:960c:2ef1:f0f3? ([2001:4958:2193:9901:6217:960c:2ef1:f0f3]) by smtp.gmail.com with ESMTPSA id af79cd13be357-891cd098e17sm24435385a.20.2025.10.17.11.49.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 17 Oct 2025 11:49:33 -0700 (PDT) Message-ID: Date: Fri, 17 Oct 2025 14:49:33 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v14 03/10] fwd: Add cache table for ARP/NDP contents To: David Gibson References: <20251015025521.1449156-1-jmaloy@redhat.com> <20251015025521.1449156-4-jmaloy@redhat.com> From: Jon Maloy In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: jNtbNw4VQl1mSjTVx2yNPzgvt8_2uDsz57X6lEaEPis_1760726974 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Message-ID-Hash: EYP62TKCT4S3AZIP4XCGVR5YB64C4K7M X-Message-ID-Hash: EYP62TKCT4S3AZIP4XCGVR5YB64C4K7M X-MailFrom: jmaloy@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: sbrivio@redhat.com, dgibson@redhat.com, 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 2025-10-16 23:05, David Gibson wrote: > On Tue, Oct 14, 2025 at 10:55:14PM -0400, Jon Maloy wrote: >> We add a cache table to keep track of the contents of the kernel ARP >> and NDP tables. The table is fed from the just introduced netlink based >> neigbour subscription function. >> >> Signed-off-by: Jon Maloy > > Reviewed-by: David Gibson > > I do see one error here, but it's fairly harmless, so I think a follow > up makes more sense than a respin. > > [snip] >> + /* Blocker entries to stop events from hosts using these addresses */ >> + if (!inany_is_unspecified4(&mhl)) >> + fwd_neigh_table_update(c, &mhl, c->our_tap_mac, true); >> + >> + if (!inany_is_unspecified4(&ggw) && !c->no_map_gw) >> + fwd_neigh_table_update(c, &ggw, c->our_tap_mac, true); >> + >> + if (!inany_is_unspecified4(&mga) && !inany_equals(&mhl, &mga)) { > > That made me realise that we should throw an error during > configuration if map_host_loopback == map_guest_addr. It doesn't make > sense for these to be the same - if they are, we have no way of > knowing if a packet should be mapped to 127.0.0.1 or to guest_addr. > *checks* looks like map_host_loopback will take precedence in this > case, because of the way nat_outbound() is ordered. I also noticed it is possible to set guest_gw to some random address while at the same time setting no_map_gw. It seems to be harmless, since no_map_gw takes precedence, but it is an inconsistency we should fence against. > > In any case I think you can drop the inany_equals() test - the > permanent bit will stop the second update from clobbering the first, > even if we are misconfigured. > >> + uint8_t mac[ETH_ALEN]; >> + int rc; >> + >> + rc = nl_link_get_mac(nl_sock, c->ifi4, mac); >> + if (rc < 0) { >> + debug("Couldn't get ip4 MAC addr: %s", strerror_(-rc)); >> + memcpy(mac, c->our_tap_mac, ETH_ALEN); >> + } > > Using the host's MAC for --map-guest-addr only makes sense if the > guest address is the same as the host address. If -a is used to make > the guest address different, then it may shadow some other random > node, not the host. We don't need special handling for that case - > the nat_inbound() you already have will do what we need. > > IIUC, the host itself doesn't appear in the neighbour table, so we do > need special handling if we want to use the host MAC when > --map-guest-addr *does* refer to the host. To handle that, I think > what we want is pseudo-codishly: > > fwd_neigh_table_update(c, nat_inbound(host_addr), host_mac, true); > > The wrinkle is that while we do get the host address at some point, > I'm not sure we keep it around (it's typically irrelevant after init). > > Strictly speaking 'permanent' isn't really correct here, but it's not > worth the hassle of setting up a whole other netlink monitor to watch > for changes in the host's MAC address. > > In fact.. I'm not sure it's worth handling this case at all. I think > it would be ok to just drop this clause. That means we'll use > our_tap_mac by default for things NATted to the (non loopback) host, > which is probably fine. > Fine with me, but I do think we need a blocker entry just in case somebody else comes up with the same address. I'll post a complementary commit once the series has been applied. ///jon