Subject: [quicwg/base-drafts] Question on ECN validation (#3733)

From: Lars Eggert <[email-masked]>
Date: Fri, 05 Jun 2020 05:14:32 -0700

So, I am finally implementing this, and have questions. Section 13.4.2.2 says:
```
   *  For validation to succeed, the total increase in ECT(0), ECT(1),
      and CE counts MUST be no smaller than the total number of QUIC
      packets sent with an ECT codepoint that are newly acknowledged in
      this ACK frame.  This step detects any network remarking from
      ECT(0), ECT(1), or CE codepoints to Not-ECT.
```
What is meant by "total increase" here? I guess it is a delta, but against what? Against the values in the last received ACK frame? Against the "local reference counts" (which aren't really described well either, see below)? Or something else?

Also, I guess the "total increase" compared here is a **sum** of these individual deltas?

```
   *  Any increase in either ECT(0) or ECT(1) counts, plus any increase
      in the CE count, MUST be no smaller than the number of packets
      sent with the corresponding ECT codepoint that are newly
      acknowledged in this ACK frame.  This step detects any erroneous
      network remarking from ECT(0) to ECT(1) (or vice versa).
```
Again, "increase" relative to what?
```
   An endpoint could miss acknowledgements for a packet when ACK frames
   are lost.  It is therefore possible for the total increase in ECT(0),
   ECT(1), and CE counts to be greater than the number of packets
   acknowledged in an ACK frame.  When this happens, and if validation
   succeeds, the local reference counts MUST be increased to match the
   counts in the ACK frame.
```
Same question about "total increase". And what are the "local reference counts", and how are they maintained?

-- 
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/issues/3733

---

From: mirjak <[email-masked]>
Date: Fri, 05 Jun 2020 06:08:48 -0700

You are supposed to keep three counters for the number of each ECT(0) and ECT(1) and CE mark reported so far on this connection (for this ack space). Increase means the difference of the count reported in the latest ACK compared to the current local counter value. Total increase is increase of ECT(0)+increase of ECT(1)+increase if CE compared to local current count of ECT(0)+ECT(1)+CE. Does that help?

-- 
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/issues/3733#issuecomment-639469310

---

From: Lars Eggert <[email-masked]>
Date: Fri, 05 Jun 2020 06:14:10 -0700

It does, thanks. it would probably be useful to introduce those local reference counts before the bullet list, describe how that are maintained, and then use them when describing how to validate.

-- 
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/issues/3733#issuecomment-639472177

---

From: mirjak <[email-masked]>
Date: Fri, 05 Jun 2020 06:15:33 -0700

Yes. Will try to propose a PR. Thanks for implementing!

-- 
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/issues/3733#issuecomment-639472908

---

From: Lars Eggert <[email-masked]>
Date: Fri, 05 Jun 2020 06:19:55 -0700

Actually, this doesn't make sense to me:
> Total increase is increase of ECT(0)+increase of ECT(1)+increase if CE compared to local current count of ECT(0)+ECT(1)+CE. 

The increase is the value in the current ACK frame minus the current local reference value. A sum of those increases is always going to be less than the sum of the total reference values?

-- 
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/issues/3733#issuecomment-639475264

---

From: mirjak <[email-masked]>
Date: Fri, 05 Jun 2020 06:22:54 -0700

Yes, sorry, in the text you don't compare the total increase to the local value of course. You compare the total increase to the number of packets  acknowledged in an ACK frame

-- 
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/issues/3733#issuecomment-639476779

---

From: mirjak <[email-masked]>
Date: Fri, 05 Jun 2020 06:25:31 -0700

and then you also compare the increase of ECT(0)+increase in CE to number of packet send with ECT(0) from the newly acknowledge packets, and same for ETC(1). So you need to remember with packet was sent with which ECN codepoint if not all packets are send with the same codepoint.

-- 
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/issues/3733#issuecomment-639478209

---

From: Lars Eggert <[email-masked]>
Date: Mon, 08 Jun 2020 03:11:35 -0700

So let me tell you what my code is doing now to verify incoming ACK-ECNs, and then you tell me if this is correct:

I maintain ECN reference counter vectors (`ecn-ref`) per pn space. These hold the last counts received in an ACK-ECN from the peer.

I could new ACKs for ECN-marked packets that I send in a `new_ack`vector. (I only send ECN_ECT0 packets though, so this isn't really a vector, just a variable.)

When I receive a new ACK-ECN with ECN counts (`ecn_cnt`), I compute the `ecn_inc` vector as `ecn_ref - ecn_cnt`.

I then do the verification per Section 13.4.2.2:

* (First bullet in 13.4.2.2 is easy, omitted here)

* Second bullet in 13.4.2.2: `ecn_inc[ECN_ECT0] + ecn_inc[ECN_ECT1] + ecn_inc[ECN_CE] <  new_ack[ECN_ECT0]` => verification failed

* Third bullet in 13.4.2.2: `ecn_inc[ECN_ECT0] + ecn_cnt[ECN_CE] < new_ack[ECN_ECT0]` => verification failed. (Since I only send ECN_ECT0, I can omit looking at ECN_ACT1, I believe.)

-- 
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/issues/3733#issuecomment-640509278

---

From: Lars Eggert <[email-masked]>
Date: Mon, 15 Jun 2020 04:19:44 -0700

@mirjak ping?

-- 
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/issues/3733#issuecomment-644067834

---

From: mirjak <[email-masked]>
Date: Mon, 15 Jun 2020 05:47:08 -0700

Sorry, long weekend in Germany. Yep this sounds right. If you only have ECT(0), you can simplify test 2 to just do what you do in 3 ('ecn_inc[ECN_ECT0] + ecn_cnt[ECN_CE] < new_ack[ECN_ECT0]') and for test 3 you just need to check if there are any ECT(1) marking at all and fail if you ever get feedback that ECT(1) was observed.

-- 
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/issues/3733#issuecomment-644113261

---

From: Martin Thomson <[email-masked]>
Date: Mon, 15 Jun 2020 15:58:35 -0700

Maybe use <= rather than < though.

-- 
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/issues/3733#issuecomment-644432136

---

From: Lars Eggert <[email-masked]>
Date: Tue, 16 Jun 2020 04:53:29 -0700

@martinthomson I'm not sure that's correct?

-- 
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/issues/3733#issuecomment-644715268

---

From: mirjak <[email-masked]>
Date: Tue, 16 Jun 2020 05:20:29 -0700

Oh yes, didn't see that. I also think it should be <= . @larseggert why do think it should be < ?

-- 
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/issues/3733#issuecomment-644727523

---

From: Lars Eggert <[email-masked]>
Date: Tue, 16 Jun 2020 05:24:10 -0700

Because if the number of marked packets in the ACK is == the number of marked packets I sent, that is not a verification failure? (Or am I misunderstanding something?)

-- 
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/issues/3733#issuecomment-644729192

---

From: mirjak <[email-masked]>
Date: Tue, 16 Jun 2020 12:53:27 +0000 (UTC)

Right... I got confused that the condition is about moving into failure mode. 

So usually the numbers should always be exactly equal; only if some marked packets get not ack'ed at all the received count could be larger than the number of marked sent and ack'ed packet. I guess, if you know that this is not the case you could be even stricter.

Otherwise if there are no unack'ed packets and the received count would be larger, it would mean you have some remarking from Not-ECT to ECT or CE. That less a problem in terms of hiding congestion but might still indicate that something is wrong... however, given that you mark all packet in your implementation with ECT(0) that case should not happen.

-- 
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/issues/3733#issuecomment-644743168

---

From: Magnus Westerlund <[email-masked]>
Date: Tue, 16 Jun 2020 05:56:35 -0700

And as noted the primary failure mode where increase of reference counter can be larger than newely ACKed packets is lost ACKs. Section 13.4.1 is clear on that duplicate packets shall be surpressed prior to increasing the relevant ECN counter.

-- 
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/issues/3733#issuecomment-644744894

---

From: Martin Thomson <[email-masked]>
Date: Tue, 23 Jun 2020 15:37:18 -0700

Closed #3733 via #3777.

-- 
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/issues/3733#event-3475068053