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 <[email protected]>
---
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
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
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 <[email protected]>
> ---
> 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
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
From: Emmanuel Grumbach <[email protected]>
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 <[email protected]>
Applied to net-next, thanks.
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 <[email protected]>
> ---
> v3: use vlan_get_protocol and call it once in tso_start
> store the result in tso_t
Acked-by: Eric Dumazet <[email protected]>
Next step, adding encapsulation support ;)
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
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 <[email protected]>
---
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
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
>
>
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 <[email protected]>
---
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