gre: Enforce IP ID verification on outer headers
Subsystems (2)
PATCH 1/4 gre: Enforce IP ID verification on outer headers
This change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
--- net/ipv4/gre_offload.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
net/ipv4/gre_offload.c +4 -2
@@ -175,8 +175,6 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head, null_compute_pseudo); } - flush =0 ; -for (p = *head; p; p = p->next) {const struct gre_base_hdr *greh2;
@@ -205,6 +203,9 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,continue ; } } + +/* Include the IP ID check from the outer IP hdr */ +NAPI_GRO_CB (p)->flush |=NAPI_GRO_CB (p)->flush_id; }skb_gro_pull (skb, grehlen);
@@ -213,6 +214,7 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,skb_gro_postpull_rcsum (skb, greh, grehlen); pp = ptype->callbacks.gro_receive (head, skb); + flush =0 ; out_unlock:rcu_read_unlock ();
PATCH 2/4 geneve: Enforce IP ID verification on outer headers
This change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
--- drivers/net/geneve.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/net/geneve.c +6 -5
@@ -462,8 +462,6 @@ static struct sk_buff **geneve_gro_receive(struct sk_buff **head, goto out; } - flush =0 ; -for (p = *head; p; p = p->next) {if (!NAPI_GRO_CB (p)->same_flow)continue ;
@@ -474,20 +472,23 @@ static struct sk_buff **geneve_gro_receive(struct sk_buff **head,NAPI_GRO_CB (p)->same_flow =0 ;continue ; } + +/* Include the IP ID check from the outer IP hdr */ +NAPI_GRO_CB (p)->flush |=NAPI_GRO_CB (p)->flush_id; } type = gh->proto_type;rcu_read_lock (); ptype =gro_find_receive_by_type (type); -if (!ptype) { - flush =1 ; +if (!ptype) goto out_unlock; - }skb_gro_pull (skb, gh_len);skb_gro_postpull_rcsum (skb, gh, gh_len); + pp = ptype->callbacks.gro_receive (head, skb); + flush =0 ; out_unlock:rcu_read_unlock ();
PATCH 3/4 vxlan: Enforce IP ID verification on outer headers
In reply to Alexander DuyckThis change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
--- drivers/net/vxlan.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/net/vxlan.c +4 -3
@@ -591,8 +591,6 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head,skb_gro_pull (skb,sizeof (struct vxlanhdr ));/* pull vxlan header */ - flush =0 ; -for (p = *head; p; p = p->next) {if (!NAPI_GRO_CB (p)->same_flow)continue ;
@@ -603,10 +601,13 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head,NAPI_GRO_CB (p)->same_flow =0 ;continue ; } + +/* Include the IP ID check from the outer IP hdr */ +NAPI_GRO_CB (p)->flush |=NAPI_GRO_CB (p)->flush_id; } pp =eth_gro_receive (head, skb); - + flush =0 ; out:skb_gro_remcsum_cleanup (skb, &grc);NAPI_GRO_CB (skb)->flush |= flush;
Re: [net-next PATCH 3/4] vxlan: Enforce IP ID verification on outer headers
Quoted 4 lines · Alexander Duyck
This change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
Can you please state the precise requirement for aggregation w.r.t IP IDs here? and point to where/how this is enforced, e.g for non-tunneled TCP GRO-ing?
Or.
David Miller
maintainerRe: [net-next PATCH 3/4] vxlan: Enforce IP ID verification on outer headers
From: Or Gerlitz <[email protected]>
Date: Mon, 7 Mar 2016 20:05:20 +0200
Quoted 9 lines · Alexander Duyck
Quoted 4 lines
This change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
Can you please state the precise requirement for aggregation w.r.t IP IDs here? and point to where/how this is enforced, e.g for non-tunneled TCP GRO-ing?
I also didn't see a nice "PATCH 0/4" posting explaining this series and I'd really like to see that.
Re: [net-next PATCH 3/4] vxlan: Enforce IP ID verification on outer headers
Quoted 9 lines · David Miller
From: Or Gerlitz <[email protected]>
Date: Mon, 7 Mar 2016 20:05:20 +0200Quoted 6 lines · Alexander Duyck
Quoted 4 lines
This change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
Quoted 6 lines
Quoted 3 lines
Can you please state the precise requirement for aggregation w.r.t IP IDs here? and point to where/how this is enforced, e.g for non-tunneled TCP GRO-ing?
I also didn't see a nice "PATCH 0/4" posting explaining this series and I'd really like to see that.
Sorry about that. I forgot to add the cover page when I sent this.
The enforcement is coming from the IP and TCP layers. If you take a look in inet_gro_receive we have the NAPI_GRO_CB(p)->flush_id value being populated based on the difference between the expected ID and the received one. So for IPv4 we overwrite it, and for IPv6 we set it to 0. The only consumer currently using it is TCP in tcp_gro_receive. The problem is with tunnels we lose the data for the outer when the inner overwrites it, as a result we can put whatever we want currently in the outer IP ID and it will be accepted.
The patch set is based off of a conversation several of us had on the list about doing TSO for tunnels and the fact that the IP IDs for the outer header have to advance. It makes it easier for me to validate that I am doing things properly if GRO doesn't destroy the IP ID data for the outer headers.
- Alex
Re: [net-next PATCH 3/4] vxlan: Enforce IP ID verification on outer headers
Quoted 28 lines · Alex Duyck
Quoted 9 lines · David Miller
From: Or Gerlitz <[email protected]>
Date: Mon, 7 Mar 2016 20:05:20 +0200Quoted 6 lines · Alexander Duyck
Quoted 4 lines
This change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
Quoted 6 lines
Quoted 3 lines
Can you please state the precise requirement for aggregation w.r.t IP IDs here? and point to where/how this is enforced, e.g for non-tunneled TCP GRO-ing?
I also didn't see a nice "PATCH 0/4" posting explaining this series and I'd really like to see that.
Sorry about that. I forgot to add the cover page when I sent this.
The enforcement is coming from the IP and TCP layers. If you take a look in
inet_gro_receivewe have the NAPI_GRO_CB(p)->flush_id value being populated based on the difference between the expected ID and the received one. So for IPv4 we overwrite it, and for IPv6 we set it to 0. The only consumer currently using it is TCP intcp_gro_receive. The problem is with tunnels we lose the data for the outer when the inner overwrites it, as a result we can put whatever we want currently in the outer IP ID and it will be accepted.
Quoted 5 lines
The patch set is based off of a conversation several of us had on the list about doing TSO for tunnels and the fact that the IP IDs for the outer header have to advance. It makes it easier for me to validate that I am doing things properly if GRO doesn't destroy the IP ID data for the outer headers.
In net/ipv4/af_inet.c:inet_gro_receive() there is the following comment above where NAPI_GRO_CB(p)->flush_id is set:
/* Save the IP ID check to be included later when we get to
* the transport layer so only the inner most IP ID is checked.
* This is because some GSO/TSO implementations do not
* correctly increment the IP ID for the outer hdrs. */
There was a long discussion about this a couple years ago and the conclusion was that it is the inner IP ID is really the important one in the case of encapsulation. Obviously, things like TCP/IP header compression don't apply to the outer encapsulation header.
David Miller
maintainerRe: [net-next PATCH 3/4] vxlan: Enforce IP ID verification on outer headers
From: Jesse Gross <[email protected]>
Date: Mon, 7 Mar 2016 15:42:59 -0800
Quoted 44 lines · Alex Duyck
Quoted 28 lines · David Miller
Quoted 9 lines
From: Or Gerlitz <[email protected]>
Date: Mon, 7 Mar 2016 20:05:20 +0200Quoted 6 lines · Alexander Duyck
Quoted 4 lines
This change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
Quoted 6 lines
Quoted 3 lines
Can you please state the precise requirement for aggregation w.r.t IP IDs here? and point to where/how this is enforced, e.g for non-tunneled TCP GRO-ing?
I also didn't see a nice "PATCH 0/4" posting explaining this series and I'd really like to see that.
Sorry about that. I forgot to add the cover page when I sent this.
The enforcement is coming from the IP and TCP layers. If you take a look in
inet_gro_receivewe have the NAPI_GRO_CB(p)->flush_id value being populated based on the difference between the expected ID and the received one. So for IPv4 we overwrite it, and for IPv6 we set it to 0. The only consumer currently using it is TCP intcp_gro_receive. The problem is with tunnels we lose the data for the outer when the inner overwrites it, as a result we can put whatever we want currently in the outer IP ID and it will be accepted.Quoted 5 lines
The patch set is based off of a conversation several of us had on the list about doing TSO for tunnels and the fact that the IP IDs for the outer header have to advance. It makes it easier for me to validate that I am doing things properly if GRO doesn't destroy the IP ID data for the outer headers.
In net/ipv4/af_inet.c:inet_gro_receive() there is the following comment above where NAPI_GRO_CB(p)->flush_id is set:
/* Save the IP ID check to be included later when we get to
* the transport layer so only the inner most IP ID is checked.
* This is because some GSO/TSO implementations do not
* correctly increment the IP ID for the outer hdrs. */
Quoted 4 lines
There was a long discussion about this a couple years ago and the conclusion was that it is the inner IP ID is really the important one in the case of encapsulation. Obviously, things like TCP/IP header compression don't apply to the outer encapsulation header.
That's how I remember the conversation going as wel...
Re: [net-next PATCH 3/4] vxlan: Enforce IP ID verification on outer headers
Quoted 47 lines · David Miller
From: Jesse Gross <[email protected]>
Date: Mon, 7 Mar 2016 15:42:59 -0800Quoted 44 lines · Alex Duyck
Quoted 28 lines · David Miller
Quoted 9 lines
From: Or Gerlitz <[email protected]>
Date: Mon, 7 Mar 2016 20:05:20 +0200Quoted 6 lines · Alexander Duyck
Quoted 4 lines
This change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
Quoted 6 lines
Quoted 3 lines
Can you please state the precise requirement for aggregation w.r.t IP IDs here? and point to where/how this is enforced, e.g for non-tunneled TCP GRO-ing?
I also didn't see a nice "PATCH 0/4" posting explaining this series and I'd really like to see that.
Sorry about that. I forgot to add the cover page when I sent this.
The enforcement is coming from the IP and TCP layers. If you take a look in
inet_gro_receivewe have the NAPI_GRO_CB(p)->flush_id value being populated based on the difference between the expected ID and the received one. So for IPv4 we overwrite it, and for IPv6 we set it to 0. The only consumer currently using it is TCP intcp_gro_receive. The problem is with tunnels we lose the data for the outer when the inner overwrites it, as a result we can put whatever we want currently in the outer IP ID and it will be accepted.Quoted 5 lines
The patch set is based off of a conversation several of us had on the list about doing TSO for tunnels and the fact that the IP IDs for the outer header have to advance. It makes it easier for me to validate that I am doing things properly if GRO doesn't destroy the IP ID data for the outer headers.
In net/ipv4/af_inet.c:inet_gro_receive() there is the following comment above where NAPI_GRO_CB(p)->flush_id is set:
/* Save the IP ID check to be included later when we get to
* the transport layer so only the inner most IP ID is checked.
* This is because some GSO/TSO implementations do not
* correctly increment the IP ID for the outer hdrs. */
Quoted 6 lines
Quoted 4 lines
There was a long discussion about this a couple years ago and the conclusion was that it is the inner IP ID is really the important one in the case of encapsulation. Obviously, things like TCP/IP header compression don't apply to the outer encapsulation header.
That's how I remember the conversation going as wel...
Given that then, how would you feel about a software scheme to use existing TSO support on devices to support UDP/GRE tunnel TSO by pre-computing the values for the outer headers and then having those duplicated on TSO frames?
This is kind of what got me started down the path of validating the outer IP ID. If we are okay with UDP or GRE encapsulated tunnels using the same IP ID for the outer headers then I should be able to code up something in software so that we can get more devices supporting tunnel TSO offload for tunnels.
- Alex
PATCH 4/4 gue: Enforce IP ID verification on outer headers
In reply to Alexander DuyckThis change enforces the IP ID verification on outer headers. As a result if the DF flag is not set on the outer header we will force the flow to be flushed in the event that the IP ID is out of sequence with the existing flow.
In addition I believe this fixes a bug in which it was possible for no GRO handler to be defined for a given protocol but we were not setting the flush bit as it had been cleared before the check.
--- net/ipv4/fou.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
net/ipv4/fou.c +4 -2
@@ -319,8 +319,6 @@ static struct sk_buff **gue_gro_receive(struct sk_buff **head,skb_gro_pull (skb, hdrlen); - flush =0 ; -for (p = *head; p; p = p->next) {const struct guehdr *guehdr2;
@@ -343,6 +341,9 @@ static struct sk_buff **gue_gro_receive(struct sk_buff **head,NAPI_GRO_CB (p)->same_flow =0 ;continue ; } + +/* Include the IP ID check from the outer IP hdr */ +NAPI_GRO_CB (p)->flush |=NAPI_GRO_CB (p)->flush_id; }rcu_read_lock ();
@@ -352,6 +353,7 @@ static struct sk_buff **gue_gro_receive(struct sk_buff **head, goto out_unlock; pp = ops->callbacks.gro_receive (head, skb); + flush =0 ; out_unlock:rcu_read_unlock ();