public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
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 --]

      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).