* [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
@ 2014-11-20 18:58 Rick Jones
2014-11-20 20:15 ` Vijay Subramanian
2014-11-20 20:47 ` Eric Dumazet
0 siblings, 2 replies; 6+ messages in thread
From: Rick Jones @ 2014-11-20 18:58 UTC (permalink / raw)
To: netdev; +Cc: davem
From: Rick Jones <rick.jones2@hp.com>
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.
Signed-off-by: Rick Jones <rick.jones2@hp.com>
---
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.
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d91436b..999b0a4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -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;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
2014-11-20 18:58 [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path Rick Jones
@ 2014-11-20 20:15 ` Vijay Subramanian
2014-11-20 20:47 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Vijay Subramanian @ 2014-11-20 20:15 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev, David Miller
> + * 99 times out of 10, there won't actually be any
Minor typo here.. s/10/100.
Vijay
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
2014-11-20 18:58 [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path Rick Jones
2014-11-20 20:15 ` Vijay Subramanian
@ 2014-11-20 20:47 ` Eric Dumazet
2014-11-20 21:31 ` Rick Jones
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2014-11-20 20:47 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev, davem
On Thu, 2014-11-20 at 10:58 -0800, Rick Jones wrote:
> From: Rick Jones <rick.jones2@hp.com>
>
> 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.
>
> Signed-off-by: Rick Jones <rick.jones2@hp.com>
>
> ---
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...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
2014-11-20 20:47 ` Eric Dumazet
@ 2014-11-20 21:31 ` Rick Jones
2014-11-20 21:48 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Rick Jones @ 2014-11-20 21:31 UTC (permalink / raw)
To: Eric Dumazet, Rick Jones; +Cc: netdev, davem
On 11/20/2014 12:47 PM, Eric Dumazet wrote:
> On Thu, 2014-11-20 at 10:58 -0800, Rick Jones wrote:
>> From: Rick Jones <rick.jones2@hp.com>
>>
>> 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.
>>
>> Signed-off-by: Rick Jones <rick.jones2@hp.com>
>>
>> ---
>
> So what happens if we really drop the packet ?
Do you mean when there is actually data in the SYN?
rick
>
> 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...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
2014-11-20 21:31 ` Rick Jones
@ 2014-11-20 21:48 ` Eric Dumazet
2014-11-20 21:51 ` Rick Jones
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2014-11-20 21:48 UTC (permalink / raw)
To: Rick Jones; +Cc: Rick Jones, netdev, davem
On Thu, 2014-11-20 at 13:31 -0800, Rick Jones wrote:
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
2014-11-20 21:48 ` Eric Dumazet
@ 2014-11-20 21:51 ` Rick Jones
0 siblings, 0 replies; 6+ messages in thread
From: Rick Jones @ 2014-11-20 21:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem
On 11/20/2014 01:48 PM, Eric Dumazet wrote:
> On Thu, 2014-11-20 at 13:31 -0800, Rick Jones wrote:
>
>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-20 21:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-20 18:58 [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path Rick Jones
2014-11-20 20:15 ` Vijay Subramanian
2014-11-20 20:47 ` Eric Dumazet
2014-11-20 21:31 ` Rick Jones
2014-11-20 21:48 ` Eric Dumazet
2014-11-20 21:51 ` Rick Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).