diff mbox series

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

Message ID 20230822-skb_ext-simplify-v1-1-9dd047340ab5@weissschuh.net (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: generalize calculation of skb extensions length | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1332 this patch: 1332
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1355 this patch: 1355
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 30 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Thomas Weißschuh Aug. 22, 2023, 6:51 a.m. UTC
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(-)


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

Best regards,

Comments

Jakub Kicinski Aug. 23, 2023, 1:46 a.m. UTC | #1
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.
Thomas Weißschuh Aug. 23, 2023, 8:14 a.m. UTC | #2
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
Jakub Kicinski Aug. 23, 2023, 2:53 p.m. UTC | #3
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.
Sabrina Dubroca Aug. 24, 2023, 1:02 p.m. UTC | #4
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.
diff mbox series

Patch

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)