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
next prev parent 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).