LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net-next] sctp: deduplicate identical skb_checksum_ops
@ 2019-05-29 15:39 Matteo Croce
  2019-05-29 16:59 ` Neil Horman
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Matteo Croce @ 2019-05-29 15:39 UTC (permalink / raw)
  To: linux-sctp, netdev
  Cc: Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, linux-kernel

The same skb_checksum_ops struct is defined twice in two different places,
leading to code duplication. Declare it as a global variable into a common
header instead of allocating it on the stack on each function call.
bloat-o-meter reports a slight code shrink.

add/remove: 1/1 grow/shrink: 0/10 up/down: 128/-1282 (-1154)
Function                                     old     new   delta
sctp_csum_ops                                  -     128    +128
crc32c_csum_ops                               16       -     -16
sctp_rcv                                    6616    6583     -33
sctp_packet_pack                            4542    4504     -38
nf_conntrack_sctp_packet                    4980    4926     -54
execute_masked_set_action                   6453    6389     -64
tcf_csum_sctp                                575     428    -147
sctp_gso_segment                            1292    1126    -166
sctp_csum_check                              579     412    -167
sctp_snat_handler                            957     772    -185
sctp_dnat_handler                           1321    1132    -189
l4proto_manip_pkt                           2536    2313    -223
Total: Before=359297613, After=359296459, chg -0.00%

Reviewed-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 include/net/sctp/checksum.h | 12 +++++++-----
 net/sctp/offload.c          |  7 +------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
index 314699333bec..5a9bb09f32b6 100644
--- a/include/net/sctp/checksum.h
+++ b/include/net/sctp/checksum.h
@@ -43,19 +43,21 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
 						   (__force __u32)csum2, len);
 }
 
+static const struct skb_checksum_ops sctp_csum_ops = {
+	.update  = sctp_csum_update,
+	.combine = sctp_csum_combine,
+};
+
 static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
 					unsigned int offset)
 {
 	struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
-	const struct skb_checksum_ops ops = {
-		.update  = sctp_csum_update,
-		.combine = sctp_csum_combine,
-	};
 	__le32 old = sh->checksum;
 	__wsum new;
 
 	sh->checksum = 0;
-	new = ~__skb_checksum(skb, offset, skb->len - offset, ~(__wsum)0, &ops);
+	new = ~__skb_checksum(skb, offset, skb->len - offset, ~(__wsum)0,
+			      &sctp_csum_ops);
 	sh->checksum = old;
 
 	return cpu_to_le32((__force __u32)new);
diff --git a/net/sctp/offload.c b/net/sctp/offload.c
index edfcf16e704c..dac46dfadab5 100644
--- a/net/sctp/offload.c
+++ b/net/sctp/offload.c
@@ -103,11 +103,6 @@ static const struct net_offload sctp6_offload = {
 	},
 };
 
-static const struct skb_checksum_ops crc32c_csum_ops = {
-	.update  = sctp_csum_update,
-	.combine = sctp_csum_combine,
-};
-
 int __init sctp_offload_init(void)
 {
 	int ret;
@@ -120,7 +115,7 @@ int __init sctp_offload_init(void)
 	if (ret)
 		goto ipv4;
 
-	crc32c_csum_stub = &crc32c_csum_ops;
+	crc32c_csum_stub = &sctp_csum_ops;
 	return ret;
 
 ipv4:
-- 
2.21.0


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

* Re: [PATCH net-next] sctp: deduplicate identical skb_checksum_ops
  2019-05-29 15:39 [PATCH net-next] sctp: deduplicate identical skb_checksum_ops Matteo Croce
@ 2019-05-29 16:59 ` Neil Horman
  2019-05-29 17:10 ` Marcelo Ricardo Leitner
  2019-05-30 21:36 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Neil Horman @ 2019-05-29 16:59 UTC (permalink / raw)
  To: Matteo Croce
  Cc: linux-sctp, netdev, Vlad Yasevich, Marcelo Ricardo Leitner, linux-kernel

On Wed, May 29, 2019 at 05:39:41PM +0200, Matteo Croce wrote:
> The same skb_checksum_ops struct is defined twice in two different places,
> leading to code duplication. Declare it as a global variable into a common
> header instead of allocating it on the stack on each function call.
> bloat-o-meter reports a slight code shrink.
> 
> add/remove: 1/1 grow/shrink: 0/10 up/down: 128/-1282 (-1154)
> Function                                     old     new   delta
> sctp_csum_ops                                  -     128    +128
> crc32c_csum_ops                               16       -     -16
> sctp_rcv                                    6616    6583     -33
> sctp_packet_pack                            4542    4504     -38
> nf_conntrack_sctp_packet                    4980    4926     -54
> execute_masked_set_action                   6453    6389     -64
> tcf_csum_sctp                                575     428    -147
> sctp_gso_segment                            1292    1126    -166
> sctp_csum_check                              579     412    -167
> sctp_snat_handler                            957     772    -185
> sctp_dnat_handler                           1321    1132    -189
> l4proto_manip_pkt                           2536    2313    -223
> Total: Before=359297613, After=359296459, chg -0.00%
> 
> Reviewed-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
> ---
>  include/net/sctp/checksum.h | 12 +++++++-----
>  net/sctp/offload.c          |  7 +------
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> index 314699333bec..5a9bb09f32b6 100644
> --- a/include/net/sctp/checksum.h
> +++ b/include/net/sctp/checksum.h
> @@ -43,19 +43,21 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
>  						   (__force __u32)csum2, len);
>  }
>  
> +static const struct skb_checksum_ops sctp_csum_ops = {
> +	.update  = sctp_csum_update,
> +	.combine = sctp_csum_combine,
> +};
> +
>  static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
>  					unsigned int offset)
>  {
>  	struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
> -	const struct skb_checksum_ops ops = {
> -		.update  = sctp_csum_update,
> -		.combine = sctp_csum_combine,
> -	};
>  	__le32 old = sh->checksum;
>  	__wsum new;
>  
>  	sh->checksum = 0;
> -	new = ~__skb_checksum(skb, offset, skb->len - offset, ~(__wsum)0, &ops);
> +	new = ~__skb_checksum(skb, offset, skb->len - offset, ~(__wsum)0,
> +			      &sctp_csum_ops);
>  	sh->checksum = old;
>  
>  	return cpu_to_le32((__force __u32)new);
> diff --git a/net/sctp/offload.c b/net/sctp/offload.c
> index edfcf16e704c..dac46dfadab5 100644
> --- a/net/sctp/offload.c
> +++ b/net/sctp/offload.c
> @@ -103,11 +103,6 @@ static const struct net_offload sctp6_offload = {
>  	},
>  };
>  
> -static const struct skb_checksum_ops crc32c_csum_ops = {
> -	.update  = sctp_csum_update,
> -	.combine = sctp_csum_combine,
> -};
> -
>  int __init sctp_offload_init(void)
>  {
>  	int ret;
> @@ -120,7 +115,7 @@ int __init sctp_offload_init(void)
>  	if (ret)
>  		goto ipv4;
>  
> -	crc32c_csum_stub = &crc32c_csum_ops;
> +	crc32c_csum_stub = &sctp_csum_ops;
>  	return ret;
>  
>  ipv4:
> -- 
> 2.21.0
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>


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

* Re: [PATCH net-next] sctp: deduplicate identical skb_checksum_ops
  2019-05-29 15:39 [PATCH net-next] sctp: deduplicate identical skb_checksum_ops Matteo Croce
  2019-05-29 16:59 ` Neil Horman
@ 2019-05-29 17:10 ` Marcelo Ricardo Leitner
  2019-05-30 21:36 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-05-29 17:10 UTC (permalink / raw)
  To: Matteo Croce; +Cc: linux-sctp, netdev, Vlad Yasevich, Neil Horman, linux-kernel

On Wed, May 29, 2019 at 05:39:41PM +0200, Matteo Croce wrote:
> The same skb_checksum_ops struct is defined twice in two different places,
> leading to code duplication. Declare it as a global variable into a common
> header instead of allocating it on the stack on each function call.
> bloat-o-meter reports a slight code shrink.
> 
> add/remove: 1/1 grow/shrink: 0/10 up/down: 128/-1282 (-1154)
> Function                                     old     new   delta
> sctp_csum_ops                                  -     128    +128
> crc32c_csum_ops                               16       -     -16
> sctp_rcv                                    6616    6583     -33
> sctp_packet_pack                            4542    4504     -38
> nf_conntrack_sctp_packet                    4980    4926     -54
> execute_masked_set_action                   6453    6389     -64
> tcf_csum_sctp                                575     428    -147
> sctp_gso_segment                            1292    1126    -166
> sctp_csum_check                              579     412    -167
> sctp_snat_handler                            957     772    -185
> sctp_dnat_handler                           1321    1132    -189
> l4proto_manip_pkt                           2536    2313    -223
> Total: Before=359297613, After=359296459, chg -0.00%
> 
> Reviewed-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

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

* Re: [PATCH net-next] sctp: deduplicate identical skb_checksum_ops
  2019-05-29 15:39 [PATCH net-next] sctp: deduplicate identical skb_checksum_ops Matteo Croce
  2019-05-29 16:59 ` Neil Horman
  2019-05-29 17:10 ` Marcelo Ricardo Leitner
@ 2019-05-30 21:36 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2019-05-30 21:36 UTC (permalink / raw)
  To: mcroce
  Cc: linux-sctp, netdev, vyasevich, nhorman, marcelo.leitner, linux-kernel

From: Matteo Croce <mcroce@redhat.com>
Date: Wed, 29 May 2019 17:39:41 +0200

> The same skb_checksum_ops struct is defined twice in two different places,
> leading to code duplication. Declare it as a global variable into a common
> header instead of allocating it on the stack on each function call.
> bloat-o-meter reports a slight code shrink.
> 
> add/remove: 1/1 grow/shrink: 0/10 up/down: 128/-1282 (-1154)
> Function                                     old     new   delta
> sctp_csum_ops                                  -     128    +128
> crc32c_csum_ops                               16       -     -16
> sctp_rcv                                    6616    6583     -33
> sctp_packet_pack                            4542    4504     -38
> nf_conntrack_sctp_packet                    4980    4926     -54
> execute_masked_set_action                   6453    6389     -64
> tcf_csum_sctp                                575     428    -147
> sctp_gso_segment                            1292    1126    -166
> sctp_csum_check                              579     412    -167
> sctp_snat_handler                            957     772    -185
> sctp_dnat_handler                           1321    1132    -189
> l4proto_manip_pkt                           2536    2313    -223
> Total: Before=359297613, After=359296459, chg -0.00%
> 
> Reviewed-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>

Applied.

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

end of thread, other threads:[~2019-05-30 21:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29 15:39 [PATCH net-next] sctp: deduplicate identical skb_checksum_ops Matteo Croce
2019-05-29 16:59 ` Neil Horman
2019-05-29 17:10 ` Marcelo Ricardo Leitner
2019-05-30 21:36 ` David Miller

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 NNTP newsgroup(s).