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

23 Commits

Author SHA1 Message Date
Luca Niccolini
ed9d1bc3e1 correctly start/stop qlog collection when setQlogger is called multiple times
Reviewed By: yangchi

Differential Revision: D23579772

fbshipit-source-id: abe8e5dac3ba46fdccba2b39055e5589179cb8a1
2020-09-09 00:20:09 -07:00
Luca Niccolini
c47c3cf5c6 Revert PMTU and size-enforced packet builder
Differential Revision: D23283619

fbshipit-source-id: b7fe31871dad5711016234a2d10ae84edc4fd24c
2020-08-22 16:55:54 -07:00
Xiaoting Tang
4762cfb927 Introduce PMTU as a variable
Summary:
First step towards d6d. Semantically we need to separate the old `udpSendPacketLen` into `peerMaxPacketSize` as well as `currPMTU`. The former is directly tied to the peer's max_packet_size transport parameter whereas the second is controlled by d6d. To get the actual udp mss, call `conn_->getUdpSendPacketLen()`, which will use the minimum of the two if d6d is enabled, otherwise it will fallback to use `peerMaxPacketSize` only.

During processClientInitialParams and processServerInitialParams, we no longer need to check whether `canIgnorePathMTU` is set because that logic is moved to `setUdpSendPacketLen`. If d6d is enabled, we set both `peerMaxPacketSize` and `currPMTU` to `packetSize` because receiving an initial packet of size x indicates both that the peer accepts x-sized packet and that the PMTU is at least x.

Many call sites and tests are changed.

Faebook:
For now, d6d is considered enabled if `canIgnorePathMTU==false` and `turnoffPMTUD==true`. Down the road, from semantic & practical POV at least one of them should be renamed to something like `enableD6D`, since enabling d6d implies turning off PMTUD and that we should not ignore PMTU. We can keep one for the sake of testing.

Reviewed By: mjoras

Differential Revision: D22049806

fbshipit-source-id: 7a9b30b7e2519c132101509be56a9e63b803dc93
2020-08-17 16:15:24 -07:00
Yang Chi
06083595e3 Do not enable pacing if transport doesn't have pacing timer
Summary:
LOG an error and fallback to no pacing. This diff also stops
supporting automaticaly set pacingEnabled to true when BBR is enabled.

Reviewed By: mjoras

Differential Revision: D22875904

fbshipit-source-id: f8c8c9ea252f6e5e86f83174309b159ce93b3919
2020-08-03 10:38:39 -07:00
Yang Chi
5d74a54259 refactor setCipherOverhead -> accountForCipherOverhead
Summary: as title

Reviewed By: mjoras

Differential Revision: D22640467

fbshipit-source-id: 063dbd6aebd323ab6396d5e408057ce16aaf51b9
2020-07-22 15:16:12 -07:00
Yang Chi
344e028687 Close quic client connection upon read error
Summary: set this to be default behavior

Reviewed By: vchynarov

Differential Revision: D22432915

fbshipit-source-id: 3ac527b206ccb798d63336bf559f7d164e2959c7
2020-07-08 13:01:34 -07:00
Junqi Wang
499b5fb0ce Remove continueOnNetworkUnreachable
Reviewed By: yangchi

Differential Revision: D22333283

fbshipit-source-id: b35fb9e6ba31e88faf2c92805edc2738cfde93f1
2020-07-01 19:35:26 -07:00
Yang Chi
51b917b0b3 PingFrame is not a simple frame
Summary:
The problem with Ping being a simple frame:
(1) All SimpleFrames are in the same scheduler. So sending ping means we may
also send other frames which can be problematic if we send in Initial or
Handshake space
(2) Ping isn't retranmisttable. But other Simple frames are. So we are
certainly setting this wrong when we send pure Ping packet today.

That being said, there are cases where we need to treat Ping as retransmittable.
One is when it comes to update ack state: If peer sends us Ping, we may want to
Ack early rather than late. so it makes sense to treat Ping as retransmittable.
Another place is insertion into OutstandingPackets list. When our API user sends
Ping, then also add a Ping timeout. Without adding pure Ping packets into OP list,
we won't be able to track the acks to our Pings.

Reviewed By: mjoras

Differential Revision: D21763935

fbshipit-source-id: a04e97b50cf4dd4e3974320a4d2cc16eda48eef9
2020-06-18 15:30:44 -07:00
Yang Chi
b8fef40c6d Clone Quic handshake packets
Summary:
On loss timer, currently we knock all handshake packets out of the OP
list and resend everything. This means miss RTT sampling opportunities during
handshake if loss timer fires, and given our initial loss timer is likely not a
good fit for many networks, it probably fires a lot.

This diff keeps handshake packets in the OP list, and add packet cloning
support to handshake packets so we can clone them and send as probes.

With this, the handshake alarm is finally removed. PTO will take care of all
packet number space.

The diff also fixes a bug in the CloningScheduler where we missed cipher
overhead setting. That broke a few unit tests once we started to clone
handshake packets.

The writeProbingDataToSocket API is also changed to support passing a token to
it so when we clone Initial, token is added correctly. This is because during
packet cloning, we only clone frames. Headers are fresh built.

The diff also changed the cloning behavior when there is only one outstanding
packet. Currently we clone it twice and send two packets. There is no point of
doing that. Now when loss timer fires and when there is only one outstanding
packet, we only clone once.

The PacketEvent, which was an alias of PacketNumber, is now a real type that
has both PacketNumber and PacketNumberSpace to support cloning of handshake
packets. I think in the long term we should refactor PacketNumber itself into a
real type.

Reviewed By: mjoras

Differential Revision: D19863693

fbshipit-source-id: e427bb392021445a9388c15e7ea807852ddcbd08
2020-06-18 15:30:44 -07:00
Andrii Vasylevskyi
adb48de101 Add client side packet drop stats bumps
Summary:
^

(Note: this ignores all push blocking failures!)

Differential Revision: D22042953

fbshipit-source-id: e435065210e5f21dd2c8d4a1e1c9ac3013c9d41e
2020-06-18 09:51:24 -07:00
Matt Joras
42bba01005 Draft-29 support.
Summary:
This implements the connection ID validation via transport parameters. Note we don't do anything with the retry transport parameter yet.

This will probably require further surgery to tests when we want the MVFST version to do this, but for now I'm punting on that test plumbing.

This retains support for h3-27.

Reviewed By: yangchi

Differential Revision: D22045631

fbshipit-source-id: e93841e734c0683655c751d808fd90b3b391eb3e
2020-06-16 17:05:41 -07:00
Xiaoting Tang
2d00d56fbd Put outstanding packets, events and associated counters in one class
Summary: ^

Reviewed By: yangchi

Differential Revision: D21956286

fbshipit-source-id: 305b879ad11df23aae8e0c3aac4645c0136b3012
2020-06-10 12:45:28 -07:00
Andrii Vasylevskyi
5d89358075 Add client side onRead/onPacketReceived stats callbacks
Summary: Invoking callbacks onRead and onPacketReceived in QuicClientTransport when reading UDP packets.

Reviewed By: mjoras

Differential Revision: D21904927

fbshipit-source-id: b2bdee6330a0ddebbdbe677a6779b14d36b15f07
2020-06-09 12:14:52 -07:00
Yang Chi
b18ffd1b6a Limit Quic Client close to be one close per RTT
Summary: I think this is still within specs requirement

Reviewed By: mjoras

Differential Revision: D21648049

fbshipit-source-id: 990e84740e1022955d7dd8957ce9131602536a63
2020-05-19 19:37:57 -07:00
Yang Chi
cd7339e454 Give caller some control over if writeStreamFrameHeader should write length
Summary:
Becuase when we clone an existing packet, the logic inside the current
writetStreamFrameHeader is no longer correct.

Reviewed By: mjoras

Differential Revision: D21383828

fbshipit-source-id: 8e6bbb048eefd97ca7cf17b89edc2f395f274a73
2020-05-07 10:56:26 -07:00
Matt Joras
50d5c29346 Cipher dropping take 2
Summary:
Now we won't have a zero PTO and we will properly clear out the outstanding packets.

Note that this cipher dropping is not what the draft prescribes, instead dropping both the initial and handshake ciphers when we know 1-rtt communication is functioning.

Reviewed By: yangchi

Differential Revision: D20388737

fbshipit-source-id: 0b89eb80c8faa796ab09eda3eaa10a00dcf7bae9
2020-05-06 11:14:20 -07:00
Matt Joras
d83d7f3024 Change default zero rtt matching policy
Summary:
This is a safer default than allowing limited on the source address not matching.

While here, also change the attemptEarlyData setting to false, since 0-rtt should be opt-in.

Reviewed By: yangchi, JunqiWang

Differential Revision: D21383402

fbshipit-source-id: b60fbbbe9438861eea894cb11ccb8bae2243a174
2020-05-04 14:45:47 -07:00
Yang Chi
2a1529068d Move Quic packet header encoding out of builder constructor
Summary:
Currently the packet builder contructor will encode the packet
builder. This is fine when the builder creates its own output buffer. If later
on we decides not to use this builder, or it fails to build packet, the buffer
will be thrown away. But once the builder uses a buffer provided by caller, and
will be reused, we can no longer just throw it away if we decide not to use
this builder. So we have to delay the header encoding until we know we will use
the builder.

This is still not enough to solve the case where we want to use this builder,
it builds, then it fails . For that, we will need to retreat the tail position
of the IOBuf.

Reviewed By: mjoras

Differential Revision: D21000658

fbshipit-source-id: 4d758b3e260463b17c870618ba68bd4b898a7d4c
2020-04-28 22:14:21 -07:00
Matt Joras
f795324507 Manually backout remove recvmsg polling.
Summary: We may have a bug in recvmmsg usage, so keep this around.

Reviewed By: lnicco

Differential Revision: D21246413

fbshipit-source-id: e39d74b8e856339d0022ba403969b83dc29ff63f
2020-04-25 15:29:45 -07:00
Matt Joras
fcc6efe68d Remove recvmsg polling.
Summary: This is essentially duplicated code, as if recvmmsg is not available the netops wrapper will simulate it with multiple calls.

Reviewed By: yangchi

Differential Revision: D21150890

fbshipit-source-id: 0e48e8a80f4ddc90df69c3e57cecc96dbc3f0913
2020-04-21 23:29:04 -07:00
Aman Sharma
a84d2e5fcb Make client use new stateless retry
Summary: This makes the change for the client to use stateless retries

Reviewed By: mjoras

Differential Revision: D19657433

fbshipit-source-id: d4b34087d15e49153860a7833ed54e28c6cd6777
2020-04-08 20:58:41 -07:00
Aman Sharma
918574c6eb Functionality to verify retry integrity token
Summary: This adds the ability to verify the integrity token present in a retry packet, as per section 5.8 of the QUIC-TLS draft (https://fburl.com/kw9l8dvu). This doesn't change any existing functionality.

Reviewed By: mjoras

Differential Revision: D19631864

fbshipit-source-id: 2ff8288986b3e27c85fe885b132ab6753fed3be8
2020-04-08 13:07:14 -07:00
Amaury Séchet
fb0b6b1cc4 Move fizz specific part of the client in quic/fizz/client (#120)
Summary:
This create a separate library for the fizz client. This allows complete separation of the fizz part of the client, and make it swapable for something else.

Depends on https://github.com/facebookincubator/mvfst/issues/118
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/120

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

 ---
## Proxygen Canary
Traffic Canary: https://our.intern.facebook.com/intern/traffic/canary?fbid=528194164778784
* elb.prod.muc2c01 - binary - 2020-04-05 14:59 - https://fburl.com/dyndash/ywntlz9n
* flb.prod.fceb2c02 - binary - 2020-04-05 14:59 - https://fburl.com/dyndash/ns1vzm1j
* olb.prod.ratn0c01.p2 - binary - 2020-04-05 14:59 - https://fburl.com/dyndash/0vxebqop
* slb.prod_regional.rnao0c00 - binary - 2020-04-05 14:59 - https://fburl.com/dyndash/a8syav0w
* slb.regional.rvll0c01.p2 - binary - 2020-04-05 14:59 - https://fburl.com/dyndash/igneyshj
 ---

Reviewed By: mjoras

Differential Revision: D20769060

Pulled By: yangchi

fbshipit-source-id: ad5d66c23b3a9723ad3f8c8091981df99339761e
2020-04-06 11:43:31 -07:00