[PATCH v7 00/31] Introduce discontiguous frames management

older
[PATCH] style: Add parentheses to...

Laurent Vivier

23 Jun 2025 23 Jun '25

1:06 p.m.

This series introduces iov_tail to convey frame information between functions. This is only an API change, for the moment the memory pool is only able to store contiguous buffer, so, except for vhost-user in a special case, we only play with iovec array with only one entry. v7: - Add a patch to fix comment style of 'Return:' - Fix ignore_arp()/accept_arp() - Fix coverity error - Fix several comments v6: - Replaced iov_slice() with the clearer iov_tail_clone() for creating iovec subsets. - Standardized local header variable names (to *_storage suffix). - Renamed functions for better semantics (e.g., ignore_arp to accept_arp, packet_data to packet_get). - Corrected OPTLEN_MAX definition in TCP. - Addressed minor logic issues (e.g., DHCPv6 FQDN flags, NDP null check). - Updated ipv6_l4hdr() return type to boolean. - Improved comments and documentation across several modules. v5: - store in the pool iovec array with several entries v4: Prepare to introduce iovec array in the pool: - passe iov_tail rather than pool to ndp,icmp, dhcp, dhcpv6 and arp - remove unused pool macros - add memory regions in the pool structure, this will allow us to use the buf pointer to store the iovec array for vhost-user v3: Address comments from David Laurent Vivier (31): style: Fix 'Return' comment style arp: Don't mix incoming and outgoing buffers iov: Introduce iov_tail_clone() and iov_tail_drop(). iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() tap: Use iov_tail with tap_add_packet() packet: Use iov_tail with packet_add() packet: Add packet_data() arp: Convert to iov_tail ndp: Convert to iov_tail icmp: Convert to iov_tail udp: Convert to iov_tail tcp: Convert tcp_tap_handler() to use iov_tail tcp: Convert tcp_data_from_tap() to use iov_tail dhcpv6: move offset initialization out of dhcpv6_opt() dhcpv6: Extract sending of NotOnLink status dhcpv6: Convert to iov_tail dhcpv6: Use iov_tail in dhcpv6_opt() dhcp: Convert to iov_tail ip: Use iov_tail in ipv6_l4hdr() tap: Convert tap4_handler() to iov_tail tap: Convert tap6_handler() to iov_tail packet: rename packet_data() to packet_get() arp: use iov_tail rather than pool dhcp: use iov_tail rather than pool dhcpv6: use iov_tail rather than pool icmp: use iov_tail rather than pool ndp: use iov_tail rather than pool packet: remove PACKET_POOL() and PACKET_POOL_P() packet: remove unused parameter from PACKET_POOL_DECL() packet: add memory regions information into pool packet: use buf to store iovec array arp.c | 86 ++++++++++++------- arp.h | 2 +- checksum.c | 4 +- conf.c | 12 +-- dhcp.c | 48 ++++++----- dhcp.h | 2 +- dhcpv6.c | 223 +++++++++++++++++++++++++++++++------------------- dhcpv6.h | 2 +- flow.c | 4 +- flow_table.h | 2 +- icmp.c | 43 ++++++---- icmp.h | 2 +- inany.c | 4 +- iov.c | 118 ++++++++++++++++++++++---- iov.h | 58 +++++++++---- ip.c | 32 ++++---- ip.h | 3 +- lineread.c | 2 +- ndp.c | 16 +++- ndp.h | 4 +- packet.c | 227 ++++++++++++++++++++++++++++++++++++++------------- packet.h | 57 ++++++------- pcap.c | 1 + tap.c | 134 +++++++++++++++++------------- tap.h | 8 +- tcp.c | 67 ++++++++++----- tcp_buf.c | 2 +- tcp_splice.c | 4 +- tcp_vu.c | 4 +- udp.c | 37 ++++++--- udp_vu.c | 6 +- util.c | 6 +- util.h | 4 +- vhost_user.c | 72 ++++++++-------- virtio.c | 6 +- virtio.h | 24 ++++-- vu_common.c | 48 ++++------- 37 files changed, 870 insertions(+), 504 deletions(-) -- 2.49.0

Show replies by date

Laurent Vivier

...

In vhost-user mode, where `pool->buf` doesn't store packet data directly, this patch repurposes it to hold the `struct iovec` array describing a scattered packet. This enables pools to manage true scatter-gather descriptors, with iovecs pointing to guest memory.

`p->pkt[idx].iov_base` is now an index into this `pool->buf` iovec array, and `p->pkt[idx].iov_len` is the count of iovecs. `packet_add_do` uses `iov_tail_slice` to store these iovec descriptors from an input iov_tail into `pool->buf`. `packet_data_do` reconstructs the iov_tail for a packet by pointing to this stored array of iovecs.

Signed-off-by: Laurent Vivier --- packet.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 148 insertions(+), 16 deletions(-)

diff --git a/packet.c b/packet.c index fd3b6db57c30..8dbe00af12c6 100644 --- a/packet.c +++ b/packet.c @@ -84,6 +84,122 @@ bool pool_full(const struct pool *p) return p->count >= p->size; }

+/** + * packet_iov_max_cnt() - Return the maximum number of iovec entries we can + * store + * @p: Pointer to packet pool + * + * Return: the maximum number of iovec entries we can store in the memory of + * the pool buffer + */ +static size_t packet_iov_max_cnt(const struct pool *p) +{ + return p->buf_size / sizeof(struct iovec); +} + +/** + * packet_iov_idx() - For a given packet index, return the iovec index and + * the number of iovec entry of the packet + * @p: Pointer to packet pool + * @idx: Index of packet descriptor in pool + * @iov_cnt: Pointer to store the number of the iovec entry of the packet + * @func: For tracing: name of calling function + * @line: For tracing: caller line of function call + * + * Return: the iovec index for the given packet index, @iov_cnt is set + * to the number of the iovec entry of the packet + */ +static int packet_iov_idx(const struct pool *p, size_t idx, size_t *iov_cnt, + const char *func, int line) +{ + size_t iov_idx, max = packet_iov_max_cnt(p); + + iov_idx = (size_t)p->pkt[idx].iov_base; + *iov_cnt = p->pkt[idx].iov_len; + + ASSERT_WITH_MSG(iov_idx + *iov_cnt <= max, + "Corrupt iov entry: (%zu, %zu), max: %zu, %s:%i", + iov_idx, *iov_cnt, max, func, line); + + return iov_idx; +} + +/** + * packet_iov_next_idx() - Give the the next available iovec index

I'm taking the liberty to fix this comment while applying ("Get the next available iovec index"). -- Stefano

Stefano Brivio

8:45 p.m.

On Mon, 23 Jun 2025 13:06:04 +0200 Laurent Vivier wrote:

...

This series introduces iov_tail to convey frame information between functions.

This is only an API change, for the moment the memory pool is only able to store contiguous buffer, so, except for vhost-user in a special case, we only play with iovec array with only one entry.

v7: - Add a patch to fix comment style of 'Return:' - Fix ignore_arp()/accept_arp() - Fix coverity error - Fix several comments

I was about to apply this without 1/31 (I applied the v2 of it you sent outside of this series instead, which is actually up to date) and with the minor comment fix to 31/31... but the test perf/passt_vu_tcp fails rather consistently now (and I triple checked without this series): - "TCP throughput over IPv6: guest to host" with MTU 1500 and 9000 bytes now reports between 0 and 0.6 Gbps. The guest kernel prints a series of two messages with ~1-10 µs interval: [ 21.159827] TCP: out of memory -- consider tuning tcp_mem [ 21.159831] TCP: out of memory -- consider tuning tcp_mem - "TCP throughput over IPv4: guest to host" never reports 0 Gbps, but the throughput figure for large MTU (65520 bytes) is very low (5.4 Gbps in the last run). Here I'm getting four messages: [ 40.807818] TCP: out of memory -- consider tuning tcp_mem [ 40.807829] TCP: out of memory -- consider tuning tcp_mem [ 40.807829] TCP: out of memory -- consider tuning tcp_mem [ 40.807830] TCP: out of memory -- consider tuning tcp_mem - on the reverse direction, "TCP throughput over IPv4: host to guest" (but not with IPv6), the iperf3 client gets SIGSEGV, but not consistently, it happened once out of five times. To me it smells a bit like we're leaking virtqueue slots but I looked again at the whole series and I couldn't find anything obvious... at least not yet. UDP tests never fail and the throughput is the same as before. I'm stopping here for the moment, let me know if I should run more tests or anything. By the way, I'm running just those tests by enclosing the rest of the sequence in test/run in a dummy function, say: --- diff --git a/test/run b/test/run index f73c311..97b96a4 100755 --- a/test/run +++ b/test/run @@ -67,6 +67,7 @@ run() { perf_init [ ${CI} -eq 1 ] && video_start ci +dont() { setup build test build/all test build/cppcheck @@ -178,6 +179,8 @@ run() { test perf/pasta_udp test passt_in_ns/shutdown teardown passt_in_ns +} + VALGRIND=0 VHOST_USER=1 setup passt_in_ns --- ...you could also pass the setup steps plus test to ./run, but you would need to set VHOST_USER=1 as well. -- Stefano

Age (days ago)

Last active (days ago)

List overview

Download

33 comments

2 participants

participants (2)

Laurent Vivier
Stefano Brivio

[PATCH v7 00/31] Introduce discontiguous frames management

tags

participants (2)