On Sat, Jan 06, 2024 at 04:59:11PM +0100, Stefano Brivio wrote:On Thu, 21 Dec 2023 17:53:19 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Right.Linux ICMP "ping" sockets are very specific in what they do. They let userspace send ping requests (ICMP_ECHO or ICMP6_ECHO_REQUEST), and receive matching replies (ICMP_ECHOREPLY or ICMP6_ECHO_REPLY). They don't let you intercept or handle incoming ping requests.Right... I don't know exactly what I was trying to do with this, back then. By the way this is the main reason why it took me a while to review this series... did I really write all those checks without a purpose? :) Well, it looks like it. Anyway, if you look at ping_err() in net/ipv4/ping.c, you'll see that among the messages which can be sent back as error (they're received on the socket causing the error -- same as ICMP messages you get on a UDP socket for port unreachable), ICMP_ECHO is allowed (see ping_supported()). I think I just used that ping_supported() function to find out which messages we could get on the socket. But we're not going to get those anyway.By the way, ICMP{,V6}_EXT_ECHO{,_REQUEST} support (RFC 8335, PROBE messages) was added a while ago (kernel commit 08baf54f01f5 "net: add support for sending RFC 8335 PROBE messages")... we should add that at some point, it's kind of trivial.Indeed. Want to make a BZ for it so we don't forget?Uh... no.. that change is right: [snip]In the case of passt/pasta that means we can process echo requests from tap and forward them to a ping socket, then take the replies from the ping socket and forward them to tap. We can't do the reverse: take echo requests from the host and somehow forward them to the guest. There's really no way for something outside to initiate a ping to a passt/pasta connected guest and if there was we'd need an entirely different mechanism to handle it. However, we have some logic to deal with packets going in that reverse direction. Remove it, since it can't ever be used that way. While we're there use defines for the ICMPv6 types, instead of open coded type values.I guess this last sentence only applied to a previous version of your patch. It doesn't matter so much, but it would be nice to drop if you re-spin.> @@ -222,7 +219,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af, > if (!ih) > return 1; > > - if (ih->icmp6_type != 128 && ih->icmp6_type != 129) > + if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)here. -- 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