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

Issue 780293 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: 14
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature
Launch-Accessibility: NA
Launch-Legal: NotReviewed
Launch-M-Approved: 64-Dev
Launch-M-Target: 64-Dev
Launch-Privacy: NotReviewed
Launch-Security: NotReviewed
Launch-Test: NotReviewed
Launch-UI: NotReviewed



Sign in to add a comment

Switch over Intel wifi devices to using Intel's cfg80211

Project Member Reported by kirtika@google.com, Oct 31 2017

Issue description

Feature description: 
cfg80211 is a layer in the wireless stack. Currently, Chrome OS picks up the cfg80211, like all other things in the kernel, from upstream, when a kernel version is fixed. We continue to use that cfg80211 with sporadic patches and bugfixes on top. 
The problem is that with 5 active kernels shipping [1], this implies we are often shipping very old cfg80211 and the Intel driver folks have trouble shipping their driver+mac80211 with the old cfg80211. The proposal is to get Intel's cfg80211 but apply it only on Intel devices. 

This or an alternate solution should launch in R64, as this is blocking the Core30 Intel driver drop. 


[1] 3.8, 3.10, 3.14 and 4.4 - only 4 of these kernels have Intel wifi devices. 
3.10 doesn't have any Intel devices on it anymore.


Eng owner:
Product owner:

PRD:
Mocks:
Design doc (send to chrome-design-docs@):

Groups link to chrome-product-review@ discussion:
Summary of chrome-product-review@ feedback:

Metrics: Please list the histograms and user actions that you intend to
analyze for this launch. Include both the metrics that you will use to
define success and the metrics that you will monitor to identify
regressions. If possible, provide a sample analysis showing the current
metrics on Dev. See https://goto.google.com/chrome-launch-metrics for
examples.

If you need help defining metrics or using Finch, you should ask for a
Metrics ambassador. Go to https://goto.google.com/chrome-metrics-ambassador
to get a Metrics ambassador and CC them on this bug.

#################################################################
# Fill these surveys out as you are ready for various reviews.  #
#################################################################

Accessibility survey: The accessibility survey is included in a review bug
that will be filed by lpalmaro@. Please answer all questions there.

Legal survey: Email ctanaka@ (for non-Chrome OS) or jlchen@ (for Chrome OS)
to request a legal review.

Privacy survey: When you flip Launch-Status to Review-Requested, the
privacy team will be notified. Once they've triaged your
launch, a blocking privacy review bug will be filed. Fill out the privacy
survey included in that bug. Email yitingc@ for any questions.

Test survey (https://goto.google.com/chrome-test-questions): Fill in the
survey at the go link https://goto.google.com/chrome-test-questions and
paste the filled in survey as an update comment on this launch bug. Make
sure to fill complete this when flipping to review-requested status. Email
ananthak@ with any questions.

UI survey: Email chrome-ui-review@ (for non-Chrome OS) or chromeos-ui-
review@ (for Chrome OS) to request a UI review if your launch will change
any user-visible strings, assets, animations, or workflows.

 
Labels: TEST-aashutoshk TEST-harpreet
IIRC, you said there's a patch stack somewhere already for this? Any pointers?

Personally, I'd rather not see yet another forked component where possible. For 4.4 especially (and to some extent, 3.18) we've kept an upstream-facing (rather than regressive, "stable"-targeted) approach for all our drivers. For the driver I've spent the most time on (mwifiex), I expect we can manage the upgrade without too much trouble. I'm less sure about (and familiar with) ath10k/ar10k.

Anyway, if we have any preliminary patches, I'm interested in at least looking before committing.
Cc: emmanuel...@intel.com johannes...@intel.com matt.c...@intel.com luciano....@intel.com
Adding Intel folks as FYI. 

@Brian: You can start with the following commit on the 4.4 kernel (which was reverted for the Core28 drop). I don't know if this is complete in itself. 

commit d8c2a9f07826d3c6f8ba52a7c885e27b256cc47a
Author: Johannes Berg <johannes.berg@intel.com>
Date:   Thu May 18 09:54:19 2017 +0200

    CHROMIUM: iwl7000: chromeOS: include cfg80211
    
    In order to simplify maintenance of the iwl7000 driver, include a
    copy of cfg80211. This copy is renamed to iwl7000_cfg80211 and, just
    like we already had with mac80211, all symbols within it are renamed
    so that the regular cfg80211 and this one can be installed at the
    same time, and the module loader will pick the correct one at runtime.
    
    This significantly reduces the amount of divergence/patching of the
    iwl7000 driver from the regular mainline/Intel-internal driver, and
    the amount of "glue code" needed. More new glue code is added to make
    cfg80211 work on older kernels, but that code is mostly taken from the
    backports project and well-tested already.
    
    This does, however, remove the ability to have multiple wireless NICs
    in use at the same time.
    
    Signed-off-by: Johannes Berg <johannes.berg@intel.com>
    iwl7000-tree: 95d76605d09a9d4f4937591ce9481d140c1a5bfc

Brian, some clarifications.  We develop the iwlwifi driver close to the Linux mainline.  We push out our patches there about every week and apply the changes from upstream to our internal tree very often too.

To do this, we rely on the backports project (basically, our iwlwifi tree is a backport that uses that project).  This allows us to use the latest version of our driver on any kernel from 3.10 up to the latest release.

The advantage of this is that we can give you the latest version of our driver out-of-the box.  The backports project includes cfg80211/mac80211/iwlwifi.  The backport we have been releasing to ChromeOS until now includes only mac80211/iwlwifi.  So we need to maintain one extra backport layer between our mac80211 and your cfg80211.

If we can avoid this extra layer, we can keep the iwl7000 driver much closer to the mainline and to our internal trees.  And that is the whole purpose of this addition.
Labels: -Launch-Accessibility-NotReviewed Launch-Accessibility-NA
Sounds like there isn't a11y impact here, but please reach out to me directly if that is not the case. 

Comment 6 by dchan@chromium.org, Nov 7 2017

Feature freeze for M64 is this Friday Nov 10, 2017
This launch feature is missing the test review.  Please file a test review ASAP and feel free to contact the test owner if you need help (see TEST- label below for owner)
Labels: -Type-Launch Pri-1 Type-Feature
I don't think this issue needs to be a launch bug -- it's updating internal plumbing that has no user visible impact. Changing to feature instead of launch.
All the launch labels are still intact :(

Kirtika, I would recommend closing this issue and filing another issue as a "feature" so that we don't cause unneeded work for privacy / security / legal etc. teams.
Labels: -Type-Feature Type-Bug-Security
I'm moving this from a feature to a bug-security since I was informed this shouldn't be reviewed as a new feature for M64.  Please modify if there's a better option.  

Thanks
Labels: -Type-Bug-Security Type-Feature
There is no need for this to be a Type=Bug-Security, since it's not a security risk to users. Type=Feature is exactly what we want. Note that Type=Feature != Type=Launch. Without Type=Launch, this won't show in chromefeatures. I still think Sameer is right and this should likely be closed in favour of something that never was a Type=Launch bug in the first place, but as is this will not show up in any release dashboards.
Status: WontFix (was: Assigned)
Closing for now. Will let Kirtika file a new issue as needed.
Filed https://bugs.chromium.org/p/chromium/issues/detail?id=782368
Sorry about the confusion - I filed this only to get the right nags/reminders for M64. 

Cc: -yoshi@chromium.org

Sign in to add a comment