[PATCH] gso: Handle Trans-Ether-Bridging protocol in skb_network_protocol()

[PATCH] gso: Handle Trans-Ether-Bridging protocol in skb_network_protocol()

[Pravin B Shelar]

Rather than having logic to calculate inner protocol in every
tunnel gso handler move it to gso code. This simplifies code.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Cong Wang <amwang@redhat.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/core/dev.c |   11 +++++++++++
 net/ipv4/gre.c |    8 +-------
 net/ipv4/udp.c |    4 +---
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 40b1fad..fc1e289 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2213,6 +2213,17 @@ __be16 skb_network_protocol(struct sk_buff *skb)
 	__be16 type = skb->protocol;
 	int vlan_depth = ETH_HLEN;
 
+	/* Tunnel gso handlers can set protocol to ethernet. */
+	if (type == htons(ETH_P_TEB)) {
+		struct ethhdr *eth;
+
+		if (unlikely(!pskb_may_pull(skb, sizeof(struct ethhdr))))
+			return 0;
+
+		eth = (struct ethhdr *)skb_mac_header(skb);
+		type = eth->h_proto;
+	}
+
 	while (type == htons(ETH_P_8021Q) || type == htons(ETH_P_8021AD)) {
 		struct vlan_hdr *vh;
 
diff --git a/net/ipv4/gre.c b/net/ipv4/gre.c
index cc22363..b2e805a 100644
--- a/net/ipv4/gre.c
+++ b/net/ipv4/gre.c
@@ -150,13 +150,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
 		csum = false;
 
 	/* setup inner skb. */
-	if (greh->protocol == htons(ETH_P_TEB)) {
-		struct ethhdr *eth = (struct ethhdr *)skb_inner_mac_header(skb);
-		skb->protocol = eth->h_proto;
-	} else {
-		skb->protocol = greh->protocol;
-	}
-
+	skb->protocol = greh->protocol;
 	skb->encapsulation = 0;
 
 	if (unlikely(!pskb_may_pull(skb, ghl)))
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0ae038a..0bf5d39 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2311,7 +2311,6 @@ static struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
 	struct sk_buff *segs = ERR_PTR(-EINVAL);
 	int mac_len = skb->mac_len;
 	int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
-	struct ethhdr *inner_eth = (struct ethhdr *)skb_inner_mac_header(skb);
 	__be16 protocol = skb->protocol;
 	netdev_features_t enc_features;
 	int outer_hlen;
@@ -2324,8 +2323,7 @@ static struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
 	skb_reset_mac_header(skb);
 	skb_set_network_header(skb, skb_inner_network_offset(skb));
 	skb->mac_len = skb_inner_network_offset(skb);
-	inner_eth = (struct ethhdr *)skb_mac_header(skb);
-	skb->protocol = inner_eth->h_proto;
+	skb->protocol = htons(ETH_P_TEB);
 
 	/* segment inner packet. */
 	enc_features = skb->dev->hw_enc_features & netif_skb_features(skb);
-- 
1.7.1

Re: [PATCH] gso: Handle Trans-Ether-Bridging protocol in skb_network_protocol()

[PJ Waskiewicz]

On 5/7/2013 11:41 PM, Pravin B Shelar wrote:
> Rather than having logic to calculate inner protocol in every
> tunnel gso handler move it to gso code. This simplifies code.

Moving code that isn't used on a regular basis to hotpath for most 
networking to simplify code isn't always a good thing.

Have you run tests on non-tunneled streams using GSO to make sure there 
isn't an adverse impact to performance due to the new, untaken branch?

-PJ

Re: [PATCH] gso: Handle Trans-Ether-Bridging protocol in skb_network_protocol()

[Stephen Hemminger]

On Wed, 08 May 2013 00:42:49 -0700
PJ Waskiewicz <peter.p.waskiewicz.jr@linux.intel.com> wrote:

> On 5/7/2013 11:41 PM, Pravin B Shelar wrote:
> > Rather than having logic to calculate inner protocol in every
> > tunnel gso handler move it to gso code. This simplifies code.
> 
> Moving code that isn't used on a regular basis to hotpath for most 
> networking to simplify code isn't always a good thing.
> 
> Have you run tests on non-tunneled streams using GSO to make sure there 
> isn't an adverse impact to performance due to the new, untaken branch?
> 
> -PJ
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Maybe use unlikely here?
It looks like more and more of this tunnel unwrapping needs to be handled here
(vxlan? ipip?) probably want more general switch statement and per-protocol functions.

Re: [PATCH] gso: Handle Trans-Ether-Bridging protocol in skb_network_protocol()

[David Miller]

From: Pravin B Shelar <pshelar@nicira.com>
Date: Tue,  7 May 2013 23:41:07 -0700

> Rather than having logic to calculate inner protocol in every
> tunnel gso handler move it to gso code. This simplifies code.
> 
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Cong Wang <amwang@redhat.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>

Applied.

Re: [PATCH] gso: Handle Trans-Ether-Bridging protocol in skb_network_protocol()

[Pravin Shelar]

On Wed, May 8, 2013 at 1:15 PM, David Miller <davem@davemloft.net> wrote:
> From: Pravin B Shelar <pshelar@nicira.com>
> Date: Tue,  7 May 2013 23:41:07 -0700
>
>> Rather than having logic to calculate inner protocol in every
>> tunnel gso handler move it to gso code. This simplifies code.
>>
>> Cc: Eric Dumazet <eric.dumazet@gmail.com>
>> Cc: Cong Wang <amwang@redhat.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
>
> Applied.

Hi David,
Can you queue this patch for -stable ?

Thanks,
Pravin.

Re: [PATCH] gso: Handle Trans-Ether-Bridging protocol in skb_network_protocol()

[David Miller]

From: Pravin Shelar <pshelar@nicira.com>
Date: Wed, 8 May 2013 19:32:40 -0700

> On Wed, May 8, 2013 at 1:15 PM, David Miller <davem@davemloft.net> wrote:
>> From: Pravin B Shelar <pshelar@nicira.com>
>> Date: Tue,  7 May 2013 23:41:07 -0700
>>
>>> Rather than having logic to calculate inner protocol in every
>>> tunnel gso handler move it to gso code. This simplifies code.
>>>
>>> Cc: Eric Dumazet <eric.dumazet@gmail.com>
>>> Cc: Cong Wang <amwang@redhat.com>
>>> Cc: David S. Miller <davem@davemloft.net>
>>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
>>
>> Applied.
> 
> Hi David,
> Can you queue this patch for -stable ?

Done.

Re: [PATCH] gso: Handle Trans-Ether-Bridging protocol in skb_network_protocol()

[Eric Dumazet]

On Wed, 2013-05-08 at 19:32 -0700, Pravin Shelar wrote:

> Hi David,
> Can you queue this patch for -stable ?

BTW you should always include in the changelog :

Bug added in commit 121212121212
("foo: some fancy patch")

This eases David and all stable/distro guys work.

Thanks