[Telemetry] Android forwarder bug fixes.

[Telemetry] Android forwarder bug fixes.

- updated GetProp() calls to go through DeviceUtils
- changed iptable DNAT rules to explicitly specify the host ip in
  destination address.  This is to fix the case where 127.0.0.1 is
  sometimes used as the dest ip

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284229

[chrishenry, 2014-07-18 01:06:18]

https://codereview.chromium.org/398003007/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/forwarders/android_forwarder.py (right):

https://codereview.chromium.org/398003007/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/forwarders/android_forwarder.py:452:
self._device.RunShellCommand('ip route flush cache')
Does these commands require root or does it work everywhere?

[tonyg, 2014-07-18 01:21:29]

https://codereview.chromium.org/398003007/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/forwarders/android_forwarder.py (right):

https://codereview.chromium.org/398003007/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/forwarders/android_forwarder.py:100: (protocol,
port_pair.remote_port, self.host_ip, port_pair.local_port))
This part and the parts that add the parens around the device() lg. Could we
split those into a separate patch and land that first?

The routing policy parts I'd like to understand a little better and verify that
they'll work on J, K and L.

[wuhu, 2014-07-18 01:39:04]

On 2014/07/18 01:06:18, chrishenry wrote:
>
https://codereview.chromium.org/398003007/diff/1/tools/telemetry/telemetry/co...
> File tools/telemetry/telemetry/core/forwarders/android_forwarder.py (right):
> 
>
https://codereview.chromium.org/398003007/diff/1/tools/telemetry/telemetry/co...
> tools/telemetry/telemetry/core/forwarders/android_forwarder.py:452:
> self._device.RunShellCommand('ip route flush cache')
> Does these commands require root or does it work everywhere?

Yes, these commands will require root access.  It's assumed the device is rooted
or we wouldn't be able to install the rndis interface for recording anyways.

[wuhu, 2014-07-18 16:59:23]

The CQ bit was checked by wuhu@google.com

[wuhu, 2014-07-18 16:59:23]

The CQ bit was unchecked by wuhu@google.com

[tonyg, 2014-07-18 17:02:05]

lgtm

[commit-bot: I haz the power, 2014-07-18 17:03:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wuhu@google.com/398003007/20001

[chromium-reviews, 2014-07-18 17:15:00]

The changes related to the routing policy and netd are now in
https://codereview.chromium.org/404803003

Thanks


On Thu, Jul 17, 2014 at 6:21 PM, <tonyg@chromium.org> wrote:

>
> https://codereview.chromium.org/398003007/diff/1/tools/
> telemetry/telemetry/core/forwarders/android_forwarder.py
> File tools/telemetry/telemetry/core/forwarders/android_forwarder.py
> (right):
>
> https://codereview.chromium.org/398003007/diff/1/tools/
> telemetry/telemetry/core/forwarders/android_forwarder.py#newcode100
> tools/telemetry/telemetry/core/forwarders/android_forwarder.py:100:
> (protocol, port_pair.remote_port, self.host_ip, port_pair.local_port))
> This part and the parts that add the parens around the device() lg.
> Could we split those into a separate patch and land that first?
>
> The routing policy parts I'd like to understand a little better and
> verify that they'll work on J, K and L.
>
> https://codereview.chromium.org/398003007/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chrishenry, 2014-07-18 17:16:59]

Do you want to update the code review description and subject?

[tonyg, 2014-07-18 17:24:17]

Do you want to update the code review description and subject?
>

I took care of that before approving.

> The changes related to the routing policy and netd are now in
https://codereview.chromium.org/404803003

For future reference, after you upload a patch, please add reviewers then
hit the publish and mail comments button to notify them.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-07-18 17:25:40]

thanks, will do


On Fri, Jul 18, 2014 at 10:23 AM, Tony Gentilcore <tonyg@chromium.org>
wrote:

>
>
> Do you want to update the code review description and subject?
>>
>
> I took care of that before approving.
>
> > The changes related to the routing policy and netd are now in
> https://codereview.chromium.org/404803003
>
> For future reference, after you upload a patch, please add reviewers then
> hit the publish and mail comments button to notify them.
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-07-18 22:40:06]

Change committed as 284229