* [PATCH] udp: Fix breakage of UDP error handling by PKTINFO support
@ 2025-04-14 3:58 David Gibson
2025-04-14 9:56 ` Stefano Brivio
0 siblings, 1 reply; 3+ messages in thread
From: David Gibson @ 2025-04-14 3:58 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: Jon Maloy, David Gibson
We recently enabled the IP_PKTINFO / IPV6_RECVPKTINFO socket options on our
UDP sockets. This lets us obtain and properly handle the specific local
address used when we're "listening" with a socket on 0.0.0.0 or ::.
However, the PKTINFO cmsgs this option generates appear on error queue
messages as well as regular datagrams. udp_sock_recverr() doesn't expect
this and so flags an unrecoverable error when it can't parse the control
message.
Correct this by adding space in udp_sock_recverr()s control buffer for the
additional PKTINFO data, and scan through all cmsgs for the RECVERR, rather
than only looking at the first one.
Link: https://bugs.passt.top/show_bug.cgi?id=99
Fixes: f4b0dd8b06 ("udp: Use PKTINFO cmsgs to get destination address..")
Reported-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
udp.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/udp.c b/udp.c
index 40af7dfc..f5fb98c2 100644
--- a/udp.c
+++ b/udp.c
@@ -155,6 +155,10 @@ __attribute__ ((aligned(32)))
#endif
udp_meta[UDP_MAX_FRAMES];
+#define PKTINFO_SPACE \
+ MAX(CMSG_SPACE(sizeof(struct in_pktinfo)), \
+ CMSG_SPACE(sizeof(struct in6_pktinfo)))
+
/**
* enum udp_iov_idx - Indices for the buffers making up a single UDP frame
* @UDP_IOV_TAP tap specific header
@@ -476,10 +480,10 @@ static int udp_sock_recverr(const struct ctx *c, union epoll_ref ref)
struct sock_extended_err ee;
union sockaddr_inany saddr;
};
- const struct errhdr *eh;
- const struct cmsghdr *hdr;
- char buf[CMSG_SPACE(sizeof(struct errhdr))];
+ char buf[PKTINFO_SPACE + CMSG_SPACE(sizeof(struct errhdr))];
char data[ICMP6_MAX_DLEN];
+ const struct errhdr *eh;
+ struct cmsghdr *hdr;
int s = ref.fd;
struct iovec iov = {
.iov_base = data,
@@ -507,12 +511,16 @@ static int udp_sock_recverr(const struct ctx *c, union epoll_ref ref)
return -1;
}
- hdr = CMSG_FIRSTHDR(&mh);
- if (!((hdr->cmsg_level == IPPROTO_IP &&
- hdr->cmsg_type == IP_RECVERR) ||
- (hdr->cmsg_level == IPPROTO_IPV6 &&
- hdr->cmsg_type == IPV6_RECVERR))) {
- err("Unexpected cmsg reading error queue");
+ for (hdr = CMSG_FIRSTHDR(&mh); hdr; hdr = CMSG_NXTHDR(&mh, hdr)) {
+ if ((hdr->cmsg_level == IPPROTO_IP &&
+ hdr->cmsg_type == IP_RECVERR) ||
+ (hdr->cmsg_level == IPPROTO_IPV6 &&
+ hdr->cmsg_type == IPV6_RECVERR))
+ break;
+ }
+
+ if (!hdr) {
+ err("Missing RECVERR cmsg in error queue");
return -1;
}
@@ -587,10 +595,6 @@ static int udp_sock_errs(const struct ctx *c, union epoll_ref ref)
return n_err;
}
-#define PKTINFO_SPACE \
- MAX(CMSG_SPACE(sizeof(struct in_pktinfo)), \
- CMSG_SPACE(sizeof(struct in6_pktinfo)))
-
/**
* udp_peek_addr() - Get source address for next packet
* @s: Socket to get information from
--
@@ -155,6 +155,10 @@ __attribute__ ((aligned(32)))
#endif
udp_meta[UDP_MAX_FRAMES];
+#define PKTINFO_SPACE \
+ MAX(CMSG_SPACE(sizeof(struct in_pktinfo)), \
+ CMSG_SPACE(sizeof(struct in6_pktinfo)))
+
/**
* enum udp_iov_idx - Indices for the buffers making up a single UDP frame
* @UDP_IOV_TAP tap specific header
@@ -476,10 +480,10 @@ static int udp_sock_recverr(const struct ctx *c, union epoll_ref ref)
struct sock_extended_err ee;
union sockaddr_inany saddr;
};
- const struct errhdr *eh;
- const struct cmsghdr *hdr;
- char buf[CMSG_SPACE(sizeof(struct errhdr))];
+ char buf[PKTINFO_SPACE + CMSG_SPACE(sizeof(struct errhdr))];
char data[ICMP6_MAX_DLEN];
+ const struct errhdr *eh;
+ struct cmsghdr *hdr;
int s = ref.fd;
struct iovec iov = {
.iov_base = data,
@@ -507,12 +511,16 @@ static int udp_sock_recverr(const struct ctx *c, union epoll_ref ref)
return -1;
}
- hdr = CMSG_FIRSTHDR(&mh);
- if (!((hdr->cmsg_level == IPPROTO_IP &&
- hdr->cmsg_type == IP_RECVERR) ||
- (hdr->cmsg_level == IPPROTO_IPV6 &&
- hdr->cmsg_type == IPV6_RECVERR))) {
- err("Unexpected cmsg reading error queue");
+ for (hdr = CMSG_FIRSTHDR(&mh); hdr; hdr = CMSG_NXTHDR(&mh, hdr)) {
+ if ((hdr->cmsg_level == IPPROTO_IP &&
+ hdr->cmsg_type == IP_RECVERR) ||
+ (hdr->cmsg_level == IPPROTO_IPV6 &&
+ hdr->cmsg_type == IPV6_RECVERR))
+ break;
+ }
+
+ if (!hdr) {
+ err("Missing RECVERR cmsg in error queue");
return -1;
}
@@ -587,10 +595,6 @@ static int udp_sock_errs(const struct ctx *c, union epoll_ref ref)
return n_err;
}
-#define PKTINFO_SPACE \
- MAX(CMSG_SPACE(sizeof(struct in_pktinfo)), \
- CMSG_SPACE(sizeof(struct in6_pktinfo)))
-
/**
* udp_peek_addr() - Get source address for next packet
* @s: Socket to get information from
--
2.49.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] udp: Fix breakage of UDP error handling by PKTINFO support
2025-04-14 3:58 [PATCH] udp: Fix breakage of UDP error handling by PKTINFO support David Gibson
@ 2025-04-14 9:56 ` Stefano Brivio
[not found] ` <20250414191251.3623406b@elisabeth>
0 siblings, 1 reply; 3+ messages in thread
From: Stefano Brivio @ 2025-04-14 9:56 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev, Jon Maloy
On Mon, 14 Apr 2025 13:58:53 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> We recently enabled the IP_PKTINFO / IPV6_RECVPKTINFO socket options on our
> UDP sockets. This lets us obtain and properly handle the specific local
> address used when we're "listening" with a socket on 0.0.0.0 or ::.
>
> However, the PKTINFO cmsgs this option generates appear on error queue
> messages as well as regular datagrams. udp_sock_recverr() doesn't expect
> this and so flags an unrecoverable error when it can't parse the control
> message.
>
> Correct this by adding space in udp_sock_recverr()s control buffer for the
> additional PKTINFO data, and scan through all cmsgs for the RECVERR, rather
> than only looking at the first one.
>
> Link: https://bugs.passt.top/show_bug.cgi?id=99
> Fixes: f4b0dd8b06 ("udp: Use PKTINFO cmsgs to get destination address..")
> Reported-by: Stefano Brivio <sbrivio@redhat.com>
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The patch looks good to me, but I'm hitting something in my tests (with
my recent DNS fix) that's perhaps not intended.
On the host:
$ cat /etc/resolv.conf
nameserver 127.0.0.1
...and nobody is listening on that address. Podman passes
--dns-forward 169.254.1.1 (default) and in the container:
$ podman run --net=pasta:-d,-l,/tmp/pasta.log --rm -ti alpine sh
I do:
/ # nslookup google.com 169.254.1.1
;; connection timed out; no servers could be reached
which is expected. But logs show a warning:
49.5377: Flow 0 (NEW): FREE -> NEW
49.5377: Flow 0 (INI): NEW -> INI
49.5377: Flow 0 (INI): TAP [88.198.0.164]:43458 -> [169.254.1.1]:53 => ?
49.5377: Flow 0 (TGT): INI -> TGT
49.5377: Flow 0 (TGT): TAP [88.198.0.164]:43458 -> [169.254.1.1]:53 => HOST [0.0.0.0]:43458 -> [127.0.0.1]:53
49.5377: Flow 0 (UDP flow): TGT -> TYPED
49.5377: Flow 0 (UDP flow): TAP [88.198.0.164]:43458 -> [169.254.1.1]:53 => HOST [0.0.0.0]:43458 -> [127.0.0.1]:53
49.5378: Flow 0 (UDP flow): Side 0 hash table insert: bucket: 148325
49.5378: Flow 0 (UDP flow): Side 1 hash table insert: bucket: 309967
49.5378: Flow 0 (UDP flow): TYPED -> ACTIVE
49.5378: Flow 0 (UDP flow): TAP [88.198.0.164]:43458 -> [169.254.1.1]:53 => HOST [127.0.0.1]:43458 -> [127.0.0.1]:53
49.5378: WARNING: Error peeking at socket address: Connection refused
49.5379: ICMP error on UDP socket 208: Connection refused
49.5379: ICMP error on UDP socket 208: Connection refused
52.0404: ICMP error on UDP socket 208: Connection refused
52.0404: ICMP error on UDP socket 208: Connection refused
and I'm not sure if that warning is intended. By the way, I have the
feeling that it now takes longer (with the whole IP_PKTINFO thing) for
nslookup to fail, as if those ICMP errors were not relayed anymore, but
I'm not sure about this, and I didn't investigate yet.
--
Stefano
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-15 7:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-14 3:58 [PATCH] udp: Fix breakage of UDP error handling by PKTINFO support David Gibson
2025-04-14 9:56 ` Stefano Brivio
[not found] ` <20250414191251.3623406b@elisabeth>
2025-04-15 5:28 ` David Gibson
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).