From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id 2FA355A026F for ; Wed, 17 Jan 2024 21:07:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1705522055; 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=BMTi9QMhbT36dSwUlnfCejcJT7qmNaIHXYDvYzt6V3I=; b=NtTQ0bxNQTzoWd8a2XBut27q42pGViOitm0k290dSJh6y2E/MOdOOIxWwjXP2m18m/9qHE Wc+MvHUvGGhPxDAe29awFdl0ksiGJ6hfO4qMsgEYoK/LGmoYSYgq9nnwW1FK39QIiBY8MG EF2mu3mQg/VvPkpuf34FBsylU+/KDxA= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-269-8TtiQhy-N3K3InWYfKVfUA-1; Wed, 17 Jan 2024 15:00:14 -0500 X-MC-Unique: 8TtiQhy-N3K3InWYfKVfUA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7ED5B3C00096; Wed, 17 Jan 2024 19:59:44 +0000 (UTC) Received: from elisabeth (unknown [10.39.208.3]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AE42151E3; Wed, 17 Jan 2024 19:59:43 +0000 (UTC) Date: Wed, 17 Jan 2024 20:59:41 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v3 12/15] icmp: Populate and use host side flow information Message-ID: <20240117205941.2dffe678@elisabeth> In-Reply-To: <20231221070237.1422557-13-david@gibson.dropbear.id.au> References: <20231221070237.1422557-1-david@gibson.dropbear.id.au> <20231221070237.1422557-13-david@gibson.dropbear.id.au> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: AZ23PV6SRXFXHM55FWYHFZCTTYZ4J366 X-Message-ID-Hash: AZ23PV6SRXFXHM55FWYHFZCTTYZ4J366 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 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, 21 Dec 2023 18:02:34 +1100 David Gibson wrote: > Complete filling out the common flow information for "ping" flows by > storing the host side information for the ping socket. We can only > retrieve this from the socket after we send the echo-request, because > that's when the kernel will assign an ID. > > Signed-off-by: David Gibson > --- > icmp.c | 36 +++++++++++++++++++++++++++++++++--- > 1 file changed, 33 insertions(+), 3 deletions(-) > > diff --git a/icmp.c b/icmp.c > index 53ad087..917c810 100644 > --- a/icmp.c > +++ b/icmp.c > @@ -310,11 +310,41 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af, > if (sendto(pingf->sock, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0) { > debug("%s: failed to relay request to socket: %s", > pname, strerror(errno)); > - } else { > - debug("%s: echo request to socket, ID: %"PRIu16", seq: %"PRIu16, > - pname, id, seq); > + if (flow) > + goto cancel; > + } > + > + debug("%s: echo request to socket, ID: %"PRIu16", seq: %"PRIu16, > + pname, id, seq); > + > + if (!flow) > + /* Nothing more to do for an existing flow */ > + return 1; > + > + /* We need to wait until after the sendto() to fill in the SOCKSIDE > + * information, so that we can find out the host side id the kernel > + * assigned. If there's no bind address specified, this will still have > + * 0.0.0.0 or :: as the host side forwarding address. There's not > + * really anything we can do to fill that in, which means we can never > + * insert the SOCKSIDE of a ping flow into the hash table. > + */ Well, if it was just because of that we could argue at some point that, with wildcards or suchlike, we could insert that in the hash table as well. But that wouldn't make sense anyway, right? That is, unless I'm missing something, we would have no use for a hash table lookup of the SOCKSIDE of a ping flow anyway. Patches 13/15 to 15/15 all look good to me. -- Stefano