-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: More migration tests and related fixes #2170
base: main
Are you sure you want to change the base?
Changes from 11 commits
b782d9d
63a1e22
e536595
3be6f96
46a4205
3d0b55e
eb958cd
ac247fa
88bdc9b
b61524e
91c0bb4
976ea0a
406cd12
bff2c92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -14,7 +14,7 @@ | |||||||||||||||||||||||
rc::Rc, | ||||||||||||||||||||||||
}; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
use neqo_common::{hex, hex_with_len, qinfo, Decoder, Encoder}; | ||||||||||||||||||||||||
use neqo_common::{hex, hex_with_len, qdebug, qinfo, Decoder, Encoder}; | ||||||||||||||||||||||||
use neqo_crypto::{random, randomize}; | ||||||||||||||||||||||||
use smallvec::{smallvec, SmallVec}; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
@@ -314,6 +314,10 @@ | |||||||||||||||||||||||
stats.new_connection_id += 1; | ||||||||||||||||||||||||
true | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
pub fn is_empty(&self) -> bool { | ||||||||||||||||||||||||
self.seqno == CONNECTION_ID_SEQNO_EMPTY || self.cid.is_empty() | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
impl ConnectionIdEntry<()> { | ||||||||||||||||||||||||
|
@@ -405,7 +409,7 @@ | |||||||||||||||||||||||
pub fn retire_prior_to(&mut self, retire_prior: u64) -> Vec<u64> { | ||||||||||||||||||||||||
let mut retired = Vec::new(); | ||||||||||||||||||||||||
self.cids.retain(|e| { | ||||||||||||||||||||||||
larseggert marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
if e.seqno < retire_prior { | ||||||||||||||||||||||||
if !e.is_empty() && e.seqno < retire_prior { | ||||||||||||||||||||||||
retired.push(e.seqno); | ||||||||||||||||||||||||
false | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
|
@@ -510,8 +514,18 @@ | |||||||||||||||||||||||
pub fn retire(&mut self, seqno: u64) { | ||||||||||||||||||||||||
// TODO(mt) - consider keeping connection IDs around for a short while. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
self.connection_ids.retire(seqno); | ||||||||||||||||||||||||
self.lost_new_connection_id.retain(|cid| cid.seqno != seqno); | ||||||||||||||||||||||||
let empty_cid = seqno == CONNECTION_ID_SEQNO_EMPTY | ||||||||||||||||||||||||
|| self | ||||||||||||||||||||||||
.connection_ids | ||||||||||||||||||||||||
.cids | ||||||||||||||||||||||||
.iter() | ||||||||||||||||||||||||
.any(|c| c.seqno == seqno && c.cid.is_empty()); | ||||||||||||||||||||||||
Comment on lines
+517
to
+522
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Do I understand correctly, that this would be functionally the same, but less efficient? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. (I'm not in love with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure. I mostly kept it in so the |
||||||||||||||||||||||||
if empty_cid { | ||||||||||||||||||||||||
qdebug!("Connection ID {seqno} is zero-length, not retiring"); | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
self.connection_ids.retire(seqno); | ||||||||||||||||||||||||
self.lost_new_connection_id.retain(|cid| cid.seqno != seqno); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/// During the handshake, a server needs to regard the client's choice of destination | ||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1536,7 +1536,7 @@ impl Connection { | |
/// This takes two times: when the datagram was received, and the current time. | ||
fn input(&mut self, d: Datagram<impl AsRef<[u8]>>, received: Instant, now: Instant) { | ||
// First determine the path. | ||
let path = self.paths.find_path_with_rebinding( | ||
let path = self.paths.find_path( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For others, relevant discussion happening on #2176 (review). |
||
d.destination(), | ||
d.source(), | ||
self.conn_params.get_cc_algorithm(), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RFC 9000 refers to this as "zero-length Connection ID", right? How about adopting that wording here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to calling these "empty", e.g.,
CONNECTION_ID_SEQNO_EMPTY
so I stuck with that. (You're right that that's different from the RFC.)