public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top
Subject: Re: [PATCH v3 12/15] icmp: Populate and use host side flow information
Date: Wed, 17 Jan 2024 20:59:41 +0100	[thread overview]
Message-ID: <20240117205941.2dffe678@elisabeth> (raw)
In-Reply-To: <20231221070237.1422557-13-david@gibson.dropbear.id.au>

On Thu, 21 Dec 2023 18:02:34 +1100
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> ---
>  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


  reply	other threads:[~2024-01-17 20:07 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21  7:02 [PATCH v3 00/15] RFC: Unified flow table David Gibson
2023-12-21  7:02 ` [PATCH v3 01/15] flow: Common data structures for tracking flow addresses David Gibson
2024-01-13 22:50   ` Stefano Brivio
2024-01-16  6:14     ` David Gibson
2023-12-21  7:02 ` [PATCH v3 02/15] tcp, flow: Maintain guest side flow information David Gibson
2024-01-13 22:51   ` Stefano Brivio
2024-01-16  6:23     ` David Gibson
2023-12-21  7:02 ` [PATCH v3 03/15] tcp, flow: Maintain host " David Gibson
2023-12-21  7:02 ` [PATCH v3 04/15] tcp_splice,flow: Maintain flow information for spliced connections David Gibson
2024-01-17 19:59   ` Stefano Brivio
2024-01-18  1:01     ` David Gibson
2023-12-21  7:02 ` [PATCH v3 05/15] flow, tcp, tcp_splice: Uniform debug helpers for new flows David Gibson
2024-01-17 19:59   ` Stefano Brivio
2024-01-18  1:04     ` David Gibson
2024-01-18 15:40       ` Stefano Brivio
2023-12-21  7:02 ` [PATCH v3 06/15] tcp, flow: Replace TCP specific hash function with general flow hash David Gibson
2024-01-17 19:59   ` Stefano Brivio
2024-01-18  1:15     ` David Gibson
2024-01-18 15:42       ` Stefano Brivio
2024-01-18 23:55         ` David Gibson
2023-12-21  7:02 ` [PATCH v3 07/15] flow: Add helper to determine a flow's protocol David Gibson
2023-12-21  7:02 ` [PATCH v3 08/15] flow, tcp: Generalise TCP hash table to general flow hash table David Gibson
2023-12-21  7:02 ` [PATCH v3 09/15] tcp: Re-use flow hash for initial sequence number generation David Gibson
2023-12-21  7:02 ` [PATCH v3 10/15] icmp: Store ping socket information in the flow table David Gibson
2023-12-21  7:02 ` [PATCH v3 11/15] icmp: Populate guest side information for ping flows David Gibson
2023-12-21  7:02 ` [PATCH v3 12/15] icmp: Populate and use host side flow information David Gibson
2024-01-17 19:59   ` Stefano Brivio [this message]
2024-01-18  1:22     ` David Gibson
2024-01-18 15:43       ` Stefano Brivio
2024-01-18 23:58         ` David Gibson
2023-12-21  7:02 ` [PATCH v3 13/15] icmp: Use 'flowside' epoll references for ping sockets David Gibson
2023-12-21  7:02 ` [PATCH v3 14/15] icmp: Merge EPOLL_TYPE_ICMP and EPOLL_TYPE_ICMPV6 David Gibson
2023-12-21  7:02 ` [PATCH v3 15/15] icmp: Eliminate icmp_id_map David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240117205941.2dffe678@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).