[PATCH v5 0/9] Add vhost-user support to passt (part 1)
v5: - add a patch to cleanup before change: udp: little cleanup in udp_update_hdrX() to prepare future changes - see detailed v5 history log in each patch v4: - rebase - see detailed v4 history log in each patch v3: - add a patch that has been extracted from: "tcp: extract buffer management from tcp_send_flag()" -> "tcp: Introduce ipv4_fill_headers()/ipv6_fill_headers()" - see detailed v3 history log in each patch - I didn't address the alignment problem when we provide a pointer to a sub-structure in the internal buffer structure. (for the last patches of the series). v2 comparing to vhost-user full part: - part 1 includes only preliminary patches (checksum, iovec, cleanup) - see detailed v2 history log in each patch. Full series v1 available at: [PATCH 00/24] Add vhost-user support to passt. https://url.corp.redhat.com/passt-vhost-user-v1 Thanks, Laurent Laurent Vivier (9): pcap: add pcap_iov() checksum: align buffers checksum: add csum_iov() util: move IP stuff from util.[ch] to ip.[ch] udp: little cleanup in udp_update_hdrX() to prepare future changes checksum: use csum_ip4_header() in udp.c and tcp.c checksum: introduce functions to compute the header part checksum for TCP/UDP tap: make tap_update_mac() generic tcp: Introduce ipv4_fill_headers()/ipv6_fill_headers() Makefile | 10 +-- checksum.c | 174 +++++++++++++++++++++++++++++----------- checksum.h | 10 ++- conf.c | 1 + dhcp.c | 1 + flow.c | 1 + fwd.c | 1 + icmp.c | 1 + inany.c | 1 + ip.c | 72 +++++++++++++++++ ip.h | 86 ++++++++++++++++++++ ndp.c | 1 + pcap.c | 26 +++++- pcap.h | 1 + qrap.c | 1 + tap.c | 13 +-- tap.h | 2 +- tcp.c | 221 +++++++++++++++++++++++++++++---------------------- tcp_splice.c | 1 + udp.c | 82 ++++++++----------- util.c | 55 ------------- util.h | 76 ------------------ 22 files changed, 503 insertions(+), 334 deletions(-) create mode 100644 ip.c create mode 100644 ip.h -- 2.42.0
Introduce a new function pcap_iov() to capture packet desribed by an IO
vector.
Update pcap_frame() to manage iovcnt > 1.
Signed-off-by: Laurent Vivier
If buffer is not aligned use sum_16b() only on the not aligned
part, and then use csum_avx2() on the remaining part
Remove unneeded now function csum_unaligned().
Signed-off-by: Laurent Vivier
Introduce the function csum_unfolded() that computes the unfolded
32-bit checksum of a data buffer, and call it from csum() that returns
the folded value.
Introduce csum_iov() that computes the checksum using csum_folded() on
all vectors of the iovec array and returns the folded result.
Signed-off-by: Laurent Vivier
Introduce ip.[ch] file to encapsulate IP protocol handling functions and
structures. Modify various files to include the new header ip.h when
it's needed.
Signed-off-by: Laurent Vivier
in udp_update_hdr4():
Assign the source address to src, either b->s_in.sin_addr,
c->ip4.dns_match or c->ip4.gw and then set b->iph.saddr to src->s_addr.
in udp_update_hdr6():
Assign the source address to src, either b->s_in6.sin6_addr,
c->ip6.dns_match, c->ip6.gw or c->ip6.addr_ll.
Assign the destination to dst, either c->ip6.addr_seen or
&c->ip6.addr_ll_seen.
Then set dst to b->ip6h.daddr and src to b->ip6h.saddr.
Signed-off-by: Laurent Vivier
On Sun, Mar 03, 2024 at 02:51:10PM +0100, Laurent Vivier wrote:
in udp_update_hdr4():
Assign the source address to src, either b->s_in.sin_addr, c->ip4.dns_match or c->ip4.gw and then set b->iph.saddr to src->s_addr.
in udp_update_hdr6():
Assign the source address to src, either b->s_in6.sin6_addr, c->ip6.dns_match, c->ip6.gw or c->ip6.addr_ll. Assign the destination to dst, either c->ip6.addr_seen or &c->ip6.addr_ll_seen. Then set dst to b->ip6h.daddr and src to b->ip6h.saddr.
Signed-off-by: Laurent Vivier
Heh, this does something quite similar to one of the patches in my
small udp cleanups series.
Reviewed-by: David Gibson
---
Notes: v5: - new patch to introduce the use of struct in_addr with csum_ip4_header()
udp.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/udp.c b/udp.c index 26774df7018c..2b41a1bdff61 100644 --- a/udp.c +++ b/udp.c @@ -588,7 +588,7 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport, const struct timespec *now) { struct udp4_l2_buf_t *b = &udp4_l2_buf[n]; - struct in_addr *src; + const struct in_addr *src; in_port_t src_port; size_t ip_len;
@@ -602,10 +602,9 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) && IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && src_port == 53) { - b->iph.saddr = c->ip4.dns_match.s_addr; + src = &c->ip4.dns_match; } else if (IN4_IS_ADDR_LOOPBACK(src) || IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr_seen)) { - b->iph.saddr = c->ip4.gw.s_addr; udp_tap_map[V4][src_port].ts = now->tv_sec; udp_tap_map[V4][src_port].flags |= PORT_LOCAL;
@@ -615,9 +614,10 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport, udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK;
bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port); - } else { - b->iph.saddr = src->s_addr; + + src = &c->ip4.gw; } + b->iph.saddr = src->s_addr;
udp_update_check4(b); b->uh.source = b->s_in.sin_port; @@ -640,10 +640,11 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport, const struct timespec *now) { struct udp6_l2_buf_t *b = &udp6_l2_buf[n]; - struct in6_addr *src; + const struct in6_addr *src, *dst; in_port_t src_port; size_t ip_len;
+ dst = &c->ip6.addr_seen; src = &b->s_in6.sin6_addr; src_port = ntohs(b->s_in6.sin6_port);
@@ -652,23 +653,14 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport, b->ip6h.payload_len = htons(udp6_l2_mh_sock[n].msg_len + sizeof(b->uh));
if (IN6_IS_ADDR_LINKLOCAL(src)) { - b->ip6h.daddr = c->ip6.addr_ll_seen; - b->ip6h.saddr = b->s_in6.sin6_addr; + dst = &c->ip6.addr_ll_seen; } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) && IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) && src_port == 53) { - b->ip6h.daddr = c->ip6.addr_seen; - b->ip6h.saddr = c->ip6.dns_match; + src = &c->ip6.dns_match; } else if (IN6_IS_ADDR_LOOPBACK(src) || IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen) || IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr)) { - b->ip6h.daddr = c->ip6.addr_ll_seen; - - if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw)) - b->ip6h.saddr = c->ip6.gw; - else - b->ip6h.saddr = c->ip6.addr_ll; - udp_tap_map[V6][src_port].ts = now->tv_sec; udp_tap_map[V6][src_port].flags |= PORT_LOCAL;
@@ -683,10 +675,17 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport, udp_tap_map[V6][src_port].flags &= ~PORT_GUA;
bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port); - } else { - b->ip6h.daddr = c->ip6.addr_seen; - b->ip6h.saddr = b->s_in6.sin6_addr; + + dst = &c->ip6.addr_ll_seen; + + if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw)) + src = &c->ip6.gw; + else + src = &c->ip6.addr_ll; + } + b->ip6h.daddr = *dst; + b->ip6h.saddr = *src;
b->uh.source = b->s_in6.sin6_port; b->uh.dest = htons(dstport);
-- 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
We can find the same function to compute the IPv4 header
checksum in tcp.c, udp.c and tap.c
Use the function defined for tap.c, csum_ip4_header(), but
with the code used in tcp.c and udp.c as it doesn't need a fully
initialiazed IPv4 header, only protocol, tot_len, saddr and daddr.
Signed-off-by: Laurent Vivier
The TCP and UDP checksums are computed using the data in the TCP/UDP
payload but also some informations in the IP header (protocol,
length, source and destination addresses).
We add two functions, proto_ipv4_header_psum() and
proto_ipv6_header_psum(), to compute the checksum of the IP
header part.
Signed-off-by: Laurent Vivier
On Sun, Mar 03, 2024 at 02:51:12PM +0100, Laurent Vivier wrote:
The TCP and UDP checksums are computed using the data in the TCP/UDP payload but also some informations in the IP header (protocol, length, source and destination addresses).
We add two functions, proto_ipv4_header_psum() and proto_ipv6_header_psum(), to compute the checksum of the IP header part.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
---
Notes: v5: - use struct in_addr - update tcp_update_check_tcp4()/tcp_update_check_tcp6() function comment and add tcphdr as a parameter
v4: - fix payload length endianness
v3: - function parameters provide tot_len, saddr, daddr and protocol rather than an iphdr/ipv6hdr
v2: - move new function to checksum.c - use _psum rather than _checksum in the name - replace csum_udp4() and csum_udp6() by the new function
v5: - use struct in_addr
v4: - fix payload length endianness
v3: - function parameters provide tot_len, saddr, daddr and protocol rather than an iphdr/ipv6hdr
v2: - move new function to checksum.c - use _psum rather than _checksum in the name - replace csum_udp4() and csum_udp6() by the new function
checksum.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------ checksum.h | 5 ++++ tcp.c | 50 +++++++++++++++++++--------------------- udp.c | 18 ++++++++------- 4 files changed, 90 insertions(+), 50 deletions(-)
diff --git a/checksum.c b/checksum.c index ad81a7104a07..7625d7a41303 100644 --- a/checksum.c +++ b/checksum.c @@ -139,6 +139,29 @@ uint16_t csum_ip4_header(uint16_t tot_len, uint8_t protocol, return ~csum_fold(sum); }
+/** + * proto_ipv4_header_psum() - Calculates the partial checksum of an + * IPv4 header for UDP or TCP + * @tot_len: IPv4 Payload length (host order) + * @proto: Protocol number (host order)
"host order" is meaningless here, because it's a single byte value. That can be fixed in followup though.
+ * @saddr: Source address (network order) + * @daddr: Destination address (network order) + * Returns: Partial checksum of the IPv4 header + */ +uint32_t proto_ipv4_header_psum(uint16_t tot_len, uint8_t protocol, + struct in_addr saddr, struct in_addr daddr) +{ + uint32_t psum = htons(protocol); + + psum += (saddr.s_addr >> 16) & 0xffff; + psum += saddr.s_addr & 0xffff; + psum += (daddr.s_addr >> 16) & 0xffff; + psum += daddr.s_addr & 0xffff; + psum += htons(tot_len); + + return psum; +} + /** * csum_udp4() - Calculate and set checksum for a UDP over IPv4 packet * @udp4hr: UDP header, initialised apart from checksum @@ -155,14 +178,10 @@ void csum_udp4(struct udphdr *udp4hr, udp4hr->check = 0;
if (UDP4_REAL_CHECKSUMS) { - /* UNTESTED: if we did want real UDPv4 checksums, this - * is roughly what we'd need */ - uint32_t psum = csum_fold(saddr.s_addr) - + csum_fold(daddr.s_addr) - + htons(len + sizeof(*udp4hr)) - + htons(IPPROTO_UDP); - /* Add in partial checksum for the UDP header alone */ - psum += sum_16b(udp4hr, sizeof(*udp4hr)); + uint16_t tot_len = len + sizeof(struct udphdr); + uint32_t psum = proto_ipv4_header_psum(tot_len, IPPROTO_UDP, + saddr, daddr); + psum = csum_unfolded(udp4hr, sizeof(struct udphdr), psum); udp4hr->check = csum(payload, len, psum); } } @@ -185,6 +204,27 @@ void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len) icmp4hr->checksum = csum(payload, len, psum); }
+/** + * proto_ipv6_header_psum() - Calculates the partial checksum of an + * IPv6 header for UDP or TCP + * @payload_len: IPv6 payload length (host order) + * @proto: Protocol number (host order) + * @saddr: Source address (network order) + * @daddr: Destination address (network order) + * Returns: Partial checksum of the IPv6 header + */ +uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol, + const struct in6_addr *saddr, + const struct in6_addr *daddr) +{ + uint32_t sum = htons(protocol) + htons(payload_len); + + sum += sum_16b(saddr, sizeof(*saddr)); + sum += sum_16b(daddr, sizeof(*daddr)); + + return sum; +} + /** * csum_udp6() - Calculate and set checksum for a UDP over IPv6 packet * @udp6hr: UDP header, initialised apart from checksum @@ -195,14 +235,11 @@ void csum_udp6(struct udphdr *udp6hr, const struct in6_addr *saddr, const struct in6_addr *daddr, const void *payload, size_t len) { - /* Partial checksum for the pseudo-IPv6 header */ - uint32_t psum = sum_16b(saddr, sizeof(*saddr)) + - sum_16b(daddr, sizeof(*daddr)) + - htons(len + sizeof(*udp6hr)) + htons(IPPROTO_UDP); - + uint32_t psum = proto_ipv6_header_psum(len + sizeof(struct udphdr), + IPPROTO_UDP, saddr, daddr); udp6hr->check = 0; - /* Add in partial checksum for the UDP header alone */ - psum += sum_16b(udp6hr, sizeof(*udp6hr)); + + psum = csum_unfolded(udp6hr, sizeof(struct udphdr), psum); udp6hr->check = csum(payload, len, psum); }
diff --git a/checksum.h b/checksum.h index e990fefa7f39..d4e563c85f4f 100644 --- a/checksum.h +++ b/checksum.h @@ -15,10 +15,15 @@ uint16_t csum_fold(uint32_t sum); uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init); uint16_t csum_ip4_header(uint16_t tot_len, uint8_t protocol, struct in_addr saddr, struct in_addr daddr); +uint32_t proto_ipv4_header_psum(uint16_t tot_len, uint8_t protocol, + struct in_addr saddr, struct in_addr daddr); void csum_udp4(struct udphdr *udp4hr, struct in_addr saddr, struct in_addr daddr, const void *payload, size_t len); void csum_icmp4(struct icmphdr *ih, const void *payload, size_t len); +uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol, + const struct in6_addr *saddr, + const struct in6_addr *daddr); void csum_udp6(struct udphdr *udp6hr, const struct in6_addr *saddr, const struct in6_addr *daddr, const void *payload, size_t len); diff --git a/tcp.c b/tcp.c index 7fb9dba95a6a..53da8b84780f 100644 --- a/tcp.c +++ b/tcp.c @@ -937,41 +937,33 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s)
/** * tcp_update_check_tcp4() - Update TCP checksum from stored one - * @buf: L2 packet buffer with final IPv4 header + * @iph: IPv4 header + * @th: TCP header followed by TCP payload */ -static void tcp_update_check_tcp4(struct tcp4_l2_buf_t *buf) +static void tcp_update_check_tcp4(struct iphdr *iph, struct tcphdr *th) { - uint16_t tlen = ntohs(buf->iph.tot_len) - 20; - uint32_t sum = htons(IPPROTO_TCP); + uint16_t tlen = ntohs(iph->tot_len) - sizeof(struct iphdr); + uint32_t sum = proto_ipv4_header_psum(tlen, IPPROTO_TCP, + (struct in_addr){ .s_addr = iph->saddr }, + (struct in_addr){ .s_addr = iph->daddr });
- sum += (buf->iph.saddr >> 16) & 0xffff; - sum += buf->iph.saddr & 0xffff; - sum += (buf->iph.daddr >> 16) & 0xffff; - sum += buf->iph.daddr & 0xffff; - sum += htons(ntohs(buf->iph.tot_len) - 20); - - buf->th.check = 0; - buf->th.check = csum(&buf->th, tlen, sum); + th->check = 0; + th->check = csum(th, tlen, sum); }
/** * tcp_update_check_tcp6() - Calculate TCP checksum for IPv6 - * @buf: L2 packet buffer with final IPv6 header + * @ip6h: IPv6 header + * @th: TCP header followed by TCP payload */ -static void tcp_update_check_tcp6(struct tcp6_l2_buf_t *buf) +static void tcp_update_check_tcp6(struct ipv6hdr *ip6h, struct tcphdr *th) { - int len = ntohs(buf->ip6h.payload_len) + sizeof(struct ipv6hdr); - - buf->ip6h.hop_limit = IPPROTO_TCP; - buf->ip6h.version = 0; - buf->ip6h.nexthdr = 0; + uint16_t payload_len = ntohs(ip6h->payload_len); + uint32_t sum = proto_ipv6_header_psum(payload_len, IPPROTO_TCP, + &ip6h->saddr, &ip6h->daddr);
- buf->th.check = 0; - buf->th.check = csum(&buf->ip6h, len, 0); - - buf->ip6h.hop_limit = 255; - buf->ip6h.version = 6; - buf->ip6h.nexthdr = IPPROTO_TCP; + th->check = 0; + th->check = csum(th, payload_len, sum); }
/** @@ -1383,7 +1375,7 @@ do { \
SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq);
- tcp_update_check_tcp4(b); + tcp_update_check_tcp4(&b->iph, &b->th);
tlen = tap_iov_len(c, &b->taph, ip_len); } else { @@ -1402,7 +1394,11 @@ do { \
SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq);
- tcp_update_check_tcp6(b); + tcp_update_check_tcp6(&b->ip6h, &b->th); + + b->ip6h.hop_limit = 255; + b->ip6h.version = 6; + b->ip6h.nexthdr = IPPROTO_TCP;
b->ip6h.flow_lbl[0] = (conn->sock >> 16) & 0xf; b->ip6h.flow_lbl[1] = (conn->sock >> 8) & 0xff; diff --git a/udp.c b/udp.c index fb8373beba40..2fd67925f368 100644 --- a/udp.c +++ b/udp.c @@ -625,6 +625,7 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport, { struct udp6_l2_buf_t *b = &udp6_l2_buf[n]; const struct in6_addr *src, *dst; + uint16_t payload_len; in_port_t src_port; size_t ip_len;
@@ -634,7 +635,8 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
ip_len = udp6_l2_mh_sock[n].msg_len + sizeof(b->ip6h) + sizeof(b->uh);
- b->ip6h.payload_len = htons(udp6_l2_mh_sock[n].msg_len + sizeof(b->uh)); + payload_len = udp6_l2_mh_sock[n].msg_len + sizeof(b->uh); + b->ip6h.payload_len = htons(payload_len);
if (IN6_IS_ADDR_LINKLOCAL(src)) { dst = &c->ip6.addr_ll_seen; @@ -670,17 +672,17 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport, } b->ip6h.daddr = *dst; b->ip6h.saddr = *src; + b->ip6h.version = 6; + b->ip6h.nexthdr = IPPROTO_UDP; + b->ip6h.hop_limit = 255;
b->uh.source = b->s_in6.sin6_port; b->uh.dest = htons(dstport); b->uh.len = b->ip6h.payload_len; - - b->ip6h.hop_limit = IPPROTO_UDP; - b->ip6h.version = b->ip6h.nexthdr = b->uh.check = 0; - b->uh.check = csum(&b->ip6h, ip_len, 0); - b->ip6h.version = 6; - b->ip6h.nexthdr = IPPROTO_UDP; - b->ip6h.hop_limit = 255; + b->uh.check = 0; + b->uh.check = csum(&b->uh, payload_len, + proto_ipv6_header_psum(payload_len, IPPROTO_UDP, + src, dst));
return tap_iov_len(c, &b->taph, ip_len); }
-- 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
Use ethhdr rather than tap_hdr.
Signed-off-by: Laurent Vivier
Replace the macro SET_TCP_HEADER_COMMON_V4_V6() by a new function
tcp_fill_header().
Move IPv4 and IPv6 code from tcp_l2_buf_fill_headers() to
tcp_fill_ipv4_header() and tcp_fill_ipv6_header()
Signed-off-by: Laurent Vivier
On Sun, Mar 03, 2024 at 02:51:14PM +0100, Laurent Vivier wrote:
Replace the macro SET_TCP_HEADER_COMMON_V4_V6() by a new function tcp_fill_header().
Move IPv4 and IPv6 code from tcp_l2_buf_fill_headers() to tcp_fill_ipv4_header() and tcp_fill_ipv6_header()
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
---
Notes: v5: - update function comments - rename tcp_fill_ipv[46]_header() to tcp_fill_headers [46]() - add TCP header pointer in the parameters of the functions
v4: - group all the ip6g initialisations together and remove flow_lbl preset to 0 - add ASSERT(a4)
v3: - add to sub-series part 1
v2: - extract header filling functions from "tcp: extract buffer management from tcp_send_flag()" - rename them tcp_fill_flag_header()/tcp_fill_ipv4_header(), tcp_fill_ipv6_header() - use upside-down Christmas tree arguments order - replace (void *) by (struct tcphdr *)
tcp.c | 156 +++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 106 insertions(+), 50 deletions(-)
diff --git a/tcp.c b/tcp.c index df27e2d4e33d..6cf19ba97fa8 100644 --- a/tcp.c +++ b/tcp.c @@ -1327,6 +1327,108 @@ void tcp_defer_handler(struct ctx *c) tcp_l2_data_buf_flush(c); }
+/** + * tcp_fill_header() - Fill the TCP header fields for a given TCP segment. + * + * @th: Pointer to the TCP header structure + * @conn: Pointer to the TCP connection structure + * @seq: Sequence number + */ +static void tcp_fill_header(struct tcphdr *th, + const struct tcp_tap_conn *conn, uint32_t seq) +{ + th->source = htons(conn->fport); + th->dest = htons(conn->eport); + th->seq = htonl(seq); + th->ack_seq = htonl(conn->seq_ack_to_tap); + if (conn->events & ESTABLISHED) { + th->window = htons(conn->wnd_to_tap); + } else { + unsigned wnd = conn->wnd_to_tap << conn->ws_to_tap; + + th->window = htons(MIN(wnd, USHRT_MAX)); + } +} + +/** + * tcp_fill_headers4() - Fill 802.3, IPv4, TCP headers in pre-cooked buffers + * @c: Execution context + * @conn: Connection pointer + * @iph: Pointer to IPv4 header + * @th: Pointer to TCP header + * @plen: Payload length (including TCP header options) + * @check: Checksum, if already known + * @seq: Sequence number for this segment + * + * Return: The total length of the IPv4 packet, host order + */ +static size_t tcp_fill_headers4(const struct ctx *c, + const struct tcp_tap_conn *conn, + struct iphdr *iph, struct tcphdr *th, + size_t plen, const uint16_t *check, + uint32_t seq) +{ + size_t ip_len = plen + sizeof(struct iphdr) + sizeof(struct tcphdr); + const struct in_addr *a4 = inany_v4(&conn->faddr); + + ASSERT(a4); + + iph->tot_len = htons(ip_len); + iph->saddr = a4->s_addr; + iph->daddr = c->ip4.addr_seen.s_addr; + + iph->check = check ? *check : + csum_ip4_header(iph->tot_len, IPPROTO_TCP, + *a4, c->ip4.addr_seen); + + tcp_fill_header(th, conn, seq); + + tcp_update_check_tcp4(iph, th); + + return ip_len; +} + +/** + * tcp_fill_headers6() - Fill 802.3, IPv6, TCP headers in pre-cooked buffers + * @c: Execution context + * @conn: Connection pointer + * @ip6h: Pointer to IPv6 header + * @th: Pointer to TCP header + * @plen: Payload length (including TCP header options) + * @check: Checksum, if already known + * @seq: Sequence number for this segment + * + * Return: The total length of the IPv6 packet, host order + */ +static size_t tcp_fill_headers6(const struct ctx *c, + const struct tcp_tap_conn *conn, + struct ipv6hdr *ip6h, struct tcphdr *th, + size_t plen, uint32_t seq) +{ + size_t ip_len = plen + sizeof(struct ipv6hdr) + sizeof(struct tcphdr); + + ip6h->payload_len = htons(plen + sizeof(struct tcphdr)); + ip6h->saddr = conn->faddr.a6; + if (IN6_IS_ADDR_LINKLOCAL(&ip6h->saddr)) + ip6h->daddr = c->ip6.addr_ll_seen; + else + ip6h->daddr = c->ip6.addr_seen; + + ip6h->hop_limit = 255; + ip6h->version = 6; + ip6h->nexthdr = IPPROTO_TCP; + + ip6h->flow_lbl[0] = (conn->sock >> 16) & 0xf; + ip6h->flow_lbl[1] = (conn->sock >> 8) & 0xff; + ip6h->flow_lbl[2] = (conn->sock >> 0) & 0xff; + + tcp_fill_header(th, conn, seq); + + tcp_update_check_tcp6(ip6h, th); + + return ip_len; +} + /** * tcp_l2_buf_fill_headers() - Fill 802.3, IP, TCP headers in pre-cooked buffers * @c: Execution context @@ -1346,67 +1448,21 @@ static size_t tcp_l2_buf_fill_headers(const struct ctx *c, const struct in_addr *a4 = inany_v4(&conn->faddr); size_t ip_len, tlen;
-#define SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq) \ -do { \ - b->th.source = htons(conn->fport); \ - b->th.dest = htons(conn->eport); \ - b->th.seq = htonl(seq); \ - b->th.ack_seq = htonl(conn->seq_ack_to_tap); \ - if (conn->events & ESTABLISHED) { \ - b->th.window = htons(conn->wnd_to_tap); \ - } else { \ - unsigned wnd = conn->wnd_to_tap << conn->ws_to_tap; \ - \ - b->th.window = htons(MIN(wnd, USHRT_MAX)); \ - } \ -} while (0) - if (a4) { struct tcp4_l2_buf_t *b = (struct tcp4_l2_buf_t *)p;
- ip_len = plen + sizeof(struct iphdr) + sizeof(struct tcphdr); - b->iph.tot_len = htons(ip_len); - b->iph.saddr = a4->s_addr; - b->iph.daddr = c->ip4.addr_seen.s_addr; - - b->iph.check = check ? *check : - csum_ip4_header(b->iph.tot_len, IPPROTO_TCP, - *a4, c->ip4.addr_seen); - - SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq); - - tcp_update_check_tcp4(&b->iph, &b->th); + ip_len = tcp_fill_headers4(c, conn, &b->iph, &b->th, plen, + check, seq);
tlen = tap_iov_len(c, &b->taph, ip_len); } else { struct tcp6_l2_buf_t *b = (struct tcp6_l2_buf_t *)p;
- ip_len = plen + sizeof(struct ipv6hdr) + sizeof(struct tcphdr); - - b->ip6h.payload_len = htons(plen + sizeof(struct tcphdr)); - b->ip6h.saddr = conn->faddr.a6; - if (IN6_IS_ADDR_LINKLOCAL(&b->ip6h.saddr)) - b->ip6h.daddr = c->ip6.addr_ll_seen; - else - b->ip6h.daddr = c->ip6.addr_seen; - - memset(b->ip6h.flow_lbl, 0, 3); - - SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq); - - tcp_update_check_tcp6(&b->ip6h, &b->th); - - b->ip6h.hop_limit = 255; - b->ip6h.version = 6; - b->ip6h.nexthdr = IPPROTO_TCP; - - b->ip6h.flow_lbl[0] = (conn->sock >> 16) & 0xf; - b->ip6h.flow_lbl[1] = (conn->sock >> 8) & 0xff; - b->ip6h.flow_lbl[2] = (conn->sock >> 0) & 0xff; + ip_len = tcp_fill_headers6(c, conn, &b->ip6h, &b->th, plen, + seq);
tlen = tap_iov_len(c, &b->taph, ip_len); } -#undef SET_TCP_HEADER_COMMON_V4_V6
return tlen; }
-- 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
On Sun, 3 Mar 2024 14:51:05 +0100
Laurent Vivier
v5: - add a patch to cleanup before change: udp: little cleanup in udp_update_hdrX() to prepare future changes - see detailed v5 history log in each patch
I'm about to apply this, but cppcheck (2.10) tells me (with make cppcheck): checksum.c:195:33: style: inconclusive: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. [funcArgNamesDifferent] void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len) ^ checksum.h:23:33: note: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. void csum_icmp4(struct icmphdr *ih, const void *payload, size_t len); ^ checksum.c:195:33: note: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len) ^ pcap.c:145:47: style: inconclusive: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. [funcArgNamesDifferent] void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ pcap.h:12:47: note: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. void pcap_iov(const struct iovec *iov, size_t n); ^ pcap.c:145:47: note: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ tcp.c:947:45: error: Expression 'tlen,IPPROTO_TCP,(struct in_addr){.s_addr=iph->saddr},(struct in_addr){.s_addr=iph->daddr}' depends on order of evaluation of side effects [unknownEvaluationOrder] (struct in_addr){ .s_addr = iph->saddr }, ^ checksum.c:506:0: style: The function 'csum_iov' is never used. [unusedFunction] uint16_t csum_iov(struct iovec *iov, size_t n, uint32_t init) ^ pcap.c:145:0: style: The function 'pcap_iov' is never used. [unusedFunction] void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ iov.c:150:0: information: Unmatched suppression: unusedFunction [unmatchedSuppression] size_t iov_size(const struct iovec *iov, size_t iov_cnt) ^ they are almost all trivial and I plan to fix them up on merge, but I still have to look into the initialisation in tcp_update_check_tcp4() (tcp.c:947:45). -- Stefano
On Tue, Mar 05, 2024 at 11:12:18PM +0100, Stefano Brivio wrote:
On Sun, 3 Mar 2024 14:51:05 +0100 Laurent Vivier
wrote: v5: - add a patch to cleanup before change: udp: little cleanup in udp_update_hdrX() to prepare future changes - see detailed v5 history log in each patch
I'm about to apply this, but cppcheck (2.10) tells me (with make cppcheck):
checksum.c:195:33: style: inconclusive: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. [funcArgNamesDifferent] void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len) ^ checksum.h:23:33: note: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. void csum_icmp4(struct icmphdr *ih, const void *payload, size_t len); ^ checksum.c:195:33: note: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len) ^ pcap.c:145:47: style: inconclusive: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. [funcArgNamesDifferent] void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ pcap.h:12:47: note: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. void pcap_iov(const struct iovec *iov, size_t n); ^ pcap.c:145:47: note: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ tcp.c:947:45: error: Expression 'tlen,IPPROTO_TCP,(struct in_addr){.s_addr=iph->saddr},(struct in_addr){.s_addr=iph->daddr}' depends on order of evaluation of side effects [unknownEvaluationOrder] (struct in_addr){ .s_addr = iph->saddr }, ^ checksum.c:506:0: style: The function 'csum_iov' is never used. [unusedFunction] uint16_t csum_iov(struct iovec *iov, size_t n, uint32_t init) ^ pcap.c:145:0: style: The function 'pcap_iov' is never used. [unusedFunction] void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ iov.c:150:0: information: Unmatched suppression: unusedFunction [unmatchedSuppression] size_t iov_size(const struct iovec *iov, size_t iov_cnt) ^
they are almost all trivial and I plan to fix them up on merge, but I still have to look into the initialisation in tcp_update_check_tcp4() (tcp.c:947:45).
I'm pretty sure this is a false positive from cppcheck: I think it's misinterpreting this (initializer in struct literal) as an assignment expression. Since there are then two apparent assignments to the same apparent target, we have that warning. The obvious workaround to me is to introduce a couple of struct in_addr typed temporaries, instead of using struct literals. I think that's kind of more readable anyway. -- 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
participants (3)
-
David Gibson
-
Laurent Vivier
-
Stefano Brivio