[PATCH net] geneve: initialize needed_headroom

[PATCH net] geneve: initialize needed_headroom

[Paolo Abeni]

Currently the needed_headroom field for the geneve device is left
to the default value.

This patch set it to space required for basic geneve encapsulation,
so that we can avoid the skb head re-allocation on xmit.

This give a 6% speedup for unsegment traffic on geneve tunnel.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 drivers/net/geneve.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index c2b79f5..bb3e6ed 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1155,7 +1155,7 @@ static int geneve_configure(struct net *net, struct net_device *dev,
 	struct geneve_net *gn = net_generic(net, geneve_net_id);
 	struct geneve_dev *t, *geneve = netdev_priv(dev);
 	bool tun_collect_md, tun_on_same_port;
-	int err;
+	int err, encap_len;
 
 	if (!remote)
 		return -EINVAL;
@@ -1187,6 +1187,14 @@ static int geneve_configure(struct net *net, struct net_device *dev,
 	if (t)
 		return -EBUSY;
 
+	/* make enough headroom for basic scenario */
+	encap_len = GENEVE_BASE_HLEN + ETH_HLEN;
+	if (remote->sa.sa_family == AF_INET)
+		encap_len += sizeof(struct iphdr);
+	else
+		encap_len += sizeof(struct ipv6hdr);
+	dev->needed_headroom = encap_len;
+
 	if (metadata) {
 		if (tun_on_same_port)
 			return -EPERM;
-- 
1.8.3.1

Re: [PATCH net] geneve: initialize needed_headroom

[John W. Linville]

On Wed, Dec 23, 2015 at 04:21:48PM +0100, Paolo Abeni wrote:
> Currently the needed_headroom field for the geneve device is left
> to the default value.
> 
> This patch set it to space required for basic geneve encapsulation,
> so that we can avoid the skb head re-allocation on xmit.
> 
> This give a 6% speedup for unsegment traffic on geneve tunnel.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Acked-by: John W. Linville <linville@tuxdriver.com>

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH net] geneve: initialize needed_headroom

[Hannes Frederic Sowa]

On 23.12.2015 16:21, Paolo Abeni wrote:
> Currently the needed_headroom field for the geneve device is left
> to the default value.
> 
> This patch set it to space required for basic geneve encapsulation,
> so that we can avoid the skb head re-allocation on xmit.
> 
> This give a 6% speedup for unsegment traffic on geneve tunnel.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

(Sorry, just saw this.)

> ---
>  drivers/net/geneve.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index c2b79f5..bb3e6ed 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1155,7 +1155,7 @@ static int geneve_configure(struct net *net, struct net_device *dev,
>  	struct geneve_net *gn = net_generic(net, geneve_net_id);
>  	struct geneve_dev *t, *geneve = netdev_priv(dev);
>  	bool tun_collect_md, tun_on_same_port;
> -	int err;
> +	int err, encap_len;
>  
>  	if (!remote)
>  		return -EINVAL;
> @@ -1187,6 +1187,14 @@ static int geneve_configure(struct net *net, struct net_device *dev,
>  	if (t)
>  		return -EBUSY;
>  
> +	/* make enough headroom for basic scenario */
> +	encap_len = GENEVE_BASE_HLEN + ETH_HLEN;
> +	if (remote->sa.sa_family == AF_INET)
> +		encap_len += sizeof(struct iphdr);
> +	else
> +		encap_len += sizeof(struct ipv6hdr);
> +	dev->needed_headroom = encap_len;
> +

We are missing one time ETH_HLEN or hard_header_len of the lower_dev (if
available) in here AFAIK.

Thanks,
Hannes

Re: [PATCH net] geneve: initialize needed_headroom

[Paolo Abeni]

On Wed, 2015-12-23 at 16:35 +0100, Hannes Frederic Sowa wrote:
> On 23.12.2015 16:21, Paolo Abeni wrote:
> > @@ -1187,6 +1187,14 @@ static int geneve_configure(struct net *net, struct net_device *dev,
> >  	if (t)
> >  		return -EBUSY;
> >  
> > +	/* make enough headroom for basic scenario */
> > +	encap_len = GENEVE_BASE_HLEN + ETH_HLEN;
> > +	if (remote->sa.sa_family == AF_INET)
> > +		encap_len += sizeof(struct iphdr);
> > +	else
> > +		encap_len += sizeof(struct ipv6hdr);
> > +	dev->needed_headroom = encap_len;
> > +
> 
> We are missing one time ETH_HLEN or hard_header_len of the lower_dev (if
> available) in here AFAIK.

You are right. Most probably performance numbers where good even with
the missing ETH_HLEN due to some rounding. I'll send a v2.

Paolo