[PATCH 2/2] ipv4: Initialize on-stack cork more efficiently.

[PATCH 2/2] ipv4: Initialize on-stack cork more efficiently.

[David Miller]

ip_setup_cork() explicitly initializes every member of
inet_cork except flags, addr, and opt.  So we can simply
set those three members to zero instead of using a
memset() via an empty struct assignment.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv4/ip_output.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index eb0647a..5f5fe4f 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1408,7 +1408,7 @@ struct sk_buff *ip_make_skb(struct sock *sk,
 			    struct ipcm_cookie *ipc, struct rtable **rtp,
 			    unsigned int flags)
 {
-	struct inet_cork cork = {};
+	struct inet_cork cork;
 	struct sk_buff_head queue;
 	int err;
 
@@ -1417,6 +1417,9 @@ struct sk_buff *ip_make_skb(struct sock *sk,
 
 	__skb_queue_head_init(&queue);
 
+	cork.flags = 0;
+	cork.addr = 0;
+	cork.opt = 0;
 	err = ip_setup_cork(sk, &cork, ipc, rtp);
 	if (err)
 		return ERR_PTR(err);
-- 
1.7.5.1

Re: [PATCH 2/2] ipv4: Initialize on-stack cork more efficiently.

[Eric Dumazet]

Le vendredi 06 mai 2011 à 15:26 -0700, David Miller a écrit :
> ip_setup_cork() explicitly initializes every member of
> inet_cork except flags, addr, and opt.  So we can simply
> set those three members to zero instead of using a
> memset() via an empty struct assignment.
> 
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  net/ipv4/ip_output.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index eb0647a..5f5fe4f 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1408,7 +1408,7 @@ struct sk_buff *ip_make_skb(struct sock *sk,
>  			    struct ipcm_cookie *ipc, struct rtable **rtp,
>  			    unsigned int flags)
>  {
> -	struct inet_cork cork = {};
> +	struct inet_cork cork;
>  	struct sk_buff_head queue;
>  	int err;
>  
> @@ -1417,6 +1417,9 @@ struct sk_buff *ip_make_skb(struct sock *sk,
>  
>  	__skb_queue_head_init(&queue);
>  
> +	cork.flags = 0;
> +	cork.addr = 0;
> +	cork.opt = 0;
>  	err = ip_setup_cork(sk, &cork, ipc, rtp);
>  	if (err)
>  		return ERR_PTR(err);

Very nice, thanks for finishing this stuff :)

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Re: [PATCH 2/2] ipv4: Initialize on-stack cork more efficiently.

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 07 May 2011 00:35:17 +0200

> Very nice, thanks for finishing this stuff :)
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

No problem, thanks for reviewing :-)

Re: [PATCH 2/2] ipv4: Initialize on-stack cork more efficiently.

[Joe Perches]

On Fri, 2011-05-06 at 15:26 -0700, David Miller wrote:
> ip_setup_cork() explicitly initializes every member of
> inet_cork except flags, addr, and opt.  So we can simply
> set those three members to zero instead of using a
> memset() via an empty struct assignment.
> Signed-off-by: David S. Miller <davem@davemloft.net>
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
[]
> @@ -1417,6 +1417,9 @@ struct sk_buff *ip_make_skb(struct sock *sk,
>  
>  	__skb_queue_head_init(&queue);
>  
> +	cork.flags = 0;
> +	cork.addr = 0;
> +	cork.opt = 0;

	cork.opt = NULL;

>  	err = ip_setup_cork(sk, &cork, ipc, rtp);
>  	if (err)
>  		return ERR_PTR(err);

Perhaps it'd be better to move the initialization
of all cork fields to ip_setup_cork.

Re: [PATCH 2/2] ipv4: Initialize on-stack cork more efficiently.

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Fri, 06 May 2011 15:44:54 -0700

> Perhaps it'd be better to move the initialization
> of all cork fields to ip_setup_cork.

We can't, the flags and other fields can be setup way up high in the
call chain when the inet->cork instance is used.

I'll fix the NULL assignment, thanks.

Re: [PATCH 2/2] ipv4: Initialize on-stack cork more efficiently.

[Eric Dumazet]

Le samedi 07 mai 2011 à 00:35 +0200, Eric Dumazet a écrit :
> Le vendredi 06 mai 2011 à 15:26 -0700, David Miller a écrit :
> > ip_setup_cork() explicitly initializes every member of
> > inet_cork except flags, addr, and opt.  So we can simply
> > set those three members to zero instead of using a
> > memset() via an empty struct assignment.
> > 
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > ---
> >  net/ipv4/ip_output.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> > 
> > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> > index eb0647a..5f5fe4f 100644
> > --- a/net/ipv4/ip_output.c
> > +++ b/net/ipv4/ip_output.c
> > @@ -1408,7 +1408,7 @@ struct sk_buff *ip_make_skb(struct sock *sk,
> >  			    struct ipcm_cookie *ipc, struct rtable **rtp,
> >  			    unsigned int flags)
> >  {
> > -	struct inet_cork cork = {};
> > +	struct inet_cork cork;
> >  	struct sk_buff_head queue;
> >  	int err;
> >  
> > @@ -1417,6 +1417,9 @@ struct sk_buff *ip_make_skb(struct sock *sk,
> >  
> >  	__skb_queue_head_init(&queue);
> >  
> > +	cork.flags = 0;
> > +	cork.addr = 0;
> > +	cork.opt = 0;
> >  	err = ip_setup_cork(sk, &cork, ipc, rtp);
> >  	if (err)
> >  		return ERR_PTR(err);
> 
> Very nice, thanks for finishing this stuff :)
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> 

By the way, when I spotted this "struct inet_cork cork = {};" to be
optimized, my idea was to add yet another case of fastpath to UDP send :

For small datagrams (most UDP uses : RTP, DNS...),
perform the user->kernel copy before route lookup, so that we can
perform an RCU route lookup. This would tremendously speedup UDP, since
the refcount handling is our last hot spot (not counting qdisc if
present)

   PerfTop:   16142 irqs/sec  kernel:97.5%  exact:  0.0% [1000Hz cycles],  (all, 16 CPUs)
-----------------------------------------------------------------------------------------------------------

             samples  pcnt function                 DSO
             _______ _____ ________________________ ______________________

            16735.00 24.2% __ip_route_output_key    vmlinux               
             9706.00 14.1% dst_release              vmlinux               
             6754.00  9.8% __ip_make_skb            vmlinux               
             5737.00  8.3% udp_send_skb             vmlinux               
             5384.00  7.8% ip_finish_output         vmlinux               
             3578.00  5.2% udp_sendmsg              vmlinux               
             1435.00  2.1% copy_user_generic_string vmlinux               
             1358.00  2.0% ia32_sysenter_target     vmlinux               
             1095.00  1.6% __ip_append_data         vmlinux               
              832.00  1.2% kfree                    vmlinux               
              794.00  1.2% __memset                 vmlinux               
              677.00  1.0% fget_light               vmlinux               
              641.00  0.9% sock_wfree               vmlinux               
              637.00  0.9% dev_queue_xmit           vmlinux               

Re: [PATCH 2/2] ipv4: Initialize on-stack cork more efficiently.

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 07 May 2011 07:11:44 +0200

> By the way, when I spotted this "struct inet_cork cork = {};" to be
> optimized, my idea was to add yet another case of fastpath to UDP send :
> 
> For small datagrams (most UDP uses : RTP, DNS...),
> perform the user->kernel copy before route lookup, so that we can
> perform an RCU route lookup. This would tremendously speedup UDP, since
> the refcount handling is our last hot spot (not counting qdisc if
> present)

Interesting idea.

This reminds me, remember about the input noref route lookup stuff
going away with the routing cache removal?  It turns out that when we
do my "routes embedded in fib nexthop" for input, the noref stuff can
be used. :)