tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
Subsystems (3)
PATCH 1/1 tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Rick Jones <[email protected]>
When a system is the passive accepter of many connections, for example when the target of a netperf TCP_CC or TCP_CRR test, or as say a web server, the discard of the skb containing the TCP SYN being processed for the LISTEN endpoint should be a consume_skb() rather than a kfree_skb() to avoid cluttering a dropped packet profile.
---
perf top -a -g -e skb:kfree_sk output from a system which is the target of a netperf TCP_CC test
before:
- 100.00% 100.00% [kernel] [k] kfree_skb
- kfree_skb
+ 68.68% sk_stream_kill_queues
+ 31.32% tcp_rcv_state_process
after:
- 100.00% 100.00% [kernel] [k] kfree_skb
- kfree_skb
99.89% sk_stream_kill_queues
Presumably, the consume_skb() versus kfree_skb() could be made conditional on there actually being data in the TCP SYN segment, but the odds of there actually being data seemed low enough to not warrant it.
net/ipv4/tcp_input.c +5 -1
@@ -5624,8 +5624,12 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,* against this problem. So, we drop the data * in the interest of security over speed unless * it's still in use. +* +* 99 times out of 10, there won't actually be any +* data and so we aren't really dropping anything. +* So, we consume_skb() rather than kfree_skb(). */ -kfree_skb (skb); +consume_skb (skb);return 0 ; } goto discard;
Quoted 1 line
+ * 99 times out of 10, there won't actually be any
Minor typo here.. s/10/100.
Vijay
Quoted 11 lines · Rick Jones
From: Rick Jones <[email protected]>
When a system is the passive accepter of many connections, for example when the target of a netperf
TCP_CCorTCP_CRRtest, or as say a web server, the discard of the skb containing the TCP SYN being processed for the LISTEN endpoint should be aconsume_skb()rather than akfree_skb()to avoid cluttering a dropped packet profile.Signed-off-by Rick Jones <[email protected]>---
So what happens if we really drop the packet ?
TCP stack at this point owns the packet, it is possible to mark a bit in it to either call consume_skb() or kfree_skb()
I attempted this once but gave up because it was a quite intrusive patch...
Quoted 9 lines · Eric Dumazet
Quoted 8 lines · Rick Jones
From: Rick Jones <[email protected]>
When a system is the passive accepter of many connections, for example when the target of a netperf
TCP_CCorTCP_CRRtest, or as say a web server, the discard of the skb containing the TCP SYN being processed for the LISTEN endpoint should be aconsume_skb()rather than akfree_skb()to avoid cluttering a dropped packet profile.
Quoted 5 lines
Quoted 3 lines
Signed-off-by Rick Jones <[email protected]>---
So what happens if we really drop the packet ?
Do you mean when there is actually data in the SYN?
rick
Quoted 6 lines
TCP stack at this point owns the packet, it is possible to mark a bit in it to either call
consume_skb()orkfree_skb()I attempted this once but gave up because it was a quite intrusive patch...
Quoted 1 line · Rick Jones
Do you mean when there is actually data in the SYN?
No, I mean the packet can be _dropped_ here, really.
SYN flood for example.
Quoted 7 lines · Eric Dumazet
Quoted 1 line · Rick Jones
Do you mean when there is actually data in the SYN?
No, I mean the packet can be _dropped_ here, really.
SYN flood for example.
Ah. I did not pick-up on that. Seeing the kfree_skb() in the original code I thought it was the end of the line for the segment.
rick