From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH 7/7] udp: Single buffer for IPv4, IPv6 headers and metadata
Date: Wed, 1 May 2024 12:54:33 +1000 [thread overview]
Message-ID: <ZjGu6eT_BxTJvoDq@zatzit> (raw)
In-Reply-To: <20240430221634.5f7dbf39@elisabeth>
[-- Attachment #1: Type: text/plain, Size: 4833 bytes --]
On Tue, Apr 30, 2024 at 10:16:34PM +0200, Stefano Brivio wrote:
> On Tue, 30 Apr 2024 20:05:41 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > Currently we have separate arrays for IPv4 and IPv6 which contain the
> > headers for guest-bound packets, and also the originating socket address.
> > We can combine these into a single array of "metadata" structures with
> > space for both pre-cooked IPv4 and IPv6 headers, as well as shared space
> > for the tap specific header and socket address (using sockaddr_inany).
>
> This is neat, definitely, I just wonder: will we need to essentially
> revert this if we implement a flow-based mechanism where frames can be
> sent to different guests/containers?
No, I don't think so. There's still a separate IPv4 and IPv6 header,
and socket address for each frame. It's just that those are stored in
a parallel array, rather than alongside the paylaod buffers.
If we are handling multiple guests, we can probably additionally merge
the IPv4 and IPv6 header buffers, since I don't think there will be
anything we can pre-fill any more.
> On the other hand, chances are it would help instead because we would
> have tables of metadata instead of those being buried into frames.
>
> > Because we're using IOVs to separately address the pieces of each frame,
> > these structures don't need to be packed to keep the headers contiguous
> > so we can more naturally arrange for the alignment we want.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > udp.c | 131 ++++++++++++++++++++++++----------------------------------
> > 1 file changed, 55 insertions(+), 76 deletions(-)
> >
> > diff --git a/udp.c b/udp.c
> > index 64e7dcef..a9cc6ed2 100644
> > --- a/udp.c
> > +++ b/udp.c
> > @@ -184,44 +184,27 @@ udp_payload_buf[UDP_MAX_FRAMES];
> > /* Ethernet header for IPv4 frames */
> > static struct ethhdr udp4_eth_hdr;
> >
> > -/**
> > - * udp4_l2_buf_t - Pre-cooked IPv4 packet buffers for tap connections
> > - * @s_in: Source socket address, filled in by recvmmsg()
> > - * @taph: Tap backend specific header
> > - * @iph: Pre-filled IP header (except for tot_len and saddr)
> > - */
> > -static struct udp4_l2_buf_t {
> > - struct sockaddr_in s_in;
> > -
> > - struct tap_hdr taph;
> > - struct iphdr iph;
> > -} __attribute__ ((packed, aligned(__alignof__(unsigned int))))
> > -udp4_l2_buf[UDP_MAX_FRAMES];
> > -
> > /* Ethernet header for IPv6 frames */
> > static struct ethhdr udp6_eth_hdr;
> >
> > /**
> > - * udp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections
> > - * @s_in6: Source socket address, filled in by recvmmsg()
> > + * udp_meta - Pre-cooked headers and metadata for UDP packets
>
> Pre-existing, but... kernel-doc format wants *struct* x - Description.
Adjusted. I also changed to udp_meta_t and udp_meta[] for consistency
with the payload buffers.
> > + * @ip6h: Pre-filled IPv6 header (except for payload_len and addresses)
> > + * @ip4h: Pre-filled IPv4 header (except for tot_len and saddr)
> > * @taph: Tap backend specific header
> > - * @ip6h: Pre-filled IP header (except for payload_len and addresses)
> > + * @s_in: Source socket address, filled in by recvmmsg()
> > */
> > -struct udp6_l2_buf_t {
> > - struct sockaddr_in6 s_in6;
> > -#ifdef __AVX2__
> > - /* Align ip6h to 32-byte boundary. */
> > - uint8_t pad[64 - (sizeof(struct sockaddr_in6) + sizeof(struct tap_hdr))];
> > -#endif
> > -
> > - struct tap_hdr taph;
> > +static struct udp_meta {
> > struct ipv6hdr ip6h;
> > + struct iphdr ip4h;
> > + struct tap_hdr taph;
> > +
> > + union sockaddr_inany s_in;
> > +}
> > #ifdef __AVX2__
> > -} __attribute__ ((packed, aligned(32)))
> > -#else
> > -} __attribute__ ((packed, aligned(__alignof__(unsigned int))))
> > +__attribute__ ((aligned(32)))
> > #endif
> > -udp6_l2_buf[UDP_MAX_FRAMES];
> > +udp_meta_buf[UDP_MAX_FRAMES];
> >
> > /**
> > * enum udp_iov_idx - Indices for the buffers making up a single UDP frame
> > @@ -315,49 +298,46 @@ void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s)
> > static void udp_iov_init_one(const struct ctx *c, size_t i)
> > {
> > struct udp_payload *payload = &udp_payload_buf[i];
> > + struct udp_meta *meta = &udp_meta_buf[i];
> > struct iovec *siov = &udp_l2_iov_sock[i];
> >
> > + *meta = (struct udp_meta) {
> > + .ip4h = L2_BUF_IP4_INIT(IPPROTO_UDP),
> > + .ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP),
> > + };
> > +
> > +
>
> One extra newline should be enough.
Fixed.
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
prev parent reply other threads:[~2024-05-01 2:54 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-30 10:05 [PATCH 0/7] Rework UDP buffers David Gibson
2024-04-30 10:05 ` [PATCH 1/7] test: Allow sftp via vsock-ssh in tests David Gibson
2024-04-30 10:05 ` [PATCH 2/7] udp: Split tap-bound UDP packets into multiple buffers using io vector David Gibson
2024-04-30 20:15 ` Stefano Brivio
2024-05-01 2:35 ` David Gibson
2024-04-30 10:05 ` [PATCH 3/7] udp: Combine initialisation of IPv4 and IPv6 iovs David Gibson
2024-04-30 10:05 ` [PATCH 4/7] udp: Explicitly set checksum in guest-bound UDP headers David Gibson
2024-04-30 10:05 ` [PATCH 5/7] udp: Share payload buffers between IPv4 and IPv6 David Gibson
2024-04-30 20:16 ` Stefano Brivio
2024-05-01 2:46 ` David Gibson
2024-04-30 10:05 ` [PATCH 6/7] udp: Use the same buffer for the L2 header for all frames David Gibson
2024-04-30 10:05 ` [PATCH 7/7] udp: Single buffer for IPv4, IPv6 headers and metadata David Gibson
2024-04-30 20:16 ` Stefano Brivio
2024-05-01 2:54 ` David Gibson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZjGu6eT_BxTJvoDq@zatzit \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).