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
Reviewed-by: David Gibson
--- 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