public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] xfrm: reduce struct sec_path size
@ 2026-02-05 16:44 Paolo Abeni
  2026-02-06  0:11 ` Florian Westphal
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2026-02-05 16:44 UTC (permalink / raw)
  To: netdev
  Cc: Steffen Klassert, Herbert Xu, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Simon Horman

The mentioned struct has an hole and uses unnecessary wide type to
store a MAC length.

It's also embedded into the skb_extensions, and the latter, due
to recent CAN changes, may exceeds the 192 bytes mark (3 cachelines
on x86_64 arch) on some reasonable configuration.

Reordering and the sec_path fields and shrinking xfrm_offload.orig_mac_len
to 16 bits, we can save 8 bytes and keep skb_extensions size under
control.

Before:

struct sec_path {
	int                        len;
	int                        olen;
	int                        verified_cnt;

	/* XXX 4 bytes hole, try to pack */$
	struct xfrm_state *        xvec[6];
	struct xfrm_offload ovec[1];

	/* size: 88, cachelines: 2, members: 5 */
	/* sum members: 84, holes: 1, sum holes: 4 */
	/* last cacheline: 24 bytes */
};

After:

struct sec_path {
	int                        len;
	int                        olen;
	struct xfrm_offload        ovec[1];
	int                        verified_cnt;
	struct xfrm_state *        xvec[6];

	/* size: 80, cachelines: 2, members: 5 */
	/* last cacheline: 16 bytes */
};

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
only build tested, any feedback appreciated.
---
 include/net/xfrm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 0a14daaa5dd4..41122cb83901 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1156,7 +1156,7 @@ struct xfrm_offload {
 #define CRYPTO_INVALID_PROTOCOL			128
 
 	/* Used to keep whole l2 header for transport mode GRO */
-	__u32			orig_mac_len;
+	__u16			orig_mac_len;
 
 	__u8			proto;
 	__u8			inner_ipproto;
@@ -1165,10 +1165,10 @@ struct xfrm_offload {
 struct sec_path {
 	int			len;
 	int			olen;
+	struct xfrm_offload	ovec[XFRM_MAX_OFFLOAD_DEPTH];
 	int			verified_cnt;
 
 	struct xfrm_state	*xvec[XFRM_MAX_DEPTH];
-	struct xfrm_offload	ovec[XFRM_MAX_OFFLOAD_DEPTH];
 };
 
 struct sec_path *secpath_set(struct sk_buff *skb);
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] xfrm: reduce struct sec_path size
  2026-02-05 16:44 [RFC PATCH] xfrm: reduce struct sec_path size Paolo Abeni
@ 2026-02-06  0:11 ` Florian Westphal
  2026-02-06  9:37   ` Paolo Abeni
  0 siblings, 1 reply; 6+ messages in thread
From: Florian Westphal @ 2026-02-06  0:11 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: netdev, Steffen Klassert, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Simon Horman

Paolo Abeni <pabeni@redhat.com> wrote:
> The mentioned struct has an hole and uses unnecessary wide type to
> store a MAC length.

Good catch.

> struct sec_path {
> 	int                        len;
> 	int                        olen;
> 	struct xfrm_offload        ovec[1];
> 	int                        verified_cnt;

Why not s/int/u8/ while at it?

> 	struct xfrm_state *        xvec[6];

len is xvec, olen is the offload length which can only be 0 or 1 at this time.

verified_cnt also refers to xvec[].

>  	/* Used to keep whole l2 header for transport mode GRO */
> -	__u32			orig_mac_len;
> +	__u16			orig_mac_len;

Matches mac_len in sk_buff; I think this change is fine.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] xfrm: reduce struct sec_path size
  2026-02-06  0:11 ` Florian Westphal
@ 2026-02-06  9:37   ` Paolo Abeni
  2026-02-06  9:48     ` Steffen Klassert
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2026-02-06  9:37 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netdev, Steffen Klassert, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Simon Horman

On 2/6/26 1:11 AM, Florian Westphal wrote:
> Paolo Abeni <pabeni@redhat.com> wrote:
>> The mentioned struct has an hole and uses unnecessary wide type to
>> store a MAC length.
> 
> Good catch.
> 
>> struct sec_path {
>> 	int                        len;
>> 	int                        olen;
>> 	struct xfrm_offload        ovec[1];
>> 	int                        verified_cnt;
> 
> Why not s/int/u8/ while at it?
> 
>> 	struct xfrm_state *        xvec[6];
> 
> len is xvec, olen is the offload length which can only be 0 or 1 at this time.

I did not look closely at the other fields semantic, other than
`orig_mac_len`, because just the latter looked an low hanging fruit to me.

Thanks for the insight, that will save an additional 8 bytes! Which in
turn is IMHO very nice because it will keep skb_extension size inside
the 3 cacheline boundary even with all extensions enabled.

I'm trying to understand why XFRM_MAX_OFFLOAD_DEPTH is 6 exactly, but
it's not obvious to me skimming over the code.

/P


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] xfrm: reduce struct sec_path size
  2026-02-06  9:37   ` Paolo Abeni
@ 2026-02-06  9:48     ` Steffen Klassert
  2026-02-06 14:36       ` Paolo Abeni
  0 siblings, 1 reply; 6+ messages in thread
From: Steffen Klassert @ 2026-02-06  9:48 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Florian Westphal, netdev, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Simon Horman

On Fri, Feb 06, 2026 at 10:37:51AM +0100, Paolo Abeni wrote:
> 
> I'm trying to understand why XFRM_MAX_OFFLOAD_DEPTH is 6 exactly, but
> it's not obvious to me skimming over the code.

That is beause we allow 6 transformations per packet as a maximum.
But for offloading we currently support just one transformation,
and we probably won't support more in future. This transfomation
bundle stuff if from the old RFC 2401. This was obsoleted by RFC
4301 which does not have the concept of transformation bundles.

I'm currently looking how to move our inplementation from RFC 2401
to RFC 4301. This should remove a lot of complexity that came with
the old RFC 2401.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] xfrm: reduce struct sec_path size
  2026-02-06  9:48     ` Steffen Klassert
@ 2026-02-06 14:36       ` Paolo Abeni
  2026-02-06 14:42         ` Steffen Klassert
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2026-02-06 14:36 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Florian Westphal, netdev, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Simon Horman

On 2/6/26 10:48 AM, Steffen Klassert wrote:
> On Fri, Feb 06, 2026 at 10:37:51AM +0100, Paolo Abeni wrote:
>> I'm trying to understand why XFRM_MAX_OFFLOAD_DEPTH is 6 exactly, but
>> it's not obvious to me skimming over the code.
> 
> That is beause we allow 6 transformations per packet as a maximum.
> But for offloading we currently support just one transformation,
> and we probably won't support more in future. This transfomation
> bundle stuff if from the old RFC 2401. This was obsoleted by RFC
> 4301 which does not have the concept of transformation bundles.
> 
> I'm currently looking how to move our inplementation from RFC 2401
> to RFC 4301. This should remove a lot of complexity that came with
> the old RFC 2401.

Thanks for the insights! Looking forward to complexity reduction :)

BTW are you ok if I send a formal, non RFC patch directly targeting
net-next? The goal would be to keep skb_extensions under control for
6.20, countering a very recent size increase for CAN's sake.

Cheers,

Paolo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] xfrm: reduce struct sec_path size
  2026-02-06 14:36       ` Paolo Abeni
@ 2026-02-06 14:42         ` Steffen Klassert
  0 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2026-02-06 14:42 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Florian Westphal, netdev, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Simon Horman

On Fri, Feb 06, 2026 at 03:36:53PM +0100, Paolo Abeni wrote:
> On 2/6/26 10:48 AM, Steffen Klassert wrote:
> > On Fri, Feb 06, 2026 at 10:37:51AM +0100, Paolo Abeni wrote:
> >> I'm trying to understand why XFRM_MAX_OFFLOAD_DEPTH is 6 exactly, but
> >> it's not obvious to me skimming over the code.
> > 
> > That is beause we allow 6 transformations per packet as a maximum.
> > But for offloading we currently support just one transformation,
> > and we probably won't support more in future. This transfomation
> > bundle stuff if from the old RFC 2401. This was obsoleted by RFC
> > 4301 which does not have the concept of transformation bundles.
> > 
> > I'm currently looking how to move our inplementation from RFC 2401
> > to RFC 4301. This should remove a lot of complexity that came with
> > the old RFC 2401.
> 
> Thanks for the insights! Looking forward to complexity reduction :)
> 
> BTW are you ok if I send a formal, non RFC patch directly targeting
> net-next? The goal would be to keep skb_extensions under control for
> 6.20, countering a very recent size increase for CAN's sake.

Sure, go ahead.

Steffen

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-06 14:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-05 16:44 [RFC PATCH] xfrm: reduce struct sec_path size Paolo Abeni
2026-02-06  0:11 ` Florian Westphal
2026-02-06  9:37   ` Paolo Abeni
2026-02-06  9:48     ` Steffen Klassert
2026-02-06 14:36       ` Paolo Abeni
2026-02-06 14:42         ` Steffen Klassert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox