- dev - passt

tcp_splice SO_RCVLOWAT code; never invoked?
by David Gibson 11 Apr '25

11 Apr '25

Hi Stefano, When debugging the splice EINTR bug I fixed the other day, I found the whole tcp_splice_sock_handler() pretty confusing to follow. So, I was working on some cleanups. But then I noticed something more specifically odd here. We've discussed the use of SO_RCVLOWAT previously. AIUI, you found it essential to achieve reasonable throughput and load for spliced connections. I think we've agreed before that it's not entirely the right tool for the job; just the only one available. Except... as far as I can tell, it's never invoked. AFAICT the only place we enable the RCVLOWAT stuff is in a block under this if: if (!(conn->flags & lowat_set_flag) && readlen > (long)c->tcp.pipe_size / 10) { But... this occurs immediately after: if (readlen >= (long)c->tcp.pipe_size * 10 / 100) continue; .. which is a strictly more inclusive condition, so we'll never reach the RCVLOWAT block. To confirm, I tried putting an ASSERT(0) in that block, and didn't hit it with spliced iperf3 runs. Am I missing something? -- 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

2 2

[PATCH 0/3] Properly preseve local addresses for UDP flows
by David Gibson 11 Apr '25

11 Apr '25

Add the pieces we need so that we'll reliably send replies from the same local address that the originating datagram was delivered to. Fixes bug 99, and will (I think) be necessary for a bunch more of the flexible forwarding stuff we have in mind. David Gibson (3): udp: Use PKTINFO cmsgs to get destination address for received datagrams inany: Improve ASSERT message for bad socket family udp, udp_flow: Track our specific address on socket interfaces flow.c | 14 +++++++++++--- flow.h | 3 ++- flow_table.h | 1 + inany.h | 3 ++- tcp.c | 2 +- udp.c | 41 +++++++++++++++++++++++++++++++++++++---- udp_flow.c | 36 ++++++++++++++++++++++++++++++++---- udp_flow.h | 3 ++- util.c | 8 ++++++-- util.h | 10 ++++++++++ 10 files changed, 104 insertions(+), 17 deletions(-) -- 2.49.0

2 4

[PATCH 0/2] Fix bugs in tcp_splice_sock_handler()
by David Gibson 10 Apr '25

10 Apr '25

Patch 1/2 here fixes what I think is the bug causing the latest issue in: https://github.com/containers/podman/issues/23686 While fixing that I spotted the bug fixed in 2/2. Definitely a bug, although I suspect we'll mostly get away with it if the --trace option is not enabled. David Gibson (2): tcp_splice: Don't double could bytes read on EINTR tcp_splice: Don't clobber errno before checking for EAGAIN tcp_splice.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) -- 2.49.0

2 3

[PATCH] conf: Add missing return in conf_nat(), fix --map-guest-addr none
by Stefano Brivio 08 Apr '25

08 Apr '25

As reported by somebody on IRC: $ pasta --map-guest-addr none Invalid address to remap to host: none that's because once we parsed "none", we try to parse it as an address as well. But we already handled it, so stop once we're done. Fixes: e813a4df7da2 ("conf: Allow address remapped to host to be configured") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conf.c b/conf.c index b54c55d..168646f 100644 --- a/conf.c +++ b/conf.c @@ -1272,6 +1272,8 @@ static void conf_nat(const char *arg, struct in_addr *addr4, *addr6 = in6addr_any; if (no_map_gw) *no_map_gw = 1; + + return; } if (inet_pton(AF_INET6, arg, addr6) && -- 2.43.0

2 1

[PATCH] udp_flow: Save 8 bytes in struct udp_flow on 64-bit architectures
by Stefano Brivio 08 Apr '25

08 Apr '25

Shuffle the fields just added by commits a7775e9550fa ("udp: support traceroute in direction tap-socket") and 9725e7988837 ("udp_flow: Don't discard packets that arrive between bind() and connect()"). On x86_64, as reported by pahole(1), before: struct udp_flow { struct flow_common f; /* 0 76 */ /* --- cacheline 1 boundary (64 bytes) was 12 bytes ago --- */ _Bool closed:1; /* 76: 0 1 */ /* XXX 7 bits hole, try to pack */ _Bool flush0; /* 77 1 */ _Bool flush1:1; /* 78: 0 1 */ /* XXX 7 bits hole, try to pack */ /* XXX 1 byte hole, try to pack */ time_t ts; /* 80 8 */ int s[2]; /* 88 8 */ uint8_t ttl[2]; /* 96 2 */ /* size: 104, cachelines: 2, members: 7 */ /* sum members: 95, holes: 1, sum holes: 1 */ /* sum bitfield members: 2 bits, bit holes: 2, sum bit holes: 14 bits */ /* padding: 6 */ /* last cacheline: 40 bytes */ }; and after: struct udp_flow { struct flow_common f; /* 0 76 */ /* --- cacheline 1 boundary (64 bytes) was 12 bytes ago --- */ uint8_t ttl[2]; /* 76 2 */ _Bool closed:1; /* 78: 0 1 */ _Bool flush0:1; /* 78: 1 1 */ _Bool flush1:1; /* 78: 2 1 */ /* XXX 5 bits hole, try to pack */ /* XXX 1 byte hole, try to pack */ time_t ts; /* 80 8 */ int s[2]; /* 88 8 */ /* size: 96, cachelines: 2, members: 7 */ /* sum members: 94, holes: 1, sum holes: 1 */ /* sum bitfield members: 3 bits, bit holes: 1, sum bit holes: 5 bits */ /* last cacheline: 32 bytes */ }; It doesn't matter much because anyway the typical storage for struct udp_flow is given by union flow: union flow { struct flow_common f; /* 0 76 */ struct flow_free_cluster free; /* 0 84 */ struct tcp_tap_conn tcp; /* 0 120 */ struct tcp_splice_conn tcp_splice; /* 0 120 */ struct icmp_ping_flow ping; /* 0 96 */ struct udp_flow udp; /* 0 96 */ }; but it still improves data locality somewhat, so let me fix this up now that commits are fresh. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- udp_flow.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/udp_flow.h b/udp_flow.h index 90d3b29..e289122 100644 --- a/udp_flow.h +++ b/udp_flow.h @@ -10,22 +10,25 @@ /** * struct udp_flow - Descriptor for a flow of UDP packets * @f: Generic flow information + * @ttl: TTL or hop_limit for both sides * @closed: Flow is already closed * @flush0: @s[0] may have datagrams queued for other flows * @flush1: @s[1] may have datagrams queued for other flows * @ts: Activity timestamp * @s: Socket fd (or -1) for each side of the flow - * @ttl: TTL or hop_limit for both sides */ struct udp_flow { /* Must be first element */ struct flow_common f; - bool closed :1; - bool flush0, flush1 :1; + uint8_t ttl[SIDES]; + + bool closed :1, + flush0 :1, + flush1 :1; + time_t ts; int s[SIDES]; - uint8_t ttl[SIDES]; }; struct udp_flow *udp_at_sidx(flow_sidx_t sidx); -- 2.43.0

2 1

[PATCH v7] udp: support traceroute in direction tap-socket
by Jon Maloy 07 Apr '25

07 Apr '25

Now that ICMP pass-through from socket-to-tap is in place, it is easy to support UDP based traceroute functionality in direction tap-to-socket. We fix that in this commit. Link: https://bugs.passt.top/show_bug.cgi?id=64 Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: - Using ancillary data instead of setsockopt to transfer outgoing TTL. - Support IPv6 v3: - Storing ttl per packet instead of per flow. This may not be elegant, but much less intrusive than changing the flow criteria. This eliminates the need for the extra, flow-changing patch we introduced in v2. v4: - Going back to something similar to the original solution, but storing current ttl in struct udp_flow, plus ensuring that all packets in a struct tap4_l4_t/tap6_l4_t instance have the same ttl. After input from David Gibson. v5: - Some minor fixes after feedback from Stefano Brivio. v6: - Ensured that socket TTL is initialized at flow creation. - Ensured that setting of hop limit on IPv6 socket works. v7: - Setting of IPv6 hop_limit fails sometimes unless it is 4 bytes. --- tap.c | 17 +++++++++++++---- udp.c | 22 +++++++++++++++++++++- udp.h | 3 ++- udp_flow.c | 1 + udp_flow.h | 4 +++- 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/tap.c b/tap.c index 3a6fcbe..d630f6d 100644 --- a/tap.c +++ b/tap.c @@ -559,6 +559,7 @@ PACKET_POOL_DECL(pool_l4, UIO_MAXIOV, pkt_buf); * struct l4_seq4_t - Message sequence for one protocol handler call, IPv4 * @msgs: Count of messages in sequence * @protocol: Protocol number + * @ttl: Time to live * @source: Source port * @dest: Destination port * @saddr: Source address @@ -567,6 +568,7 @@ PACKET_POOL_DECL(pool_l4, UIO_MAXIOV, pkt_buf); */ static struct tap4_l4_t { uint8_t protocol; + uint8_t ttl; uint16_t source; uint16_t dest; @@ -586,6 +588,7 @@ static struct tap4_l4_t { * @dest: Destination port * @saddr: Source address * @daddr: Destination address + * @hop_limit: Hop limit * @msg: Array of messages that can be handled in a single call */ static struct tap6_l4_t { @@ -598,6 +601,8 @@ static struct tap6_l4_t { struct in6_addr saddr; struct in6_addr daddr; + uint8_t hop_limit; + struct pool_l4_t p; } tap6_l4[TAP_SEQS /* Arbitrary: TAP_MSGS in theory, so limit in users */]; @@ -786,7 +791,8 @@ resume: #define L4_MATCH(iph, uh, seq) \ ((seq)->protocol == (iph)->protocol && \ (seq)->source == (uh)->source && (seq)->dest == (uh)->dest && \ - (seq)->saddr.s_addr == (iph)->saddr && (seq)->daddr.s_addr == (iph)->daddr) + (seq)->saddr.s_addr == (iph)->saddr && \ + (seq)->daddr.s_addr == (iph)->daddr && (seq)->ttl == (iph)->ttl) #define L4_SET(iph, uh, seq) \ do { \ @@ -795,6 +801,7 @@ resume: (seq)->dest = (uh)->dest; \ (seq)->saddr.s_addr = (iph)->saddr; \ (seq)->daddr.s_addr = (iph)->daddr; \ + (seq)->ttl = (iph)->ttl; \ } while (0) if (seq && L4_MATCH(iph, uh, seq) && seq->p.count < UIO_MAXIOV) @@ -843,7 +850,7 @@ append: for (k = 0; k < p->count; ) k += udp_tap_handler(c, PIF_TAP, AF_INET, &seq->saddr, &seq->daddr, - p, k, now); + seq->ttl, p, k, now); } } @@ -966,7 +973,8 @@ resume: (seq)->dest == (uh)->dest && \ (seq)->flow_lbl == ip6_get_flow_lbl(ip6h) && \ IN6_ARE_ADDR_EQUAL(&(seq)->saddr, saddr) && \ - IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr)) + IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr) && \ + (seq)->hop_limit == (ip6h)->hop_limit) #define L4_SET(ip6h, proto, uh, seq) \ do { \ @@ -976,6 +984,7 @@ resume: (seq)->flow_lbl = ip6_get_flow_lbl(ip6h); \ (seq)->saddr = *saddr; \ (seq)->daddr = *daddr; \ + (seq)->hop_limit = (ip6h)->hop_limit; \ } while (0) if (seq && L4_MATCH(ip6h, proto, uh, seq) && @@ -1026,7 +1035,7 @@ append: for (k = 0; k < p->count; ) k += udp_tap_handler(c, PIF_TAP, AF_INET6, &seq->saddr, &seq->daddr, - p, k, now); + seq->hop_limit, p, k, now); } } diff --git a/udp.c b/udp.c index b275db3..3600336 100644 --- a/udp.c +++ b/udp.c @@ -818,6 +818,7 @@ fail: * @af: Address family, AF_INET or AF_INET6 * @saddr: Source address * @daddr: Destination address + * @ttl: TTL or hop limit for packets to be sent in this call * @p: Pool of UDP packets, with UDP headers * @idx: Index of first packet to process * @now: Current timestamp @@ -828,7 +829,8 @@ fail: */ int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, int idx, const struct timespec *now) + uint8_t ttl, const struct pool *p, int idx, + const struct timespec *now) { const struct flowside *toside; struct mmsghdr mm[UIO_MAXIOV]; @@ -907,6 +909,24 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, mm[i].msg_hdr.msg_controllen = 0; mm[i].msg_hdr.msg_flags = 0; + if (ttl != uflow->ttl[tosidx.sidei]) { + uflow->ttl[tosidx.sidei] = ttl; + if (af == AF_INET) { + if (setsockopt(s, IPPROTO_IP, IP_TTL, + &ttl, sizeof(ttl)) < 0) + flow_perror(uflow, + "setsockopt IP_TTL"); + } else { + /* IPv6 hop_limit cannot be only 1 byte */ + int hop_limit = ttl; + + if (setsockopt(s, SOL_IPV6, IPV6_UNICAST_HOPS, + &hop_limit, sizeof(hop_limit)) < 0) + flow_perror(uflow, + "setsockopt IPV6_UNICAST_HOPS"); + } + } + count++; } diff --git a/udp.h b/udp.h index 8fc4283..8f8531a 100644 --- a/udp.h +++ b/udp.h @@ -15,7 +15,8 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, const struct timespec *now); int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, int idx, const struct timespec *now); + uint8_t ttl, const struct pool *p, int idx, + const struct timespec *now); int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr, const char *ifname, in_port_t port); int udp_init(struct ctx *c); diff --git a/udp_flow.c b/udp_flow.c index af15d7f..d84d170 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -136,6 +136,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, uflow = FLOW_SET_TYPE(flow, FLOW_UDP, udp); uflow->ts = now->tv_sec; uflow->s[INISIDE] = uflow->s[TGTSIDE] = -1; + uflow->ttl[INISIDE] = uflow->ttl[TGTSIDE] = 0; flow_foreach_sidei(sidei) { if (pif_is_socket(uflow->f.pif[sidei])) diff --git a/udp_flow.h b/udp_flow.h index d518737..90d3b29 100644 --- a/udp_flow.h +++ b/udp_flow.h @@ -8,13 +8,14 @@ #define UDP_FLOW_H /** - * struct udp - Descriptor for a flow of UDP packets + * struct udp_flow - Descriptor for a flow of UDP packets * @f: Generic flow information * @closed: Flow is already closed * @flush0: @s[0] may have datagrams queued for other flows * @flush1: @s[1] may have datagrams queued for other flows * @ts: Activity timestamp * @s: Socket fd (or -1) for each side of the flow + * @ttl: TTL or hop_limit for both sides */ struct udp_flow { /* Must be first element */ @@ -24,6 +25,7 @@ struct udp_flow { bool flush0, flush1 :1; time_t ts; int s[SIDES]; + uint8_t ttl[SIDES]; }; struct udp_flow *udp_at_sidx(flow_sidx_t sidx); -- 2.48.1

3 2

[PATCH v6] udp: support traceroute in direction tap-socket
by Jon Maloy 04 Apr '25

04 Apr '25

Now that ICMP pass-through from socket-to-tap is in place, it is easy to support UDP based traceroute functionality in direction tap-to-socket. We fix that in this commit. Link: https://bugs.passt.top/show_bug.cgi?id=64 Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: - Using ancillary data instead of setsockopt to transfer outgoing TTL. - Support IPv6 v3: - Storing ttl per packet instead of per flow. This may not be elegant, but much less intrusive than changing the flow criteria. This eliminates the need for the extra, flow-changing patch we introduced in v2. v4: - Going back to something similar to the original solution, but storing current ttl in struct udp_flow, plus ensuring that all packets in a struct tap4_l4_t/tap6_l4_t instance have the same ttl. After input from David Gibson. v5: - Some minor fixes after feedback from Stefano Brivio. v6: - Ensured that socket TTL is initialized at flow creation. - Ensured that setting of hop limit on IPv6 socket works. --- tap.c | 17 +++++++++++++---- udp.c | 19 ++++++++++++++++++- udp.h | 3 ++- udp_flow.c | 1 + udp_flow.h | 4 +++- 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/tap.c b/tap.c index 3a6fcbe..d630f6d 100644 --- a/tap.c +++ b/tap.c @@ -559,6 +559,7 @@ PACKET_POOL_DECL(pool_l4, UIO_MAXIOV, pkt_buf); * struct l4_seq4_t - Message sequence for one protocol handler call, IPv4 * @msgs: Count of messages in sequence * @protocol: Protocol number + * @ttl: Time to live * @source: Source port * @dest: Destination port * @saddr: Source address @@ -567,6 +568,7 @@ PACKET_POOL_DECL(pool_l4, UIO_MAXIOV, pkt_buf); */ static struct tap4_l4_t { uint8_t protocol; + uint8_t ttl; uint16_t source; uint16_t dest; @@ -586,6 +588,7 @@ static struct tap4_l4_t { * @dest: Destination port * @saddr: Source address * @daddr: Destination address + * @hop_limit: Hop limit * @msg: Array of messages that can be handled in a single call */ static struct tap6_l4_t { @@ -598,6 +601,8 @@ static struct tap6_l4_t { struct in6_addr saddr; struct in6_addr daddr; + uint8_t hop_limit; + struct pool_l4_t p; } tap6_l4[TAP_SEQS /* Arbitrary: TAP_MSGS in theory, so limit in users */]; @@ -786,7 +791,8 @@ resume: #define L4_MATCH(iph, uh, seq) \ ((seq)->protocol == (iph)->protocol && \ (seq)->source == (uh)->source && (seq)->dest == (uh)->dest && \ - (seq)->saddr.s_addr == (iph)->saddr && (seq)->daddr.s_addr == (iph)->daddr) + (seq)->saddr.s_addr == (iph)->saddr && \ + (seq)->daddr.s_addr == (iph)->daddr && (seq)->ttl == (iph)->ttl) #define L4_SET(iph, uh, seq) \ do { \ @@ -795,6 +801,7 @@ resume: (seq)->dest = (uh)->dest; \ (seq)->saddr.s_addr = (iph)->saddr; \ (seq)->daddr.s_addr = (iph)->daddr; \ + (seq)->ttl = (iph)->ttl; \ } while (0) if (seq && L4_MATCH(iph, uh, seq) && seq->p.count < UIO_MAXIOV) @@ -843,7 +850,7 @@ append: for (k = 0; k < p->count; ) k += udp_tap_handler(c, PIF_TAP, AF_INET, &seq->saddr, &seq->daddr, - p, k, now); + seq->ttl, p, k, now); } } @@ -966,7 +973,8 @@ resume: (seq)->dest == (uh)->dest && \ (seq)->flow_lbl == ip6_get_flow_lbl(ip6h) && \ IN6_ARE_ADDR_EQUAL(&(seq)->saddr, saddr) && \ - IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr)) + IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr) && \ + (seq)->hop_limit == (ip6h)->hop_limit) #define L4_SET(ip6h, proto, uh, seq) \ do { \ @@ -976,6 +984,7 @@ resume: (seq)->flow_lbl = ip6_get_flow_lbl(ip6h); \ (seq)->saddr = *saddr; \ (seq)->daddr = *daddr; \ + (seq)->hop_limit = (ip6h)->hop_limit; \ } while (0) if (seq && L4_MATCH(ip6h, proto, uh, seq) && @@ -1026,7 +1035,7 @@ append: for (k = 0; k < p->count; ) k += udp_tap_handler(c, PIF_TAP, AF_INET6, &seq->saddr, &seq->daddr, - p, k, now); + seq->hop_limit, p, k, now); } } diff --git a/udp.c b/udp.c index 39431d7..026a7ee 100644 --- a/udp.c +++ b/udp.c @@ -849,6 +849,7 @@ fail: * @af: Address family, AF_INET or AF_INET6 * @saddr: Source address * @daddr: Destination address + * @ttl: TTL or hop limit for packets to be sent in this call * @p: Pool of UDP packets, with UDP headers * @idx: Index of first packet to process * @now: Current timestamp @@ -859,7 +860,8 @@ fail: */ int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, int idx, const struct timespec *now) + uint8_t ttl, const struct pool *p, int idx, + const struct timespec *now) { const struct flowside *toside; struct mmsghdr mm[UIO_MAXIOV]; @@ -938,6 +940,21 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, mm[i].msg_hdr.msg_controllen = 0; mm[i].msg_hdr.msg_flags = 0; + if (ttl != uflow->ttl[tosidx.sidei]) { + uflow->ttl[tosidx.sidei] = ttl; + if (af == AF_INET) { + if (setsockopt(s, IPPROTO_IP, IP_TTL, + &ttl, sizeof(ttl)) < 0) + flow_perror(uflow, + "setsockopt IP_TTL"); + } else { + if (setsockopt(s, SOL_IPV6, IPV6_UNICAST_HOPS, + &ttl, sizeof(ttl)) < 0) + flow_perror(uflow, + "setsockopt IPV6_UNICAST_HOPS"); + } + } + count++; } diff --git a/udp.h b/udp.h index de2df6d..a811475 100644 --- a/udp.h +++ b/udp.h @@ -15,7 +15,8 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, const struct timespec *now); int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, int idx, const struct timespec *now); + uint8_t ttl, const struct pool *p, int idx, + const struct timespec *now); int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr, const char *ifname, in_port_t port); int udp_init(struct ctx *c); diff --git a/udp_flow.c b/udp_flow.c index bf4b896..99ae490 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -137,6 +137,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, uflow = FLOW_SET_TYPE(flow, FLOW_UDP, udp); uflow->ts = now->tv_sec; uflow->s[INISIDE] = uflow->s[TGTSIDE] = -1; + uflow->ttl[INISIDE] = uflow->ttl[TGTSIDE] = 0; if (s_ini >= 0) { /* When using auto port-scanning the listening port could go diff --git a/udp_flow.h b/udp_flow.h index 9a1b059..520de62 100644 --- a/udp_flow.h +++ b/udp_flow.h @@ -8,11 +8,12 @@ #define UDP_FLOW_H /** - * struct udp - Descriptor for a flow of UDP packets + * struct udp_flow - Descriptor for a flow of UDP packets * @f: Generic flow information * @closed: Flow is already closed * @ts: Activity timestamp * @s: Socket fd (or -1) for each side of the flow + * @ttl: TTL or hop_limit for both sides */ struct udp_flow { /* Must be first element */ @@ -21,6 +22,7 @@ struct udp_flow { bool closed :1; time_t ts; int s[SIDES]; + uint8_t ttl[SIDES]; }; struct udp_flow *udp_at_sidx(flow_sidx_t sidx); -- 2.48.1

1 0

[PATCH v4] udp: support traceroute
by Jon Maloy 04 Apr '25

04 Apr '25

Now that ICMP pass-through from socket-to-tap is in place, it is easy to support UDP based traceroute functionality in direction tap-to-socket. We fix that in this commit. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: - Using ancillary data instead of setsockopt to transfer outgoing TTL. - Support IPv6 v3: - Storing ttl per packet instead of per flow. This may not be elegant, but much less intrusive than changing the flow criteria. This eliminates the need for the extra, flow-changing patch we introduced in v2. v4: - Going back to something similar to the original solution, but storing current ttl in struct udp_flow, plus ensuring that all packets in a struct tap4_l4_t/tap6_l4_t instance, have the same ttl. After input from David Gibson. --- packet.h | 2 ++ tap.c | 18 ++++++++++++++---- udp.c | 17 ++++++++++++++++- udp.h | 3 ++- udp_flow.c | 1 + udp_flow.h | 1 + 6 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packet.h b/packet.h index c94780a..e84e123 100644 --- a/packet.h +++ b/packet.h @@ -11,6 +11,8 @@ /* Maximum size of a single packet stored in pool, including headers */ #define PACKET_MAX_LEN ((size_t)UINT16_MAX) +#define DEFAULT_TTL 64 + /** * struct pool - Generic pool of packets stored in a buffer * @buf: Buffer storing packet descriptors, diff --git a/tap.c b/tap.c index 3a6fcbe..e65d592 100644 --- a/tap.c +++ b/tap.c @@ -563,6 +563,7 @@ PACKET_POOL_DECL(pool_l4, UIO_MAXIOV, pkt_buf); * @dest: Destination port * @saddr: Source address * @daddr: Destination address + * @ttl: Time to live * @msg: Array of messages that can be handled in a single call */ static struct tap4_l4_t { @@ -574,6 +575,8 @@ static struct tap4_l4_t { struct in_addr saddr; struct in_addr daddr; + uint8_t ttl; + struct pool_l4_t p; } tap4_l4[TAP_SEQS /* Arbitrary: TAP_MSGS in theory, so limit in users */]; @@ -586,6 +589,7 @@ static struct tap4_l4_t { * @dest: Destination port * @saddr: Source address * @daddr: Destination address + * @hop_limit: Hop limit * @msg: Array of messages that can be handled in a single call */ static struct tap6_l4_t { @@ -598,6 +602,8 @@ static struct tap6_l4_t { struct in6_addr saddr; struct in6_addr daddr; + uint8_t hop_limit; + struct pool_l4_t p; } tap6_l4[TAP_SEQS /* Arbitrary: TAP_MSGS in theory, so limit in users */]; @@ -786,7 +792,8 @@ resume: #define L4_MATCH(iph, uh, seq) \ ((seq)->protocol == (iph)->protocol && \ (seq)->source == (uh)->source && (seq)->dest == (uh)->dest && \ - (seq)->saddr.s_addr == (iph)->saddr && (seq)->daddr.s_addr == (iph)->daddr) + (seq)->saddr.s_addr == (iph)->saddr && \ + (seq)->daddr.s_addr == (iph)->daddr && (seq)->ttl == (iph)->ttl) #define L4_SET(iph, uh, seq) \ do { \ @@ -795,6 +802,7 @@ resume: (seq)->dest = (uh)->dest; \ (seq)->saddr.s_addr = (iph)->saddr; \ (seq)->daddr.s_addr = (iph)->daddr; \ + (seq)->ttl = (iph)->ttl; \ } while (0) if (seq && L4_MATCH(iph, uh, seq) && seq->p.count < UIO_MAXIOV) @@ -843,7 +851,7 @@ append: for (k = 0; k < p->count; ) k += udp_tap_handler(c, PIF_TAP, AF_INET, &seq->saddr, &seq->daddr, - p, k, now); + seq->ttl, p, k, now); } } @@ -966,7 +974,8 @@ resume: (seq)->dest == (uh)->dest && \ (seq)->flow_lbl == ip6_get_flow_lbl(ip6h) && \ IN6_ARE_ADDR_EQUAL(&(seq)->saddr, saddr) && \ - IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr)) + IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr) && \ + (seq)->hop_limit == (ip6h)->hop_limit) #define L4_SET(ip6h, proto, uh, seq) \ do { \ @@ -976,6 +985,7 @@ resume: (seq)->flow_lbl = ip6_get_flow_lbl(ip6h); \ (seq)->saddr = *saddr; \ (seq)->daddr = *daddr; \ + (seq)->hop_limit = (ip6h)->hop_limit; \ } while (0) if (seq && L4_MATCH(ip6h, proto, uh, seq) && @@ -1026,7 +1036,7 @@ append: for (k = 0; k < p->count; ) k += udp_tap_handler(c, PIF_TAP, AF_INET6, &seq->saddr, &seq->daddr, - p, k, now); + seq->hop_limit, p, k, now); } } diff --git a/udp.c b/udp.c index 39431d7..bc93292 100644 --- a/udp.c +++ b/udp.c @@ -849,6 +849,7 @@ fail: * @af: Address family, AF_INET or AF_INET6 * @saddr: Source address * @daddr: Destination address + * @ttl: TTL or hop limit for packets to be sent in this call * @p: Pool of UDP packets, with UDP headers * @idx: Index of first packet to process * @now: Current timestamp @@ -859,7 +860,8 @@ fail: */ int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, int idx, const struct timespec *now) + uint8_t ttl, const struct pool *p, int idx, + const struct timespec *now) { const struct flowside *toside; struct mmsghdr mm[UIO_MAXIOV]; @@ -938,6 +940,19 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, mm[i].msg_hdr.msg_controllen = 0; mm[i].msg_hdr.msg_flags = 0; + if (ttl != uflow->ttl[tosidx.sidei]) { + uflow->ttl[tosidx.sidei] = ttl; + if (af == AF_INET) { + if (setsockopt(s, IPPROTO_IP, IP_TTL, + &ttl, sizeof(ttl)) < 0) + perror("setsockopt (IP_TTL)"); + } else { + if (setsockopt(s, IPPROTO_IPV6, IPV6_HOPLIMIT, + &ttl, sizeof(ttl)) < 0) + perror("setsockopt (IP_TTL)"); + } + } + count++; } diff --git a/udp.h b/udp.h index de2df6d..041fad4 100644 --- a/udp.h +++ b/udp.h @@ -15,7 +15,8 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, const struct timespec *now); int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, int idx, const struct timespec *now); + uint8_t ttl, const struct pool *p, int idx, + const struct timespec *now); int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr, const char *ifname, in_port_t port); int udp_init(struct ctx *c); diff --git a/udp_flow.c b/udp_flow.c index bf4b896..39372c2 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -137,6 +137,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, uflow = FLOW_SET_TYPE(flow, FLOW_UDP, udp); uflow->ts = now->tv_sec; uflow->s[INISIDE] = uflow->s[TGTSIDE] = -1; + uflow->ttl[INISIDE] = uflow->ttl[TGTSIDE] = DEFAULT_TTL; if (s_ini >= 0) { /* When using auto port-scanning the listening port could go diff --git a/udp_flow.h b/udp_flow.h index 9a1b059..606ac08 100644 --- a/udp_flow.h +++ b/udp_flow.h @@ -21,6 +21,7 @@ struct udp_flow { bool closed :1; time_t ts; int s[SIDES]; + uint8_t ttl[SIDES]; }; struct udp_flow *udp_at_sidx(flow_sidx_t sidx); -- 2.48.1

3 9

[PATCH v5] udp: support traceroute
by Jon Maloy 04 Apr '25

04 Apr '25

Now that ICMP pass-through from socket-to-tap is in place, it is easy to support UDP based traceroute functionality in direction tap-to-socket. We fix that in this commit. Link: https://bugs.passt.top/show_bug.cgi?id=64 Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: - Using ancillary data instead of setsockopt to transfer outgoing TTL. - Support IPv6 v3: - Storing ttl per packet instead of per flow. This may not be elegant, but much less intrusive than changing the flow criteria. This eliminates the need for the extra, flow-changing patch we introduced in v2. v4: - Going back to something similar to the original solution, but storing current ttl in struct udp_flow, plus ensuring that all packets in a struct tap4_l4_t/tap6_l4_t instance have the same ttl. After input from David Gibson. v5: - Some minor fixes after feedback from Stefano Brivio. --- packet.h | 2 ++ tap.c | 17 +++++++++++++---- udp.c | 19 ++++++++++++++++++- udp.h | 3 ++- udp_flow.c | 1 + udp_flow.h | 4 +++- 6 files changed, 39 insertions(+), 7 deletions(-) diff --git a/packet.h b/packet.h index c94780a..e84e123 100644 --- a/packet.h +++ b/packet.h @@ -11,6 +11,8 @@ /* Maximum size of a single packet stored in pool, including headers */ #define PACKET_MAX_LEN ((size_t)UINT16_MAX) +#define DEFAULT_TTL 64 + /** * struct pool - Generic pool of packets stored in a buffer * @buf: Buffer storing packet descriptors, diff --git a/tap.c b/tap.c index 3a6fcbe..d630f6d 100644 --- a/tap.c +++ b/tap.c @@ -559,6 +559,7 @@ PACKET_POOL_DECL(pool_l4, UIO_MAXIOV, pkt_buf); * struct l4_seq4_t - Message sequence for one protocol handler call, IPv4 * @msgs: Count of messages in sequence * @protocol: Protocol number + * @ttl: Time to live * @source: Source port * @dest: Destination port * @saddr: Source address @@ -567,6 +568,7 @@ PACKET_POOL_DECL(pool_l4, UIO_MAXIOV, pkt_buf); */ static struct tap4_l4_t { uint8_t protocol; + uint8_t ttl; uint16_t source; uint16_t dest; @@ -586,6 +588,7 @@ static struct tap4_l4_t { * @dest: Destination port * @saddr: Source address * @daddr: Destination address + * @hop_limit: Hop limit * @msg: Array of messages that can be handled in a single call */ static struct tap6_l4_t { @@ -598,6 +601,8 @@ static struct tap6_l4_t { struct in6_addr saddr; struct in6_addr daddr; + uint8_t hop_limit; + struct pool_l4_t p; } tap6_l4[TAP_SEQS /* Arbitrary: TAP_MSGS in theory, so limit in users */]; @@ -786,7 +791,8 @@ resume: #define L4_MATCH(iph, uh, seq) \ ((seq)->protocol == (iph)->protocol && \ (seq)->source == (uh)->source && (seq)->dest == (uh)->dest && \ - (seq)->saddr.s_addr == (iph)->saddr && (seq)->daddr.s_addr == (iph)->daddr) + (seq)->saddr.s_addr == (iph)->saddr && \ + (seq)->daddr.s_addr == (iph)->daddr && (seq)->ttl == (iph)->ttl) #define L4_SET(iph, uh, seq) \ do { \ @@ -795,6 +801,7 @@ resume: (seq)->dest = (uh)->dest; \ (seq)->saddr.s_addr = (iph)->saddr; \ (seq)->daddr.s_addr = (iph)->daddr; \ + (seq)->ttl = (iph)->ttl; \ } while (0) if (seq && L4_MATCH(iph, uh, seq) && seq->p.count < UIO_MAXIOV) @@ -843,7 +850,7 @@ append: for (k = 0; k < p->count; ) k += udp_tap_handler(c, PIF_TAP, AF_INET, &seq->saddr, &seq->daddr, - p, k, now); + seq->ttl, p, k, now); } } @@ -966,7 +973,8 @@ resume: (seq)->dest == (uh)->dest && \ (seq)->flow_lbl == ip6_get_flow_lbl(ip6h) && \ IN6_ARE_ADDR_EQUAL(&(seq)->saddr, saddr) && \ - IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr)) + IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr) && \ + (seq)->hop_limit == (ip6h)->hop_limit) #define L4_SET(ip6h, proto, uh, seq) \ do { \ @@ -976,6 +984,7 @@ resume: (seq)->flow_lbl = ip6_get_flow_lbl(ip6h); \ (seq)->saddr = *saddr; \ (seq)->daddr = *daddr; \ + (seq)->hop_limit = (ip6h)->hop_limit; \ } while (0) if (seq && L4_MATCH(ip6h, proto, uh, seq) && @@ -1026,7 +1035,7 @@ append: for (k = 0; k < p->count; ) k += udp_tap_handler(c, PIF_TAP, AF_INET6, &seq->saddr, &seq->daddr, - p, k, now); + seq->hop_limit, p, k, now); } } diff --git a/udp.c b/udp.c index 39431d7..618a4e2 100644 --- a/udp.c +++ b/udp.c @@ -849,6 +849,7 @@ fail: * @af: Address family, AF_INET or AF_INET6 * @saddr: Source address * @daddr: Destination address + * @ttl: TTL or hop limit for packets to be sent in this call * @p: Pool of UDP packets, with UDP headers * @idx: Index of first packet to process * @now: Current timestamp @@ -859,7 +860,8 @@ fail: */ int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, int idx, const struct timespec *now) + uint8_t ttl, const struct pool *p, int idx, + const struct timespec *now) { const struct flowside *toside; struct mmsghdr mm[UIO_MAXIOV]; @@ -938,6 +940,21 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, mm[i].msg_hdr.msg_controllen = 0; mm[i].msg_hdr.msg_flags = 0; + if (ttl != uflow->ttl[tosidx.sidei]) { + uflow->ttl[tosidx.sidei] = ttl; + if (af == AF_INET) { + if (setsockopt(s, IPPROTO_IP, IP_TTL, + &ttl, sizeof(ttl)) < 0) + flow_perror(uflow, + "setsockopt IP_TTL"); + } else { + if (setsockopt(s, IPPROTO_IPV6, IPV6_HOPLIMIT, + &ttl, sizeof(ttl)) < 0) + flow_perror(uflow, + "setsockopt IPV6_HOPLIMIT"); + } + } + count++; } diff --git a/udp.h b/udp.h index de2df6d..a811475 100644 --- a/udp.h +++ b/udp.h @@ -15,7 +15,8 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, const struct timespec *now); int udp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, int idx, const struct timespec *now); + uint8_t ttl, const struct pool *p, int idx, + const struct timespec *now); int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr, const char *ifname, in_port_t port); int udp_init(struct ctx *c); diff --git a/udp_flow.c b/udp_flow.c index bf4b896..39372c2 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -137,6 +137,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, uflow = FLOW_SET_TYPE(flow, FLOW_UDP, udp); uflow->ts = now->tv_sec; uflow->s[INISIDE] = uflow->s[TGTSIDE] = -1; + uflow->ttl[INISIDE] = uflow->ttl[TGTSIDE] = DEFAULT_TTL; if (s_ini >= 0) { /* When using auto port-scanning the listening port could go diff --git a/udp_flow.h b/udp_flow.h index 9a1b059..520de62 100644 --- a/udp_flow.h +++ b/udp_flow.h @@ -8,11 +8,12 @@ #define UDP_FLOW_H /** - * struct udp - Descriptor for a flow of UDP packets + * struct udp_flow - Descriptor for a flow of UDP packets * @f: Generic flow information * @closed: Flow is already closed * @ts: Activity timestamp * @s: Socket fd (or -1) for each side of the flow + * @ttl: TTL or hop_limit for both sides */ struct udp_flow { /* Must be first element */ @@ -21,6 +22,7 @@ struct udp_flow { bool closed :1; time_t ts; int s[SIDES]; + uint8_t ttl[SIDES]; }; struct udp_flow *udp_at_sidx(flow_sidx_t sidx); -- 2.48.1

3 4

[PATCH] passt-repair: Ensure that read buffer is NULL-terminated
by Stefano Brivio 04 Apr '25

04 Apr '25

After 3d41e4d83895 ("passt-repair: Correct off-by-one error verifying name"), Coverity Scan isn't convinced anymore about the fact that the ev->name used in the snprintf() is NULL-terminated. It comes from a read() call, and read() of course doesn't terminate it, but we already check that the byte at ev->len - 1 is a NULL terminator, so this is actually a false positive. In any case, the logic ensuring that ev->name is NULL-terminated isn't necessarily obvious, and additionally checking that the last byte in the buffer we read is a NULL terminator is harmless, so do that explicitly, even if it's redundant. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt-repair.c | 1 + 1 file changed, 1 insertion(+) diff --git a/passt-repair.c b/passt-repair.c index 440c77a..256a8c9 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -137,6 +137,7 @@ int main(int argc, char **argv) fprintf(stderr, "inotify read: %i", errno); _exit(1); } + buf[n - 1] = '\0'; if (n < (ssize_t)sizeof(*ev)) { fprintf(stderr, "Short inotify read: %zi", n); -- 2.43.0

2 1