Re: [PATCH net-next 1/2] tcp: measure rwnd-limited time
On Wed, Sep 7, 2016 at 5:27 PM, David Millerwrote: > > From: "Francis Y. Yan" > Date: Tue, 6 Sep 2016 18:32:40 -0700 > > > diff --git a/include/linux/tcp.h b/include/linux/tcp.h > > index 7be9b12..f5b588e 100644 > > --- a/include/linux/tcp.h > > +++ b/include/linux/tcp.h > > @@ -176,6 +176,7 @@ struct tcp_sock { > >* were acked. > >*/ > > struct u64_stats_sync syncp; /* protects 64bit vars (cf > > tcp_get_info()) */ > > + seqcount_t seqcnt; /* proctects rwnd-limited-related vars, etc. > > */ > > > > u32 snd_una;/* First byte we want an ack for*/ > > u32 snd_sml;/* Last byte of the most recently transmitted > > small packet */ > > @@ -204,6 +205,8 @@ struct tcp_sock { > > > > u32 window_clamp; /* Maximal window to advertise */ > > u32 rcv_ssthresh; /* Current window clamp */ > > + struct skb_mstamp rwnd_limited_ts; /* Last timestamp limited by rwnd > > */ > > + u64 rwnd_limited; /* Total time (us) limited by rwnd */ > > > > /* Information of the most recently (s)acked skb */ > > struct tcp_rack { > > I understand completely the usefulness of this change, but wow that is a lot > of > new TCP socket space taken up just to export some time values in tcp_info for > debugging and statistics. Thanks for raising the concern. The space requirement and timing resolution should indeed be justified better on the motivation and the dev plan. We would resubmit a v2 with a cover letter detailing those.
Re: [PATCH net-next 1/2] tcp: measure rwnd-limited time
From: "Francis Y. Yan"Date: Tue, 6 Sep 2016 18:32:40 -0700 > diff --git a/include/linux/tcp.h b/include/linux/tcp.h > index 7be9b12..f5b588e 100644 > --- a/include/linux/tcp.h > +++ b/include/linux/tcp.h > @@ -176,6 +176,7 @@ struct tcp_sock { >* were acked. >*/ > struct u64_stats_sync syncp; /* protects 64bit vars (cf tcp_get_info()) > */ > + seqcount_t seqcnt; /* proctects rwnd-limited-related vars, etc. */ > > u32 snd_una;/* First byte we want an ack for*/ > u32 snd_sml;/* Last byte of the most recently transmitted > small packet */ > @@ -204,6 +205,8 @@ struct tcp_sock { > > u32 window_clamp; /* Maximal window to advertise */ > u32 rcv_ssthresh; /* Current window clamp */ > + struct skb_mstamp rwnd_limited_ts; /* Last timestamp limited by rwnd */ > + u64 rwnd_limited; /* Total time (us) limited by rwnd */ > > /* Information of the most recently (s)acked skb */ > struct tcp_rack { I understand completely the usefulness of this change, but wow that is a lot of new TCP socket space taken up just to export some time values in tcp_info for debugging and statistics.
Re: [PATCH net-next 1/2] tcp: measure rwnd-limited time
On Tue, 2016-09-06 at 18:32 -0700, Francis Y. Yan wrote: > This patch measures the total time when TCP transmission is limited > by receiver's advertised window (rwnd), and exports it in tcp_info as > tcpi_rwnd_limited. > > The rwnd-limited time is defined as the period when the next segment > to send by TCP cannot fit into rwnd. To measure it, we record the last > timestamp when limited by rwnd (rwnd_limited_ts) and the total > rwnd-limited time (rwnd_limited) in tcp_sock. > > Then we export the total rwnd-limited time so far in tcp_info, where > by so far, we mean that if TCP transmission is still being limited by > rwnd, the time interval since rwnd_limited_ts needs to be counted as > well; otherwise, we simply export rwnd_limited. > > It is worth noting that we also have to add a new sequence counter > (seqcnt) in tcp_sock to carefully handle tcp_info's reading of > rwnd_limited_ts and rwnd_limited in order to get a consistent snapshot > of both variables together. > > Signed-off-by: Francis Y. Yan> Signed-off-by: Yuchung Cheng > --- Acked-by: Eric Dumazet Thanks Francis !
Re: [PATCH net-next 1/2] tcp: measure rwnd-limited time
On Tue, Sep 6, 2016 at 9:32 PM, Francis Y. Yanwrote: > This patch measures the total time when TCP transmission is limited > by receiver's advertised window (rwnd), and exports it in tcp_info as > tcpi_rwnd_limited. > > The rwnd-limited time is defined as the period when the next segment > to send by TCP cannot fit into rwnd. To measure it, we record the last > timestamp when limited by rwnd (rwnd_limited_ts) and the total > rwnd-limited time (rwnd_limited) in tcp_sock. > > Then we export the total rwnd-limited time so far in tcp_info, where > by so far, we mean that if TCP transmission is still being limited by > rwnd, the time interval since rwnd_limited_ts needs to be counted as > well; otherwise, we simply export rwnd_limited. > > It is worth noting that we also have to add a new sequence counter > (seqcnt) in tcp_sock to carefully handle tcp_info's reading of > rwnd_limited_ts and rwnd_limited in order to get a consistent snapshot > of both variables together. > > Signed-off-by: Francis Y. Yan > Signed-off-by: Yuchung Cheng Acked-by: Soheil Hassas Yeganeh