* [PATCH] udp_flow: Save 8 bytes in struct udp_flow on 64-bit architectures
@ 2025-04-08 5:56 Stefano Brivio
2025-04-08 6:39 ` David Gibson
0 siblings, 1 reply; 2+ messages in thread
From: Stefano Brivio @ 2025-04-08 5:56 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson, Jon Maloy
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@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);
--
@@ -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
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] udp_flow: Save 8 bytes in struct udp_flow on 64-bit architectures
2025-04-08 5:56 [PATCH] udp_flow: Save 8 bytes in struct udp_flow on 64-bit architectures Stefano Brivio
@ 2025-04-08 6:39 ` David Gibson
0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2025-04-08 6:39 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, Jon Maloy
[-- Attachment #1: Type: text/plain, Size: 4468 bytes --]
On Tue, Apr 08, 2025 at 07:56:24AM +0200, Stefano Brivio wrote:
> 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@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> 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);
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-04-08 6:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-08 5:56 [PATCH] udp_flow: Save 8 bytes in struct udp_flow on 64-bit architectures Stefano Brivio
2025-04-08 6:39 ` David Gibson
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).