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=MXFF4I6v; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 234145A0619 for ; Wed, 08 Oct 2025 12:01:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1759917684; 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=pK2efbMs2QbvTxfB5+lRSSmaVEr7mpCMJuKPE9nVh84=; b=MXFF4I6vUFzaWpRiZk+9Lz6MNBdJiGQU9Msuj3u+dbxEgRBDcsIhyVrA1Clvb3c4aAjd3x yE8Yd9oOvO2F3Mhq8BQ1faSElnr44930HvHoUKfhysZZX2h8EGDb1OQIBl+kh1Pt7Bug2g FBtxkoKBwr/IWPW4jaJ4O5VxBn2wL10= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-300-8qM04AOhNXeyTlPfpf7kOg-1; Wed, 08 Oct 2025 06:01:22 -0400 X-MC-Unique: 8qM04AOhNXeyTlPfpf7kOg-1 X-Mimecast-MFC-AGG-ID: 8qM04AOhNXeyTlPfpf7kOg_1759917681 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-41066f050a4so2570280f8f.1 for ; Wed, 08 Oct 2025 03:01:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1759917681; x=1760522481; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=pK2efbMs2QbvTxfB5+lRSSmaVEr7mpCMJuKPE9nVh84=; b=JS8jAD3uZmqV1yG6tDd3T2EOS1uzqmJ4Zo0YDhs4unq0jN4pdHnmBHpbYFtw2VEKwn Tg/NrQxGxRgakZx92BcWcrFPrp7EO9RzixtbcOi8zSWNSoWpuS/M4zQfqCDCmoaFEfDQ G8Bkmf73uTP1vV9hhEP4FLlA/ZEKA8AQbQbbHDNOXLxOSyAhhwlbczsxPqdWgpkJ/GMC GQRytPB4+eOZ5XSDntKlRwviQLV3Q10t99Jg4Ysy8TEQq0G3pm4qbOjeNPwcR3/I9RDp wO1KoI2m/kV9HCM7YLohL7RnS1Lg231s01Mm4DPB5+S8xfSuwzmJdQJ4HwIem4RkCCih KjNw== X-Forwarded-Encrypted: i=1; AJvYcCXFHx62X5+ysExOqtmfRgQuWKLYd7z5mDb3LXuAxYcULbKuI8vvlcJoFgsjwE79JOFwhXaKfLLTXKc=@passt.top X-Gm-Message-State: AOJu0Yxfre3Qn4ASAtxeFohWJico8M/qfz36/Te2G6weq5V3HNkUnmAi 82wm4jXKBhfjs6MwdzfgalptT/u6HtV/Zg5CcQuVnIbLdTfkWQhRJDIterEWK4lPotf9ML2Esdf koK+GY/TvmouHkUzakqlVrS5HKEZ1UjPvWLkJCf1zjFLLnKx/AkHvzA== X-Gm-Gg: ASbGnctc6KqMNbYHQNevP0w8DC8DF9x24a+XvVXpAFV90nuEUH/5c0Lf6Gr/W4gu6OM xVuvC2J+/eKNAEv19sRd/qfOjATFSrYiBByoGSkDKARB0vvWF3HrRfjsvdhGgZk6oaAlanP7AVl CcwaYtRKPjUxFMk0iyzsLlkJtI2SRUAVEBjAlWwqmQwyAP9VmhG+2M+nPipl0rTMy/QUk7rhukP 9NO54l41ljAQkJpo8EFC4CZCpjHdQCbGijbM7O8mam2B/o/J2yfLnmr8Kzvl4d9X45YPzl8wIKq /EMMuyfUdH25LDqoXwqlnd3hSJal6PWXmJ5Ug1fM+OvmlRdqSGags6DCGmP+lgkFjfUKdxEtlw= = X-Received: by 2002:a05:6000:2485:b0:425:8133:ec6c with SMTP id ffacd0b85a97d-42666ac634cmr1419690f8f.9.1759917681195; Wed, 08 Oct 2025 03:01:21 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHrDidugR43oocJYSzBjzRu/QOCQ63+snFaOqsPVB0w/I3TEihoZKw/L9I0zS6XpcsfdgBG4g== X-Received: by 2002:a05:6000:2485:b0:425:8133:ec6c with SMTP id ffacd0b85a97d-42666ac634cmr1419666f8f.9.1759917680661; Wed, 08 Oct 2025 03:01:20 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-46fa9d6fb41sm30096055e9.17.2025.10.08.03.01.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Oct 2025 03:01:19 -0700 (PDT) Date: Wed, 8 Oct 2025 12:01:18 +0200 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v12 3/9] arp/ndp: send ARP announcement / unsolicited NA when neigbour entry added Message-ID: <20251008120118.046339db@elisabeth> In-Reply-To: References: <20251003003412.588801-1-jmaloy@redhat.com> <20251003003412.588801-4-jmaloy@redhat.com> <20251007121022.353a44fc@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: UxYmbvC8FclYUN_hMpnCbfcr2-M6BAgFmbC6rGkwlSE_1759917681 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: AC4KXHVHCWKSGWXDGS66U4AAON5PB5IW X-Message-ID-Hash: AC4KXHVHCWKSGWXDGS66U4AAON5PB5IW 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: Jon Maloy , 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 Wed, 8 Oct 2025 11:27:32 +1100 David Gibson wrote: > On Tue, Oct 07, 2025 at 12:10:22PM +0200, Stefano Brivio wrote: > > On Fri, 3 Oct 2025 14:41:56 +1000 > > David Gibson wrote: > > > > > On Thu, Oct 02, 2025 at 08:34:06PM -0400, Jon Maloy wrote: > > > > ARP announcements and unsolicited NAs should be handled with caution > > > > because of the risk of malignant users emitting them to disturb > > > > network communication. > > > > > > > > There is however one case we where we know it is legitimate > > > > and safe for us to send out such messages: The one time we switch > > > > from using ctx->own_tap_mac to a MAC address received via the > > > > recently added neigbour subscription function. Later changes to > > > > the MAC address of a host in an existing entry cannot be fully > > > > trusted, so we abstain from doing it in such cases. > > > > > > > > When sending this type of messages, we notice that the guest accepts > > > > the update, but shortly later asks for a confirmation in the form of > > > > a regular ARP/NS request. This is responded to with the new value, > > > > and we have exactly the effect we wanted. > > > > > > > > This commit adds this functionality. > > > > > > > > Signed-off-by: Jon Maloy > > > > > > > > --- > > > > v10: -Made small changes based of feedback from David G. > > > > v11: -Moved from 'Gratuitous ARP reply' model to 'ARP Announcement' > > > > model. > > > > v12: -Excluding loopback and default GW addresses from the ARP/NA > > > > announcement to be sent to the guest > > > > --- > > > > arp.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > > > arp.h | 2 ++ > > > > fwd.c | 16 ++++++++++++++++ > > > > ndp.c | 10 ++++++++++ > > > > ndp.h | 1 + > > > > 5 files changed, 71 insertions(+) > > > > > > > > diff --git a/arp.c b/arp.c > > > > index ad088b1..b08780f 100644 > > > > --- a/arp.c > > > > +++ b/arp.c > > > > @@ -146,3 +146,45 @@ void arp_send_init_req(const struct ctx *c) > > > > debug("Sending initial ARP request for guest MAC address"); > > > > tap_send_single(c, &req, sizeof(req)); > > > > } > > > > + > > > > +/** > > > > + * arp_announce() - Send an ARP announcement for an IPv4 host > > > > + * @c: Execution context > > > > + * @ip: IPv4 address we announce as owned by @mac > > > > + * @mac: MAC address to advertise for @ip > > > > + */ > > > > +void arp_announce(const struct ctx *c, struct in_addr *ip, > > > > + const unsigned char *mac) > > > > +{ > > > > + char ip_str[INET_ADDRSTRLEN]; > > > > + char mac_str[ETH_ADDRSTRLEN]; > > > > + struct { > > > > + struct ethhdr eh; > > > > + struct arphdr ah; > > > > + struct arpmsg am; > > > > + } __attribute__((__packed__)) annc; > > > > + > > > > + /* Ethernet header */ > > > > + annc.eh.h_proto = htons(ETH_P_ARP); > > > > + memcpy(annc.eh.h_dest, MAC_BROADCAST, sizeof(annc.eh.h_dest)); > > > > + memcpy(annc.eh.h_source, mac, sizeof(annc.eh.h_source)); > > > > + > > > > + /* ARP header */ > > > > + annc.ah.ar_op = htons(ARPOP_REQUEST); > > > > + annc.ah.ar_hrd = htons(ARPHRD_ETHER); > > > > + annc.ah.ar_pro = htons(ETH_P_IP); > > > > + annc.ah.ar_hln = ETH_ALEN; > > > > + annc.ah.ar_pln = 4; > > > > + > > > > + /* ARP message */ > > > > + memcpy(annc.am.sha, mac, sizeof(annc.am.sha)); > > > > + memcpy(annc.am.sip, ip, sizeof(annc.am.sip)); > > > > + memcpy(annc.am.tha, MAC_BROADCAST, sizeof(annc.am.tha)); > > > > + memcpy(annc.am.tip, ip, sizeof(annc.am.tip)); > > > > > > As noted in several earlier revisions, having sip == tip (but with > > > different mac addresses) looks odd. Is that what the RFCs say to do > > > for ARP announcements? > > > > > > > + inet_ntop(AF_INET, ip, ip_str, sizeof(ip_str)); > > > > + eth_ntop(mac, mac_str, sizeof(mac_str)); > > > > + debug("Announcing ARP for %s / %s", ip_str, mac_str); > > > > + > > > > + tap_send_single(c, &annc, sizeof(annc)); > > > > +} > > > > diff --git a/arp.h b/arp.h > > > > index d5ad0e1..4862e90 100644 > > > > --- a/arp.h > > > > +++ b/arp.h > > > > @@ -22,5 +22,7 @@ struct arpmsg { > > > > > > > > int arp(const struct ctx *c, struct iov_tail *data); > > > > void arp_send_init_req(const struct ctx *c); > > > > +void arp_announce(const struct ctx *c, struct in_addr *ip, > > > > + const unsigned char *mac); > > > > > > > > #endif /* ARP_H */ > > > > diff --git a/fwd.c b/fwd.c > > > > index c34bb1c..ade97c8 100644 > > > > --- a/fwd.c > > > > +++ b/fwd.c > > > > @@ -26,6 +26,8 @@ > > > > #include "passt.h" > > > > #include "lineread.h" > > > > #include "flow_table.h" > > > > +#include "arp.h" > > > > +#include "ndp.h" > > > > > > > > /* Empheral port range: values from RFC 6335 */ > > > > static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14); > > > > @@ -140,6 +142,20 @@ void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr, > > > > > > > > memcpy(&e->addr, addr, sizeof(*addr)); > > > > memcpy(e->mac, mac, ETH_ALEN); > > > > + > > > > + if (inany_equals(addr, &inany_loopback4)) > > > > + return; > > > > + if (inany_equals(addr, &inany_loopback6)) > > > > + return; > > > > > > Since you need these explicit checks anyway, there's not much point to > > > the dummy entries you created - you could exit on these addresses > > > before even looking up the table. > > > > I guess those entries make sense if we can drop all these checks as a > > result. I think we should be able to. > > We couldn't in this version, because that might have allowed the > entries for loopback to be updated, which is certainly wrong. But > it will all need re-examination after moving everything over to guest > side addresses which AIUI is the plan for the next spin. Yes, I was talking about the next version. For context, when we first discussed about the possibility of these entries with Jon, my assumption was that the whole series used guest-side link-layer addresses exclusively, but that wasn't the case, hence (I think) the current struggle. If we go in that direction, I hope it's possible. By the way, while they are probably more elegant because we can skip explicit cases, they might be a bit more complicated to manage compared to those explicit cases the day we get to change addresses and routes dynamically using a netlink monitor, because at that point we might need to remove some entries based on old addresses / default gateways. But given that this is already complicated enough, we can keep that problem for later, and just go with the simplest possible approach (whatever it is) for the moment. -- Stefano