Message ID | 1427358484-16402-1-git-send-email-alexey.kodanev@oracle.com |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, 2015-03-26 at 11:28 +0300, Alexey Kodanev wrote: > Regression from the following commit: 2dc49d1680. > > tcp_v6_fill_cb() will be called twice if socket's state changes from > TCP_TIME_WAIT to TCP_LISTEN. That can result in control buffer data > corruption because in the second tcp_v6_fill_cb() call it's not copying > IP6CB(skb) anymore, but 'seq', 'end_seq', etc., so we can get weird and > unpredictable results. Performance loss of up to 1200% has been observed > in LTP/vxlan03 test. > > This can be fixed by copying inet6_skb_parm to the beginning of 'cb' > only if xfrm6_policy_check() and tcp_v6_fill_cb() are going to be > called again. > > Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> > --- Thanks Alexey ! Fixes: 2dc49d1680b53 ("tcp6: don't move IP6CB before xfrm6_policy_check()") Acked-by: Eric Dumazet <edumazet@google.com> -- 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
Hello. On 3/26/2015 11:28 AM, Alexey Kodanev wrote: > Regression from the following commit: 2dc49d1680. Please also specify that commit's summary line in parens. > tcp_v6_fill_cb() will be called twice if socket's state changes from > TCP_TIME_WAIT to TCP_LISTEN. That can result in control buffer data > corruption because in the second tcp_v6_fill_cb() call it's not copying > IP6CB(skb) anymore, but 'seq', 'end_seq', etc., so we can get weird and > unpredictable results. Performance loss of up to 1200% has been observed > in LTP/vxlan03 test. > This can be fixed by copying inet6_skb_parm to the beginning of 'cb' > only if xfrm6_policy_check() and tcp_v6_fill_cb() are going to be > called again. > Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> [...] WBR, Sergei -- 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
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 5d46832..7d21a78 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1411,6 +1411,15 @@ static void tcp_v6_fill_cb(struct sk_buff *skb, const struct ipv6hdr *hdr, TCP_SKB_CB(skb)->sacked = 0; } +static void tcp_v6_restore_cb(struct sk_buff *skb) +{ + /* We need to move header back to the beginning if xfrm6_policy_check() + * and tcp_v6_fill_cb() are going to be called again. + */ + memmove(IP6CB(skb), &TCP_SKB_CB(skb)->header.h6, + sizeof(struct inet6_skb_parm)); +} + static int tcp_v6_rcv(struct sk_buff *skb) { const struct tcphdr *th; @@ -1543,6 +1552,7 @@ do_time_wait: inet_twsk_deschedule(tw, &tcp_death_row); inet_twsk_put(tw); sk = sk2; + tcp_v6_restore_cb(skb); goto process; } /* Fall through to ACK */ @@ -1551,6 +1561,7 @@ do_time_wait: tcp_v6_timewait_ack(sk, skb); break; case TCP_TW_RST: + tcp_v6_restore_cb(skb); goto no_tcp_socket; case TCP_TW_SUCCESS: ;
Regression from the following commit: 2dc49d1680. tcp_v6_fill_cb() will be called twice if socket's state changes from TCP_TIME_WAIT to TCP_LISTEN. That can result in control buffer data corruption because in the second tcp_v6_fill_cb() call it's not copying IP6CB(skb) anymore, but 'seq', 'end_seq', etc., so we can get weird and unpredictable results. Performance loss of up to 1200% has been observed in LTP/vxlan03 test. This can be fixed by copying inet6_skb_parm to the beginning of 'cb' only if xfrm6_policy_check() and tcp_v6_fill_cb() are going to be called again. Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> --- net/ipv6/tcp_ipv6.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)