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

Issue 658831 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

shill RTNL dump requests extremely flaky on kernel v3.18

Project Member Reported by gdk@chromium.org, Oct 24 2016

Issue description

Shill currently uses the rtgenmsg header when requesting RTNL dumps.  This is actually the wrong header to use for these requests, but the kernel has backwards-compatibility logic for older versions of iproute2 which we intermittently trigger.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/86535c9f0e74aad6be5fe98132044df1456b0901

commit 86535c9f0e74aad6be5fe98132044df1456b0901
Author: Garret Kelly <gdk@google.com>
Date: Mon Oct 24 19:33:04 2016

shill: use correct header when requesting an RTNL dump

When requesting a dump the correct sub-header is ifinfomsg.  The kernel
has logic to detect older versions of iproute2 which use the same header
here and provide compatability.  However, we don't always trigger this
logic (and in fact trigger it very sporadically on v3.18).  Also drops
the now-unused rtgenmsg header from RTNLMessage.

BUG= chromium:658831 
TEST=unit tests on gale and samus, and manually on v3.14- and
  v3.18-kernel devices

Change-Id: I6b9423898dfb9b2186ad62b074d196d329403092
Reviewed-on: https://chromium-review.googlesource.com/402292
Commit-Ready: Garret Kelly <gdk@chromium.org>
Tested-by: Garret Kelly <gdk@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/86535c9f0e74aad6be5fe98132044df1456b0901/net/rtnl_message.cc

Comment 2 Deleted

Comment 3 Deleted

Comment 4 by gdk@chromium.org, Oct 25 2016

Cc: hyehia@chromium.org
Labels: M-55 Merge-Request-55
Status: Fixed (was: Started)
Jetstream would like to get this into M55, but I'm unsure who the shill TPMs are.

Comment 5 by gdk@chromium.org, Oct 25 2016

FTR the deleted comments were just bugdroid reposting the same commit message for reasons unknown.

Comment 6 by hyehia@chromium.org, Oct 25 2016

Cc: bhthompson@google.com
Labels: Merge-Requested-55
+ Bernie
Labels: -Merge-Request-55 -Merge-Requested-55 Merge-Approved-55
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 25 2016

Labels: merge-merged-release-R55-8872.B
The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/dba1752c5453d56a4ed7ede9f7379a91a9495e65

commit dba1752c5453d56a4ed7ede9f7379a91a9495e65
Author: Garret Kelly <gdk@google.com>
Date: Mon Oct 24 19:33:04 2016

shill: use correct header when requesting an RTNL dump

When requesting a dump the correct sub-header is ifinfomsg.  The kernel
has logic to detect older versions of iproute2 which use the same header
here and provide compatability.  However, we don't always trigger this
logic (and in fact trigger it very sporadically on v3.18).  Also drops
the now-unused rtgenmsg header from RTNLMessage.

BUG= chromium:658831 
TEST=unit tests on gale and samus, and manually on v3.14- and
  v3.18-kernel devices

Change-Id: I6b9423898dfb9b2186ad62b074d196d329403092
Reviewed-on: https://chromium-review.googlesource.com/402292
Commit-Ready: Garret Kelly <gdk@chromium.org>
Tested-by: Garret Kelly <gdk@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>
(cherry picked from commit 86535c9f0e74aad6be5fe98132044df1456b0901)

[modify] https://crrev.com/dba1752c5453d56a4ed7ede9f7379a91a9495e65/net/rtnl_message.cc

Project Member

Comment 9 by sheriffbot@chromium.org, Oct 29 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by sheriffbot@chromium.org, Nov 1 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 11 by gdk@chromium.org, Nov 1 2016

Labels: -Merge-Approved-55
Do we require any manual testing for verification?

Comment 13 by gdk@chromium.org, Dec 2 2016

Status: Verified (was: Fixed)
No, we've manually verified this on gale.

Sign in to add a comment