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=DmTyMGLr; 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 5ABA15A026F for ; Thu, 09 Oct 2025 21:29:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1760038157; 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=6+lOJDB4Sgbfyz6DkKVYMDmsCVumbGX/MBVAQYL4AaA=; b=DmTyMGLrHtyE19epEFoQBTkk5H428wRWa4CW9Rs2zGM8ycajDf5Zg3nsMq3FJXTdC4HerC fS+obg2lf1dY1nSneR5/EhrdoWrHJIoD2wfGQGfYCH2/iamTGE8KuFy85ysGuh6c9cdCnT edkMOk0j3ZaRgGHS2oJx9k89niNiLLk= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-292-0ThC1efhNxeAkY-dpTRspw-1; Thu, 09 Oct 2025 15:29:15 -0400 X-MC-Unique: 0ThC1efhNxeAkY-dpTRspw-1 X-Mimecast-MFC-AGG-ID: 0ThC1efhNxeAkY-dpTRspw_1760038155 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-46e45899798so9023065e9.3 for ; Thu, 09 Oct 2025 12:29:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1760038155; x=1760642955; 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=6+lOJDB4Sgbfyz6DkKVYMDmsCVumbGX/MBVAQYL4AaA=; b=Twq4l304tFIcoquYZSDgBpQvftA5Lc7g+4DlhBSy55FgFe1sNbakuXLnvdxSAThybm cx9spOU7r+0U0WtmzDtU919x4r54S8+2EWiHIgB6Tarian7sv2/ZeMpiXchqPjFaDcyB nDMEhApkpn2Bhv7s6aIa2PccLXe6NYbNOmC80Aazwmnn6NKFf6YccPZ9moRRKavF/xnk hOCqci+KIw/+WCAfnyJ4L+dG66Mj9SOuGXT6nc0tLrMmUgE2tgEsVbUKgtjXPEBnYGFj kdApR2bHtH1zRgY6r++dqMjsPqLcllnM3DK6RJZ5J91/jTXpyvCYENmF/+FXqZgJLj5Y 3jKQ== X-Forwarded-Encrypted: i=1; AJvYcCWYUeoQ998fYJnyai45gMW3Dp1KhOB+iMIJRNLbKYhLU/omZ2SN0AldYGum2LO+iTo287050+lIcLw=@passt.top X-Gm-Message-State: AOJu0YxoA4ApxpeHfccInlwg+2bqgMknzwwPTjbQtgJDzHfDnWmdplCe ahglQzKBi01oXap5hh86f9byRpiOEfGCAGfCBjN9dIj0ivTwQWpFGQ+PyPboFNNjhZaKKxeFOHV v3o5K5JLvxqHa9kS8N+dHIqjqmcLCH+m3jTFrF3+PQ2jB04QJo7g5yQ== X-Gm-Gg: ASbGncuRlmKLz3duoPLUek4p5zfH0JzkpdcCqavmUrFPiBY2mNBuOHmMOB6rDWOveTa 3dCoiHdOpZAtWbpzY5mydj3dsVX+4ZQ06rRHBshmfssgguyUVtA0GoB5Yd1yCX75hRQL7pc2bcX JtIrWqIfN1j2UHH27vLxn+hj4LupIKOv/aDOLuptLAgPo+BdOPCsL6qKYfLI3T+ZAqKGHzpJWXF e0gZaCtwclUpjLPY91MsRveoIrX28Kv6nYhFaKVPkIW/z+5uVktbcHZ5BUgzQa4xtYDAuDLWtVX w+3LbHhua+9s6OMUAF0XsrqHYO33LZX8DyYLxpEmbV7zn2B+I/qcivxZ X-Received: by 2002:a05:600c:5285:b0:46e:4f25:aace with SMTP id 5b1f17b1804b1-46fa9a8c43emr64553185e9.6.1760038154475; Thu, 09 Oct 2025 12:29:14 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG0SbHM57BoLN0jkFiC2qFFsUwu/RoCAFvq9aVzL+fspKWlnwD1FZQIcxsZJYYTyCNgO3NdYg== X-Received: by 2002:a05:600c:5285:b0:46e:4f25:aace with SMTP id 5b1f17b1804b1-46fa9a8c43emr64553095e9.6.1760038154003; Thu, 09 Oct 2025 12:29:14 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-46fb489ac60sm11217325e9.16.2025.10.09.12.29.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Oct 2025 12:29:13 -0700 (PDT) Date: Thu, 9 Oct 2025 21:29:12 +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: <20251009212912.510f860e@elisabeth> In-Reply-To: References: <20251003003412.588801-1-jmaloy@redhat.com> <20251003003412.588801-4-jmaloy@redhat.com> <20251007121022.353a44fc@elisabeth> <20251008120118.046339db@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: IEeyl0M6CHut38yogaTQTW4QQaXmZ4hkrdHC0gf8EJk_1760038155 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: L645GIYDURQNH6EQOD7KSOL3J7YTYMCH X-Message-ID-Hash: L645GIYDURQNH6EQOD7KSOL3J7YTYMCH 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 Thu, 9 Oct 2025 14:51:02 +1100 David Gibson wrote: > On Wed, Oct 08, 2025 at 12:01:18PM +0200, Stefano Brivio wrote: > > 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, > > We did use guest-side link-layer addresses - host-side LL addresses > might not even exist. The question is about whether we use guest side > or host side IP addresses to index the table. Sorry, yes, I meant to write network and I wrote link-layer. > > but that wasn't the case, hence (I think) the > > current struggle. If we go in that direction, I hope it's possible. > > Thinking a bit more closely, I don't think it is, for much the same > reason it wasn't in this draft. > > According to the rules Jon and I thrashed out elsewhere in the thread, > there are certain guest side addresses that must be locked to use > our_tap_mac. We're essentially shadowing something that might exist > on the host side, so we should use our MAC not the MAC of whatever is > shadowed. > > Just pre-populating an entry won't do the trick, because it could be > overwritten if the right events occur for the shadowed host. Right, sorry, I omitted another bit of context: I've been suggesting to Jon that he'd introduce some kind of "permanent" or "administrative" bit, and keep those entries at the beginning of the chain, exactly for the reason you mention. I can imagine we'll need those at some point if we ever want to offer explicit link-layer address mapping in the future, and they're probably convenient the day one can change map_guest_addr and map_host_loopback at runtime. We can also happily skip that for the moment, though, it's another problem we can keep for later. > > 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 -- Stefano