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=Zc8H/yrf; 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 21CEB5A0287 for ; Wed, 11 Jun 2025 14:52:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1749646332; 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=3dWo6BZ00P+cLe4XdPuCyJDyhS8p183RWb0LcC7Kse8=; b=Zc8H/yrfe3HsiQWC7BwzHpzIJIEKelueADbCFfdPSvwB9YQSfhyA46Ch7x4n65iw2hI8YM 1rrdUXWzmchSOLGHBpcZgK6ToeJ6VP3e0FuRjqACamanLd1O36COUAuXBMmpNq7qn74oaV ArHgahvyhCRwmyzGpSolZisERk3GpSw= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-558-5_FUX6BONgWazRpqbwgP6Q-1; Wed, 11 Jun 2025 08:52:10 -0400 X-MC-Unique: 5_FUX6BONgWazRpqbwgP6Q-1 X-Mimecast-MFC-AGG-ID: 5_FUX6BONgWazRpqbwgP6Q_1749646330 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-3a4f7f1b932so3900150f8f.2 for ; Wed, 11 Jun 2025 05:52:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1749646329; x=1750251129; 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=3dWo6BZ00P+cLe4XdPuCyJDyhS8p183RWb0LcC7Kse8=; b=CJ4BpaDUXhfiKWDeHqvX84C/Qy0V3Vr9/KRbkruLPp9W+Lm9HnJ1KhYB9R1NgZ9b32 KrsGduY76Gd8a9QMbqLy67nrPQ3B0z/MyW6Q7v9qaJqmzWkWmK6syo+4taI/EaO6FBVd gxsI11AQs5kbX6WVCA/TIAadKkkqzkqVxb0uK28jFAk9dujd8tEDltYKP+2iDOdaCKYs se+ls+USbiKZRma/c5wJmXrKHphvUY1goifYUAbwB14ZeaAkUYXfkF4bdaFUb5CX3sOY m2LgpVft0zW1lmpdtXJOUIpeOxQGkEAF26tl2f25VP5f61H6Ay1ou6t6kzmhOiBBWQop jq7w== X-Gm-Message-State: AOJu0YxBGSYKXggsigkrvIO1hInznRhruhuzTNQPzaNquj0T1oiRwb0q xWlKl4zHqgghqGt1GY2Vb99KqbR4Dsn3PrHZczemqPATiayXH0MPc+bZPrW64ZUkwPERzEIO2ee X5Q027KTs+TieOWFuhX0ROMBdyZ960GyhNPy2CJNmRyetlcHjk3optQ== X-Gm-Gg: ASbGnctJghCTe3U8/ZzfuN85tgb2VJeLqhZHjVYdjbLMQENuXoc+8kWoPAU8CmXaC6P yoy3SjgXqBLd+Bb5j7/3UtygopVi+m2TJpeA0KG/A0WYGFaRx+kw2CK4PYZyRvOTNqFgin//ZMp Dx6LImII94orQTIxWPZtjY5gDdfci3nYbHkf+PX3OFsplPciVu2rsx6thlzPR1xkAH/8VTOTNxG VokI/DIRumdOXwCnRc61XdKt9+eLuU+KGmSMh9roB+eB2FJGtwx/wqPvFXFfeQ/FGOmwkIpLBqY kaHY1DWYse03pfJOxD+8ak9pbUF8o40l9w== X-Received: by 2002:a5d:5f55:0:b0:3a4:dd8e:e16d with SMTP id ffacd0b85a97d-3a5586efea2mr2477283f8f.15.1749646329527; Wed, 11 Jun 2025 05:52:09 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEfq+ANFdCoukoVAj1eK3LWbxMPMljkYQeypREX5KP5I4YpFfZhh4IOiItxRwvv86KKO37sfw== X-Received: by 2002:a5d:5f55:0:b0:3a4:dd8e:e16d with SMTP id ffacd0b85a97d-3a5586efea2mr2477256f8f.15.1749646329048; Wed, 11 Jun 2025 05:52:09 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-45325205479sm20386055e9.24.2025.06.11.05.52.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Jun 2025 05:52:08 -0700 (PDT) Date: Wed, 11 Jun 2025 14:52:06 +0200 From: Stefano Brivio To: Laurent Vivier Subject: Re: [PATCH v6 01/30] arp: Don't mix incoming and outgoing buffers Message-ID: <20250611145206.39e7a1f2@elisabeth> In-Reply-To: <20250604130834.3868010-2-lvivier@redhat.com> References: <20250604130834.3868010-1-lvivier@redhat.com> <20250604130834.3868010-2-lvivier@redhat.com> 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: w5mJk9LdzyNKUpZTLZtl7E4ihw_XzanWJOly82e_PIg_1749646330 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: LYVRIK3QUB4HGTQDUKFJPXYCWOE7P27P X-Message-ID-Hash: LYVRIK3QUB4HGTQDUKFJPXYCWOE7P27P 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: passt-dev@passt.top, David Gibson 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, 4 Jun 2025 15:08:05 +0200 Laurent Vivier wrote: > Don't use the memory of the incoming packet to build the outgoing buffer > as it can be memory of the TX queue in the case of vhost-user. > > Moreover with vhost-user, the packet can be split across several > iovec and it's easier to rebuild it in a buffer than updating an > existing iovec array. > > Signed-off-by: Laurent Vivier > Reviewed-by: David Gibson > --- > arp.c | 84 ++++++++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 55 insertions(+), 29 deletions(-) > > diff --git a/arp.c b/arp.c > index fc482bbd9938..721ff2d529b5 100644 > --- a/arp.c > +++ b/arp.c > @@ -31,56 +31,82 @@ > #include "tap.h" > > /** > - * arp() - Check if this is a supported ARP message, reply as needed > + * accept_arp() - Check if we should accept this ARP message > * @c: Execution context > - * @p: Packet pool, single packet with Ethernet buffer > + * @ah: ARP header > + * @am: ARP message > * > - * Return: 1 if handled, -1 on failure > + * Return: true if the message is supported, false otherwise. ...wait, the function name is reversed now, but this isn't. You actually return true if the message should be discarded, not if the message "is supported". On v5, I mentioned I'd change the name of the function to accept_arp() rather than reversing the return value... but I missed this part, sorry. I guess if it's accept_arp(), it should return true *if we want to do something with the message*. If it's ignore_arp(), the opposite. I don't have a particular preference, as long as the function doesn't return the opposite of what the verb in its name says. :) > */ > -int arp(const struct ctx *c, const struct pool *p) > +static bool accept_arp(const struct ctx *c, > + const struct arphdr *ah, const struct arpmsg *am) > { > - unsigned char swap[4]; > - struct ethhdr *eh; > - struct arphdr *ah; > - struct arpmsg *am; > - size_t l2len; > - > - eh = packet_get(p, 0, 0, sizeof(*eh), NULL); > - ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL); > - am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL); > - > - if (!eh || !ah || !am) > - return -1; > - > if (ah->ar_hrd != htons(ARPHRD_ETHER) || > ah->ar_pro != htons(ETH_P_IP) || > ah->ar_hln != ETH_ALEN || > ah->ar_pln != 4 || > ah->ar_op != htons(ARPOP_REQUEST)) > - return 1; > + return true; > > /* Discard announcements, but not 0.0.0.0 "probes" */ > if (memcmp(am->sip, &in4addr_any, sizeof(am->sip)) && > !memcmp(am->sip, am->tip, sizeof(am->sip))) > - return 1; > + return true; > > /* Don't resolve the guest's assigned address, either. */ > if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) > + return true; > + > + return false; > +} > + > +/** > + * arp() - Check if this is a supported ARP message, reply as needed > + * @c: Execution context > + * @p: Packet pool, single packet with Ethernet buffer > + * > + * Return: 1 if handled, -1 on failure > + */ > +int arp(const struct ctx *c, const struct pool *p) > +{ > + struct { > + struct ethhdr eh; > + struct arphdr ah; > + struct arpmsg am; > + } __attribute__((__packed__)) resp; > + const struct ethhdr *eh; > + const struct arphdr *ah; > + const struct arpmsg *am; > + > + eh = packet_get(p, 0, 0, sizeof(*eh), NULL); > + ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL); > + am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL); > + > + if (!eh || !ah || !am) > + return -1; > + > + if (accept_arp(c, ah, am)) > return 1; > > - ah->ar_op = htons(ARPOP_REPLY); > - memcpy(am->tha, am->sha, sizeof(am->tha)); > - memcpy(am->sha, c->our_tap_mac, sizeof(am->sha)); > + /* Ethernet header */ > + resp.eh.h_proto = htons(ETH_P_ARP); > + memcpy(resp.eh.h_dest, eh->h_source, sizeof(resp.eh.h_dest)); > + memcpy(resp.eh.h_source, c->our_tap_mac, sizeof(resp.eh.h_source)); > > - memcpy(swap, am->tip, sizeof(am->tip)); > - memcpy(am->tip, am->sip, sizeof(am->tip)); > - memcpy(am->sip, swap, sizeof(am->sip)); > + /* ARP header */ > + resp.ah.ar_op = htons(ARPOP_REPLY); > + resp.ah.ar_hrd = ah->ar_hrd; > + resp.ah.ar_pro = ah->ar_pro; > + resp.ah.ar_hln = ah->ar_hln; > + resp.ah.ar_pln = ah->ar_pln; > > - l2len = sizeof(*eh) + sizeof(*ah) + sizeof(*am); > - memcpy(eh->h_dest, eh->h_source, sizeof(eh->h_dest)); > - memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source)); > + /* ARP message */ > + memcpy(resp.am.sha, c->our_tap_mac, sizeof(resp.am.sha)); > + memcpy(resp.am.sip, am->tip, sizeof(resp.am.sip)); > + memcpy(resp.am.tha, am->sha, sizeof(resp.am.tha)); > + memcpy(resp.am.tip, am->sip, sizeof(resp.am.tip)); > > - tap_send_single(c, eh, l2len); > + tap_send_single(c, &resp, sizeof(resp)); > > return 1; > } -- Stefano