New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 914444 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Dec 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 649417



Sign in to add a comment

[network_nightly] network_VPNConnect.l2tpipsec_psk and network_VPNConnect.l2tpipsec_cert tests failing on all boards "vpn connection failed"

Project Member Reported by jmuppala@google.com, Dec 12

Issue description

network_VPNConnect.l2tpipsec_psk and network_VPNConnect.l2tpipsec_cert	tests failing since Dec 10th 2018 on all boards with "vpn connection failed" error.

stainless link:
https://stainless.corp.google.com/search?view=matrix&row=queued_date&col=build&first_date=2018-12-06&last_date=2018-12-12&test=network_VPNConnect.l2tpipsec_psk%7Cnetwork_VPNConnect.l2tpipsec_cert&status=GOOD&status=WARN&status=FAIL&status=ERROR&status=ABORT&status=ALERT&status=NOT_RUN&exclude_cts=true&exclude_not_run=false&exclude_non_release=true&exclude_au=true&exclude_acts=true&exclude_retried=true&exclude_non_production=false


sample failure:
Traceback (most recent call last):
  File "/usr/local/autotest/common_lib/test.py", line 600, in _exec
    _call_test_function(self.execute, *p_args, **p_dargs)
  File "/usr/local/autotest/common_lib/test.py", line 800, in _call_test_function
    return func(*args, **dargs)
  File "/usr/local/autotest/common_lib/test.py", line 464, in execute
    postprocess_profiled_run, args, dargs)
  File "/usr/local/autotest/common_lib/test.py", line 371, in _call_run_once
    self.run_once(*args, **dargs)
  File "/usr/local/autotest/tests/network_VPNConnect/network_VPNConnect.py", line 219, in run_once
    self.run_vpn_test(vpn_type)
  File "/usr/local/autotest/tests/network_VPNConnect/network_VPNConnect.py", line 255, in run_vpn_test
    if self.connect_vpn():
  File "/usr/local/autotest/tests/network_VPNConnect/network_VPNConnect.py", line 208, in connect_vpn
    raise error.TestFail('VPN connection failed')
TestFail: VPN connection failed
12/11 08:24:30.023 DEBUG|              test:0611| Running cleanup for test.
12/11 08:24:30.024 DEBUG|   logging_manager:0627| Logging subprocess finished
12/11 08:24:30.029 DEBUG|   logging_manager:0627| Logging subprocess finished

 
Cc: mortonm@chromium.org benchan@chromium.org
Owner: mortonm@chromium.org
Status: Assigned (was: Untriaged)
These are squarely in that range:

https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1365088/
https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1365089/
2018-12-11T19:26:46.335016+00:00 INFO shill[1578]: [INFO:l2tp_ipsec_driver.cc(228)] L2TP/IPSec VPN process options: --psk_file=/run/shill/.org.chromium.Chromium.AWGDez --remote_host=10.9.8.1 --pppd_plugin=/usr/lib/shill/shims/shill-pppd-plugin.so --nosystemconfig --user=chapuser
2018-12-11T19:26:46.335089+00:00 INFO shill[1578]: [INFO:rpc_task.cc(26)] RPCTask 2 created.
2018-12-11T19:26:46.350825+00:00 INFO l2tpipsec_vpn[2079]: Starting starter
2018-12-11T19:26:46.350865+00:00 ERR l2tpipsec_vpn[2079]: Unable to fstat fd 2: 9
2018-12-11T19:26:46.350877+00:00 ERR l2tpipsec_vpn[2079]: Failing to start because pipe creation failed
2018-12-11T19:26:46.350888+00:00 ERR l2tpipsec_vpn[2079]: Starter did not start successfully
2018-12-11T19:26:46.350900+00:00 ERR l2tpipsec_vpn[2079]: Unable to start IPsec layer
Blocking: 649417
Labels: -Pri-2 ReleaseBlock-Stable M-72 Pri-1
M-72 is broken too, since we ported those CLs.
Labels: ReleaseBlock-Beta
Status: Started (was: Assigned)
Interesting, looks like stopping allowing shill VPN sub-processes to inherit file descriptors (CL:1365088) is a fix for the OpenVPN bug ( crbug.com/911234 ) but breaks L2TP/IPsec VPN. Should have tested L2TP/IPsec when I tested OpenVPN after the change. I guess I'll mark as RBB and try to get a fix in today.
Can you please provide an update for this issue?

M72 Beta is targeted for early next week and this issue is currently blocking. Thanks
Is anyone able to pick this up since mortonm@ is OOO today?
Owner: briannorris@chromium.org
Status: Assigned (was: Started)
I can probably finish it up. He has CLs out that are mostly ready. Didn't know he was OOO.

(Micah: if you are working on this today, feel free to steal back. I'll mark Started if I really get to it myself.)

Also, I marked RBS. Not sure if enough people use L2TP/IPSEC on Beta for this to be a true Beta blocker.
Owner: mortonm@chromium.org
(Didn't get time for it today)
Labels: -ReleaseBlock-Beta
Yeah lets just take away RBB. It should be a quick fix. Just need to land CL:1374533 and CL:1375052 and merge back and we've mostly settled on those CLs. Will probably beat the beta release unless its on Monday. briannorris@ has the better sense of whether this should be RBS or RBB anyway.
Heres a reference UMA lookup for a few days after the first M71 Beta push went out as to how many users we had on L2TP/IPSec: https://uma.googleplex.com/histograms?endDate=20181012&dayCount=1&histograms=Network.Shill.Vpn.Driver&fixupData=true&showMax=true&filters=platform%2Ceq%2CC%2Cchannel%2Ceq%2C3%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

Later beta pushes in that milestone show about the same numbers.

According to https://sites.google.com/a/google.com/chromeos/resources/engineering/weekly-release-plan?pli=1, M72 beta push is scheduled for Wednesday, so hopefully we can get the CLs in and merged back before then. They're headed to the CQ now
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/589c096d3635e926f70515f5a40785bc8357773b

commit 589c096d3635e926f70515f5a40785bc8357773b
Author: Micah Morton <mortonm@chromium.org>
Date: Tue Dec 18 04:47:11 2018

libbrillo: add PreserveFd function to brillo::Minijail

Wrapper around the minijail_preserve_fd function in libminijail.

BUG= chromium:914444 
TEST=tested as part of fix for chromium:914444

Change-Id: I25998fc640bab7f9107abf6bab8ce600d4036c6d
Reviewed-on: https://chromium-review.googlesource.com/1375052
Commit-Ready: Micah Morton <mortonm@chromium.org>
Tested-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/589c096d3635e926f70515f5a40785bc8357773b/libbrillo/brillo/minijail/mock_minijail.h
[modify] https://crrev.com/589c096d3635e926f70515f5a40785bc8357773b/libbrillo/brillo/minijail/minijail.cc
[modify] https://crrev.com/589c096d3635e926f70515f5a40785bc8357773b/libbrillo/brillo/minijail/minijail.h

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4e5589c7e530af99170369f68bcc4f0b0564f531

commit 4e5589c7e530af99170369f68bcc4f0b0564f531
Author: Micah Morton <mortonm@chromium.org>
Date: Tue Dec 18 08:42:59 2018

shill: add close_nonstd_fds arg to StartProcessInMinijail

Shill starts tc, logger, dhcpcd, openvpn, and l2tpipsec_vpn in a
minijail. openvpn needs to have its non-standard file descriptors
cleared by its parent (shill) before starting (see  crbug.com/911234 ).
l2tpipsec_vpn seems to work with just the standard file descriptors as
well. For now not messing with the file descriptors inherited by tc,
logger or dhcpcd in this CL, because we want to get this change into
M72. We can take away the non-standard fds from those other processes in
M73.

CQ-DEPEND=CL:1375052
BUG= chromium:914444 
TEST=network_VPNConnect.openvpn and network_VPNConnect.l2tpipsec_*
autotests. Also performed the debugging steps outlined in CL:386364 to
verify that tc works as expected.

Change-Id: I0147e3f123bbcc30852947e7981ef549983a1922
Reviewed-on: https://chromium-review.googlesource.com/1374533
Commit-Ready: Micah Morton <mortonm@chromium.org>
Tested-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/process_manager.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/vpn/openvpn_driver_test.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/throttler_test.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/dhcp/dhcp_config.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/process_manager.h
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/external_task.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/vpn/l2tp_ipsec_driver_test.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/dhcp/dhcpv4_config_test.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/dhcp/dhcp_config_test.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/dhcp/dhcpv6_config_test.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/external_task.h
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/mock_process_manager.h
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/vpn/l2tp_ipsec_driver.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/process_manager_test.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/vpn/openvpn_driver.cc
[modify] https://crrev.com/4e5589c7e530af99170369f68bcc4f0b0564f531/shill/throttler.cc

Labels: Merge-Request-72
Status: Fixed (was: Assigned)
Just verified that this is fixed on R73-11424.0.0. Need to merge back to M72.
Project Member

Comment 15 by sheriffbot@chromium.org, Dec 18

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-72 Merge-Approved-72
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 18

Labels: merge-merged-release-R72-11316.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/107124984f240eaa50475e3b766a6efe552634ed

commit 107124984f240eaa50475e3b766a6efe552634ed
Author: Micah Morton <mortonm@chromium.org>
Date: Tue Dec 18 17:03:58 2018

libbrillo: add PreserveFd function to brillo::Minijail

Wrapper around the minijail_preserve_fd function in libminijail.

BUG= chromium:914444 
TEST=tested as part of fix for chromium:914444

Change-Id: I25998fc640bab7f9107abf6bab8ce600d4036c6d
Reviewed-on: https://chromium-review.googlesource.com/1375052
Commit-Ready: Micah Morton <mortonm@chromium.org>
Tested-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit 589c096d3635e926f70515f5a40785bc8357773b)
Reviewed-on: https://chromium-review.googlesource.com/c/1382637
Commit-Queue: Micah Morton <mortonm@chromium.org>

[modify] https://crrev.com/107124984f240eaa50475e3b766a6efe552634ed/libbrillo/brillo/minijail/mock_minijail.h
[modify] https://crrev.com/107124984f240eaa50475e3b766a6efe552634ed/libbrillo/brillo/minijail/minijail.cc
[modify] https://crrev.com/107124984f240eaa50475e3b766a6efe552634ed/libbrillo/brillo/minijail/minijail.h

Project Member

Comment 19 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8

commit d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8
Author: Micah Morton <mortonm@chromium.org>
Date: Tue Dec 18 21:37:45 2018

shill: add close_nonstd_fds arg to StartProcessInMinijail

Shill starts tc, logger, dhcpcd, openvpn, and l2tpipsec_vpn in a
minijail. openvpn needs to have its non-standard file descriptors
cleared by its parent (shill) before starting (see  crbug.com/911234 ).
l2tpipsec_vpn seems to work with just the standard file descriptors as
well. For now not messing with the file descriptors inherited by tc,
logger or dhcpcd in this CL, because we want to get this change into
M72. We can take away the non-standard fds from those other processes in
M73.

CQ-DEPEND=CL:1382637
BUG= chromium:914444 
TEST=network_VPNConnect.openvpn and network_VPNConnect.l2tpipsec_*
autotests. Also performed the debugging steps outlined in CL:386364 to
verify that tc works as expected.

Change-Id: I0147e3f123bbcc30852947e7981ef549983a1922
Reviewed-on: https://chromium-review.googlesource.com/1374533
Commit-Ready: Micah Morton <mortonm@chromium.org>
Tested-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
(cherry picked from commit 4e5589c7e530af99170369f68bcc4f0b0564f531)
Reviewed-on: https://chromium-review.googlesource.com/c/1382638
Commit-Queue: Micah Morton <mortonm@chromium.org>

[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/process_manager.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/vpn/openvpn_driver_test.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/throttler_test.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/dhcp/dhcp_config.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/process_manager.h
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/external_task.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/vpn/l2tp_ipsec_driver_test.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/dhcp/dhcpv4_config_test.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/dhcp/dhcp_config_test.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/dhcp/dhcpv6_config_test.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/external_task.h
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/mock_process_manager.h
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/vpn/l2tp_ipsec_driver.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/process_manager_test.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/vpn/openvpn_driver.cc
[modify] https://crrev.com/d9ff56978b0292bfeb4b2782e29b7a7139dfc7c8/shill/throttler.cc

Status: Verified (was: Fixed)
Labels: -Merge-Approved-72

Sign in to add a comment