Skip to content

Align with recent cfg80211 header #63

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

Merged
merged 4 commits into from
May 21, 2024
Merged

Conversation

jychen0611
Copy link
Collaborator

Starting from kernel version 6.7, several significant
changes have been made to the cfg80211 subsystem.
1.
The scan_width field is no longer present in the
cfg80211_inform_bss structure.
There really isn't any support for scanning at different
channel widths than 20 MHz since there's no way to set it.
2.
The parameters in the change_beacon function have been
updated to cfg80211_ap_update.
The change_beacon function now includes two additional
parameters, fils_discovery and unsol_bcast_probe_resp,
which arepart of the cfg80211_ap_update structure.

Starting from kernel version 6.7, several significant
changes have been made to the cfg80211 subsystem.
1.
The scan_width field is no longer present in the
cfg80211_inform_bss structure.
There really isn't any support for scanning at different
channel widths than 20 MHz since there's no way to set it.
2.
The parameters in the change_beacon function have been
updated to cfg80211_ap_update.
The change_beacon function now includes two additional
parameters, fils_discovery and unsol_bcast_probe_resp,
which arepart of the cfg80211_ap_update structure.
@jserv jserv requested a review from rickywu0421 May 20, 2024 09:11
vwifi.c Outdated
@@ -1611,7 +1613,41 @@ static int vwifi_change_beacon(struct wiphy *wiphy,

return 0;
}
#else
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, surrounding the changed lines with the #if directives is better since there are only a few lines changed.

That is:

static int vwifi_change_beacon(struct wiphy *wiphy,
                               struct net_device *ndev,
#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 7, 0)
                               struct cfg80211_beacon_data *info
#else
                               struct cfg80211_ap_update *info)
#endif

Copy link
Collaborator Author

@jychen0611 jychen0611 May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, due to the changes within this function, the frequency of using #if will significantly increase.
Is this acceptable? If so, I will proceed with the modification immediately.

vwifi.c Outdated
static int vwifi_change_beacon(struct wiphy *wiphy,
struct net_device *ndev,
struct cfg80211_beacon_data *info)
#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 7, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the code fitting recent kernel versions earlier. That is,

#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 7, 0)
    struct cfg80211_ap_update *info
#else
    struct cfg80211_beacon_data *info
#endif

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be modified?

#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 7, 0)
            .scan_width = NL80211_BSS_CHAN_WIDTH_20,
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be modified?

Yes, make it consistent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later, we might introduce version check macro later. See https://github.com/sysprog21/simplefs/blob/master/simplefs.h#L78-L81

vwifi.c Outdated
@@ -529,7 +532,8 @@ static enum hrtimer_restart vwifi_beacon(struct hrtimer *timer)
.boottime_ns = ktime_get_boottime_ns(),
.chan = vif->channel,
};

#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 7, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrite as following:

#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 7, 0)
...
#endif

You should explain the above changes in git commit messages.

Adopt rickywu0421's suggestion:
Surrounding the changed lines with #if directives is better since
there are only a few lines changed.
Adopt jserv's suggestion:
Place the newer kernel versions at the top to maintain consistency
with the coding style.
Adopt jserv's suggestion:
When there are no newer versions in the program statement,
the form "#if LINUX_VERSION_CODE < KERNEL_VERSION" can be
used for writing.
@jserv jserv merged commit 35ca058 into sysprog21:main May 21, 2024
4 checks passed
@jserv
Copy link
Contributor

jserv commented May 21, 2024

Thank @jychen0611 for contributing! I have amended the git commit messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants