Summary: The new controller happens to be the exact same address
Reviewed By: lnicco
Differential Revision: D26566423
fbshipit-source-id: 88fa04d2cf8b02ef20e7979f0055ee84903d33e9
Summary:
Before the change, there's no good way to recreate Cubic CC instance with custom CC factory, because Cubic is created by default.
On client side this requires calling setCongestionControl() or setTransportSettings() after calling setCongestionControllerFactory(), which is normally the case.
Reviewed By: yangchi
Differential Revision: D26401996
fbshipit-source-id: dfda39be835c67b9db42f726b3ac64c7b3d37c2f
Summary: this param is passed to transport then ignored
Reviewed By: avasylev
Differential Revision: D26133327
fbshipit-source-id: 459dd0132185513215ba034f213d4137d7b56ba1
Summary:
(1) Receiving a valid retry packet is a HappyEyeballs signal. We should
use that to update the happy eyeballs state.
(2) HappyEyeballs state should also be preserved when we undo client connection
state for retry.
Reviewed By: mjoras
Differential Revision: D25728713
fbshipit-source-id: 4ff06879f5a05e6fb4faeb1e9f330e251d3dbcb6
Summary:
I think this should just work without the trailing `_E`. It was added
when we mixed up our own union based variant and boost::variant. Some compiler
flags didn't like that. Now we no longer have mixed up cases, this should be
fine
Reviewed By: lnicco
Differential Revision: D25589393
fbshipit-source-id: 6430dc20f8e81af0329d89e6990c16826da168b8
Summary:
The test case relies on the client transport hits max PTO within 10s
to trigger a read error callback. Sometimes, it takes longer than 10s to hit
max PTO under stress runs
Reviewed By: mjoras
Differential Revision: D25065653
fbshipit-source-id: c728084178126452d0c7c885cb7215878cf4e3b7
Summary: Adds a top level API to set stream priorities, mirroring what is currently proposed in httpbis. For now, default all streams to the highest urgency, round-robin, which mirrors the current behavior in mvfst.
Reviewed By: mjoras
Differential Revision: D20318260
fbshipit-source-id: eec625e2ab641f7fa6266517776a2ca9798e8f89
Summary:
When I tested against picoquic, I found that an exception was thrown here: https://fburl.com/diffusion/qsnzingx.
The reason is that we modify `conn.initialDestinationConnectionId` in the event of a retry, so it's no longer the original destination connection id. By separately keeping track of the original destination connection id, we can solve this issue.
Reviewed By: mjoras
Differential Revision: D23722127
fbshipit-source-id: 94be08812e675feaf46f5af86e7304af84c1910c
Summary: This switches the client to use the FizzRetryIntegrityTagGenerator and the PseudoRetryPacketBuilder, to avoid duplication of a lot of the work.
Reviewed By: mjoras
Differential Revision: D21489881
fbshipit-source-id: 8aa3af26f1090eeb9f2f04eb4defd785ad555df1
Summary: It's useful to be able to use the v4-mapped addresses sometimes.
Reviewed By: avasylev
Differential Revision: D24371807
fbshipit-source-id: 74fe13fa4bef9c77cc51a18137559facda9bdbeb
Summary: The D6DEnabled integration test was flaky because of a race condition between stats callback and client's transportReady callback. Simple fix by ~~send some data and wait for echo~~ calling terminateLoopSoon in onConnectionD6DStarted. Still, it makes sense to make transportReady the last callback in server so I switched the order.
Reviewed By: yangchi
Differential Revision: D24198017
fbshipit-source-id: 36a7b6377c970b2f2f5d072f03d8bc8b0837fd79
Summary: We need to increase those parameter Ids above 0xFF00, and xmit them.
Reviewed By: mjoras
Differential Revision: D24115117
fbshipit-source-id: cf8b4970ba8640a83bde22c1f9fdfa1d3e53e3f9
Summary: Since the DCID is often empty for clients, this is otherwise not a very useful field.
Reviewed By: yangchi
Differential Revision: D23998639
fbshipit-source-id: b8949ca6913ed270e5ebd0a0c5335b224f817774
Summary:
There is a pretty tight coupling between ClientHandshake and FizzClientHandshake . The later can mess up pretty bad with the former's state, especially since th code has quite a lot of temporal dependencies and is very stateful.
This constraint he API through which the subclass can interact with the parent class and prevent some obviously bad misuses. It also make it clear when something is to made available for testing exclusively.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/182
Test Plan:
Imported from GitHub, without a `Test Plan:` line.
---
## Traffic Canary
https://our.intern.facebook.com/intern/traffic/canary?fbid=119893296332660
* elb.prod.hel3c01 - binary - 2020-09-24 23:01 - https://fburl.com/dyndash/q948xh5b
* flb.prod.fgdl1c03 - binary - 2020-09-24 23:01 - https://fburl.com/dyndash/1p582qcv
* olb.prod.rftw0c01.p2 - binary - 2020-09-24 23:01 - https://fburl.com/dyndash/h23ldteg
* slb.prod_regional.rash0c00 - binary - 2020-09-24 23:01 - https://fburl.com/dyndash/6cjn5fxn
* slb.regional.rash0c01.p2 - binary - 2020-09-24 23:01 - https://fburl.com/dyndash/kadbkkdf
---
Reviewed By: mjoras
Differential Revision: D23816801
Pulled By: bschlinker
fbshipit-source-id: 52c7fa3e3f436d3317f2a13bef8ae4d596f6a25b
Summary: This is a temporary hack until we properly implement 0-rtt transport parameter updating. Now after 0-rtt we will use the packet size from the peer as the max packet size, if the can ignore setting is set.
Reviewed By: xttjsn
Differential Revision: D23690019
fbshipit-source-id: b4dbf5702e81e52ccd437e0fa68f4d156c7123be
Summary: This was probably a premature optimization and introduces complexity for dubious gain. Additionally a sequence of losses could potentially cause multiple updates to be delayed.
Reviewed By: yangchi
Differential Revision: D23628058
fbshipit-source-id: d6cf70baec8c34f0209ea791dadc724795fe0c21
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
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
Summary: set this to be default behavior
Reviewed By: vchynarov
Differential Revision: D22432915
fbshipit-source-id: 3ac527b206ccb798d63336bf559f7d164e2959c7
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
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
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
Summary: I think this is still within specs requirement
Reviewed By: mjoras
Differential Revision: D21648049
fbshipit-source-id: 990e84740e1022955d7dd8957ce9131602536a63
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
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
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
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
Summary: We may have a bug in recvmmsg usage, so keep this around.
Reviewed By: lnicco
Differential Revision: D21246413
fbshipit-source-id: e39d74b8e856339d0022ba403969b83dc29ff63f
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
Summary: This makes the change for the client to use stateless retries
Reviewed By: mjoras
Differential Revision: D19657433
fbshipit-source-id: d4b34087d15e49153860a7833ed54e28c6cd6777
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
