| Message ID | 20190911223148.89808-2-tph@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v3,1/2] tcp: Add TCP_INFO counter for packets received out-of-order | expand |
On Wed, Sep 11, 2019 at 6:32 PM Thomas Higdon <tph@fb.com> wrote: > > Neal Cardwell mentioned that rcv_wnd would be useful for helping > diagnose whether a flow is receive-window-limited at a given instant. > > This serves the purpose of adding an additional __u32 to avoid the > would-be hole caused by the addition of the tcpi_rcvi_ooopack field. > > Signed-off-by: Thomas Higdon <tph@fb.com> > --- Thanks, Thomas. I know that when I mentioned this before I mentioned the idea of both tp->snd_wnd (send-side receive window) and tp->rcv_wnd (receive-side receive window) in tcp_info, and did not express a preference between the two. Now that we are faced with a decision between the two, personally I think it would be a little more useful to start with tp->snd_wnd. :-) Two main reasons: (1) Usually when we're diagnosing TCP performance problems, we do so from the sender, since the sender makes most of the performance-critical decisions (cwnd, pacing, TSO size, TSQ, etc). From the sender-side the thing that would be most useful is to see tp->snd_wnd, the receive window that the receiver has advertised to the sender. (2) From the receiver side, "ss" can already show a fair amount of info about receive-side buffer/window limits, like: info->tcpi_rcv_ssthresh, info->tcpi_rcv_space, skmeminfo[SK_MEMINFO_RMEM_ALLOC], skmeminfo[SK_MEMINFO_RCVBUF]. Often the rwin can be approximated by combining those. Hopefully Eric, Yuchung, and Soheil can weigh in on the question of snd_wnd vs rcv_wnd. Or we can perhaps think of another field, and add the tcpi_rcvi_ooopack, snd_wnd, rcv_wnd, and that final field, all together. thanks, neal
On Thu, Sep 12, 2019 at 1:59 AM Neal Cardwell <ncardwell@google.com> wrote: > > On Wed, Sep 11, 2019 at 6:32 PM Thomas Higdon <tph@fb.com> wrote: > > > > Neal Cardwell mentioned that rcv_wnd would be useful for helping > > diagnose whether a flow is receive-window-limited at a given instant. > > > > This serves the purpose of adding an additional __u32 to avoid the > > would-be hole caused by the addition of the tcpi_rcvi_ooopack field. > > > > Signed-off-by: Thomas Higdon <tph@fb.com> > > --- > > Thanks, Thomas. > > I know that when I mentioned this before I mentioned the idea of both > tp->snd_wnd (send-side receive window) and tp->rcv_wnd (receive-side > receive window) in tcp_info, and did not express a preference between > the two. Now that we are faced with a decision between the two, > personally I think it would be a little more useful to start with > tp->snd_wnd. :-) > > Two main reasons: > > (1) Usually when we're diagnosing TCP performance problems, we do so > from the sender, since the sender makes most of the > performance-critical decisions (cwnd, pacing, TSO size, TSQ, etc). > From the sender-side the thing that would be most useful is to see > tp->snd_wnd, the receive window that the receiver has advertised to > the sender. I am under the impression, that particularly in the mobile space, that network behavior is often governed by rcv_wnd. At least, there's been so many papers on this that I'd tended to assume so. Given a desire to do both vars, is there a *third* u32 we could add to fill in the next hole? :) ecn marks? > > (2) From the receiver side, "ss" can already show a fair amount of > info about receive-side buffer/window limits, like: > info->tcpi_rcv_ssthresh, info->tcpi_rcv_space, > skmeminfo[SK_MEMINFO_RMEM_ALLOC], skmeminfo[SK_MEMINFO_RCVBUF]. Often > the rwin can be approximated by combining those. > > Hopefully Eric, Yuchung, and Soheil can weigh in on the question of > snd_wnd vs rcv_wnd. Or we can perhaps think of another field, and add > the tcpi_rcvi_ooopack, snd_wnd, rcv_wnd, and that final field, all > together. > > thanks, > neal
On Thu, Sep 12, 2019 at 10:14:33AM +0100, Dave Taht wrote: > On Thu, Sep 12, 2019 at 1:59 AM Neal Cardwell <ncardwell@google.com> wrote: > > > > On Wed, Sep 11, 2019 at 6:32 PM Thomas Higdon <tph@fb.com> wrote: > > > > > > Neal Cardwell mentioned that rcv_wnd would be useful for helping > > > diagnose whether a flow is receive-window-limited at a given instant. > > > > > > This serves the purpose of adding an additional __u32 to avoid the > > > would-be hole caused by the addition of the tcpi_rcvi_ooopack field. > > > > > > Signed-off-by: Thomas Higdon <tph@fb.com> > > > --- > > > > Thanks, Thomas. > > > > I know that when I mentioned this before I mentioned the idea of both > > tp->snd_wnd (send-side receive window) and tp->rcv_wnd (receive-side > > receive window) in tcp_info, and did not express a preference between > > the two. Now that we are faced with a decision between the two, > > personally I think it would be a little more useful to start with > > tp->snd_wnd. :-) > > > > Two main reasons: > > > > (1) Usually when we're diagnosing TCP performance problems, we do so > > from the sender, since the sender makes most of the > > performance-critical decisions (cwnd, pacing, TSO size, TSQ, etc). > > From the sender-side the thing that would be most useful is to see > > tp->snd_wnd, the receive window that the receiver has advertised to > > the sender. > > I am under the impression, that particularly in the mobile space, that > network behavior > is often governed by rcv_wnd. At least, there's been so many papers on > this that I'd > tended to assume so. > > Given a desire to do both vars, is there a *third* u32 we could add to > fill in the next hole? :) > ecn marks? Neal makes some good points -- there is a fair amount of existing information for deriving receive window. It seems like snd_wnd would be more valuable at this moment. For the purpose of pairing up these __u32s to get something we can commit, I propose that we go with the rcv_ooopack/snd_wnd pair for now, and when something comes up later, one might consider pairing up rcv_wnd.
On Fri, Sep 13, 2019 at 10:29 AM Thomas Higdon <tph@fb.com> wrote: > > On Thu, Sep 12, 2019 at 10:14:33AM +0100, Dave Taht wrote: > > On Thu, Sep 12, 2019 at 1:59 AM Neal Cardwell <ncardwell@google.com> wrote: > > > > > > On Wed, Sep 11, 2019 at 6:32 PM Thomas Higdon <tph@fb.com> wrote: > > > > > > > > Neal Cardwell mentioned that rcv_wnd would be useful for helping > > > > diagnose whether a flow is receive-window-limited at a given instant. > > > > > > > > This serves the purpose of adding an additional __u32 to avoid the > > > > would-be hole caused by the addition of the tcpi_rcvi_ooopack field. > > > > > > > > Signed-off-by: Thomas Higdon <tph@fb.com> > > > > --- > > > > > > Thanks, Thomas. > > > > > > I know that when I mentioned this before I mentioned the idea of both > > > tp->snd_wnd (send-side receive window) and tp->rcv_wnd (receive-side > > > receive window) in tcp_info, and did not express a preference between > > > the two. Now that we are faced with a decision between the two, > > > personally I think it would be a little more useful to start with > > > tp->snd_wnd. :-) > > > > > > Two main reasons: > > > > > > (1) Usually when we're diagnosing TCP performance problems, we do so > > > from the sender, since the sender makes most of the > > > performance-critical decisions (cwnd, pacing, TSO size, TSQ, etc). > > > From the sender-side the thing that would be most useful is to see > > > tp->snd_wnd, the receive window that the receiver has advertised to > > > the sender. > > > > I am under the impression, that particularly in the mobile space, that > > network behavior > > is often governed by rcv_wnd. At least, there's been so many papers on > > this that I'd > > tended to assume so. > > > > Given a desire to do both vars, is there a *third* u32 we could add to > > fill in the next hole? :) > > ecn marks? > > Neal makes some good points -- there is a fair amount of existing > information for deriving receive window. It seems like snd_wnd would be > more valuable at this moment. For the purpose of pairing up these __u32s > to get something we can commit, I propose that we go with > the rcv_ooopack/snd_wnd pair for now, and when something comes up later, > one might consider pairing up rcv_wnd. FWIW that sounds like a great plan to me. Thanks, Thomas! neal
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h index 20237987ccc8..8a0d1d1af622 100644 --- a/include/uapi/linux/tcp.h +++ b/include/uapi/linux/tcp.h @@ -272,6 +272,7 @@ struct tcp_info { __u32 tcpi_reord_seen; /* reordering events seen */ __u32 tcpi_rcv_ooopack; /* Out-of-order packets received */ + __u32 tcpi_rcv_wnd; /* Receive window size */ }; /* netlink attributes types for SCM_TIMESTAMPING_OPT_STATS */ diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 4cf58208270e..c980145c4247 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -3297,6 +3297,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info) info->tcpi_dsack_dups = tp->dsack_dups; info->tcpi_reord_seen = tp->reord_seen; info->tcpi_rcv_ooopack = tp->rcv_ooopack; + info->tcpi_rcv_wnd = tp->rcv_wnd; unlock_sock_fast(sk, slow); } EXPORT_SYMBOL_GPL(tcp_get_info);
Neal Cardwell mentioned that rcv_wnd would be useful for helping diagnose whether a flow is receive-window-limited at a given instant. This serves the purpose of adding an additional __u32 to avoid the would-be hole caused by the addition of the tcpi_rcvi_ooopack field. Signed-off-by: Thomas Higdon <tph@fb.com> --- include/uapi/linux/tcp.h | 1 + net/ipv4/tcp.c | 1 + 2 files changed, 2 insertions(+)