From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 27FF45A0265 for ; Thu, 27 Oct 2022 00:08:40 +0200 (CEST) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4MyNHJ2wk4z4xGk; Thu, 27 Oct 2022 09:08:28 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1666822108; bh=xt6Hx7eli/3A36HHgmA2CA62imhJzswEixdpZNdgF6Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=hBZhj9L5xPHmoCcxr1b3IVa0GQfHop6Oeu1SC/3aktUbmQZYc1Ytyqs9U5w6HQh8r uJNg/sm2UxRwQVhNQG/kVxJmqfO3mZ/BiJcYHS8Ti20xajNnyD2A8xDHk4/DAjedy/ JzUlDacPKI+q95LdbIDKOJO8vvzkVkqdQW39SYVE= Date: Thu, 27 Oct 2022 08:59:26 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 4/4] icmp: Don't discard first reply sequence for a given echo ID Message-ID: References: <20221026162531.545374-1-sbrivio@redhat.com> <20221026162531.545374-5-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="I7QhdOkd3FUkanpO" Content-Disposition: inline In-Reply-To: <20221026162531.545374-5-sbrivio@redhat.com> Message-ID-Hash: MWHDS4LHNXIR5XY2QOMNQFRM2XAACRHP X-Message-ID-Hash: MWHDS4LHNXIR5XY2QOMNQFRM2XAACRHP X-MailFrom: dgibson@gandalf.ozlabs.org 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, Paul Holzinger X-Mailman-Version: 3.3.3 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: --I7QhdOkd3FUkanpO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 26, 2022 at 06:25:31PM +0200, Stefano Brivio wrote: > In pasta mode, ICMP and ICMPv6 echo sockets relay back to us any > reply we send: we're on the same host as the target, after all. We > discard them by comparing the last sequence we sent with the sequence > we receive. >=20 > However, on the first reply for a given identifier, the sequence > might be zero, depending on the implementation of ping(8): we need > another value to indicate we haven't sent any sequence number, yet. >=20 > Use -1 as initialiser in the echo identifier map. >=20 > This is visible with Busybox's ping, and was reported by Paul on the > integration at https://github.com/containers/podman/pull/16141, with: >=20 > $ podman run --net=3Dpasta alpine ping -c 2 192.168.188.1 >=20 > ...where only the second reply would be routed back. >=20 > Reported-by: Paul Holzinger > Fixes: 33482d5bf293 ("passt: Add PASTA mode, major rework") > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > icmp.c | 16 ++++++++++++++-- > icmp.h | 1 + > passt.c | 3 +++ > 3 files changed, 18 insertions(+), 2 deletions(-) >=20 > diff --git a/icmp.c b/icmp.c > index 9caa7e6..4ee847f 100644 > --- a/icmp.c > +++ b/icmp.c > @@ -44,12 +44,12 @@ > /** > * struct icmp_id_sock - Tracking information for single ICMP echo ident= ifier > * @sock: Bound socket for identifier > - * @seq: Last sequence number sent to tap, host order > + * @seq: Last sequence number sent to tap, host order, -1: not sent yet > * @ts: Last associated activity from tap, seconds > */ > struct icmp_id_sock { > int sock; > - uint16_t seq; > + int seq; > time_t ts; > }; > =20 > @@ -273,6 +273,7 @@ static void icmp_timer_one(const struct ctx *c, int v= 6, uint16_t id, > epoll_ctl(c->epollfd, EPOLL_CTL_DEL, id_map->sock, NULL); > close(id_map->sock); > id_map->sock =3D 0; > + id_map->seq =3D -1; > } > =20 > /** > @@ -301,3 +302,14 @@ v6: > goto v6; > } > } > + > +/** > + * icmp_init() - Initialise sequences in ID map to -1 (no sequence sent = yet) > + */ > +void icmp_init(void) > +{ > + unsigned i; > + > + for (i =3D 0; i < ICMP_NUM_IDS; i++) > + icmp_id_map[V4][i].seq =3D icmp_id_map[V6][i].seq =3D -1; > +} > diff --git a/icmp.h b/icmp.h > index 458ce31..275486d 100644 > --- a/icmp.h > +++ b/icmp.h > @@ -15,6 +15,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll= _ref ref, > int icmp_tap_handler(const struct ctx *c, int af, const void *addr, > const struct pool *p, const struct timespec *now); > void icmp_timer(const struct ctx *c, const struct timespec *ts); > +void icmp_init(void); > =20 > /** > * union icmp_epoll_ref - epoll reference portion for ICMP tracking > diff --git a/passt.c b/passt.c > index ff4ee5d..34cd832 100644 > --- a/passt.c > +++ b/passt.c > @@ -256,6 +256,9 @@ int main(int argc, char **argv) > if ((!c.no_udp && udp_init(&c)) || (!c.no_tcp && tcp_init(&c))) > exit(EXIT_FAILURE); > =20 > + if (!c.no_icmp) > + icmp_init(); > + > proto_update_l2_buf(c.mac_guest, c.mac, &c.ip4.addr); > =20 > if (c.ifi4 && !c.no_dhcp) --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --I7QhdOkd3FUkanpO Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmNZrbcACgkQgypY4gEw YSLwTBAAmNfDjmBooxs8Fzelg7Yr61Xid2ZAnhtWSeL/sqgUGpg38CLZEQ2w3gyn 2wJRzXQA+ranWAvr+7xlpPAq1IcpTqFrMbPp4boSSSpnBIAKxlkkYiGQv1iDiTnK gGc5crl0vy2qZeO/v3FEKrQexmQHFP++vQ8xfpXWXK6jEpmvZ0KpvML0Rb1SBV7B rPZteVGJHkCJzheGIvOlaYlXHEohq2b1m5gQXlRQDJLR6sun22OaAjjfVFj61t0I G3UUDdAxRV293CTNhMBdgYhrlU9hrFisSfKgxdOAfSyamYMvVgja2iY1JYIXQRF3 59+C/p4gQ83Y7a1Ap76jKsmwxz1HlXahywtPHcw5CD/pFEBF7J1y+JFuqtIwUCU7 R18xM1U45FKTYLTTc1jjuY/ci/Z5LIc9PyLhjWYbmqRiSnb6KZbX+KiHwoANH8Sj 5FtoGZXzSGXPot9Sws/KuzETBXZpN+d2kxyIu0x9SfDhytOSWLcdGmpwQLQLQejS Yy5K5zop4p4kpAtDBHdIN5vEymbCNersfZvyeM/CD4iflCx3IjJiOPfIb4E2T5To 23oqF0m54jNzG85L1Io6qXU7zCDCGo3L5SMIyDkAw2brwFzlAdn6yZNwkO9zUPLP kSViJ+OVo4kS0bZtxWqGqDVUqyKGcXh3bk5g6iSm/LTZOOoI+gI= =rjOx -----END PGP SIGNATURE----- --I7QhdOkd3FUkanpO--