[PATCH] net: tso: add support for IPv6

[PATCH] net: tso: add support for IPv6

[Emmanuel Grumbach]

Adding IPv6 for the TSO helper API is trivial:
* Don't play with the id (which doesn't exist in IPv6)
* Correctly update the payload_len (don't include the
  length of the IP header itself)

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/core/tso.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/core/tso.c b/net/core/tso.c
index 630b30b..ece4605 100644
--- a/net/core/tso.c
+++ b/net/core/tso.c
@@ -14,18 +14,25 @@ EXPORT_SYMBOL(tso_count_descs);
 void tso_build_hdr(struct sk_buff *skb, char *hdr, struct tso_t *tso,
 		   int size, bool is_last)
 {
-	struct iphdr *iph;
 	struct tcphdr *tcph;
 	int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
 	int mac_hdr_len = skb_network_offset(skb);
 
 	memcpy(hdr, skb->data, hdr_len);
-	iph = (struct iphdr *)(hdr + mac_hdr_len);
-	iph->id = htons(tso->ip_id);
-	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
+	if (skb->protocol == htons(ETH_P_IP)) {
+		struct iphdr *iph = (void *)(hdr + mac_hdr_len);
+
+		iph->id = htons(tso->ip_id);
+		iph->tot_len = htons(size + hdr_len - mac_hdr_len);
+		tso->ip_id++;
+	}
+	if (skb->protocol == htons(ETH_P_IPV6)) {
+		struct ipv6hdr *iph = (void *)(hdr + mac_hdr_len);
+
+		iph->payload_len = htons(size + tcp_hdrlen(skb));
+	}
 	tcph = (struct tcphdr *)(hdr + skb_transport_offset(skb));
 	put_unaligned_be32(tso->tcp_seq, &tcph->seq);
-	tso->ip_id++;
 
 	if (!is_last) {
 		/* Clear all special flags for not last packet */
-- 
2.1.4

[PATCH v2] net: tso: add support for IPv6

[Emmanuel Grumbach]

Adding IPv6 for the TSO helper API is trivial:
* Don't play with the id (which doesn't exist in IPv6)
* Correctly update the payload_len (don't include the
  length of the IP header itself)

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
v2: add else if

NOTE: instead of checking the skb->protocol, I can add a bool
	in tso_t. This might be better in terms of cache
	handling. Let me know if you prefer that option
---
 net/core/tso.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/core/tso.c b/net/core/tso.c
index 630b30b..bf71bdc5 100644
--- a/net/core/tso.c
+++ b/net/core/tso.c
@@ -14,18 +14,24 @@ EXPORT_SYMBOL(tso_count_descs);
 void tso_build_hdr(struct sk_buff *skb, char *hdr, struct tso_t *tso,
 		   int size, bool is_last)
 {
-	struct iphdr *iph;
 	struct tcphdr *tcph;
 	int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
 	int mac_hdr_len = skb_network_offset(skb);
 
 	memcpy(hdr, skb->data, hdr_len);
-	iph = (struct iphdr *)(hdr + mac_hdr_len);
-	iph->id = htons(tso->ip_id);
-	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
+	if (skb->protocol == htons(ETH_P_IP)) {
+		struct iphdr *iph = (void *)(hdr + mac_hdr_len);
+
+		iph->id = htons(tso->ip_id);
+		iph->tot_len = htons(size + hdr_len - mac_hdr_len);
+		tso->ip_id++;
+	} else if (skb->protocol == htons(ETH_P_IPV6)) {
+		struct ipv6hdr *iph = (void *)(hdr + mac_hdr_len);
+
+		iph->payload_len = htons(size + tcp_hdrlen(skb));
+	}
 	tcph = (struct tcphdr *)(hdr + skb_transport_offset(skb));
 	put_unaligned_be32(tso->tcp_seq, &tcph->seq);
-	tso->ip_id++;
 
 	if (!is_last) {
 		/* Clear all special flags for not last packet */
-- 
2.1.4

Re: [PATCH v2] net: tso: add support for IPv6

[Toshiaki Makita]

On 2015/10/26 5:02, Emmanuel Grumbach wrote:
> Adding IPv6 for the TSO helper API is trivial:
> * Don't play with the id (which doesn't exist in IPv6)
> * Correctly update the payload_len (don't include the
>   length of the IP header itself)
...
>  	memcpy(hdr, skb->data, hdr_len);
> -	iph = (struct iphdr *)(hdr + mac_hdr_len);
> -	iph->id = htons(tso->ip_id);
> -	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
> +	if (skb->protocol == htons(ETH_P_IP)) {

I guess this should be vlan_get_protocol(skb).

> +		struct iphdr *iph = (void *)(hdr + mac_hdr_len);
> +
> +		iph->id = htons(tso->ip_id);
> +		iph->tot_len = htons(size + hdr_len - mac_hdr_len);
> +		tso->ip_id++;
> +	} else if (skb->protocol == htons(ETH_P_IPV6)) {

Likewise.

Toshiaki Makita

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] net: tso: add support for IPv6

[Grumbach, Emmanuel]

On 10/26/2015 06:03 AM, Toshiaki Makita wrote:
> On 2015/10/26 5:02, Emmanuel Grumbach wrote:
>> Adding IPv6 for the TSO helper API is trivial:
>> * Don't play with the id (which doesn't exist in IPv6)
>> * Correctly update the payload_len (don't include the
>>   length of the IP header itself)
> ...
>>  	memcpy(hdr, skb->data, hdr_len);
>> -	iph = (struct iphdr *)(hdr + mac_hdr_len);
>> -	iph->id = htons(tso->ip_id);
>> -	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
>> +	if (skb->protocol == htons(ETH_P_IP)) {
> 
> I guess this should be vlan_get_protocol(skb).

I truly don't know. I guess we could have VLANs, but I'd need to check
how the packet would look like after it exits mac80211.
If we need that, I'll likely do this check once in tso_start() and add a
variable to struct tso_t.

> 
>> +		struct iphdr *iph = (void *)(hdr + mac_hdr_len);
>> +
>> +		iph->id = htons(tso->ip_id);
>> +		iph->tot_len = htons(size + hdr_len - mac_hdr_len);
>> +		tso->ip_id++;
>> +	} else if (skb->protocol == htons(ETH_P_IPV6)) {
> 
> Likewise.
> 
> Toshiaki Makita
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] net: tso: add support for IPv6

[Toshiaki Makita]

On 2015/10/26 16:47, Grumbach, Emmanuel wrote:
> On 10/26/2015 06:03 AM, Toshiaki Makita wrote:
>> On 2015/10/26 5:02, Emmanuel Grumbach wrote:
>>> Adding IPv6 for the TSO helper API is trivial:
>>> * Don't play with the id (which doesn't exist in IPv6)
>>> * Correctly update the payload_len (don't include the
>>>   length of the IP header itself)
>> ...
>>>  	memcpy(hdr, skb->data, hdr_len);
>>> -	iph = (struct iphdr *)(hdr + mac_hdr_len);
>>> -	iph->id = htons(tso->ip_id);
>>> -	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
>>> +	if (skb->protocol == htons(ETH_P_IP)) {
>>
>> I guess this should be vlan_get_protocol(skb).
> 
> I truly don't know. I guess we could have VLANs, but I'd need to check
> how the packet would look like after it exits mac80211.

I don't know much about mac80211.

What I see is that mvneta has TSO in vlan_features and it uses
tso_build_hdr(). When vlan device is used, we cannot access network
protocol by skb->protocol without HW vlan acceleration.
So it looks like this change corrupts TSO functionality on mvneta.

> If we need that, I'll likely do this check once in tso_start() and add a
> variable to struct tso_t.

I'm not sure if an additional variable is needed.
At least, skb_network_offset()/ip_hdr() should correctly handle (skip)
vlan headers.

Toshiaki Makita

Re: [PATCH v2] net: tso: add support for IPv6

[Toshiaki Makita]

On 2015/10/26 17:13, Toshiaki Makita wrote:
> On 2015/10/26 16:47, Grumbach, Emmanuel wrote:
>> On 10/26/2015 06:03 AM, Toshiaki Makita wrote:
>>> On 2015/10/26 5:02, Emmanuel Grumbach wrote:
>>>> Adding IPv6 for the TSO helper API is trivial:
>>>> * Don't play with the id (which doesn't exist in IPv6)
>>>> * Correctly update the payload_len (don't include the
>>>>   length of the IP header itself)
>>> ...
>>>>  	memcpy(hdr, skb->data, hdr_len);
>>>> -	iph = (struct iphdr *)(hdr + mac_hdr_len);
>>>> -	iph->id = htons(tso->ip_id);
>>>> -	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
>>>> +	if (skb->protocol == htons(ETH_P_IP)) {
>>>
>>> I guess this should be vlan_get_protocol(skb).
>>
>> I truly don't know. I guess we could have VLANs, but I'd need to check
>> how the packet would look like after it exits mac80211.
> 
> I don't know much about mac80211.
> 
> What I see is that mvneta has TSO in vlan_features and it uses
> tso_build_hdr(). When vlan device is used, we cannot access network
> protocol by skb->protocol without HW vlan acceleration.
> So it looks like this change corrupts TSO functionality on mvneta.
> 
>> If we need that, I'll likely do this check once in tso_start() and add a
>> variable to struct tso_t.
> 
> I'm not sure if an additional variable is needed.
> At least, skb_network_offset()/ip_hdr() should correctly handle (skip)
> vlan headers.

Ah, sorry, I misread your suggestion.
Additional variable would make sense to me.

Toshiaki Makita

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] net: tso: add support for IPv6

[Grumbach, Emmanuel]

On 10/26/2015 10:14 AM, Toshiaki Makita wrote:
> On 2015/10/26 16:47, Grumbach, Emmanuel wrote:
>> On 10/26/2015 06:03 AM, Toshiaki Makita wrote:
>>> On 2015/10/26 5:02, Emmanuel Grumbach wrote:
>>>> Adding IPv6 for the TSO helper API is trivial:
>>>> * Don't play with the id (which doesn't exist in IPv6)
>>>> * Correctly update the payload_len (don't include the
>>>>   length of the IP header itself)
>>> ...
>>>>  	memcpy(hdr, skb->data, hdr_len);
>>>> -	iph = (struct iphdr *)(hdr + mac_hdr_len);
>>>> -	iph->id = htons(tso->ip_id);
>>>> -	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
>>>> +	if (skb->protocol == htons(ETH_P_IP)) {
>>>
>>> I guess this should be vlan_get_protocol(skb).
>>
>> I truly don't know. I guess we could have VLANs, but I'd need to check
>> how the packet would look like after it exits mac80211.
> 
> I don't know much about mac80211.
> 
> What I see is that mvneta has TSO in vlan_features and it uses
> tso_build_hdr(). When vlan device is used, we cannot access network
> protocol by skb->protocol without HW vlan acceleration.
> So it looks like this change corrupts TSO functionality on mvneta.

Convincing enough :)

> 
>> If we need that, I'll likely do this check once in tso_start() and add a
>> variable to struct tso_t.
> 
> I'm not sure if an additional variable is needed.
> At least, skb_network_offset()/ip_hdr() should correctly handle (skip)
> vlan headers.
> 

Adding a bool to tso_t is more an optimisation than something needed for
correctness. It avoid to call vlan_get_protocol(skb) for each MSS.

> Toshiaki Makita
> 
> 

Re: [PATCH] net: tso: add support for IPv6

[Sergei Shtylyov]

Hello.

On 10/25/2015 10:58 PM, Emmanuel Grumbach wrote:

> Adding IPv6 for the TSO helper API is trivial:
> * Don't play with the id (which doesn't exist in IPv6)
> * Correctly update the payload_len (don't include the
>    length of the IP header itself)
>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> ---
>   net/core/tso.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/net/core/tso.c b/net/core/tso.c
> index 630b30b..ece4605 100644
> --- a/net/core/tso.c
> +++ b/net/core/tso.c
> @@ -14,18 +14,25 @@ EXPORT_SYMBOL(tso_count_descs);
>   void tso_build_hdr(struct sk_buff *skb, char *hdr, struct tso_t *tso,
>   		   int size, bool is_last)
>   {
> -	struct iphdr *iph;
>   	struct tcphdr *tcph;
>   	int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
>   	int mac_hdr_len = skb_network_offset(skb);
>
>   	memcpy(hdr, skb->data, hdr_len);
> -	iph = (struct iphdr *)(hdr + mac_hdr_len);
> -	iph->id = htons(tso->ip_id);
> -	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
> +	if (skb->protocol == htons(ETH_P_IP)) {
> +		struct iphdr *iph = (void *)(hdr + mac_hdr_len);
> +
> +		iph->id = htons(tso->ip_id);
> +		iph->tot_len = htons(size + hdr_len - mac_hdr_len);
> +		tso->ip_id++;
> +	}
> +	if (skb->protocol == htons(ETH_P_IPV6)) {

    *else* needed.

> +		struct ipv6hdr *iph = (void *)(hdr + mac_hdr_len);
> +
> +		iph->payload_len = htons(size + tcp_hdrlen(skb));
> +	}

    The above asks to be a *switch* statement.

[...]

MBR, Sergei

[PATCH v3] net: tso: add support for IPv6

[Emmanuel Grumbach]

Adding IPv6 for the TSO helper API is trivial:
* Don't play with the id (which doesn't exist in IPv6)
* Correctly update the payload_len (don't include the
  length of the IP header itself)

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
v3: use vlan_get_protocol and call it once in tso_start
    store the result in tso_t
---
 include/net/tso.h |  1 +
 net/core/tso.c    | 18 +++++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/net/tso.h b/include/net/tso.h
index 47e5444..b7be852 100644
--- a/include/net/tso.h
+++ b/include/net/tso.h
@@ -8,6 +8,7 @@ struct tso_t {
 	void *data;
 	size_t size;
 	u16 ip_id;
+	bool ipv6;
 	u32 tcp_seq;
 };
 
diff --git a/net/core/tso.c b/net/core/tso.c
index 630b30b..5dca7ce 100644
--- a/net/core/tso.c
+++ b/net/core/tso.c
@@ -1,4 +1,5 @@
 #include <linux/export.h>
+#include <linux/if_vlan.h>
 #include <net/ip.h>
 #include <net/tso.h>
 #include <asm/unaligned.h>
@@ -14,18 +15,24 @@ EXPORT_SYMBOL(tso_count_descs);
 void tso_build_hdr(struct sk_buff *skb, char *hdr, struct tso_t *tso,
 		   int size, bool is_last)
 {
-	struct iphdr *iph;
 	struct tcphdr *tcph;
 	int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
 	int mac_hdr_len = skb_network_offset(skb);
 
 	memcpy(hdr, skb->data, hdr_len);
-	iph = (struct iphdr *)(hdr + mac_hdr_len);
-	iph->id = htons(tso->ip_id);
-	iph->tot_len = htons(size + hdr_len - mac_hdr_len);
+	if (!tso->ipv6) {
+		struct iphdr *iph = (void *)(hdr + mac_hdr_len);
+
+		iph->id = htons(tso->ip_id);
+		iph->tot_len = htons(size + hdr_len - mac_hdr_len);
+		tso->ip_id++;
+	} else {
+		struct ipv6hdr *iph = (void *)(hdr + mac_hdr_len);
+
+		iph->payload_len = htons(size + tcp_hdrlen(skb));
+	}
 	tcph = (struct tcphdr *)(hdr + skb_transport_offset(skb));
 	put_unaligned_be32(tso->tcp_seq, &tcph->seq);
-	tso->ip_id++;
 
 	if (!is_last) {
 		/* Clear all special flags for not last packet */
@@ -61,6 +68,7 @@ void tso_start(struct sk_buff *skb, struct tso_t *tso)
 	tso->ip_id = ntohs(ip_hdr(skb)->id);
 	tso->tcp_seq = ntohl(tcp_hdr(skb)->seq);
 	tso->next_frag_idx = 0;
+	tso->ipv6 = vlan_get_protocol(skb) == htons(ETH_P_IPV6);
 
 	/* Build first data */
 	tso->size = skb_headlen(skb) - hdr_len;
-- 
2.1.4

Re: [PATCH v3] net: tso: add support for IPv6

[Eric Dumazet]

On Mon, 2015-10-26 at 10:31 +0200, Emmanuel Grumbach wrote:
> Adding IPv6 for the TSO helper API is trivial:
> * Don't play with the id (which doesn't exist in IPv6)
> * Correctly update the payload_len (don't include the
>   length of the IP header itself)
> 
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> ---
> v3: use vlan_get_protocol and call it once in tso_start
>     store the result in tso_t

Acked-by: Eric Dumazet <edumazet@google.com>

Next step, adding encapsulation support ;)

Re: [PATCH v3] net: tso: add support for IPv6

[David Miller]

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Date: Mon, 26 Oct 2015 10:31:29 +0200

> Adding IPv6 for the TSO helper API is trivial:
> * Don't play with the id (which doesn't exist in IPv6)
> * Correctly update the payload_len (don't include the
>   length of the IP header itself)
> 
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

Applied to net-next, thanks.