[PATCH net-next] net: generalize calculation of skb extensions length

[PATCH net-next] net: generalize calculation of skb extensions length

[Thomas Weißschuh]

Remove the necessity to modify skb_ext_total_length() when new extension
types are added.
Also reduces the line count a bit.

With optimizations enabled the function is folded down to a constant
value as before.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 net/core/skbuff.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index faa6c86da2a5..45707059082f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4785,23 +4785,13 @@ static const u8 skb_ext_type_len[] = {
 
 static __always_inline unsigned int skb_ext_total_length(void)
 {
-	return SKB_EXT_CHUNKSIZEOF(struct skb_ext) +
-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
-		skb_ext_type_len[SKB_EXT_BRIDGE_NF] +
-#endif
-#ifdef CONFIG_XFRM
-		skb_ext_type_len[SKB_EXT_SEC_PATH] +
-#endif
-#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
-		skb_ext_type_len[TC_SKB_EXT] +
-#endif
-#if IS_ENABLED(CONFIG_MPTCP)
-		skb_ext_type_len[SKB_EXT_MPTCP] +
-#endif
-#if IS_ENABLED(CONFIG_MCTP_FLOWS)
-		skb_ext_type_len[SKB_EXT_MCTP] +
-#endif
-		0;
+	unsigned int l = SKB_EXT_CHUNKSIZEOF(struct skb_ext);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(skb_ext_type_len); i++)
+		l += skb_ext_type_len[i];
+
+	return l;
 }
 
 static void skb_extensions_init(void)

---
base-commit: 90308679c297ffcbb317c715ef434e9fb3c881dc
change-id: 20230822-skb_ext-simplify-3d93ceb95f62

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

Re: [PATCH net-next] net: generalize calculation of skb extensions length

[Jakub Kicinski]

On Tue, 22 Aug 2023 08:51:57 +0200 Thomas Weißschuh wrote:
> Remove the necessity to modify skb_ext_total_length() when new extension
> types are added.
> Also reduces the line count a bit.
> 
> With optimizations enabled the function is folded down to a constant
> value as before.

Could you include more info about the compiler versions you tried
and maybe some objdump? We'll have to take your word for it getting
optimized out, would be great if we had more proof in the commit msg.
-- 
pw-bot: cr

Re: [PATCH net-next] net: generalize calculation of skb extensions length

[linux]

Hi Jakub,


Aug 23, 2023 03:46:48 Jakub Kicinski <kuba@kernel.org>:

> On Tue, 22 Aug 2023 08:51:57 +0200 Thomas Weißschuh wrote:
>> Remove the necessity to modify skb_ext_total_length() when new extension
>> types are added.
>> Also reduces the line count a bit.
>>
>> With optimizations enabled the function is folded down to a constant
>> value as before.
>
> Could you include more info about the compiler versions you tried
> and maybe some objdump? We'll have to take your word for it getting
> optimized out, would be great if we had more proof in the commit msg.
> --
> pw-bot: cr

Thanks for the feedback.
I'll send a v2 with more background soon.

On the other hand this function is only ever
executed once, so even if it is slightly inefficient
it shouldn't matter.

Thomas

Re: [PATCH net-next] net: generalize calculation of skb extensions length

[Jakub Kicinski]

On Wed, 23 Aug 2023 10:14:48 +0200 (GMT+02:00) linux@weissschuh.net
wrote:
> > Could you include more info about the compiler versions you tried
> > and maybe some objdump? We'll have to take your word for it getting
> > optimized out, would be great if we had more proof in the commit msg.
> > --
> > pw-bot: cr  
> 
> Thanks for the feedback.
> I'll send a v2 with more background soon.
> 
> On the other hand this function is only ever
> executed once, so even if it is slightly inefficient
> it shouldn't matter.

Oh you're right, somehow I thought it was for every alloc.
You can mention it's only run at init in the commit msg if 
that's easier.

Re: [PATCH net-next] net: generalize calculation of skb extensions length

[Sabrina Dubroca]

2023-08-23, 07:53:18 -0700, Jakub Kicinski wrote:
> On Wed, 23 Aug 2023 10:14:48 +0200 (GMT+02:00) linux@weissschuh.net
> wrote:
> > > Could you include more info about the compiler versions you tried
> > > and maybe some objdump? We'll have to take your word for it getting
> > > optimized out, would be great if we had more proof in the commit msg.
> > > --
> > > pw-bot: cr  
> > 
> > Thanks for the feedback.
> > I'll send a v2 with more background soon.
> > 
> > On the other hand this function is only ever
> > executed once, so even if it is slightly inefficient
> > it shouldn't matter.
> 
> Oh you're right, somehow I thought it was for every alloc.
> You can mention it's only run at init in the commit msg if 
> that's easier.

We could also add __init annotations to skb_ext_total_length and
skb_extensions_init to make that clearer.

-- 
Sabrina