Subject: [quicwg/base-drafts] Clarify bytes_in_flight (#551)

From: ianswett <[email-masked]>
Date: Mon, 22 May 2017 13:03:40 -0700

Fixed Issue #550 
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/551

-- Commit Summary --

  * Clarify bytes_in_flight

-- File Changes --

    M draft-ietf-quic-recovery.md (2)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/551.patch
https://github.com/quicwg/base-drafts/pull/551.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/551

---

From: Martin Thomson <[email-masked]>
Date: Mon, 22 May 2017 17:18:43 -0700

martinthomson commented on this pull request.

> @@ -615,6 +615,8 @@ are described in this section.
 bytes_in_flight:
 : The sum of the size in bytes of all sent packets that contain at least
   one retransmittable frame, and have not been acked or declared lost.
+  The size does not include IP or UDP overhead.  Ack only frames do not
+  count towards byte_in_flight.

Ouch, that's kinda ugly.  You are saying that if you include a PING, plus a 999 octet ACK, then you have 1000 octets in flight, but drop the ping and you have 0.  I wonder what impact that has on traffic analysis.

I understand that TCP doesn't do this, but is there any reason why we need to copy that?  A packet that is sent is still creating congestion.

In any case, I think that this needs to be clearer.  This counts the entire QUIC packet, including the header and any overhead added by packet protection or integrity.  QUIC packets that only contain ACK frames are not counted.

What - if any - impact does PADDING have on this?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/551#pullrequestreview-39615529

---

From: ianswett <[email-masked]>
Date: Mon, 22 May 2017 18:05:04 -0700

ianswett commented on this pull request.

 bytes_in_flight:

We don't need to copy what TCP does, but I thought it makes sense to document a reasonable option, and make that similar to TCP.  

I'll also note there's not much evidence we have to agree on the same bytes_in_flight accounting across implementations.  The most important thing is internal consistency.

The reason acks shouldn't count towards bytes in flight is one never wants to delay an ack due to lack of congestion window.  We could say they count, but you send them immediately anyway, but I think it's equally ugly.

Good point on PADDING, because they are not retransmittable, but we do count them towards bytes in flight in the GQUIC implementation, and I tend to think they should be counted.

So I'd say we have three options I can think of: 
1) Land an updated version of this in order to document a TCP like approach and discuss this further at the interim.
2) Do nothing, and hope we get to this at the interim.
3) Do nothing, because we're happy letting implementations do what they want, just like TCP implementations do.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/551#discussion_r117878386

---

From: Martin Thomson <[email-masked]>
Date: Mon, 22 May 2017 18:14:03 -0700

bytes_in_flight:

> The reason acks shouldn't count towards bytes in flight is one never wants to delay an ack due to lack of congestion window.

You can count acknowledgment-only packets, and it will only cause bytes_in_flight to hit the critical threshold faster.  If you want to send an ACK without paying attention to the congestion window, then you could move the special logic to the sending of a packet:

```
if (!congested) {
  packet.send();
} else if (packet.contains(ACK)) {
  packet.strip_everything_except(ACK);
  packet.send();
}
```

I think that we should probably discuss at some point.  I have no problem with landing something that is more concrete than what we currently have in the meantime.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/551#discussion_r117879147

---

From: mirjak <[email-masked]>
Date: Thu, 25 May 2017 09:53:28 -0700

mirjak commented on this pull request.

 bytes_in_flight:

Isn't this basically the same question as the question which packets should be congestion controlled? All bytes that are congestion controlled should be counted for bytes_in_flight, no?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/551#discussion_r118530298

---

From: ianswett <[email-masked]>
Date: Thu, 25 May 2017 10:04:57 -0700

bytes_in_flight:

That is most of it, though if we wanted to include ack-only packets, it would be important to allow those to be sent immediately, even if congestion control indicated no data should be sent.

Martin, in regards to the pseudocode, I think it's simpler to think of not adding non-ACK data to packets if you're congested, rather than adding it and stripping it out.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/551#discussion_r118532920

---

From: Martin Thomson <[email-masked]>
Date: Thu, 25 May 2017 18:05:22 -0700

bytes_in_flight:

You are right, you probably want to send an ACK on a separate code path:

```
def send_ack():
  make_packet([construct_ack()]).send_direct();

def send():
  if congestion_window > 0:
    ack = construct_ack()
    other_frames = make_frames(min(congestion_window, pmtu) - len(ack))
    make_packet(ack + other_frames).send_direct();
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/551#discussion_r118617064