Re: [PATCH net-next 05/14] tcp: track data delivery rate for a TCP connection

[Neal Cardwell]

On Fri, Sep 16, 2016 at 5:38 PM, kbuild test robot  wrote:
> Hi Yuchung,
>
> [auto build test WARNING on net-next/master]
> All warnings (new ones prefixed by >>):
>
>In file included from net/ipv4/route.c:103:0:
>>> include/net/tcp.h:769:11: warning: 'packed' attribute ignored for field of 
>>> type 'struct skb_mstamp' [-Wattributes]
>struct skb_mstamp first_tx_mstamp __packed;
>   ^~
>include/net/tcp.h:771:11: warning: 'packed' attribute ignored for field of 
> type 'struct skb_mstamp' [-Wattributes]
>struct skb_mstamp delivered_mstamp __packed;
>   ^~

We have a fix for this, and we'll post it with the v2 series.

thanks,
neal

Re: [PATCH net-next 05/14] tcp: track data delivery rate for a TCP connection

[kbuild test robot]

Hi Yuchung,

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Neal-Cardwell/tcp-BBR-congestion-control-algorithm/20160917-025323
config: cris-etrax-100lx_v2_defconfig (attached as .config)
compiler: cris-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=cris 

All warnings (new ones prefixed by >>):

   In file included from net/ipv4/route.c:103:0:
>> include/net/tcp.h:769:11: warning: 'packed' attribute ignored for field of 
>> type 'struct skb_mstamp' [-Wattributes]
   struct skb_mstamp first_tx_mstamp __packed;
  ^~
   include/net/tcp.h:771:11: warning: 'packed' attribute ignored for field of 
type 'struct skb_mstamp' [-Wattributes]
   struct skb_mstamp delivered_mstamp __packed;
  ^~

vim +769 include/net/tcp.h

   753  #define TCPCB_TAGBITS   0x07/* All tag bits 
*/
   754  #define TCPCB_REPAIRED  0x10/* SKB repaired (no skb_mstamp) 
*/
   755  #define TCPCB_EVER_RETRANS  0x80/* Ever retransmitted frame 
*/
   756  #define TCPCB_RETRANS   
(TCPCB_SACKED_RETRANS|TCPCB_EVER_RETRANS| \
   757  TCPCB_REPAIRED)
   758  
   759  __u8ip_dsfield; /* IPv4 tos or IPv6 dsfield 
*/
   760  __u8txstamp_ack:1,  /* Record TX timestamp for ack? 
*/
   761  eor:1,  /* Is skb MSG_EOR marked? */
   762  unused:6;
   763  __u32   ack_seq;/* Sequence number ACK'd
*/
   764  union {
   765  struct {
   766  /* There is space for up to 24 bytes */
   767  __u32 in_flight;/* Bytes in flight when packet 
sent */
   768  /* start of send pipeline phase */
 > 769  struct skb_mstamp first_tx_mstamp __packed;
   770  /* when we reached the "delivered" count */
   771  struct skb_mstamp delivered_mstamp __packed;
   772  /* pkts S/ACKed so far upon tx of skb, incl 
retrans: */
   773  __u32 delivered;
   774  } tx;   /* only used for outgoing skbs */
   775  union {
   776  struct inet_skb_parmh4;
   777  #if IS_ENABLED(CONFIG_IPV6)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH net-next 05/14] tcp: track data delivery rate for a TCP connection

[Neal Cardwell]

From: Yuchung Cheng 

This patch generates data delivery rate (throughput) samples on a
per-ACK basis. These rate samples can be used by congestion control
modules, and specifically will be used by TCP BBR in later patches in
this series.

Key state:

tp->delivered: Tracks the total number of data packets (original or not)
   delivered so far. This is an already-existing field.

tp->delivered_mstamp: the last time tp->delivered was updated.

Algorithm:

A rate sample is calculated as (d1 - d0)/(t1 - t0) on a per-ACK basis:

  d1: the current tp->delivered after processing the ACK
  t1: the current time after processing the ACK

  d0: the prior tp->delivered when the acked skb was transmitted
  t0: the prior tp->delivered_mstamp when the acked skb was transmitted

When an skb is transmitted, we snapshot d0 and t0 in its control
block in tcp_rate_skb_sent().

When an ACK arrives, it may SACK and ACK some skbs. For each SACKed
or ACKed skb, tcp_rate_skb_delivered() updates the rate_sample struct
to reflect the latest (d0, t0).

Finally, tcp_rate_gen() generates a rate sample by storing
(d1 - d0) in rs->delivered and (t1 - t0) in rs->interval_us.

One caveat: if an skb was sent with no packets in flight, then
tp->delivered_mstamp may be either invalid (if the connection is
starting) or outdated (if the connection was idle). In that case,
we'll re-stamp tp->delivered_mstamp.

At first glance it seems t0 should always be the time when an skb was
transmitted, but actually this could over-estimate the rate due to
phase mismatch between transmit and ACK events. To track the delivery
rate, we ensure that if packets are in flight then t0 and and t1 are
times at which packets were marked delivered.

If the initial and final RTTs are different then one may be corrupted
by some sort of noise. The noise we see most often is sending gaps
caused by delayed, compressed, or stretched acks. This either affects
both RTTs equally or artificially reduces the final RTT. We approach
this by recording the info we need to compute the initial RTT
(duration of the "send phase" of the window) when we recorded the
associated inflight. Then, for a filter to avoid bandwidth
overestimates, we generalize the per-sample bandwidth computation
from:

bw = delivered / ack_phase_rtt

to the following:

bw = delivered / max(send_phase_rtt, ack_phase_rtt)

In large-scale experiments, this filtering approach incorporating
send_phase_rtt is effective at avoiding bandwidth overestimates due to
ACK compression or stretched ACKs.

Signed-off-by: Van Jacobson 
Signed-off-by: Neal Cardwell 
Signed-off-by: Yuchung Cheng 
Signed-off-by: Nandita Dukkipati 
Signed-off-by: Eric Dumazet 
Signed-off-by: Soheil Hassas Yeganeh 
---
 include/linux/tcp.h   |   2 +
 include/net/tcp.h |  35 +++-
 net/ipv4/Makefile |   2 +-
 net/ipv4/tcp.c|   5 ++
 net/ipv4/tcp_input.c  |  46 +++-
 net/ipv4/tcp_output.c |   4 ++
 net/ipv4/tcp_rate.c   | 149 ++
 7 files changed, 227 insertions(+), 16 deletions(-)
 create mode 100644 net/ipv4/tcp_rate.c

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 38590fb..c50e6ae 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -268,6 +268,8 @@ struct tcp_sock {
u32 prr_out;/* Total number of pkts sent during Recovery. */
u32 delivered;  /* Total data packets delivered incl. rexmits */
u32 lost;   /* Total data packets lost incl. rexmits */
+   struct skb_mstamp first_tx_mstamp;  /* start of window send phase */
+   struct skb_mstamp delivered_mstamp; /* time we reached "delivered" */
 
u32 rcv_wnd;/* Current receiver window  */
u32 write_seq;  /* Tail(+1) of data held in tcp send buffer */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 2f1648a..4a94f64 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -763,8 +763,14 @@ struct tcp_skb_cb {
__u32   ack_seq;/* Sequence number ACK'd*/
union {
struct {
-   /* There is space for up to 20 bytes */
+   /* There is space for up to 24 bytes */
__u32 in_flight;/* Bytes in flight when packet sent */
+   /* start of send pipeline phase */
+   struct skb_mstamp first_tx_mstamp __packed;
+   /* when we reached the "delivered" count */
+   struct skb_mstamp delivered_mstamp __packed;
+   /* pkts S/ACKed so far upon tx of skb, incl retrans: */
+   __u32 delivered;
} tx;   /* only used for outgoing skbs */
union {
struct