Hi!
On 03/23/2015 04:52 PM, Eric Dumazet wrote:
On Mon, 2015-03-23 at 13:38 +0300, Alexey Kodanev wrote:
Regression introduced by 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 performance loss and
control buffer data corruption because in the second tcp_v6_fill_cb()
it's not copying the 'header' anymore, but 'seq', 'end_seq', etc.
Reproduced with LTP/tcp_fastopen test and netperf -t TCP_CRR, so when
we're constantly closing and opening TCP connections after several
requests/responses from client.
This can be fixed if we move 'header' union back to the beginning of
'struct tcp_skb_cb' and getting skb->next, TCP_SKB_CB(skb)->seq and
TCP_SKB_CB(skb)->end_seq on the same cache line by moving 'cb[48]'
closer to 'skb->next', above the *sk and *dev pointers.
NACK. DO not change sk and dev pointers.
Fix the bug yes, do not change skb layout so radically, it will have
serious performance impact for other part of the stack.
Did various testing with exactly that change and hadn't found any
"serious performance impact" on the stack (at least on my configuration).
Alright, what about other part of the patch, is it acceptable?
I came up with one more solution... not so radical :)
We can restore inet6_skb_parm with
memmove(IP6CB(skb), &TCP_SKB_CB(skb)->header.h6, ...)
in the two places in tcp_v6_rcv(), before xfrm6_policy_check() +
tcp_v6_fill_cb() are called again:
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:
Thanks,
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
