1
0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-11-09 10:00:57 +03:00
Commit Graph

133 Commits

Author SHA1 Message Date
Yang Chi
264203b87d Remove duplicate Quic logging of PARSE_ERROR in 0-rtt buffered case
Summary:
When parsePacket returns anything other than RegularPacket, we already
log the Drop even inside the switch-case blocks before checking if
RegularPacket has been parsed. So the logging of Drop with PARSE_ERROR when the
CodecResult isn't RegularPacket is wrong. For example, right now when we
buffer 0-rtt data, we always log a Drop immediatly afterwards into QLog which
is incorrect.

Reviewed By: JunqiWang

Differential Revision: D20421934

fbshipit-source-id: d836700fd691645951d5e5b49b19cbcc1e5df51a
2020-03-13 11:32:40 -07:00
Matt Joras
d1a3652a4c Iterate QuicVersion::MVFST
Summary:
This iterates the mvfst version to be semantically equivalent to draft-27, and leaves support for the old mvfst version.

The client will not yet be moved to draft-27 by default.

Reviewed By: lnicco

Differential Revision: D20182452

fbshipit-source-id: 1e11ad7296a6cd8d15ca5ed359d9ed82af79bb17
2020-03-04 22:08:34 -08:00
Matt Joras
3b2ba3fe8b Implement handshake done
Summary: This is without cipher dropping, but the frame is parseable and the server will send it at the correct time.

Reviewed By: yangchi, lnicco

Differential Revision: D20235013

fbshipit-source-id: 696c11ec573a530b3ed9f4185a2f6847ee08819f
2020-03-04 22:08:33 -08:00
Matt Joras
61cd1a7289 Back out "Implement handshake done and cipher dropping."
Summary: This caused an increase in client errors.

Reviewed By: yangchi, lnicco

Differential Revision: D20186386

fbshipit-source-id: 737122a94c97498efba61292a6c292cfe482925c
2020-03-01 18:31:40 -08:00
Matt Joras
2b3b76cc4d Remove support for MVFST_OLD.
Summary:
This eliminatees some tech debt by completely removing the notion of version from the core transport parameters structure and the app token for zero rtt.

Note that for the draft-27 changes we will need to temporarily re-introduce it, but to a different layer (the extension encoding itself).

Reviewed By: JunqiWang

Differential Revision: D20073578

fbshipit-source-id: 2b55af621566bf1c20e21dd17251116de1788fa0
2020-02-28 09:52:34 -08:00
Matt Joras
472e40a902 Implement handshake done and cipher dropping.
Summary: This implements the handshake done signal and also cipher dropping.

Reviewed By: yangchi

Differential Revision: D19584922

fbshipit-source-id: a98bec8f1076393b051ff65a2d8aae7d572b42f5
2020-02-27 12:25:52 -08:00
TJ Yin
a396f62335 Replace folly::Optional::hasValue() by has_value()
Differential Revision: D19882830

fbshipit-source-id: 031217f9890351022bc8d171f0ccd7e045dd6972
2020-02-26 08:40:44 -08:00
Yang Chi
04e487dc71 Move MockQuicStates to quic/state/test directory
Summary:
our convention has always been to put the mock in the test dir under
the real class

Reviewed By: lnicco

Differential Revision: D20104476

fbshipit-source-id: 5215ffc9af7a6d7a5ac41109723a71f68f852af7
2020-02-25 19:02:59 -08:00
Matt Joras
6827637d45 Use NiceMock in more tests
Summary: When we don't use NiceMock we end up with a ton of spam in failing tests for every callback that we didn't EXPECT. This makes failed test output extremely noisy.

Reviewed By: sharmafb

Differential Revision: D19977113

fbshipit-source-id: 1a083fba13308cd3f2859da364c8106e349775bb
2020-02-19 21:46:46 -08:00
TJ Yin
73a6de2488 Replace folly::Optional::clear() by reset()
Reviewed By: yfeldblum

Differential Revision: D19953637

fbshipit-source-id: c2135fc24084b7bba0c9d6fc978c8b6c8f9943d4
2020-02-19 12:11:47 -08:00
Amaury Séchet
2f8774c921 Only take a QuicServerConnectionState when constructing ServerHandshake (#90)
Summary:
The cryptoState can be derrived from it, so it is redundant to pass it.

This PR depends on https://github.com/facebookincubator/mvfst/issues/88
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/90

Reviewed By: mjoras

Differential Revision: D19622581

Pulled By: yangchi

fbshipit-source-id: b3c88999199673f69c4422b96fbe7f2b0656bf6c
2020-02-12 09:42:18 -08:00
Andrejs Krasilnikovs
8114c3e3d0 Changed last packet sent times to bet per-packet-number-space andthe PTO timer calculation from them
Summary:
This diff:
1) introduces `EnumArray` - effectively an `std::array` indexed by an enum
2) changes loss times and `lastRetransmittablePacketSentTime` inside `LossState`  to be an `EnumArray` indexed by `PacketNumberSpace`
3) makes the method `isHandshakeDone()` available for both client and server handshakes.
4) uses all those inputs to determine PTO timers in `earliestTimeAndSpace()`

Reviewed By: yangchi

Differential Revision: D19650864

fbshipit-source-id: d72e4a0cf61d2dcb76f0a7f4037c36a7c8156942
2020-02-10 12:28:43 -08:00
Amaury Séchet
e6e6196c86 Move the delayed destruction from Handshake to QuicConnectionStateBase
Summary: Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/88

Test Plan:
Imported from GitHub, without a `Test Plan:` line.

 ---
## Proxygen Canary
Traffic Canary: https://our.intern.facebook.com/intern/traffic/canary?fbid=224323975233396
* elb.prod.ham3c01 - binary_asan - 2020-02-05 02:00 - https://fburl.com/dyndash/u2q12hwq
* elb.prod.mia3c02 - binary - 2020-01-31 09:40 - https://fburl.com/dyndash/vmv34rpa
* elb.prod.otp1c01 - binary - 2020-02-05 02:26 - https://fburl.com/dyndash/0zttm61b
* flb.prod.fath4c02 - binary - 2020-02-05 02:26 - https://fburl.com/dyndash/6o1nqsti
* flb.prod.fgye3c01 - binary - 2020-01-31 09:40 - https://fburl.com/dyndash/nu3i5ahw
* olb.prod.rfrc0c01.p2 - binary - 2020-01-31 09:40 - https://fburl.com/dyndash/c1o6hpqw
* olb.prod.rftw0c01.p2 - binary - 2020-02-05 02:26 - https://fburl.com/dyndash/xg6qbyru
* slb.prod_regional.rcln0c00 - binary - 2020-02-05 02:26 - https://fburl.com/dyndash/e4qkbzcz
* slb.prod_regional.rfrc0c00 - binary - 2020-01-31 09:40 - https://fburl.com/dyndash/j0yxofty
* slb.prod_regional.rrva0c00 - binary_asan - 2020-02-05 02:00 - https://fburl.com/dyndash/4hsg02uj
* slb.regional.rfrc0c01.p2 - binary - 2020-01-31 09:40 - https://fburl.com/dyndash/1njxzbgf
* slb.regional.rvll0c01.p2 - binary - 2020-02-05 02:26 - https://fburl.com/dyndash/056xdmzn
 ---

Reviewed By: lnicco

Differential Revision: D19551142

Pulled By: mjoras

fbshipit-source-id: b0d14146d14384b8c37887b3e9d8fed7d6181d88
2020-02-05 06:13:33 -08:00
Yang Chi
2405111e88 New QuicSocket::ConnectionCallback for first peer packet processed
Summary: as title

Reviewed By: JunqiWang

Differential Revision: D19434037

fbshipit-source-id: 7f2b6491fcce68840dc6ff57f99657dd5d535e9f
2020-01-18 13:36:04 -08:00
Matt Joras
431acc838f Optimize ACK frame writing
Summary:
Previously we stored an `IntervalSet` in each `WriteAckFrame`. We don't need to do this, as by the time we are writing an `ACK` the `IntervalSet` is complete. Instead of bothering with the `IntervalSet` operations, we can simply serialize it to a reverse-sorted `vector.`

Additionally this has some micro-optimizations for filling the ACK frame, with a new function for getting varint size.

Reviewed By: yangchi

Differential Revision: D19397728

fbshipit-source-id: ba6958fb36a4681edaa8394b1bcbbec3472e177d
2020-01-16 10:40:40 -08:00
Udip Pant
74f98d4604 Move the fizz code into its own package
Summary:
This moves the fizz specific part of the handshake into its own folder and library.

There is a bit of smurf naming going on as a result, not sure it is worth fixing or not at this stage. Maybe this code should be a in namespace named quic::fizz .

This should be doable with the client as well as soon as the key cache situation is figured out.
 ---
## Proxygen Canary

Reviewed By: yangchi

Differential Revision: D19290919

fbshipit-source-id: 48d7f7c70db42c65f7dffe3256805c268a481198
2020-01-09 20:59:54 -08:00
Mirko Andjic
3f419b2eb2 Add vantage point to qlog.
Summary: Landing an outstanding diff

Reviewed By: yangchi

Differential Revision: D19261395

fbshipit-source-id: 437461222ff04f5c3271567d3bb064bceaf80029
2020-01-07 11:19:13 -08:00
Yang Chi
332b3c340c Use original server cid chosen by client for source addr based routing
Summary:
Currently, before server generate the destination CID, we route packets with client's address, port and client's source connection ID. But now that client can use 0-len source connection ID, the different connections from the same client address and port will be routed to the same server connections.

This diff changes it to use client's initial destination connection ID as part of the routing key.

Reviewed By: udippant

Differential Revision: D19268354

fbshipit-source-id: 837f5bd2f1e3a74957afacf7aabad922b1719219
2020-01-06 08:58:12 -08:00
Yang Chi
d7d19c74b5 Stop tracking pure ack packets in Quic
Summary:
Previously we track them since we thought we can get some additional
RTT samples. But these are bad RTT samples since peer can delays the acking of
pure acks. Now we no longer trust such RTT samples, there is no reason to keep
tracking pure ack packets.

Reviewed By: mjoras

Differential Revision: D18946081

fbshipit-source-id: 0a92d88e709edf8475d67791ba064c3e8b7f627a
2019-12-12 13:20:09 -08:00
Matt Joras
f041ec17ef Use folly::small_vector for ack blocks
Summary: By modifying `IntervalSet` a bit we can make it so it takes a `folly::small_vector` as the container. We expect that for real traffic there will not generally be a lot of ACK blocks per frame, so optimize for that.

Reviewed By: siyengar

Differential Revision: D18919975

fbshipit-source-id: 199a2ea9ba5003382e2d7d99fc7a6de7e8aafdca
2019-12-12 12:06:31 -08:00
Raghu Nallamothu
e06de27550 Merge Application Close Frame and Connection Close Frame
Summary: As a part of Draft 17, application close frame has been removed, we use connection close frame to represent both application close and connection close.

Reviewed By: mjoras

Differential Revision: D18580856

fbshipit-source-id: d274fa2d3dbc59b926bca5a2b8a20328ae582703
2019-12-03 15:02:06 -08:00
Yang Chi
3c314e04c0 Remove unused variable in QuicServerTransportTest
Summary: got a warning in cmake build

Reviewed By: JunqiWang

Differential Revision: D18765111

fbshipit-source-id: f19dc5948622e6ad065fb81dd0836f265e59769f
2019-12-02 11:27:41 -08:00
Matt Joras
b6e134fdee Use F14FastMap as the retranmission buffer.
Summary:
The retransmission buffer tracks stream frame data we have sent that is currently unacked. We keep this as a sorted `deque`. This isn't so bad for performance, but we can do better if we break ourselves of the requirement that it be sorted (removing a binary search on ACK).

To do this we make the buffer a map of offset -> `StreamBuffer`.

There were two places that were dependent on the sorted nature of the list.

1. For partial reliablity we call `shrinkBuffers` to remove all unacked buffers less than an offset. For this we now have to do it with a full traversal of the retransmission buffer instead of only having to do an O(offset) search. In the future we could make this better by only lazily deleting from the retransmission buffer on ACK or packet loss.

2. We used the start of the retransmission buffer to determine if a delivery callback could be fired for a given offset. We need some new state to track this. Instead of tracking unacked buffers, we now track acked ranges using the existing `IntervalSet`. This set should be small for the typical case, as we think most ACKs will come in order and just cause existing ranges to merge.

Reviewed By: yangchi

Differential Revision: D18609467

fbshipit-source-id: 13cd2164352f1183362be9f675c1bdc686426698
2019-11-27 00:25:25 -08:00
Viktor Chynarov
820bc48daf Fail PathChallenge if no available peer cids
Summary:
An endpoint receiving a PathChallenge will throw a
TransportException if they don't have have enough remaining
peer connection ids.

* Update PathChallenge tests

Reviewed By: JunqiWang

Differential Revision: D18600143

fbshipit-source-id: 933d62d022912754d2829cd6ff8a29bf7e57e82f
2019-11-26 03:42:18 -08:00
Viktor Chynarov
5cb4c461e7 Add method to rotate peer cid [1/x]
Summary:
Allows both client/server to update its peer connection id based on available ids.
This method returns false if there are no available ids.

Reviewed By: JunqiWang

Differential Revision: D18575129

fbshipit-source-id: 0e6969354ee9941d44f6533c003546955e11d098
2019-11-26 03:42:18 -08:00
Viktor Chynarov
fa022633a5 Store new connection id frame stateless reset tokens
Summary: ^

Reviewed By: JunqiWang

Differential Revision: D18620398

fbshipit-source-id: 26fee4e6d26df379a1f44b0704bc71311feea2f6
2019-11-21 09:19:37 -08:00
Viktor Chynarov
20807a350d Exchange active_connection_id_limit in transport parameters [2/2]
Summary:
Client will set their active_connection_id_limit to the server as 7 (so it will
have 8 conn ids in total).

Reviewed By: JunqiWang

Differential Revision: D18532441

fbshipit-source-id: b0be65cec9f7c483469b0b4a2810bc370a6945c3
2019-11-20 08:46:56 -08:00
Viktor Chynarov
3a6dfb73c3 Fix naming of self/peer active_connection_id_limit parameters [1/2]
Summary:
It was hard to understand which names refer to which limits.

This diff makes it simple:
* conn.transportSettings.selfActiveConnectionIdLimit
represents how many of its peer's connection ids it will hold. It sends this to
the peer as the active_connection_id_limit
* conn.peerActiveConnectionIdLimit represents the value the peer sends as its
active_connection_id_limit. This should be defaulted to 0 and only changed
when we receive the transport parameters

Reviewed By: udippant

Differential Revision: D18531733

fbshipit-source-id: 53709ccaa58f835fd654ac28cdd740be46e65289
2019-11-20 08:46:56 -08:00
Aman Sharma
69ac8aeb62 De-templatize stream state machine logic
Summary: The state machine logic is quite abstruse, this modifies it to make it more readable.

Reviewed By: siyengar

Differential Revision: D18488301

fbshipit-source-id: c6fd52973880931e34904713e8b147f56d0c4629
2019-11-19 20:18:11 -08:00
Viktor Chynarov
7504453972 Use Path Rate Limiter for conn migration
Summary:
Use the Path rate limiter introduced in the previous diff.

When we initialize path validation of an unvalidated peer address,
enable pathValidationRateLimit.

When we receive a proper PATH_RESPONSE frame, disable this limit.

If this limit is enabled, we will check the pathValidationLimiter for
the amount of bytes we are allowed to write.

Change the migration tests in QuicServerTransportTest to use this new limiter
instead of writableByteLimits.

Update shouldWriteData to directly use the new congestionControlWritableBytes
function.

Reviewed By: yangchi

Differential Revision: D18145774

fbshipit-source-id: 1fe4fd5be7486077c58b0d1285dfb03f6c62831c
2019-11-14 13:48:17 -08:00
Raghu Nallamothu
b62c11b841 T45316552 [quic][state] Initial Rtt measurement on Connection Migration
Summary: On Connection Migation use the time taken from pathchallenge frame to path response frame as initial rtt.

Reviewed By: JunqiWang

Differential Revision: D18388324

fbshipit-source-id: ede10484c240ca44a4eab544504461c052143bad
2019-11-11 12:17:12 -08:00
Junqi Wang
0fced3e95c No migration when 0-RTT packets come from different peer
Summary: 0-RTT packet is also in AppData pn space

Reviewed By: vchynarov

Differential Revision: D18353017

fbshipit-source-id: a39fe5aae823fbe0d7522f895cff05cc9395928b
2019-11-11 10:21:13 -08:00
Junqi Wang
52de3eb832 Reset mrtt on connection migration
Reviewed By: yangchi

Differential Revision: D18397845

fbshipit-source-id: ce1c7bcf4495b1a937892589c4c4131288f7d28f
2019-11-08 09:10:12 -08:00
Junqi Wang
a87bbc6fa8 Clear pending path challenge frame when migrating again
Summary:
When server has a pending path challenge frame to be sent, it's
possible that client can migrate again. This diff handles that case.

Reviewed By: vchynarov

Differential Revision: D18351213

fbshipit-source-id: c6887db0274ccddf267eb227de0a7de7ee103b4c
2019-11-06 10:51:56 -08:00
Viktor Chynarov
f2e09a4ab8 Server to issue up to 7 more conn ids on handshake finish [3/x]
Summary:
Use the helper function from earlier to create new connection id with sequence
number, and stateless reset token.

Add each of those new connection ids to the routing callback. Add a CHECK()
for routingCallback, because it should be set.

Add a new parametrized set of tests, to test the value of the
active_connection_id_limit transport parameter sent from client.

Reviewed By: yangchi

Differential Revision: D15178642

fbshipit-source-id: 37b4879b09a47d371100c7ac0ab4f01130245715
2019-10-28 17:46:59 -07:00
Yang Chi
9247b8f49b Change Quic qlogger vantage point to enum
Summary: To prevent adhoc string passed in

Reviewed By: sharma95

Differential Revision: D18013615

fbshipit-source-id: 6f5512468755c2b1ecbba3ba42188fa22f4e94b9
2019-10-23 10:16:58 -07:00
Raghu Nallamothu
e06c0848e0 T24905463 - [quic][ping] Implement ping in Quic
Summary: Implement ping functionality in ping

Reviewed By: yangchi

Differential Revision: D17885286

fbshipit-source-id: 4c328d14a023057d6889818250c0129c06e60874
2019-10-21 17:07:12 -07:00
Viktor Chynarov
ac053567e3 NewConnectionId frame handling
Summary:
Mainly based on Junqi's changes in D15230415

Added the start of several error checking. Section 19.15
of d-23
(https://tools.ietf.org/html/draft-ietf-quic-transport-23#section-19.15)
specifies 2 cases where PROTOCOL_VIOLATION should be thrown during processing:
* retire_prior_to is larger than sequence number
* new_connection_id is sent by peer using 0-len conn id (will handle later)

I don't think it was specified what happens if we get a NEW_CONN_ID frame
after the the endpoint already has enough of the peer's conn ids - just ignore
the frame.

Reviewed By: sharma95

Differential Revision: D17578167

fbshipit-source-id: 6e410b6110970a6e52970576ac3ff2b1c7d5c06a
2019-10-11 14:56:29 -07:00
Viktor Chynarov
0ba3b6517b Ensure server/client conn ids are synced to self/peer connIdData
Summary:
Everytime a client/server sets a client/server conn id, it adds it to the
respective self/peer connection id data collections.

Reviewed By: sharma95

Differential Revision: D17577333

fbshipit-source-id: de8b887c1f3acb142c070727fb98ca0841337369
2019-10-11 14:56:29 -07:00
Subodh Iyengar
8ad7d05693 use custom variant type for errors
Summary: Use the custom variant type for errors.

Reviewed By: yangchi

Differential Revision: D17826935

fbshipit-source-id: 2bf0c3e1cc8ca84b504d201fd6c2a4266878b715
2019-10-09 22:37:40 -07:00
Carmi Grushko
a8e76fc3ea Ping frames are retransmittable
Summary:
This will treat incoming ping frames as retransmittable frames and
update ack state accordingly.

Reviewed By: lhuang04, yangchi

Differential Revision: D17841073

fbshipit-source-id: be19ec9a9dae7f0c2d416fab5deac7487861c4e2
2019-10-09 14:59:13 -07:00
Carmi Grushko
3fc265f115 Sync quic/server
Reviewed By: lhuang04

Differential Revision: D17837374

fbshipit-source-id: 0bf3347a0dfd056b8c36973d900efbfef5e86872
2019-10-09 13:38:25 -07:00
Yang Chi
2d24a250f7 Ping frames are retransmittable
Summary:
This will treat incoming ping frames as retransmittable frames and
update ack state accordingly.

Reviewed By: siyengar

Differential Revision: D17832707

fbshipit-source-id: 69e4a95a62bf86a707bac94399588df61c947bd3
2019-10-09 13:12:12 -07:00
Amaury Séchet
a0ebc3995b Fusion QuicFizzFactory into FizzCryptoFactory (#44)
Summary:
They are strongly coupled, which indicate this is probably better to do it as one class.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/44

Reviewed By: mjoras

Differential Revision: D17590918

Pulled By: yangchi

fbshipit-source-id: 2eaca079fd760107eefd2b74fa612d7a0c8b3001
2019-10-08 22:17:02 -07:00
Subodh Iyengar
cbbc6b0c77 Custom variant for simple frames
Summary: Use a custom variant for simple frames

Reviewed By: mjoras

Differential Revision: D17781068

fbshipit-source-id: e059d33df4651c7e0a1c989cc8651c9cd661b14b
2019-10-07 22:43:32 -07:00
Subodh Iyengar
68c332acb1 Use custom variant type for write frames
Summary:
Use the custom variant type for write frames as well, now that
we use them for read frames.

Reviewed By: mjoras

Differential Revision: D17776862

fbshipit-source-id: 47093146d0f1565c22e5393ed012c70e2e23d279
2019-10-07 22:43:31 -07:00
Yang Chi
6d00ae544d Temp enum fix in variant macro to unshadow QuicSimpleFrame type alias
Summary:
It looks like under some compiler flags, type alias follows a very
different rule of name shadowing. This diff adds a trailing "_E" to generated
enum names to unbreak builds. I will send another diff to make it less ugly
later. And eventually the QuicSimpleFrame won't be a type alias once it's no
longer a boost_variant.

Reviewed By: siyengar

Differential Revision: D17766330

fbshipit-source-id: 7b3c5847fd2c1eae10757bfbf9558a38f3085f10
2019-10-04 12:57:54 -07:00
Subodh Iyengar
95f509ae54 custom variant type for read frames
Summary:
Create a custom type for read frame types. This allows us to reduce size of code.
We use a macro to generate new variant types whenever we need to.

Reviewed By: yangchi

Differential Revision: D17266468

fbshipit-source-id: 59a1183dce728e71f0924f39f95a7b78449642b0
2019-10-03 18:17:39 -07:00
Viktor Chynarov
56c18e96ef Back out "Revert D17636918: [quic] Create findFrameInPacketFunc helper function for unit tests"
Summary: Original commit changeset: dcc2b618c2a7

Reviewed By: lnicco

Differential Revision: D17688819

fbshipit-source-id: 9dd3dbcfd78549f5858a41b67db7fc4bc0647469
2019-10-01 11:11:06 -07:00
Shawn Shihang Wei
3ca024e3b8 Revert D17636918: [quic] Create findFrameInPacketFunc helper function for unit tests
Differential Revision:
D17636918

Original commit changeset: 927b3a880ee0

fbshipit-source-id: dcc2b618c2a7accde254d01b5d549a08041dd589
2019-09-30 18:49:58 -07:00