-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Deepin-Kernel-SIG] [Upstream] [linux 6.6-y] Sync some mac80211 bugfix from mainline v6.7 #614
[Deepin-Kernel-SIG] [Upstream] [linux 6.6-y] Sync some mac80211 bugfix from mainline v6.7 #614
Conversation
Reviewer's Guide by SourceryThis pull request synchronizes several bug fixes from the mainline v6.7 kernel into the Deepin Kernel SIG's linux 6.6-y branch. The changes address issues in mac80211, including debugfs handling, mesh path management, ROC cancellation, MFP, and queue flushing. It also adds support for operating class 137 in the 6GHz band. Sequence diagram for ROC cancellationsequenceDiagram
participant User Space
participant mac80211
participant Driver
User Space->>mac80211: Cancel ROC
mac80211->>Driver: drv_cancel_remain_on_channel
Driver-->>mac80211: Returns from drv_cancel_remain_on_channel
mac80211->>mac80211: wiphy_work_cancel(hw_roc_done)
Note over mac80211: Cancel the work to ensure all pending workers have completed execution
Sequence diagram for association managementsequenceDiagram
participant ieee80211_mgd_assoc
participant ieee80211_destroy_auth_data
ieee80211_mgd_assoc->>ieee80211_destroy_auth_data: destroy auth_data if association continues
Note over ieee80211_mgd_assoc,ieee80211_destroy_auth_data: Cleanup is delayed if auth_data matches
ieee80211_mgd_assoc->>ieee80211_destroy_auth_data: destroy auth_data after association
Updated class diagram for ieee80211_sub_if_dataclassDiagram
class ieee80211_sub_if_data {
+debugfs_dir
+deflink
+flags
+vif
}
class ieee80211_link_data {
+debugfs_dir
}
ieee80211_sub_if_data -- ieee80211_link_data: has a
note for ieee80211_sub_if_data "The debugfs_dir is now also assigned to deflink."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @opsiff - I've reviewed your changes - here's some feedback:
Overall Comments:
- The debugfs changes look good, but it might be worth double-checking how they interact with the driver's debugfs implementation.
- The commit message for 'wifi: mac80211: purge TX queues in flush_queues flow' could be more descriptive about why this change is needed.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -927,11 +929,24 @@ void ieee80211_debugfs_rename_netdev(struct ieee80211_sub_if_data *sdata) | |||
debugfs_rename(dir->d_parent, dir, dir->d_parent, buf); | |||
} | |||
|
|||
void ieee80211_debugfs_recreate_netdev(struct ieee80211_sub_if_data *sdata, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (complexity): Consider using early returns/guard clauses to reduce nesting and improve readability in functions like ieee80211_debugfs_recreate_netdev
and ieee80211_link_debugfs_add
.
In functions like `ieee80211_debugfs_recreate_netdev` and `ieee80211_link_debugfs_add`, consider using early returns/guard clauses to avoid extra nesting. For example, you may refactor the nested if–statements in `ieee80211_debugfs_recreate_netdev` as follows:
--- Before ---
```c
ieee80211_debugfs_remove_netdev(sdata);
ieee80211_debugfs_add_netdev(sdata, mld_vif);
if (sdata->flags & IEEE80211_SDATA_IN_DRIVER) {
drv_vif_add_debugfs(sdata->local, sdata);
if (!mld_vif)
ieee80211_link_debugfs_drv_add(&sdata->deflink);
}
--- After ---
ieee80211_debugfs_remove_netdev(sdata);
ieee80211_debugfs_add_netdev(sdata, mld_vif);
if (!(sdata->flags & IEEE80211_SDATA_IN_DRIVER))
return;
drv_vif_add_debugfs(sdata->local, sdata);
if (mld_vif)
return;
ieee80211_link_debugfs_drv_add(&sdata->deflink);
Similarly, in ieee80211_link_debugfs_add
, instead of combining multiple WARN_ON checks in one compound condition, separate them:
--- Before ---
if (WARN_ON(!link->sdata->vif.debugfs_dir || link->debugfs_dir))
return;
if (WARN_ON(!(link->sdata->local->hw.wiphy->flags & WIPHY_FLAG_SUPPORTS_MLO)))
return;
--- After ---
if (!link->sdata->vif.debugfs_dir) {
WARN_ON(true);
return;
}
if (link->debugfs_dir) {
WARN_ON(true);
return;
}
if (!(link->sdata->local->hw.wiphy->flags & WIPHY_FLAG_SUPPORTS_MLO)) {
WARN_ON(true);
return;
}
These small changes improve readability by reducing nested branches while keeping the functionality intact.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sourcery-ai[bot] The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
mainline inclusion from mainline-v6.7-rc1 category: bugfix Convert all instances of RX_DROP_UNUSABLE to indicate a better reason, and then remove RX_DROP_UNUSABLE. Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit dccc9aa) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: bugfix This has many different reasons, split the return value into the individual reasons for better traceability. Also, since symbolic tracing doesn't work for these, add a few comments for the numbering. Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit 6c02fab) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: bugfix If the association command fails then the authentication is still valid and it makes sense to keep it alive. Otherwise, we would currently get into an inconsistent state because mac80211 on the one hand is disconnected but on the other hand the state is not entirely cleared and a new authentication could not continue. Signed-off-by: Benjamin Berg <[email protected]> Signed-off-by: Gregory Greenman <[email protected]> Link: https://lore.kernel.org/r/20230928172905.c9855f46ebc8.I7f3dcd4120a186484a91b87560e9b7201d40984f@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit 6b398f1) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: bugfix In MLO, we have a per-link debugfs directory which contains the per-link files. In case of non-MLO we would like to put the per-link files in the netdev directory to keep it how it was before MLO. - Upon interface creation the netdev will be created with the per-link files in it. - Upon switching to MLO: delete the entire netdev directory and then recreate it without the per-link files. Then the per-link directories with the per-link files in it will be created in ieee80211_link_init() - Upon switching to non-MLO: delete the entire netdev directory (including the per-link directories) and recreate it with the per-link files in it. Note that this also aligns to always call the vif link debugfs method for the deflink as promised in the documentation, which wasn't done before. Signed-off-by: Miri Korenblit <[email protected]> Signed-off-by: Johannes Berg <[email protected]> Signed-off-by: Gregory Greenman <[email protected]> Link: https://lore.kernel.org/r/20230928172905.082e698caca9.I5bef7b2026e0f58b4a958b3d1f459ac5baeccfc9@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit c942398) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: bugfix Draft P802.11be_D3.1 added operating class to describe 320 MHz operation in the 6GHz band. Include this new operating class in ieee80211_operating_class_to_band(). Signed-off-by: Ilan Peer <[email protected]> Signed-off-by: Gregory Greenman <[email protected]> Link: https://lore.kernel.org/r/20230928172905.bed4a007d81b.I3eb4b8fe39c0c1a988c98a103b11a9f45a92b038@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit 256caff) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: bugfix These were mostly missing or incorrectly tagged return values. Signed-off-by: Benjamin Berg <[email protected]> Signed-off-by: Gregory Greenman <[email protected]> Link: https://lore.kernel.org/r/20230928172905.33fea2968c62.I41d197b570370ab7cad1405518512fdd36e08717@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit c00de1c) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: bugfix When the remain on channel is removed at the time it should have expired, we have a race: the driver could be handling the flow of the expiration while mac80211 is cancelling that very same remain on channel request. This wouldn't be problem in itself, but since mac80211 can send the next request to the driver in the cancellation flow, we can get to the following situation: CPU0 CPU1 expiration of roc in driver ieee80211_remain_on_channel_expired() Cancellation of the roc schedules a worker (hw_roc_done) Add next roc hw_roc_done_wk runs and ends the second roc prematurely. Since, by design, there is only one single request sent to the driver at a time, we can safely assume that after the cancel() request returns from the driver, we should not handle any worker that handles the expiration of the request. Cancel the hw_roc_done worker after the cancellation to make sure we start the next one with a clean slate. Signed-off-by: Emmanuel Grumbach <[email protected]> Signed-off-by: Gregory Greenman <[email protected]> Link: https://lore.kernel.org/r/20230928172905.4e4469be20ac.Iab0525f5cc4698acf23eab98b8b1eec02099cde0@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit 9ad08fb) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: bugfix When this flow is invoked with the "drop" parameter as true, we only drop the frames from the hw queues, but not from the sw queues. So when we call wake_queues() after hw queue purging, all the frames from the sw queues will be TX'ed, when what we actually want to do is to purge all queues in order to not TX anything... This can cause, for example, TXing data frames to the peer after the deauth frame was sent. Fix this by purging the sw queues in addition to the hw queues if the drop parameter is true. Signed-off-by: Miri Korenblit <[email protected]> Signed-off-by: Gregory Greenman <[email protected]> Link: https://lore.kernel.org/r/20230928172905.8fc2ee23e56f.I8b3f6def9c28ea96261e2d31df8786986fb5385b@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit 3831f6d) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: bugfix To be able to more easily understand the code, drop robust action frames before being associated, even if there's no MFP in the end, as they are Class 3 Frames and shouldn't be transmitted in the first place. Signed-off-by: Johannes Berg <[email protected]> Signed-off-by: Gregory Greenman <[email protected]> Link: https://lore.kernel.org/r/20231001125722.b2fd37083371.Ie9f4906e2f6c698989bce6681956ed2f9454f27c@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit f3bd593) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7 category: bugfix The driver debugfs entries still exist when the interface is re-added during reconfiguration. This can be either because of a HW restart (in_reconfig) or because we are resuming. Fixes: a1f5dcb ("wifi: mac80211: add a driver callback to add vif debugfs") Signed-off-by: Benjamin Berg <[email protected]> Reviewed-by: Gregory Greenman <[email protected]> Signed-off-by: Miri Korenblit <[email protected]> Link: https://msgid.link/20231220043149.ddd48c66ec6b.Ia81080d92129ceecf462eceb4966bab80df12060@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit 8c917f1) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7 category: bugfix When an interface is removed, we should also be deleting the driver debugfs entries (as it might still exist in DOWN state in mac80211). At the same time, when adding an interface, we can check the IEEE80211_SDATA_IN_DRIVER flag to know whether the interface was previously known to the driver and is simply being reconfigured. Fixes: a1f5dcb ("wifi: mac80211: add a driver callback to add vif debugfs") Signed-off-by: Benjamin Berg <[email protected]> Reviewed-by: Gregory Greenman <[email protected]> Signed-off-by: Miri Korenblit <[email protected]> Link: https://msgid.link/20231220043149.a9f64c359424.I7076526b5297ae8f832228079c999f7b8e147a4c@changeid Signed-off-by: Johannes Berg <[email protected]> (cherry picked from commit 0a3d898) Signed-off-by: Wentao Guan <[email protected]>
6fa3f59
to
62c6a6d
Compare
Fix patches in #69
Commit(11):
wifi: mac80211: add/remove driver debugfs entries as appropriate
wifi: mac80211: do not re-add debugfs entries during resume
wifi: mac80211: drop robust action frames before assoc
wifi: mac80211: purge TX queues in flush_queues flow
wifi: mac80211: fix a expired vs. cancel race in roc
wifi: mac80211: mesh: fix some kdoc warnings
wifi: cfg80211: Include operating class 137 in 6GHz band
wifi: mac80211: handle debugfs when switching to/from MLO
wifi: mac80211: cleanup auth_data only if association continues
wifi: mac80211: split ieee80211_drop_unencrypted_mgmt() return value
wifi: mac80211: remove RX_DROP_UNUSABLE
Summary by Sourcery
This pull request synchronizes several bug fixes and enhancements from the mainline kernel v6.7 into the Deepin Kernel, primarily focusing on mac80211 improvements. These include fixes for race conditions, debugfs handling, and memory management, as well as enhancements to mesh networking and support for the 6GHz band.
Bug Fixes:
Enhancements:
Chores: