All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] remove TWKilled counter
@ 2019-01-17  8:18 yupeng
  2019-01-17 18:07 ` Cong Wang
  0 siblings, 1 reply; 6+ messages in thread
From: yupeng @ 2019-01-17  8:18 UTC (permalink / raw)
  To: netdev, davem, kuznet, yoshfuji

The most possible way to trigger TWKilled is using tcp_tw_recycle, but
tcp_tw_recycle was removed. The TWKilled could still be triggered if
below conditions are satisfied:
1 the tw_timer expires in fin_wait_2 status
2 the timeout value of tw_timer is changed to less than 4 seconds
  (via net.ipv4.tcp_fin_timeout or TCP_LINGER2 option)
But the 4 seconds threshold makes no sense in this scenario. So remove
this counter.

Signed-off-by: yupeng <yupeng0921@gmail.com>
---
 include/net/inet_timewait_sock.h |  3 +--
 include/uapi/linux/snmp.h        |  1 -
 net/ipv4/inet_timewait_sock.c    | 31 +------------------------------
 net/ipv4/proc.c                  |  1 -
 4 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
index 78775038f011..b042c0baa2ca 100644
--- a/include/net/inet_timewait_sock.h
+++ b/include/net/inet_timewait_sock.h
@@ -69,8 +69,7 @@ struct inet_timewait_sock {
 	/* these three are in inet_sock */
 	__be16			tw_sport;
 	/* And these are ours. */
-	unsigned int		tw_kill		: 1,
-				tw_transparent  : 1,
+	unsigned int		tw_transparent  : 1,
 				tw_flowlabel	: 20,
 				tw_pad		: 2,	/* 2 bits hole */
 				tw_tos		: 8;
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index 86dc24a96c90..fb0a17337bf2 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -178,7 +178,6 @@ enum
 	LINUX_MIB_ARPFILTER,			/* ArpFilter */
 	LINUX_MIB_TIMEWAITED,			/* TimeWaited */
 	LINUX_MIB_TIMEWAITRECYCLED,		/* TimeWaitRecycled */
-	LINUX_MIB_TIMEWAITKILLED,		/* TimeWaitKilled */
 	LINUX_MIB_PAWSACTIVEREJECTED,		/* PAWSActiveRejected */
 	LINUX_MIB_PAWSESTABREJECTED,		/* PAWSEstabRejected */
 	LINUX_MIB_DELAYEDACKS,			/* DelayedACKs */
diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 88c5069b5d20..f0c4fe21909d 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -144,10 +144,7 @@ static void tw_timer_handler(struct timer_list *t)
 {
 	struct inet_timewait_sock *tw = from_timer(tw, t, tw_timer);
 
-	if (tw->tw_kill)
-		__NET_INC_STATS(twsk_net(tw), LINUX_MIB_TIMEWAITKILLED);
-	else
-		__NET_INC_STATS(twsk_net(tw), LINUX_MIB_TIMEWAITED);
+	__NET_INC_STATS(twsk_net(tw), LINUX_MIB_TIMEWAITED);
 	inet_twsk_kill(tw);
 }
 
@@ -218,32 +215,6 @@ EXPORT_SYMBOL(inet_twsk_deschedule_put);
 
 void __inet_twsk_schedule(struct inet_timewait_sock *tw, int timeo, bool rearm)
 {
-	/* timeout := RTO * 3.5
-	 *
-	 * 3.5 = 1+2+0.5 to wait for two retransmits.
-	 *
-	 * RATIONALE: if FIN arrived and we entered TIME-WAIT state,
-	 * our ACK acking that FIN can be lost. If N subsequent retransmitted
-	 * FINs (or previous seqments) are lost (probability of such event
-	 * is p^(N+1), where p is probability to lose single packet and
-	 * time to detect the loss is about RTO*(2^N - 1) with exponential
-	 * backoff). Normal timewait length is calculated so, that we
-	 * waited at least for one retransmitted FIN (maximal RTO is 120sec).
-	 * [ BTW Linux. following BSD, violates this requirement waiting
-	 *   only for 60sec, we should wait at least for 240 secs.
-	 *   Well, 240 consumes too much of resources 8)
-	 * ]
-	 * This interval is not reduced to catch old duplicate and
-	 * responces to our wandering segments living for two MSLs.
-	 * However, if we use PAWS to detect
-	 * old duplicates, we can reduce the interval to bounds required
-	 * by RTO, rather than MSL. So, if peer understands PAWS, we
-	 * kill tw bucket after 3.5*RTO (it is important that this number
-	 * is greater than TS tick!) and detect old duplicates with help
-	 * of PAWS.
-	 */
-
-	tw->tw_kill = timeo <= 4*HZ;
 	if (!rearm) {
 		BUG_ON(mod_timer(&tw->tw_timer, jiffies + timeo));
 		atomic_inc(&tw->tw_dr->tw_count);
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index c3610b37bb4c..20a29736d07e 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -186,7 +186,6 @@ static const struct snmp_mib snmp4_net_list[] = {
 	SNMP_MIB_ITEM("ArpFilter", LINUX_MIB_ARPFILTER),
 	SNMP_MIB_ITEM("TW", LINUX_MIB_TIMEWAITED),
 	SNMP_MIB_ITEM("TWRecycled", LINUX_MIB_TIMEWAITRECYCLED),
-	SNMP_MIB_ITEM("TWKilled", LINUX_MIB_TIMEWAITKILLED),
 	SNMP_MIB_ITEM("PAWSActive", LINUX_MIB_PAWSACTIVEREJECTED),
 	SNMP_MIB_ITEM("PAWSEstab", LINUX_MIB_PAWSESTABREJECTED),
 	SNMP_MIB_ITEM("DelayedACKs", LINUX_MIB_DELAYEDACKS),
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] remove TWKilled counter
  2019-01-17  8:18 [PATCH net-next] remove TWKilled counter yupeng
@ 2019-01-17 18:07 ` Cong Wang
  2019-01-17 20:26   ` peng yu
  0 siblings, 1 reply; 6+ messages in thread
From: Cong Wang @ 2019-01-17 18:07 UTC (permalink / raw)
  To: yupeng
  Cc: Linux Kernel Network Developers, David Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI

On Thu, Jan 17, 2019 at 3:43 AM yupeng <yupeng0921@gmail.com> wrote:
> diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
> index 86dc24a96c90..fb0a17337bf2 100644
> --- a/include/uapi/linux/snmp.h
> +++ b/include/uapi/linux/snmp.h
> @@ -178,7 +178,6 @@ enum
>         LINUX_MIB_ARPFILTER,                    /* ArpFilter */
>         LINUX_MIB_TIMEWAITED,                   /* TimeWaited */
>         LINUX_MIB_TIMEWAITRECYCLED,             /* TimeWaitRecycled */
> -       LINUX_MIB_TIMEWAITKILLED,               /* TimeWaitKilled */
>         LINUX_MIB_PAWSACTIVEREJECTED,           /* PAWSActiveRejected */
>         LINUX_MIB_PAWSESTABREJECTED,            /* PAWSEstabRejected */
>         LINUX_MIB_DELAYEDACKS,                  /* DelayedACKs */

This is no way to go, it is breaking user-space.

You should at least keep this definition even if this counter is unused.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] remove TWKilled counter
  2019-01-17 18:07 ` Cong Wang
@ 2019-01-17 20:26   ` peng yu
  2019-01-21 23:14     ` peng yu
  0 siblings, 1 reply; 6+ messages in thread
From: peng yu @ 2019-01-17 20:26 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI

On Thu, Jan 17, 2019 at 10:07 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Thu, Jan 17, 2019 at 3:43 AM yupeng <yupeng0921@gmail.com> wrote:
> > diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
> > index 86dc24a96c90..fb0a17337bf2 100644
> > --- a/include/uapi/linux/snmp.h
> > +++ b/include/uapi/linux/snmp.h
> > @@ -178,7 +178,6 @@ enum
> >         LINUX_MIB_ARPFILTER,                    /* ArpFilter */
> >         LINUX_MIB_TIMEWAITED,                   /* TimeWaited */
> >         LINUX_MIB_TIMEWAITRECYCLED,             /* TimeWaitRecycled */
> > -       LINUX_MIB_TIMEWAITKILLED,               /* TimeWaitKilled */
> >         LINUX_MIB_PAWSACTIVEREJECTED,           /* PAWSActiveRejected */
> >         LINUX_MIB_PAWSESTABREJECTED,            /* PAWSEstabRejected */
> >         LINUX_MIB_DELAYEDACKS,                  /* DelayedACKs */
>
> This is no way to go, it is breaking user-space.
>
> You should at least keep this definition even if this counter is unused.

From the snmp.h commit history, I didn't aware that we should keep the
user-space compatibility. For example, the commit 713bafea9292 removed
LINUX_MIB_TCPFACKREORDER, and the commit 4f693b55c3d2 inserted
LINUX_MIB_TCPBACKLOGCOALESCE to the middle of the linux mib
definitions enum. I think these commits would break the user-space
compatibility too. Did they use any method to avoid breaking the
user-space compatibility? Or if I misunderstand anything, please let
me know.

Thanks a lot.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] remove TWKilled counter
  2019-01-17 20:26   ` peng yu
@ 2019-01-21 23:14     ` peng yu
  2019-01-23  6:58       ` Cong Wang
  0 siblings, 1 reply; 6+ messages in thread
From: peng yu @ 2019-01-21 23:14 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI

Hello, I think the snmp.h didn't try to keep user-space compatibility,
so we could delete  LINUX_MIB_TIMEWAITKILLED counter. Do I
misunderstand anything?

On Thu, Jan 17, 2019 at 12:26 PM peng yu <yupeng0921@gmail.com> wrote:
>
> On Thu, Jan 17, 2019 at 10:07 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >
> > On Thu, Jan 17, 2019 at 3:43 AM yupeng <yupeng0921@gmail.com> wrote:
> > > diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
> > > index 86dc24a96c90..fb0a17337bf2 100644
> > > --- a/include/uapi/linux/snmp.h
> > > +++ b/include/uapi/linux/snmp.h
> > > @@ -178,7 +178,6 @@ enum
> > >         LINUX_MIB_ARPFILTER,                    /* ArpFilter */
> > >         LINUX_MIB_TIMEWAITED,                   /* TimeWaited */
> > >         LINUX_MIB_TIMEWAITRECYCLED,             /* TimeWaitRecycled */
> > > -       LINUX_MIB_TIMEWAITKILLED,               /* TimeWaitKilled */
> > >         LINUX_MIB_PAWSACTIVEREJECTED,           /* PAWSActiveRejected */
> > >         LINUX_MIB_PAWSESTABREJECTED,            /* PAWSEstabRejected */
> > >         LINUX_MIB_DELAYEDACKS,                  /* DelayedACKs */
> >
> > This is no way to go, it is breaking user-space.
> >
> > You should at least keep this definition even if this counter is unused.
>
> From the snmp.h commit history, I didn't aware that we should keep the
> user-space compatibility. For example, the commit 713bafea9292 removed
> LINUX_MIB_TCPFACKREORDER, and the commit 4f693b55c3d2 inserted
> LINUX_MIB_TCPBACKLOGCOALESCE to the middle of the linux mib
> definitions enum. I think these commits would break the user-space
> compatibility too. Did they use any method to avoid breaking the
> user-space compatibility? Or if I misunderstand anything, please let
> me know.
>
> Thanks a lot.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] remove TWKilled counter
  2019-01-21 23:14     ` peng yu
@ 2019-01-23  6:58       ` Cong Wang
  2019-01-28  7:35         ` peng yu
  0 siblings, 1 reply; 6+ messages in thread
From: Cong Wang @ 2019-01-23  6:58 UTC (permalink / raw)
  To: peng yu
  Cc: Linux Kernel Network Developers, David Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI

On Mon, Jan 21, 2019 at 3:14 PM peng yu <yupeng0921@gmail.com> wrote:
>
> Hello, I think the snmp.h didn't try to keep user-space compatibility,
> so we could delete  LINUX_MIB_TIMEWAITKILLED counter. Do I
> misunderstand anything?

I don't know. Maybe user-space doesn't need these definitions at all,
as like nstat can figure out the values from /proc/net/netstat. It looks
like we either could just kill snmp.h or we have to keep it for backward
compatibility.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] remove TWKilled counter
  2019-01-23  6:58       ` Cong Wang
@ 2019-01-28  7:35         ` peng yu
  0 siblings, 0 replies; 6+ messages in thread
From: peng yu @ 2019-01-28  7:35 UTC (permalink / raw)
  To: David Miller, Linux Kernel Network Developers, Cong Wang
  Cc: Alexey Kuznetsov, Hideaki YOSHIFUJI

Hi, David and Cong

I don't quite understand where this patch to go. This patch wants to
delete the LINUX_MIB_TIMEWAITKILLED counter. Moving snmp.h out of the
uapi directory is a difference thing. Do we really need to do it?

On Tue, Jan 22, 2019 at 10:58 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Jan 21, 2019 at 3:14 PM peng yu <yupeng0921@gmail.com> wrote:
> >
> > Hello, I think the snmp.h didn't try to keep user-space compatibility,
> > so we could delete  LINUX_MIB_TIMEWAITKILLED counter. Do I
> > misunderstand anything?
>
> I don't know. Maybe user-space doesn't need these definitions at all,
> as like nstat can figure out the values from /proc/net/netstat. It looks
> like we either could just kill snmp.h or we have to keep it for backward
> compatibility.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-28  7:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-17  8:18 [PATCH net-next] remove TWKilled counter yupeng
2019-01-17 18:07 ` Cong Wang
2019-01-17 20:26   ` peng yu
2019-01-21 23:14     ` peng yu
2019-01-23  6:58       ` Cong Wang
2019-01-28  7:35         ` peng yu

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.