Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2261523004:
Signal to remove remote candidates if ports are pruned. (Closed)
DescriptionSignal to remove remote candidates if ports are pruned.
Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections.
We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates.
Also updated the virtual socket server to better support our test cases.
1. Allow the virtual socket server to set transit delay for packets sent from a given IP address.
2. Ensure the ordered packet delivery for each socket (Previously the delivery order is enforced on the whole test case, so if a udp packet gets delayed based on its IP address, all TCP packets sent after the UDP packet will be delayed at least until the UDP packet is received).
BUG=webrtc:6380
R=deadbeef@webrtc.org, pthatcher@webrtc.org, skvlad@webrtc.org
Committed: https://crrev.com/c67e0f5753e9055e5fe76ae5eba76fe9efb3d769
Cr-Commit-Position: refs/heads/master@{#14297}
Patch Set 1 : . #
Total comments: 4
Patch Set 2 : inline set_pruned method #
Total comments: 36
Patch Set 3 : Address all comments #Patch Set 4 : Revised tests to include different delay cases. #
Total comments: 2
Patch Set 5 : Minor changes #Patch Set 6 : Make delay based on IP and make ordered delivery within each socket #Patch Set 7 : Merge to head and fix a comment #
Messages
Total messages: 39 (22 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
Description was changed from ========== Signal to remove remote candidates if ports are pruned. ========== to ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. ==========
skvlad@webrtc.org changed reviewers: + skvlad@webrtc.org
lgtm, but I'm not sure I completely understand the effect of this change. Let's discuss offline. https://codereview.webrtc.org/2261523004/diff/80001/webrtc/p2p/client/basicpo... File webrtc/p2p/client/basicportallocator.h (right): https://codereview.webrtc.org/2261523004/diff/80001/webrtc/p2p/client/basicpo... webrtc/p2p/client/basicportallocator.h:145: void set_pruned() { state_ = STATE_PRUNED; } This doesn't seem to be called from anywhere else. Could this be inlined into PrunePort()? https://codereview.webrtc.org/2261523004/diff/80001/webrtc/p2p/client/basicpo... File webrtc/p2p/client/basicportallocator_unittest.cc (right): https://codereview.webrtc.org/2261523004/diff/80001/webrtc/p2p/client/basicpo... webrtc/p2p/client/basicportallocator_unittest.cc:1335: virtual_socket_server()->set_delay_stddev(100); Is there a way to use deterministic delays here, to make sure it *always* happens? Non-deterministic tests may become flaky after something changes.
https://codereview.webrtc.org/2261523004/diff/80001/webrtc/p2p/client/basicpo... File webrtc/p2p/client/basicportallocator.h (right): https://codereview.webrtc.org/2261523004/diff/80001/webrtc/p2p/client/basicpo... webrtc/p2p/client/basicportallocator.h:145: void set_pruned() { state_ = STATE_PRUNED; } On 2016/09/14 02:35:11, skvlad wrote: > This doesn't seem to be called from anywhere else. Could this be inlined into > PrunePort()? Done. https://codereview.webrtc.org/2261523004/diff/80001/webrtc/p2p/client/basicpo... File webrtc/p2p/client/basicportallocator_unittest.cc (right): https://codereview.webrtc.org/2261523004/diff/80001/webrtc/p2p/client/basicpo... webrtc/p2p/client/basicportallocator_unittest.cc:1335: virtual_socket_server()->set_delay_stddev(100); On 2016/09/14 02:35:11, skvlad wrote: > Is there a way to use deterministic delays here, to make sure it *always* > happens? Non-deterministic tests may become flaky after something changes. We can use deterministic delay, to make it more deterministic, and I understand its benefit to being able to reproduce. But that only captures one specific scenario. Using a random delay can capture more possible scenarios, e.g., packets are interleaved. If this became flaky, then it captures some bugs which we will need to fix. Using a deterministic value may just have hidden the bug which may only happen under certain situations. WDYT?
deadbeef@webrtc.org changed reviewers: + deadbeef@webrtc.org
https://codereview.webrtc.org/2261523004/diff/100001/webrtc/base/virtualsocke... File webrtc/base/virtualsocketserver.h (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/base/virtualsocke... webrtc/base/virtualsocketserver.h:83: // If |delay_dist_ipv6_| is set, that will be used for the delay distribution nit: Since delay_dist_ is an implementation detail, I don't think it should be mentioned in these comments. It may be more clear to just have UpdateDelayDistribution, UpdateDelayDistributionForIPv4 and UpdateDelayDistributionForIPv6, where the first method updates both. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/base/virtualsocke... webrtc/base/virtualsocketserver.h:90: void UpdateDelayDistributionForIPv6(); I think this is good, but another idea would to have a method to set the delay for a specific 5-tuple. Then we could set the delay for one TURN server to be larger than another, even with all other things equal. It would also avoid having to keep track of 4 delay_dist variables when we start differentiating UDP vs. TCP delay. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator.cc (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:782: data.PrunePort(); In this case, we call PrunePort and set "pruned" to true, but don't add it to pruned_ports. This confused me at first. Maybe "pruned_ports" can be renamed to "pruned_ports_with_signaled_candidates" or something similar? https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:789: << " low-priority Turn ports"; nit: capitalize "TURN" https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator_unittest.cc (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:1240: // that we can control the TCP delay independently of the UDP delay. Since IPv4 vs. IPv6 is already handled in this CL, can UDP vs. TCP be handled too? I think we should avoid non-deterministic tests. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:1258: EXPECT_TRUE_WAIT(candidate_allocation_done_, 10000); nit: May want to use a constant for this timeout. Could even just increase the default timeout to 10 seconds (I've done that in other places; it reduces msan/tsan/etc flakes). https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:1271: // created before or after the UDP turn port. nit: Capitalize "TURN" https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:1288: virtual_socket_server()->set_delay_stddev(100); Since you can control the delay for IPv6 vs. IPv4 directly, why is variation needed?
pthatcher@webrtc.org changed reviewers: + pthatcher@webrtc.org
https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator.cc (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:317: std::vector<PortData*> port_data_list = Why not "ports_to_prune"? https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:318: GetUnprunedPortsOnNetworks(failed_networks); I think "GetUnprunedPorts" would be sufficient. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:321: << " ports because their networks failed"; If you do it *before* the line below, it should be "Prune X", and if you say "Pruned X", it should go *after* the line below. I'd prefer before and just change to "Prune" https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:629: PrunePortsAndRemoveCandidates(port_data_list); Same here. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:772: std::vector<PortData*> pruned_ports; If we don't prune these until the end, should we call this port_to_prune? https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:782: data.PrunePort(); On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > In this case, we call PrunePort and set "pruned" to true, but don't add it to > pruned_ports. This confused me at first. Maybe "pruned_ports" can be renamed to > "pruned_ports_with_signaled_candidates" or something similar? I think "ports_to_prune" would be a better name. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:963: std::vector<PortData*> port_data_list; Why not "unpruned_ports"? https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:964: for (PortData& data : ports_) { Why not "port : ports_"? https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator.h (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.h:146: void PrunePort() { Why not just "Prune()"? https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.h:212: // by these ports. generated by these ports => that were previously signaled from those pruned ports
Patchset #4 (id:140001) has been deleted
PTAL. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/base/virtualsocke... File webrtc/base/virtualsocketserver.h (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/base/virtualsocke... webrtc/base/virtualsocketserver.h:83: // If |delay_dist_ipv6_| is set, that will be used for the delay distribution On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > nit: Since delay_dist_ is an implementation detail, I don't think it should be > mentioned in these comments. > > It may be more clear to just have UpdateDelayDistribution, > UpdateDelayDistributionForIPv4 and UpdateDelayDistributionForIPv6, where the > first method updates both. This change has been reverted. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/base/virtualsocke... webrtc/base/virtualsocketserver.h:90: void UpdateDelayDistributionForIPv6(); On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > I think this is good, but another idea would to have a method to set the delay > for a specific 5-tuple. Then we could set the delay for one TURN server to be > larger than another, even with all other things equal. It would also avoid > having to keep track of 4 delay_dist variables when we start differentiating UDP > vs. TCP delay. I changed to set the delay for source socket address only. It is sufficient for most test cases I can think of. It would be more complicated to make it based on 5-tuple and I don't see an immediate use case yet. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator.cc (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:317: std::vector<PortData*> port_data_list = On 2016/09/15 18:54:38, pthatcher1 wrote: > Why not "ports_to_prune"? Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:318: GetUnprunedPortsOnNetworks(failed_networks); On 2016/09/15 18:54:38, pthatcher1 wrote: > I think "GetUnprunedPorts" would be sufficient. Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:321: << " ports because their networks failed"; On 2016/09/15 18:54:38, pthatcher1 wrote: > If you do it *before* the line below, it should be "Prune X", and if you say > "Pruned X", it should go *after* the line below. > > I'd prefer before and just change to "Prune" Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:629: PrunePortsAndRemoveCandidates(port_data_list); On 2016/09/15 18:54:38, pthatcher1 wrote: > Same here. Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:772: std::vector<PortData*> pruned_ports; On 2016/09/15 18:54:38, pthatcher1 wrote: > If we don't prune these until the end, should we call this port_to_prune? Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:782: data.PrunePort(); On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > In this case, we call PrunePort and set "pruned" to true, but don't add it to > pruned_ports. This confused me at first. Maybe "pruned_ports" can be renamed to > "pruned_ports_with_signaled_candidates" or something similar? Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:789: << " low-priority Turn ports"; On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > nit: capitalize "TURN" Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:963: std::vector<PortData*> port_data_list; On 2016/09/15 18:54:38, pthatcher1 wrote: > Why not "unpruned_ports"? Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.cc:964: for (PortData& data : ports_) { On 2016/09/15 18:54:38, pthatcher1 wrote: > Why not "port : ports_"? Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator.h (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.h:146: void PrunePort() { On 2016/09/15 18:54:38, pthatcher1 wrote: > Why not just "Prune()"? Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator.h:212: // by these ports. On 2016/09/15 18:54:38, pthatcher1 wrote: > generated by these ports => that were previously signaled from those pruned > ports I used that were previously signaled from these ports. They are not "pruned" yet. Using "these ports" should have no confusion from the context. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator_unittest.cc (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:1240: // that we can control the TCP delay independently of the UDP delay. On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > Since IPv4 vs. IPv6 is already handled in this CL, can UDP vs. TCP be handled > too? I think we should avoid non-deterministic tests. Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:1258: EXPECT_TRUE_WAIT(candidate_allocation_done_, 10000); On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > nit: May want to use a constant for this timeout. Could even just increase the > default timeout to 10 seconds (I've done that in other places; it reduces > msan/tsan/etc flakes). Done. Now that there is no randomness, I don't think it needs 10 seconds. But still make it larger to be 3 seconds. If we still see flakiness, we can increase to 10seconds. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:1271: // created before or after the UDP turn port. On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > nit: Capitalize "TURN" Done. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:1288: virtual_socket_server()->set_delay_stddev(100); On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > Since you can control the delay for IPv6 vs. IPv4 directly, why is variation > needed? I was hoping to use a random value to test behaviors under different realistic scenarios. vlad made a good argument on using a deterministic value and you also want a deterministic value. So removed it now with the new approach of setting delays
lgtm. I like what you did with the tests. https://codereview.webrtc.org/2261523004/diff/100001/webrtc/base/virtualsocke... File webrtc/base/virtualsocketserver.h (right): https://codereview.webrtc.org/2261523004/diff/100001/webrtc/base/virtualsocke... webrtc/base/virtualsocketserver.h:90: void UpdateDelayDistributionForIPv6(); On 2016/09/16 01:41:29, honghaiz3 wrote: > On 2016/09/15 00:43:17, Taylor Brandstetter wrote: > > I think this is good, but another idea would to have a method to set the delay > > for a specific 5-tuple. Then we could set the delay for one TURN server to be > > larger than another, even with all other things equal. It would also avoid > > having to keep track of 4 delay_dist variables when we start differentiating > UDP > > vs. TCP delay. > I changed to set the delay for source socket address only. It is sufficient for > most test cases I can think of. > It would be more complicated to make it based on 5-tuple and I don't see an > immediate use case yet. Sounds good; after thinking about it a bit I agree. https://codereview.webrtc.org/2261523004/diff/160001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator_unittest.cc (right): https://codereview.webrtc.org/2261523004/diff/160001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:520: // |ready_ports|, nit: Odd comment formatting here.
lgtm. The tests are fantastic.
lgtm
https://codereview.webrtc.org/2261523004/diff/160001/webrtc/p2p/client/basicp... File webrtc/p2p/client/basicportallocator_unittest.cc (right): https://codereview.webrtc.org/2261523004/diff/160001/webrtc/p2p/client/basicp... webrtc/p2p/client/basicportallocator_unittest.cc:520: // |ready_ports|, On 2016/09/16 17:21:17, Taylor Brandstetter wrote: > nit: Odd comment formatting here. Done.
The CQ bit was checked by honghaiz@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org, pthatcher@webrtc.org, skvlad@webrtc.org Link to the patchset: https://codereview.webrtc.org/2261523004/#ps180001 (title: "Minor changes")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
Description was changed from ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. ========== to ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. BUG=webrtc:6380 ==========
Description was changed from ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. BUG=webrtc:6380 ========== to ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. Also updated the virtual socket server to better support our test cases. 1. Allow the virtual socket server to set transit delay for packets sent from a given IP address. 2. Ensure the ordered packet delivery for each socket. BUG=webrtc:6380 ==========
On 2016/09/17 02:19:27, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) > android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) I made changes on the virtualsocketserver to make delay depend on IP address only (because TCP connection uses a new port for each connection), and make ordered delivery based on each socket. You may want to take another look.
lgtm
Description was changed from ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. Also updated the virtual socket server to better support our test cases. 1. Allow the virtual socket server to set transit delay for packets sent from a given IP address. 2. Ensure the ordered packet delivery for each socket. BUG=webrtc:6380 ========== to ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. Also updated the virtual socket server to better support our test cases. 1. Allow the virtual socket server to set transit delay for packets sent from a given IP address. 2. Ensure the ordered packet delivery for each socket (Previously the delivery order is enforced on the whole test case, so if a udp packet gets delayed based on its IP address, all TCP packets sent after the UDP packet will be delayed at least until the UDP packet is received). BUG=webrtc:6380 ==========
The CQ bit was checked by honghaiz@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org, skvlad@webrtc.org Link to the patchset: https://codereview.webrtc.org/2261523004/#ps200001 (title: "Make delay based on IP and make ordered delivery within each socket")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by honghaiz@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org, pthatcher@webrtc.org, skvlad@webrtc.org Link to the patchset: https://codereview.webrtc.org/2261523004/#ps220001 (title: "Merge to head and fix a comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Description was changed from ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. Also updated the virtual socket server to better support our test cases. 1. Allow the virtual socket server to set transit delay for packets sent from a given IP address. 2. Ensure the ordered packet delivery for each socket (Previously the delivery order is enforced on the whole test case, so if a udp packet gets delayed based on its IP address, all TCP packets sent after the UDP packet will be delayed at least until the UDP packet is received). BUG=webrtc:6380 ========== to ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. Also updated the virtual socket server to better support our test cases. 1. Allow the virtual socket server to set transit delay for packets sent from a given IP address. 2. Ensure the ordered packet delivery for each socket (Previously the delivery order is enforced on the whole test case, so if a udp packet gets delayed based on its IP address, all TCP packets sent after the UDP packet will be delayed at least until the UDP packet is received). BUG=webrtc:6380 R=deadbeef@webrtc.org, pthatcher@webrtc.org, skvlad@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/c67e0f5753e9055e5fe76ae5e... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:220001) manually as c67e0f5753e9055e5fe76ae5eba76fe9efb3d769 (presubmit successful).
Message was sent while issue was closed.
Description was changed from ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. Also updated the virtual socket server to better support our test cases. 1. Allow the virtual socket server to set transit delay for packets sent from a given IP address. 2. Ensure the ordered packet delivery for each socket (Previously the delivery order is enforced on the whole test case, so if a udp packet gets delayed based on its IP address, all TCP packets sent after the UDP packet will be delayed at least until the UDP packet is received). BUG=webrtc:6380 R=deadbeef@webrtc.org, pthatcher@webrtc.org, skvlad@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/c67e0f5753e9055e5fe76ae5e... ========== to ========== Signal to remove remote candidates if ports are pruned. Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections. We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates. Also updated the virtual socket server to better support our test cases. 1. Allow the virtual socket server to set transit delay for packets sent from a given IP address. 2. Ensure the ordered packet delivery for each socket (Previously the delivery order is enforced on the whole test case, so if a udp packet gets delayed based on its IP address, all TCP packets sent after the UDP packet will be delayed at least until the UDP packet is received). BUG=webrtc:6380 R=deadbeef@webrtc.org, pthatcher@webrtc.org, skvlad@webrtc.org Committed: https://crrev.com/c67e0f5753e9055e5fe76ae5eba76fe9efb3d769 Cr-Commit-Position: refs/heads/master@{#14297} ========== |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
