New issue
Advanced search Search tips

Issue 696142 link

Starred by 0 users
Status: Fixed
Owner:
tbansal@chromium.org
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Filter out transport RTT from local sockets

Project Member Reported by tbansal@chromium.org, Feb 25 2017 
NQE currently records RTT from all sockets including sockets whose remote end point may be local (e.g., 192.168.*.*). RTT values from such sockets do not reflect the available network quality.

NQE should consider excluding RTTs from such sockets. This will likely become more important as we expand the API to platforms beyond Android.

Comment 1 by bengr@chromium.org, Mar 14 2017

Labels: -Pri-3 M-59 Pri-2
Owner: tbansal@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by tbansal@chromium.org, May 10 2017

Labels: -Pri-2 -M-59 Pri-3
Owner: ----
Status: Available (was: Assigned)

Comment 3 by bengr@chromium.org, May 10 2017

Labels: -Pri-3 M-61 Pri-2
This seems like it could skew estimates quite considerably.

Comment 4 by bengr@chromium.org, May 10 2017

Owner: tbansal@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 24 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5cf904450ddb7fba8448914af8ff4c4800481723

commit 5cf904450ddb7fba8448914af8ff4c4800481723
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Jul 24 21:39:54 2017

NQE: clean up constructor

NQE (Network Quality Estimator): Remove args from NQE constructor
that are only used by tests.

Bug:  696142 
Change-Id: Idda7375d433f7d76e3b8857421b7baa75220fa8f
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/581612
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489083}
[modify] https://crrev.com/5cf904450ddb7fba8448914af8ff4c4800481723/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/5cf904450ddb7fba8448914af8ff4c4800481723/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/5cf904450ddb7fba8448914af8ff4c4800481723/net/nqe/network_quality_estimator_test_util.cc
[modify] https://crrev.com/5cf904450ddb7fba8448914af8ff4c4800481723/net/nqe/network_quality_estimator_unittest.cc
[modify] https://crrev.com/5cf904450ddb7fba8448914af8ff4c4800481723/net/nqe/throughput_analyzer.cc
[modify] https://crrev.com/5cf904450ddb7fba8448914af8ff4c4800481723/net/nqe/throughput_analyzer.h
[modify] https://crrev.com/5cf904450ddb7fba8448914af8ff4c4800481723/net/nqe/throughput_analyzer_unittest.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7

commit 73a04377d7dc8e9e17b9a1a5167b9f7baed016b7
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Jul 27 16:28:41 2017

NQE: Discard transport RTT observations from private sockets.

RTT from sockets that are expected to connect to private addresses
is not notified back to the network quality estimator.

Bug:  696142 
Change-Id: Ie82765adbdbfcad0d9d33a9e07713b4724a32b97
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/580253
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489964}
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/network_quality_estimator_test_util.cc
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/network_quality_estimator_unittest.cc
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/socket_watcher.cc
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/socket_watcher.h
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/socket_watcher_factory.cc
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/socket_watcher_factory.h
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/nqe/socket_watcher_unittest.cc
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/quic/chromium/quic_network_transaction_unittest.cc
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/quic/chromium/quic_stream_factory.cc
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/socket/socket_performance_watcher_factory.h
[modify] https://crrev.com/73a04377d7dc8e9e17b9a1a5167b9f7baed016b7/net/socket/transport_client_socket_pool.cc

Comment 7 by tbansal@chromium.org, Jul 27 2017

Labels: -M-61 M-62
Status: Fixed (was: Assigned)
►

Sign in to add a comment