From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202504 header.b=jVYgrA+Z; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 139715A0277 for ; Sat, 03 May 2025 06:12:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202504; t=1746245518; bh=21k5dYT4J9ohGVIhNPBXhAduTSqXInlT6aDfKP3TW8E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jVYgrA+ZO5rChl0XrJU/mxA4yUnzrKZ3y3d4DNuGXu6XNMG1gEqq4f7UeRrpXHJwS XHnzAr/9T6GpmILjZKCAhVrJhJbNcpb5rdJ2JEROkZr83wBg9vffYi9C2k3nHDFEcA N7i2VIbpC/UiKnNBKfaG4KnHka4z1CyUwDRB6cBEG9jwXo7b9TSKCEmSpLMOrvNnT0 Y0baMEbYaV6ECiRh3CSOdVlrgAQ71U4PY/Y3UG7Mx3bFmzb/3XsBleirSua2j1Tc0O q3YujMJ7lQunRrv/aFr5XOPs7AJg8nWKd07UxS2GoZmtL8ORsvdX20766j4JmtNeSd B8ZO08iL6FRBw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZqDrZ0sbzz4xQv; Sat, 3 May 2025 14:11:58 +1000 (AEST) Date: Sat, 3 May 2025 09:44:12 +0700 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] udp: Actually discard datagrams we can't forward Message-ID: References: <20250502210640.364286-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ll8hYgH4w2XAaJY1" Content-Disposition: inline In-Reply-To: <20250502210640.364286-1-sbrivio@redhat.com> Message-ID-Hash: WOEX3FC6P32HAVITCYZJPZTVUJ4TGNBQ X-Message-ID-Hash: WOEX3FC6P32HAVITCYZJPZTVUJ4TGNBQ 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 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: --ll8hYgH4w2XAaJY1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 02, 2025 at 11:06:40PM +0200, Stefano Brivio wrote: > Given that udp_sock_fwd() now loops on udp_peek_addr() to get endpoint > addresses for datagrams, if we can't forward one of these datagrams, > we need to make sure we actually discard it. Otherwise, with MSG_PEEK, > we won't dequeue and loop on it forever. >=20 > For example, if we fail to create a socket for a new flow, because, > say, the destination of an inbound packet is multicast, and we can't > bind() to a multicast address, the loop will look like this: >=20 > 18.0563: Flow 0 (NEW): FREE -> NEW > 18.0563: Flow 0 (INI): NEW -> INI > 18.0563: Flow 0 (INI): HOST [127.0.0.1]:42487 -> [127.0.0.1]:9997 =3D> ? > 18.0563: Flow 0 (TGT): INI -> TGT > 18.0563: Flow 0 (TGT): HOST [127.0.0.1]:42487 -> [ff02::c]:9997 =3D> SPLI= CE [0.0.0.0]:42487 -> [88.198.0.164]:9997 > 18.0563: Flow 0 (UDP flow): TGT -> TYPED > 18.0564: Flow 0 (UDP flow): HOST [127.0.0.1]:42487 -> [ff02::c]:9997 =3D>= SPLICE [0.0.0.0]:42487 -> [88.198.0.164]:9997 > 18.0564: Flow 0 (UDP flow): Couldn't open flow specific socket: Invalid a= rgument > 18.0564: Flow 0 (FREE): TYPED -> FREE > 18.0564: Flow 0 (FREE): HOST [127.0.0.1]:42487 -> [ff02::c]:9997 =3D> SPL= ICE [0.0.0.0]:42487 -> [88.198.0.164]:9997 > 18.0564: Discarding datagram without flow > 18.0564: Flow 0 (NEW): FREE -> NEW > 18.0564: Flow 0 (INI): NEW -> INI > 18.0564: Flow 0 (INI): HOST [127.0.0.1]:42487 -> [127.0.0.1]:9997 =3D> ? > 18.0564: Flow 0 (TGT): INI -> TGT > 18.0564: Flow 0 (TGT): HOST [127.0.0.1]:42487 -> [ff02::c]:9997 =3D> SPLI= CE [0.0.0.0]:42487 -> [88.198.0.164]:9997 > 18.0564: Flow 0 (UDP flow): TGT -> TYPED > 18.0564: Flow 0 (UDP flow): HOST [127.0.0.1]:42487 -> [ff02::c]:9997 =3D>= SPLICE [0.0.0.0]:42487 -> [88.198.0.164]:9997 > 18.0564: Flow 0 (UDP flow): Couldn't open flow specific socket: Invalid a= rgument > 18.0564: Flow 0 (FREE): TYPED -> FREE > 18.0564: Flow 0 (FREE): HOST [127.0.0.1]:42487 -> [ff02::c]:9997 =3D> SPL= ICE [0.0.0.0]:42487 -> [88.198.0.164]:9997 > 18.0564: Discarding datagram without flow >=20 > and seen from strace: >=20 > epoll_wait(3, [{events=3DEPOLLIN, data=3D0x1076c00000705}], 8, 1000) =3D 1 > recvmsg(7, {msg_name=3D{sa_family=3DAF_INET6, sin6_port=3Dhtons(55899), s= in6_flowinfo=3Dhtonl(0), inet_pton(AF_INET6, "fe80::26e8:53ff:fef3:13b6", &= sin6_addr), sin6_scope_id=3Dif_nametoindex("wlp4s0")}, msg_namelen=3D28, ms= g_iov=3DNULL, msg_iovlen=3D0, msg_control=3D[{cmsg_len=3D36, cmsg_level=3DS= OL_IPV6, cmsg_type=3D0x32, cmsg_data=3D"\xff\x02\x00\x00\x00\x00\x00\x00\x0= 0\x00\x00\x00\x00\x00\x00\x0c\x03\x00\x00\x00"}], msg_controllen=3D40, msg_= flags=3DMSG_TRUNC}, MSG_PEEK|MSG_DONTWAIT) =3D 0 > socket(AF_INET6, SOCK_DGRAM|SOCK_NONBLOCK, IPPROTO_UDP) =3D 12 > setsockopt(12, SOL_IPV6, IPV6_V6ONLY, [1], 4) =3D 0 > setsockopt(12, SOL_SOCKET, SO_REUSEADDR, [1], 4) =3D 0 > setsockopt(12, SOL_IPV6, IPV6_RECVERR, [1], 4) =3D 0 > setsockopt(12, SOL_IPV6, IPV6_RECVPKTINFO, [1], 4) =3D 0 > bind(12, {sa_family=3DAF_INET6, sin6_port=3Dhtons(1900), sin6_flowinfo=3D= htonl(0), inet_pton(AF_INET6, "ff02::c", &sin6_addr), sin6_scope_id=3D0}, 2= 8) =3D -1 EINVAL (Invalid argument) > close(12) =3D 0 > recvmsg(7, {msg_name=3D{sa_family=3DAF_INET6, sin6_port=3Dhtons(55899), s= in6_flowinfo=3Dhtonl(0), inet_pton(AF_INET6, "fe80::26e8:53ff:fef3:13b6", &= sin6_addr), sin6_scope_id=3Dif_nametoindex("wlp4s0")}, msg_namelen=3D28, ms= g_iov=3DNULL, msg_iovlen=3D0, msg_control=3D[{cmsg_len=3D36, cmsg_level=3DS= OL_IPV6, cmsg_type=3D0x32, cmsg_data=3D"\xff\x02\x00\x00\x00\x00\x00\x00\x0= 0\x00\x00\x00\x00\x00\x00\x0c\x03\x00\x00\x00"}], msg_controllen=3D40, msg_= flags=3DMSG_TRUNC}, MSG_PEEK|MSG_DONTWAIT) =3D 0 > socket(AF_INET6, SOCK_DGRAM|SOCK_NONBLOCK, IPPROTO_UDP) =3D 12 > setsockopt(12, SOL_IPV6, IPV6_V6ONLY, [1], 4) =3D 0 > setsockopt(12, SOL_SOCKET, SO_REUSEADDR, [1], 4) =3D 0 > setsockopt(12, SOL_IPV6, IPV6_RECVERR, [1], 4) =3D 0 > setsockopt(12, SOL_IPV6, IPV6_RECVPKTINFO, [1], 4) =3D 0 > bind(12, {sa_family=3DAF_INET6, sin6_port=3Dhtons(1900), sin6_flowinfo=3D= htonl(0), inet_pton(AF_INET6, "ff02::c", &sin6_addr), sin6_scope_id=3D0}, 2= 8) =3D -1 EINVAL (Invalid argument) > close(12) =3D 0 Oof, yes, that's a somewhat embarrassing oversight. Reviewed-by: David Gibson > Signed-off-by: Stefano Brivio > --- > udp.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) >=20 > diff --git a/udp.c b/udp.c > index f5a5cd1..ca28b37 100644 > --- a/udp.c > +++ b/udp.c > @@ -828,6 +828,7 @@ void udp_sock_fwd(const struct ctx *c, int s, uint8_t= frompif, > int rc; > =20 > while ((rc =3D udp_peek_addr(s, &src, &dst)) !=3D 0) { > + bool discard =3D false; > flow_sidx_t tosidx; > uint8_t topif; > =20 > @@ -861,8 +862,17 @@ void udp_sock_fwd(const struct ctx *c, int s, uint8_= t frompif, > flow_err(uflow, > "No support for forwarding UDP from %s to %s", > pif_name(frompif), pif_name(topif)); > + discard =3D true; > } else { > debug("Discarding datagram without flow"); > + discard =3D true; > + } > + > + if (discard) { > + struct msghdr msg =3D { 0 }; > + > + if (recvmsg(s, &msg, MSG_DONTWAIT) < 0) > + debug_perror("Failed to discard datagram"); > } > } > } --=20 David Gibson (he or they) | 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 --ll8hYgH4w2XAaJY1 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmgVgswACgkQzQJF27ox 2GdVeg//fJ/iY3UYpPc9zpsEtwnQolMUxr7re4i0KDQ9kFQYLeTCsFfOC6b5wejI 5y8yhXw+gzWoxd5nMThrndPVaHj+e9COnQrzR869R3YXjaB1RI5Lqhu0fSBG0BM6 3NkpCrtDS4roGIn+9uHBrq4z7bEaWT5lXylZRVqQ7aAh9CPXzXyY7e+Neq4sewdw vP9byss7KKYbhDBDAd8cwnPmLCUSqDYH8FMIoDSB0JxhGQjJbHVh7bmvG2FaVfWo 2mu+NEgIUZCuJIJQ/jSAPnPlxhqRhv4vLV/fN5kseCuyEdUmIVwVgjLhiQ98bgcM KWquBCJQFyBlNm90JSqHZYrwhLe5Y6+V6QeocIbAXUS5eHJ8Nc6M32eXECP41sbo VUPu/AwjEmDQSLjembD3s7hYrdOh8hM+65VS+bPlvARz7XGvchjDRNG5iCLLuhPU 5wZ+OrRBFXplvbuW7zwHUqlY3M9XcoXF586Oc2bnSVtHLK3r2+97lKD1PLStcD9r 9CmAvcICqWU9q6pWxsMCLi2QYDWuqIKF7QGg9GTs+0I7wmDT9Ml2SQl5yCgZl+9A iE+BmRI2aZ0AVg1r8KavWvSjDz5btAOaKoIdgOXw5pHlvDZ/Nivpp6mBaOf2+/Rp s9ksNURHQgOMcf2uZYb9GTXb2X5fHABAOuXjzyhXkeoJyXEAa6Q= =LByq -----END PGP SIGNATURE----- --ll8hYgH4w2XAaJY1--