Postpone checking peer address change after the packet is successfully

Postpone checking peer address change after the packet is successfully
decrypted. Protected behind FLAGS_check_peer_address_change_after_decryption.

Merge internal change: 113259990

R=rch@chromium.org

[ramant (doing other things), 2016-02-03 05:58:50]

On 2016/02/03 05:53:39, ramant wrote:

Hi Ryan,
  I was talking with Jana about this CL. Couple of things missing in this CL
are: 

1) no unit tests for the new code.
2) net's base code doesn't have complete support for subnet masking. Did the
crude method of comparing the first 3 bytes (24 bits) of IPAddressNumber.
3) Jana found a bug and filed it: 26945668 where the current code assumes IP
Address doesn't change. He pointed out it may be true for server but need not be
true for client.

  I am wondering if we should go with this change or revert the internal CL and
merge back after it has more tests.

thanks
raman

[ramant (doing other things), 2016-02-03 06:21:24]

The CQ bit was checked by rtenneti@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-03 06:21:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1663493003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1663493003/1

[commit-bot: I haz the power, 2016-02-03 06:33:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-03 06:33:49]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[chromium-reviews, 2016-02-03 14:44:04]

Hi Raman,

  1) This change is covered by e2e test. This code currently is flag
protected.
  2)
  3) Er, I think this should be also true for client? That is why I
postpond the checking after decryption. For multpath, self address should
not change for the same path. For single path connection migration, I am
not sure, need to discuss with Jana.

  Feel free to revert it. I would add unit tests and other missing things.

  Thanks,

  Fan

On Tue, Feb 2, 2016 at 9:58 PM, <rtenneti@chromium.org> wrote:

> On 2016/02/03 05:53:39, ramant wrote:
>
> Hi Ryan,
>   I was talking with Jana about this CL. Couple of things missing in this CL
> are:
>
> 1) no unit tests for the new code.
> 2) net's base code doesn't have complete support for subnet masking. Did the
> crude method of comparing the first 3 bytes (24 bits) of IPAddressNumber.
> 3) Jana found a bug and filed it: 26945668 where the current code assumes IP
> Address doesn't change. He pointed out it may be true for server but need not
be
> true for client.
>
>   I am wondering if we should go with this change or revert the internal CL
and
> merge back after it has more tests.
>
> thanks
> raman
> https://codereview.chromium.org/1663493003/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ryan Hamilton, 2016-02-04 00:14:06]

lgtm

[Jana, 2016-02-04 01:42:50]

jri@chromium.org changed reviewers:
+ jri@chromium.org

[Jana, 2016-02-04 01:42:50]

I think we should go ahead with merging this CL in, since Fan's offered to write
more unit tests in google3.

[ramant (doing other things), 2016-02-04 02:36:33]

On 2016/02/04 01:42:50, Jana wrote:
> I think we should go ahead with merging this CL in, since Fan's offered to
write
> more unit tests in google3.

Thanks Jana. Landing this as part of the following merge CL:

https://codereview.chromium.org/1660593004/

[chromium-reviews, 2016-02-04 03:28:23]

Have landed a CL (cl/113774550) to add test and move
DeterminePeerAddressChangeType from quic_connection to quic_utils.

Thanks, Ryan, Jana and Raman!

On Wed, Feb 3, 2016 at 6:36 PM, <rtenneti@chromium.org> wrote:

> On 2016/02/04 01:42:50, Jana wrote:
> > I think we should go ahead with merging this CL in, since Fan's offered to
> write
> > more unit tests in google3.
>
> Thanks Jana. Landing this as part of the following merge CL:
> https://codereview.chromium.org/1660593004/
> https://codereview.chromium.org/1663493003/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.