* [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST
@ 2017-04-06 13:35 gfree.wind
2017-04-06 14:00 ` Neal Cardwell
0 siblings, 1 reply; 6+ messages in thread
From: gfree.wind @ 2017-04-06 13:35 UTC (permalink / raw)
To: davem, kuznet, jmorris, kaber, netdev; +Cc: Gao Feng
From: Gao Feng <fgao@ikuai8.com>
When fail to transmit RST, don't increase TCP_MIB_OUTRSTS in func
tcp_send_active_reset like the case that it only increases
LINUX_MIB_TCPABORTFAILED when fail to alloc skb.
Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
net/ipv4/tcp_output.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 22548b5..274802f 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3014,8 +3014,8 @@ void tcp_send_active_reset(struct sock *sk, gfp_t priority)
/* Send it off. */
if (tcp_transmit_skb(sk, skb, 0, priority))
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTFAILED);
-
- TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);
+ else
+ TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);
}
/* Send a crossed SYN-ACK during socket establishment.
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST
2017-04-06 13:35 [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST gfree.wind
@ 2017-04-06 14:00 ` Neal Cardwell
2017-04-06 14:05 ` Gao Feng
0 siblings, 1 reply; 6+ messages in thread
From: Neal Cardwell @ 2017-04-06 14:00 UTC (permalink / raw)
To: Gao Feng
Cc: David Miller, Alexey Kuznetsov, James Morris, Patrick McHardy,
Netdev, Gao Feng
On Thu, Apr 6, 2017 at 9:35 AM, <gfree.wind@foxmail.com> wrote:
> From: Gao Feng <fgao@ikuai8.com>
>
> When fail to transmit RST, don't increase TCP_MIB_OUTRSTS in func
> tcp_send_active_reset like the case that it only increases
> LINUX_MIB_TCPABORTFAILED when fail to alloc skb.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
I would be concerned that this is a change in the semantics of
TCP_MIB_OUTRSTS that might break user-space monitoring tools that rely
on the current semantics. Counting attempted RSTs could be an
important signal to monitor, and it could be quite bad if that signal
is lost or hidden because the machine is so overloaded that the
transmission of the RSTs fails.
Also it would seem to muddy the semantics a bit, since both
tcp_v4_send_reset() and tcp_v6_send_response() currently increment
TCP_MIB_OUTRSTS without regard to whether the transmit actually
succeeded or not.
neal
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST
2017-04-06 14:00 ` Neal Cardwell
@ 2017-04-06 14:05 ` Gao Feng
2017-04-06 14:08 ` Neal Cardwell
0 siblings, 1 reply; 6+ messages in thread
From: Gao Feng @ 2017-04-06 14:05 UTC (permalink / raw)
To: 'Neal Cardwell', 'Gao Feng'
Cc: 'David Miller', 'Alexey Kuznetsov',
'James Morris', 'Patrick McHardy',
'Netdev'
Hi Neal,
> -----Original Message-----
> From: Neal Cardwell [mailto:ncardwell@google.com]
> Sent: Thursday, April 6, 2017 10:01 PM
> To: Gao Feng <gfree.wind@foxmail.com>
> Cc: David Miller <davem@davemloft.net>; Alexey Kuznetsov
> <kuznet@ms2.inr.ac.ru>; James Morris <jmorris@namei.org>; Patrick McHardy
> <kaber@trash.net>; Netdev <netdev@vger.kernel.org>; Gao Feng
> <fgao@ikuai8.com>
> Subject: Re: [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS
> when fail to transmit RST
>
> On Thu, Apr 6, 2017 at 9:35 AM, <gfree.wind@foxmail.com> wrote:
> > From: Gao Feng <fgao@ikuai8.com>
> >
> > When fail to transmit RST, don't increase TCP_MIB_OUTRSTS in func
> > tcp_send_active_reset like the case that it only increases
> > LINUX_MIB_TCPABORTFAILED when fail to alloc skb.
> >
> > Signed-off-by: Gao Feng <fgao@ikuai8.com>
> > ---
>
> I would be concerned that this is a change in the semantics of
> TCP_MIB_OUTRSTS that might break user-space monitoring tools that rely on
> the current semantics. Counting attempted RSTs could be an important signal
> to monitor, and it could be quite bad if that signal is lost or hidden because the
> machine is so overloaded that the transmission of the RSTs fails.
>
> Also it would seem to muddy the semantics a bit, since both
> tcp_v4_send_reset() and tcp_v6_send_response() currently increment
> TCP_MIB_OUTRSTS without regard to whether the transmit actually succeeded
> or not.
>
> neal
If so, we should increase the TCP_MIB_OUTRSTS too when fail to alloc skb.
When machine is overloaded and mem is exhausted, it may fail to alloc skb.
Regards
Feng
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST
2017-04-06 14:05 ` Gao Feng
@ 2017-04-06 14:08 ` Neal Cardwell
2017-04-06 15:11 ` Gao Feng
2017-04-06 15:24 ` Eric Dumazet
0 siblings, 2 replies; 6+ messages in thread
From: Neal Cardwell @ 2017-04-06 14:08 UTC (permalink / raw)
To: Gao Feng
Cc: David Miller, Alexey Kuznetsov, James Morris, Patrick McHardy, Netdev
On Thu, Apr 6, 2017 at 10:05 AM, Gao Feng <gfree.wind@foxmail.com> wrote:
> If so, we should increase the TCP_MIB_OUTRSTS too when fail to alloc skb.
> When machine is overloaded and mem is exhausted, it may fail to alloc skb.
Moving the increment of TCP_MIB_OUTRSTS to the top of
tcp_send_active_reset() sounds fine to me.
neal
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST
2017-04-06 14:08 ` Neal Cardwell
@ 2017-04-06 15:11 ` Gao Feng
2017-04-06 15:24 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Gao Feng @ 2017-04-06 15:11 UTC (permalink / raw)
To: 'Neal Cardwell'
Cc: 'David Miller', 'Alexey Kuznetsov',
'James Morris', 'Patrick McHardy',
'Netdev'
Hi Neal
> -----Original Message-----
>
> On Thu, Apr 6, 2017 at 10:05 AM, Gao Feng <gfree.wind@foxmail.com> wrote:
> > If so, we should increase the TCP_MIB_OUTRSTS too when fail to alloc skb.
> > When machine is overloaded and mem is exhausted, it may fail to alloc skb.
>
> Moving the increment of TCP_MIB_OUTRSTS to the top of
> tcp_send_active_reset() sounds fine to me.
>
> neal
I sent the v2 patch, and didn't change the tcp_v4_send_reset and tcp_v6_send_response.
Because I think they are only used during connecting. The rst count is not as important as
tcp_send_active_reset.
Regards
Feng
.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST
2017-04-06 14:08 ` Neal Cardwell
2017-04-06 15:11 ` Gao Feng
@ 2017-04-06 15:24 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2017-04-06 15:24 UTC (permalink / raw)
To: Neal Cardwell
Cc: Gao Feng, David Miller, Alexey Kuznetsov, James Morris,
Patrick McHardy, Netdev
On Thu, 2017-04-06 at 10:08 -0400, Neal Cardwell wrote:
> On Thu, Apr 6, 2017 at 10:05 AM, Gao Feng <gfree.wind@foxmail.com> wrote:
> > If so, we should increase the TCP_MIB_OUTRSTS too when fail to alloc skb.
> > When machine is overloaded and mem is exhausted, it may fail to alloc skb.
>
> Moving the increment of TCP_MIB_OUTRSTS to the top of
> tcp_send_active_reset() sounds fine to me.
Yes.
Keep in mind that whatever hard we try to send the packet, something
might drop it later without TCP stack knowing it.
So it is not really useful to test the immediate actions that are under
our control.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-06 15:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 13:35 [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST gfree.wind
2017-04-06 14:00 ` Neal Cardwell
2017-04-06 14:05 ` Gao Feng
2017-04-06 14:08 ` Neal Cardwell
2017-04-06 15:11 ` Gao Feng
2017-04-06 15:24 ` Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.