pull-request: mac80211-next 2021-04-20
Hi, We have a bunch more things for next, now that we got another week "for free" ;-) Pretty much all over the map, see the tag description and shortlog below. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 3cd52c1e32fe7dfee09815ced702db9ee9f84ec9: net: fealnx: use module_pci_driver to simplify the code (2021-04-07 15:15:02 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-net-next-2021-04-20 for you to fetch changes up to 010bfbe768f7ecc876ffba92db30432de4997e2a: cfg80211: scan: drop entry from hidden_list on overflow (2021-04-19 13:25:50 +0200) Another set of updates, all over the map: * set sk_pacing_shift for 802.3->802.11 encap offload * some monitor support for 802.11->802.3 decap offload * HE (802.11ax) spec updates * userspace API for TDLS HE support * along with various other small features, cleanups and fixups Aloka Dixit (1): nl80211: Add missing line in nl80211_fils_discovery_policy Avraham Stern (2): ieee80211: add the values of ranging parameters max LTF total field nl80211/cfg80211: add a flag to negotiate for LMR feedback in NDP ranging Colin Ian King (2): mac80211: remove redundant assignment of variable result mac80211: minstrel_ht: remove extraneous indentation on if statement Emmanuel Grumbach (5): cfg80211: allow specifying a reason for hw_rfkill mac80211: clear the beacon's CRC after channel switch cfg80211: fix an htmldoc warning mac80211: make ieee80211_vif_to_wdev work when the vif isn't in the driver mac80211: properly drop the connection in case of invalid CSA IE Guobin Huang (2): rfkill: use DEFINE_SPINLOCK() for spinlock mac80211_hwsim: use DEFINE_SPINLOCK() for spinlock Ilan Peer (2): cfg80211: Remove wrong RNR IE validation check nl80211: Add new RSNXE related nl80211 extended features James Prestwood (1): nl80211: better document CMD_ROAM behavior Joe Perches (1): cfg80211: constify ieee80211_get_response_rate return Johan Almbladh (1): mac80211: Set priority and queue mapping for injected frames Johannes Berg (8): mac80211: don't apply flow control on management frames mac80211: bail out if cipher schemes are invalid mac80211: properly process TXQ management frames mac80211: aes_cmac: check crypto_shash_setkey() return value wireless: align some HE capabilities with the spec wireless: align HE capabilities A-MPDU Length Exponent Extension wireless: fix spelling of A-MSDU in HE capabilities cfg80211: scan: drop entry from hidden_list on overflow Lorenzo Bianconi (1): mac80211: set sk_pacing_shift for 802.3 txpath Naftali Goldstein (1): mac80211: drop the connection if firmware crashed while in CSA Qiheng Lin (1): cfg80211: regulatory: use DEFINE_SPINLOCK() for spinlock Randy Dunlap (1): cfg80211: fix a few kernel-doc warnings Sriram R (1): mac80211: Allow concurrent monitor iface and ethernet rx decap Vamsi Krishna (1): nl80211: Add interface to indicate TDLS peer's HE capability Wei Yongjun (1): mac80211: minstrel_ht: remove unused variable 'mg' in minstrel_ht_next_jump_rate() drivers/net/wireless/ath/ath11k/mac.c | 15 +++--- drivers/net/wireless/broadcom/b43/main.c | 2 +- drivers/net/wireless/broadcom/b43legacy/main.c | 2 +- drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c | 14 +++--- drivers/net/wireless/mac80211_hwsim.c | 24 - .../net/wireless/mediatek/mt76/mt76_connac_mcu.c | 2 +- drivers/net/wireless/mediatek/mt76/mt7915/init.c | 14 +++--- drivers/net/wireless/mediatek/mt76/mt7915/mcu.c| 6 +-- drivers/net/wireless/mediatek/mt76/mt7921/main.c | 6 +-- include/linux/ieee80211.h | 33 ++-- include/net/cfg80211.h | 24 ++--- include/net/mac80211.h | 12 +++-- include/uapi/linux/nl80211.h | 22 net/mac80211/aes_cmac.c| 11 +++- net/mac80211/debugfs.c | 1 + net/mac80211/debugfs_sta.c | 37 +++--- net/mac80211/ieee80211_i.h | 2 + net/mac80211/iface.c | 3 +- net/mac80211/main.c| 16 -- net/mac80211/mlme.c| 16 +++--- net/mac80211/rc80211_minstrel_ht.c | 4 +- net/mac80211/tx.c | 58 +++--- net/mac80211/util.c
Re: iwlwifi: Microcode SW error
On Mon, 2021-04-19 at 11:54 +0200, Gonsolo wrote: > > Do you use MFP? > > I don't think so. I'm running unmodified kernels from Ubuntu PPA and > don't meddle with WIFI configs. > How can I find out? > > > Could be related to > > https://patchwork.kernel.org/project/linux-wireless/patch/20210416134702.ef8486a64293.If0a9025b39c71bb91b11dd6ac45547aba682df34@changeid/ > > I saw similar issues internally without that fix. > > A quick "git grep" didn't show any of these two patches in the 5.12- > rc7 kernel. > Yeah, my bad, I only put the offending patch into -next, I misremembered. Sorry for the noise. johannes
Re: iwlwifi: Microcode SW error
On Mon, 2021-04-19 at 11:08 +0200, Gon Solo wrote: > > [Apr19 10:50] iwlwifi :02:00.0: Queue 10 is active on fifo 1 and stuck > for 1 ms. SW [40, 93] HW [40, 93] FH TRB=0x0c010a037 > [ +0,001244] iwlwifi :02:00.0: Microcode SW error detected. Restarting > 0x200. > > The rest of the message is at the end of this message. > The kernel version is "Linux Limone 5.12.0-051200rc7-lowlatency" from > https://kernel.ubuntu.com/~kernel-ppa/mainline. > The relevant output of lspci is: > 02:00.0 Network controller: Intel Corporation Wireless 7260 (rev 73) > > I would be glad to provide additional details if somebody is interested > to fix this bug. > > Regards, > Andreas > > [[Apr19 10:50] iwlwifi :02:00.0: Queue 10 is active on fifo 1 and stuck > for 1 ms. SW [40, 93] HW [40, 93] FH TRB=0x0c010a037 > [ +0,001244] iwlwifi :02:00.0: Microcode SW error detected. Restarting > 0x200. > [ +0,000160] iwlwifi :02:00.0: Start IWL Error Log Dump: > [ +0,04] iwlwifi :02:00.0: Status: 0x0040, count: 6 > [ +0,05] iwlwifi :02:00.0: Loaded firmware version: 17.3216344376.0 > 7260-17.ucode > [ +0,05] iwlwifi :02:00.0: 0x0084 | NMI_INTERRUPT_UNKNOWN Do you use MFP? Could be related to https://patchwork.kernel.org/project/linux-wireless/patch/20210416134702.ef8486a64293.If0a9025b39c71bb91b11dd6ac45547aba682df34@changeid/ I saw similar issues internally without that fix. johannes
Re: [PATCH] mac80211_hwsim: indicate support for 60GHz channels
On Wed, 2021-04-14 at 12:06 -0300, Ramon Fontes wrote: > > Advertise 60GHz channels to mac80211. > Oh.. That was a mistake. Sorry for that. > > Anyway, can we indicate 60GHz support even though it is not being > supported by mac80211 yet? > No, that makes no sense. DMG operation is significantly different from non-DMG operation, even the A-MSDU format for example (an abbreviated format called "short A-MSDU" is used in DMG). Similarly beacons, etc., all kinds of operations are significantly different. Adding 60 GHz channels to hwsim would be essentially operating as non- DMG yet on a DMG channel, which makes no sense. I don't think anyone's planning to add DMG support to mac80211, and in the absence of a real driver requiring that it wouldn't make sense anyway. Quite possibly, it simply doesn't make sense period, because DMG operation is sufficiently different. johannes
Re: [PATCH] mac80211_hwsim: indicate support for 60GHz channels
On Mon, 2021-04-12 at 22:06 -0300, Ramon Fontes wrote: > Advertise 60GHz channels to mac80211. This is wrong. mac80211 doesn't support 60 GHz operation. johannes
pull-request: mac80211 2021-04-08.2
Hi, Yes, I'm late with this, sorry about that. I've mostly restricted this to the most necessary fixes, though the virt_wifi one isn't but since that's not used a lot, it's harmless and included since it's obvious. This updated version includes another netlink buffer overrun fix, as reported by syzbot. The only thing that's bigger is the rfkill thing, but that's just since it adds a new version of the struct for userspace to use, since the change to the existing struct caused various breakage all around. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 8a12f8836145ffe37e9c8733dce18c22fb668b66: net: hso: fix null-ptr-deref during tty device unregistration (2021-04-07 15:18:15 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2021-04-08.2 for you to fetch changes up to 9a6847ba1747858ccac53c5aba3e25c54fbdf846: nl80211: fix beacon head validation (2021-04-08 16:43:05 +0200) Various small fixes: * S1G beacon validation * potential leak in nl80211 * fast-RX confusion with 4-addr mode * erroneous WARN_ON that userspace can trigger * wrong time units in virt_wifi * rfkill userspace API breakage * TXQ AC confusing that led to traffic stopped forever * connection monitoring time after/before confusion * netlink beacon head validation buffer overrun A. Cody Schuffelen (1): virt_wifi: Return micros for BSS TSF values Ben Greear (1): mac80211: fix time-is-after bug in mlme Du Cheng (1): cfg80211: remove WARN_ON() in cfg80211_sme_connect Johannes Berg (5): rfkill: revert back to old userspace API by default mac80211: fix TXQ AC confusion cfg80211: check S1G beacon compat element length nl80211: fix potential leak of ACL params nl80211: fix beacon head validation Seevalamuthu Mariappan (1): mac80211: clear sta->fast_rx when STA removed from 4-addr VLAN drivers/net/wireless/virt_wifi.c | 5 ++- include/uapi/linux/rfkill.h | 80 ++-- net/mac80211/cfg.c | 4 +- net/mac80211/mlme.c | 5 ++- net/mac80211/tx.c| 2 +- net/rfkill/core.c| 7 ++-- net/wireless/nl80211.c | 10 +++-- net/wireless/scan.c | 14 --- net/wireless/sme.c | 2 +- 9 files changed, 99 insertions(+), 30 deletions(-)
Re: pull-request: mac80211 2021-04-08
On Thu, 2021-04-08 at 14:53 +0200, Johannes Berg wrote: > Hi, > > Yes, I'm late with this, sorry about that. I've mostly restricted this > to the most necessary fixes, though the virt_wifi one isn't but since > that's not used a lot, it's harmless and included since it's obvious. > > The only thing that's bigger is the rfkill thing, but that's just since > it adds a new version of the struct for userspace to use, since the > change to the existing struct caused various breakage all around. Wait, let me withdraw that - I have another syzbot fix coming. johannes
pull-request: mac80211 2021-04-08
Hi, Yes, I'm late with this, sorry about that. I've mostly restricted this to the most necessary fixes, though the virt_wifi one isn't but since that's not used a lot, it's harmless and included since it's obvious. The only thing that's bigger is the rfkill thing, but that's just since it adds a new version of the struct for userspace to use, since the change to the existing struct caused various breakage all around. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 8a12f8836145ffe37e9c8733dce18c22fb668b66: net: hso: fix null-ptr-deref during tty device unregistration (2021-04-07 15:18:15 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2021-04-08 for you to fetch changes up to abaf94ecc9c356d0b885a84edef4905cdd89cfdd: nl80211: fix potential leak of ACL params (2021-04-08 14:44:56 +0200) Various small fixes: * S1G beacon validation * potential leak in nl80211 * fast-RX confusion with 4-addr mode * erroneous WARN_ON that userspace can trigger * wrong time units in virt_wifi * rfkill userspace API breakage * TXQ AC confusing that led to traffic stopped forever * connection monitoring time after/before confusion A. Cody Schuffelen (1): virt_wifi: Return micros for BSS TSF values Ben Greear (1): mac80211: fix time-is-after bug in mlme Du Cheng (1): cfg80211: remove WARN_ON() in cfg80211_sme_connect Johannes Berg (4): rfkill: revert back to old userspace API by default mac80211: fix TXQ AC confusion cfg80211: check S1G beacon compat element length nl80211: fix potential leak of ACL params Seevalamuthu Mariappan (1): mac80211: clear sta->fast_rx when STA removed from 4-addr VLAN drivers/net/wireless/virt_wifi.c | 5 ++- include/uapi/linux/rfkill.h | 80 ++-- net/mac80211/cfg.c | 4 +- net/mac80211/mlme.c | 5 ++- net/mac80211/tx.c| 2 +- net/rfkill/core.c| 7 ++-- net/wireless/nl80211.c | 4 +- net/wireless/scan.c | 14 --- net/wireless/sme.c | 2 +- 9 files changed, 94 insertions(+), 29 deletions(-)
Re: [PATCH] net: netlink: fix error check in genl_family_rcv_msg_doit
On Sat, 2021-04-03 at 15:13 +, Pavel Skripkin wrote: > genl_family_rcv_msg_attrs_parse() can return NULL > pointer: > > if (!ops->maxattr) > return NULL; > > But this condition doesn't cause an error in > genl_family_rcv_msg_doit And I'm almost certain that in fact it shouldn't cause an error! If the family doesn't set maxattr then it doesn't want to have generic netlink doing the parsing, but still it should be possible to call the ops. Look at fs/dlm/netlink.c for example, it doesn't even have attributes. You're breaking it with this patch. Also, the (NULL) pointer is not actually _used_ anywhere, so why would it matter? johannes
pull-request: mac80211 2021-03-17
Hi Dave, So here's a first set of fixes for the current rc cycle, really nothing major, though the network namespace locking fix is something a few people have been running into. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 13832ae2755395b2585500c85b64f5109a44227e: mptcp: fix ADD_ADDR HMAC in case port is specified (2021-03-15 16:43:01 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2021-03-17 for you to fetch changes up to 239729a21e528466d02f5558936306ffa9314ad1: wireless/nl80211: fix wdev_id may be used uninitialized (2021-03-16 21:20:47 +0100) First round of fixes for 5.12-rc: * HE (802.11ax) elements can be extended, handle that * fix locking in network namespace changes that was broken due to the RTNL-redux work * various other small fixes Brian Norris (1): mac80211: Allow HE operation to be longer than expected. Daniel Phan (1): mac80211: Check crypto_aead_encrypt for errors Jarod Wilson (1): wireless/nl80211: fix wdev_id may be used uninitialized Johannes Berg (3): mac80211: fix rate mask reset mac80211: minstrel_ht: remove unused variable 'mg' nl80211: fix locking for wireless device netns change Karthikeyan Kathirvel (1): mac80211: choose first enabled channel for monitor Markus Theil (1): mac80211: fix double free in ibss_leave net/mac80211/aead_api.c| 5 +++-- net/mac80211/aes_gmac.c| 5 +++-- net/mac80211/cfg.c | 4 ++-- net/mac80211/ibss.c| 2 ++ net/mac80211/main.c| 13 - net/mac80211/mlme.c| 2 +- net/mac80211/rc80211_minstrel_ht.c | 2 -- net/mac80211/util.c| 2 +- net/wireless/nl80211.c | 12 9 files changed, 32 insertions(+), 15 deletions(-)
Re: [PATCH] net: wireless: search and hold bss in cfg80211_connect_done
On Tue, 2021-03-16 at 19:29 +, Abhishek Kumar wrote: > If BSS instance is not provided in __cfg80211_connect_result then > a get bss is performed. This can return NULL if the BSS for the > given SSID is expired due to delayed scheduling of connect result event > in rdev->event_work. This can cause WARN_ON(!cr->bss) in > __cfg80211_connect_result to be triggered and cause cascading > failures. To mitigate this, initiate a get bss call in > cfg80211_connect_done itself and hold it to ensure that the BSS > instance does not get expired. I'm not sure I see the value in this. You're basically picking a slightly earlier point in time where cfg80211 might know about the BSS entry still, so you're really just making the problem window a few microseconds or perhaps milliseconds (whatever ends up being the worker delay) shorter. Compared to the 30s entry lifetime, that's nothing. So what's the point? Please fix the driver instead to actually hold on to it and report it back. johannes
Re: BUG: soft lockup in ieee80211_tasklet_handler
On Tue, 2021-03-02 at 20:01 +0100, Dmitry Vyukov wrote:
>
> Looking at the reproducer that mostly contains just perf_event_open,
> It may be the old known issue of perf_event_open with some extreme
> parameters bringing down kernel.
> +perf maintainers
> And as far as I remember +Peter had some patch to restrict
> perf_event_open parameters.
>
> r0 = perf_event_open(&(0x7f01d000)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x3ff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xfffe, 0x0, @perf_config_ext}, 0x0,
> 0x0, 0x, 0x0)
Oh! Thanks for looking.
Seems that also applies to
https://syzkaller.appspot.com/bug?extid=d6219cf21f26bdfcc22e
FWIW. I was still tracking that one, but never had a chance to look at
it (also way down the list since it was reported as directly in hwsim)
johannes
Re: BUG: soft lockup in ieee80211_tasklet_handler
On Wed, 2021-02-24 at 10:30 +0800, Hillf Danton wrote:
>
> Add budget for the 80211 softint handler - it's feasible not to try to
> build the giant pyramid in a week.
>
> --- x/net/mac80211/main.c
> +++ y/net/mac80211/main.c
> @@ -224,9 +224,15 @@ static void ieee80211_tasklet_handler(un
> {
> struct ieee80211_local *local = (struct ieee80211_local *) data;
> struct sk_buff *skb;
> + int i = 0;
> +
> + while (i++ < 64) {
> + skb = skb_dequeue(&local->skb_queue);
> + if (!skb)
> + skb = skb_dequeue(&local->skb_queue_unreliable);
> + if (!skb)
> + return;
I guess that's not such a bad idea, but I do wonder how we get here,
userspace can submit packets faster than we can process?
It feels like a simulation-only case, tbh, since over the air you have
limits how much bandwidth you can get ... unless you have a very slow
CPU?
In any case, if you want anything merged you're going to have to submit
a proper patch with a real commit message and Signed-off-by, etc.
johannes
Re: [PATCH v2 2/3] lockdep: add lockdep lock state defines
> @@ -5475,7 +5476,7 @@ noinstr int lock_is_held_type(const struct lockdep_map > *lock, int read) > /* avoid false negative lockdep_assert_not_held() >* and lockdep_assert_held() >*/ > - return -1; > + return LOCK_STATE_UNKNOWN; I'd argue that then the other two return places here should also be changed. johannes
Re: [PATCH net v1 3/3] [RFC] mac80211: ieee80211_store_ack_skb(): make use of skb_clone_sk_optional()
On Mon, 2021-02-22 at 19:51 +0100, Marc Kleine-Budde wrote: > On 22.02.2021 17:30:59, Johannes Berg wrote: > > On Mon, 2021-02-22 at 16:12 +0100, Oleksij Rempel wrote: > > > This code is trying to clone the skb with optional skb->sk. But this > > > will fail to clone the skb if socket was closed just after the skb was > > > pushed into the networking stack. > > > > Which IMHO is completely fine. If we then still clone the SKB we can't > > do anything with it, since the point would be to ... send it back to the > > socket, but it's gone. > > Ok, but why is the skb cloned if there is no socket linked in skb->sk? Hm? There are two different ways to get here, one with and one without a socket. johannes
Re: [PATCH net v1 3/3] [RFC] mac80211: ieee80211_store_ack_skb(): make use of skb_clone_sk_optional()
On Mon, 2021-02-22 at 16:12 +0100, Oleksij Rempel wrote: > This code is trying to clone the skb with optional skb->sk. But this > will fail to clone the skb if socket was closed just after the skb was > pushed into the networking stack. Which IMHO is completely fine. If we then still clone the SKB we can't do anything with it, since the point would be to ... send it back to the socket, but it's gone. Nothing to fix here, I'd think. If you wanted to get a copy back that gives you the status of the SKB, it should not come as a huge surprise that you have to keep the socket open for that :) Having the ACK skb will just make us do more work by handing it back to skb_complete_wifi_ack() at TX status time, which is supposed to put it into the socket's error queue, but if the socket is closed ... no point in that. johannes
Re: [PATCH] [v13] wireless: Initial driver submission for pureLiFi STA devices
On Fri, 2021-02-19 at 05:15 +, Srinivasan Raju wrote:
> Hi,
>
> Please find a few responses to the comments , We will fix rest of the
> comments and submit v14 of the patch.
>
> > Also, you *really* need some validation here, rather than blindly
> > trusting that the file is well-formed, otherwise you immediately have a
> > security issue here.
>
> The firmware is signed and the harware validates the signature , so we are
> not validating in the software.
That wasn't my point. My point was that the kernel code trusts the
validity of the firmware image, in the sense of e.g. this piece:
> + no_of_files = *(u32 *)&fw_packed->data[0];
If the firmware file was corrupted (intentionally/maliciously or not), this
could now be say 0x.
> + for (step = 0; step < no_of_files; step++) {
> + start_addr = *(u32 *)&fw_packed->data[4 + (step * 4)];
But you trust it completely and don't even check that "4 + (step * 4)"
fits into the length of the data!
That's what I meant. Don't allow this to crash the kernel.
> > > +static const struct plf_reg_alpha2_map reg_alpha2_map[] = {
> > > + { PLF_REGDOMAIN_FCC, "US" },
> > > + { PLF_REGDOMAIN_IC, "CA" },
> > > + { PLF_REGDOMAIN_ETSI, "DE" }, /* Generic ETSI, use most restrictive
> > > */
> > > + { PLF_REGDOMAIN_JAPAN, "JP" },
> > > + { PLF_REGDOMAIN_SPAIN, "ES" },
> > > + { PLF_REGDOMAIN_FRANCE, "FR" },
> > > +};
> > You actually have regulatory restrictions on this stuff?
>
> Currently, There are no regulatory restrictions applicable for LiFi ,
> regulatory_hint setting is only for wifi core
So why do you have PLF_REGDOMAIN_* things? What does that even mean
then?
> > OTOH, I can sort of see why/how you're reusing wifi functionality here,
> > very old versions of wifi even had an infrared PHY I think.
> >
> > Conceptually, it seems odd. Perhaps we should add a new band definition?
> >
> > And what I also asked above - how much of the rate stuff is completely
> > fake? Are you really doing CCK/OFDM in some (strange?) way?
>
> Yes, your understanding is correct, and we use OFDM.
> For now we will use the existing band definition.
OFDM over infrared, nice.
Still, I'm not convinced we should piggy-back this on 2.4 GHz.
NL80211_BAND_1THZ? ;-)
Ok, I don't know what wavelength you're using, of course...
But at least thinking about this, wouldn't we be better off if we define
NL80211_BAND_INFRARED or something? That way, at least we can tell that
it's not going to interoperate with real 2.4 GHz, etc.
OTOH, this is a bit of a slippery slow - what if somebody else designs
an *incompatible* infrared PHY? I guess this is not really standardised
at this point.
Not really sure about all this, but I guess we need to think about it
more.
What are your reasons for piggy-backing on 2.4 GHz? Just practical "it's
there and we don't care"?
johannes
Re: [PATCH] kcov: Remove kcov include from sched.h and move it to its users.
On Thu, 2021-02-18 at 18:31 +0100, Sebastian Andrzej Siewior wrote: > The recent addition of in_serving_softirq() to kconv.h results in You typo'ed "kconv.h" pretty consistently ;-) But yes, that makes sense. Acked-by: Johannes Berg johannes
Re: [PATCH 1/2] lockdep: add lockdep_assert_not_held()
On Mon, 2021-02-15 at 17:04 +0100, Peter Zijlstra wrote:
> On Mon, Feb 15, 2021 at 02:12:30PM +0100, Johannes Berg wrote:
> > On Mon, 2021-02-15 at 11:44 +0100, Peter Zijlstra wrote:
> > > I think something like so will work, but please double check.
> >
> > Yeah, that looks better.
> >
> > > +++ b/include/linux/lockdep.h
> > > @@ -294,11 +294,15 @@ extern void lock_unpin_lock(struct lockdep_map
> > > *lock, struct pin_cookie);
> > >
> > > #define lockdep_depth(tsk) (debug_locks ? (tsk)->lockdep_depth : 0)
> > >
> > > -#define lockdep_assert_held(l) do {\
> > > - WARN_ON(debug_locks && !lockdep_is_held(l));\
> > > +#define lockdep_assert_held(l) do {
> > > \
> > > + WARN_ON(debug_locks && lockdep_is_held(l) == 0)); \
> > > } while (0)
> >
> > That doesn't really need to change? It's the same.
>
> Correct, but I found it more symmetric vs the not implementation below.
Fair enough. One might argue that you should have an
enum lockdep_lock_state {
LOCK_STATE_NOT_HELD, /* 0 now */
LOCK_STATE_HELD, /* 1 now */
LOCK_STATE_UNKNOWN, /* -1 with your patch but might as well be 2 */
};
:)
johannes
Re: [PATCH 1/2] lockdep: add lockdep_assert_not_held()
On Mon, 2021-02-15 at 11:44 +0100, Peter Zijlstra wrote:
>
> I think something like so will work, but please double check.
Yeah, that looks better.
> +++ b/include/linux/lockdep.h
> @@ -294,11 +294,15 @@ extern void lock_unpin_lock(struct lockdep_map *lock,
> struct pin_cookie);
>
> #define lockdep_depth(tsk) (debug_locks ? (tsk)->lockdep_depth : 0)
>
> -#define lockdep_assert_held(l) do {\
> - WARN_ON(debug_locks && !lockdep_is_held(l));\
> +#define lockdep_assert_held(l) do {
> \
> + WARN_ON(debug_locks && lockdep_is_held(l) == 0)); \
> } while (0)
That doesn't really need to change? It's the same.
> -#define lockdep_assert_held_write(l) do {\
> +#define lockdep_assert_not_held(l) do {\
> + WARN_ON(debug_locks && lockdep_is_held(l) == 1)); \
> + } while (0)
> +
> +#define lockdep_assert_held_write(l) do {\
> WARN_ON(debug_locks && !lockdep_is_held_type(l, 0));\
> } while (0)
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index c1418b47f625..983ba206f7b2 100644
> --- a/kernel/locking/lockdep.
> +++ b/kernel/locking/lockdep.c
> @@ -5467,7 +5467,7 @@ noinstr int lock_is_held_type(const struct lockdep_map
> *lock, int read)
> int ret = 0;
>
> if (unlikely(!lockdep_enabled()))
> - return 1; /* avoid false negative lockdep_assert_held() */
> + return -1; /* avoid false negative lockdep_assert_held() */
Maybe add lockdep_assert_not_held() to the comment, to explain the -1
(vs non-zero)?
johannes
pull-request: mac80211-next 2021-02-12
Hi, This is almost certainly a last update for net-next, and it's not very big - the biggest chunk here is minstrel improvements from Felix, to lower overhead. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 3c5a2fd042d0bfac71a2dfb99515723d318df47b: tcp: Sanitize CMSG flags and reserved args in tcp_zerocopy_receive. (2021-02-11 18:25:05 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-net-next-2021-02-12 for you to fetch changes up to 735a48481cca453525d9199772f9c3733a47cff4: nl80211: add documentation for HT/VHT/HE disable attributes (2021-02-12 11:00:07 +0100) Last set of updates: * more minstrel work from Felix to reduce the probing overhead * QoS for nl80211 control port frames * STBC injection support * and a couple of small fixes Ben Greear (1): cfg80211/mac80211: Support disabling HE mode Colin Ian King (1): mac80211: fix potential overflow when multiplying to u32 integers Felix Fietkau (6): mac80211: minstrel_ht: use bitfields to encode rate indexes mac80211: minstrel_ht: update total packets counter in tx status path mac80211: minstrel_ht: reduce the need to sample slower rates mac80211: minstrel_ht: significantly redesign the rate probing strategy mac80211: minstrel_ht: show sampling rates in debugfs mac80211: minstrel_ht: remove sample rate switching code for constrained devices Johannes Berg (1): nl80211: add documentation for HT/VHT/HE disable attributes Luca Coelho (1): cfg80211: initialize reg_rule in __freq_reg_info() Markus Theil (1): mac80211: enable QoS support for nl80211 ctrl port Matteo Croce (1): cfg80211: remove unused callback Philipp Borgers (1): mac80211: add STBC encoding to ieee80211_parse_tx_radiotap include/net/cfg80211.h | 2 + include/uapi/linux/nl80211.h | 13 +- net/mac80211/mesh_hwmp.c | 2 +- net/mac80211/mlme.c| 3 + net/mac80211/rc80211_minstrel_ht.c | 766 ++--- net/mac80211/rc80211_minstrel_ht.h | 47 +- net/mac80211/rc80211_minstrel_ht_debugfs.c | 22 +- net/mac80211/status.c | 8 +- net/mac80211/tx.c | 34 +- net/wireless/nl80211.c | 7 + net/wireless/reg.c | 2 +- net/wireless/sysfs.c | 7 - 12 files changed, 486 insertions(+), 427 deletions(-)
Re: [PATCH] [v13] wireless: Initial driver submission for pureLiFi STA devices
Hi,
Thanks for your patience, and thanks for sticking around!
I'm sorry I haven't reviewed this again in a long time, but I was able
to today.
> +PUREILIFI USB DRIVER
Did you mistype "PURELIFI" here, or was that intentional?
> +PUREILIFI USB DRIVER
> +M: Srinivasan Raju
Probably would be good to have an "L" entry with the linux-wireless list
here.
> +if WLAN_VENDOR_PURELIFI
> +
> +source "drivers/net/wireless/purelifi/plfxlc/Kconfig"
Seems odd to have the Makefile under purelifi/ but the Kconfig is yet
another directory deeper?
> +++ b/drivers/net/wireless/purelifi/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_WLAN_VENDOR_PURELIFI) := plfxlc/
Although this one doesn't do anything, so all you did was save a level
of Kconfig inclusion I guess ... no real objection to that.
> diff --git a/drivers/net/wireless/purelifi/plfxlc/Kconfig
> b/drivers/net/wireless/purelifi/plfxlc/Kconfig
> new file mode 100644
> index ..07a65c0fce68
> --- /dev/null
> +++ b/drivers/net/wireless/purelifi/plfxlc/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config PLFXLC
> +
> + tristate "pureLiFi X, XL, XC device support"
extra blank line.
Also, maybe that should be a bit more verbose? PURELIFI_XLC or so? I
don't think it shows up in many places, but if you see "PLFXLC"
somewhere that's not very helpful.
> + depends on CFG80211 && MAC80211 && USB
> + help
> +This driver makes the adapter appear as a normal WLAN interface.
> +
> +The pureLiFi device requires external STA firmware to be loaded.
> +
> +To compile this driver as a module, choose M here: the module will
> +be called purelifi.
But will it? Seems like it would be called "plfxlc"?
See here:
> +++ b/drivers/net/wireless/purelifi/plfxlc/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_PLFXLC) := plfxlc.o
> +plfxlc-objs += chip.o firmware.o usb.o mac.o
> +int purelifi_set_beacon_interval(struct purelifi_chip *chip, u16 interval,
> + u8 dtim_period, int type)
> +{
> + if (!interval ||
> + (chip->beacon_set && chip->beacon_interval == interval)) {
> + return 0;
> + }
Don't need braces here
> + chip->beacon_interval = interval;
> + chip->beacon_set = true;
> + return usb_write_req((const u8 *)&chip->beacon_interval,
> + sizeof(chip->beacon_interval),
> + USB_REQ_BEACON_INTERVAL_WR);
There's clearly an endian problem hiding here somewhere. You should have
"chip->beacon_interval" be (probably) __le16 since you send it to the
device as a buffer, yet the parameter to this function is just "u16".
Therefore, a conversion is missing somewhere - likely you need it to be
__le16 in the chip struct since you can't send USB requests from stack.
You should add some annotations for things getting sent to the device
like this, and then you can run sparse to validate them all over the
code.
> +}
> +
> +int purelifi_chip_init_hw(struct purelifi_chip *chip)
> +{
> + u8 *addr = purelifi_mac_get_perm_addr(purelifi_chip_to_mac(chip));
> + struct usb_device *udev = interface_to_usbdev(chip->usb.intf);
> +
> + pr_info("purelifi chip %02x:%02x v%02x %02x-%02x-%02x %s\n",
"%04x:%04x" for the USB ID?
And you should probably use %pM for the MAC address - the format you
have there looks ... strange? Why only print the vendor OUI?
> +int purelifi_chip_switch_radio(struct purelifi_chip *chip, u32 value)
> +{
> + int r;
> +
> + r = usb_write_req((const u8 *)&value, sizeof(value), USB_REQ_POWER_WR);
> + if (r)
> + dev_err(purelifi_chip_dev(chip), "POWER_WR failed (%d)\n", r);
Same endian problem here.
>
> +static inline void purelifi_mc_add_addr(struct purelifi_mc_hash *hash,
> + u8 *addr)
> +{
> + unsigned int i = addr[5] >> 2;
> +
> + if (i < 32)
> + hash->low |= 1 << i;
> + else
> + hash->high |= 1 << (i - 32);
That's UB if i == 31, you need 1U << i, or just use the BIT() macro.
> + r = request_firmware((const struct firmware **)&fw, fw_name,
Not sure why that cast should be required?
> + fpga_dmabuff = NULL;
> + fpga_dmabuff = kmalloc(PLF_FPGA_SLEN, GFP_KERNEL);
that NULL assignment is pointless :)
> +
> + if (!fpga_dmabuff) {
> + r = -ENOMEM;
> + goto error_free_fw;
> + }
> + send_vendor_request(udev, PLF_VNDR_FPGA_SET_REQ, fpga_dmabuff,
> sizeof(fpga_setting));
> + memcpy(fpga_setting, fpga_dmabuff, PLF_FPGA_SLEN);
> + kfree(fpga_dmabuff);
I'd say you should remove fpga_setting from the stack since you anyway
allocated it, save the memcpy() to the stack, and just use fpga_dmabuff
below - and then free it later?
> + for (tbuf_idx = 0; tbuf_idx < blk_tran_len; tbuf_idx++) {
> +
Re: Potential invalid ~ operator in net/mac80211/cfg.c
On Fri, 2021-02-05 at 18:20 +, Colin Ian King wrote: > > > > https://lore.kernel.org/linux-wireless/516c0c7f.3000...@openwrt.org/ > > > > > > But maybe that isn't actually quite right due to integer promotion? > > > OTOH, that's a u8, so it should do the ~ in u8 space, and then compare > > > to 0 also? > > > > rc_rateidx_vht_mcs_mask is a u64, so I think the expression could be > > expressed as: > > oops, fat fingered that, it is a u16 not a u64 Right, u16, I must've looked at some ancient version or something. But no, I was obviously wrong with what I said above. So of course the condition is always true, like you said. However, what was intended doesn't look like !, but rather == 0xff and == 0x respectively, I'll send a patch. johannes
Re: [PATCH 2/3] mac80211: Add support to trigger sta disconnect on hardware restart
On Fri, 2021-02-12 at 09:42 +0100, Johannes Berg wrote:
> On Tue, 2020-12-15 at 22:53 +0530, Youghandhar Chintala wrote:
> > The right fix would be to pull the entire data path into the host
> > +++ b/net/mac80211/ieee80211_i.h
> > @@ -748,6 +748,8 @@ struct ieee80211_if_mesh {
> > * back to wireless media and to the local net stack.
> > * @IEEE80211_SDATA_DISCONNECT_RESUME: Disconnect after resume.
> > * @IEEE80211_SDATA_IN_DRIVER: indicates interface was added to driver
> > + * @IEEE80211_SDATA_DISCONNECT_HW_RESTART: Disconnect after hardware
> > restart
> > + * recovery
>
> How did you model this on IEEE80211_SDATA_DISCONNECT_RESUME, but than
> didn't check how that's actually used?
>
> Please change it so that the two models are the same. You really don't
> need the wiphy flag.
In fact, you could even simply
generalize IEEE80211_SDATA_DISCONNECT_RESUME
and ieee80211_resume_disconnect() to _reconfig_ instead of _resume_, and
call it from the driver just before requesting HW restart.
johannes
Re: [PATCH 2/3] mac80211: Add support to trigger sta disconnect on hardware restart
On Tue, 2020-12-15 at 22:53 +0530, Youghandhar Chintala wrote:
> The right fix would be to pull the entire data path into the host
> +++ b/net/mac80211/ieee80211_i.h
> @@ -748,6 +748,8 @@ struct ieee80211_if_mesh {
> * back to wireless media and to the local net stack.
> * @IEEE80211_SDATA_DISCONNECT_RESUME: Disconnect after resume.
> * @IEEE80211_SDATA_IN_DRIVER: indicates interface was added to driver
> + * @IEEE80211_SDATA_DISCONNECT_HW_RESTART: Disconnect after hardware restart
> + * recovery
How did you model this on IEEE80211_SDATA_DISCONNECT_RESUME, but than
didn't check how that's actually used?
Please change it so that the two models are the same. You really don't
need the wiphy flag.
johannes
Re: [PATCH 2/3] mac80211: Add support to trigger sta disconnect on hardware restart
On Fri, 2021-02-05 at 13:51 -0800, Abhishek Kumar wrote: > Since using DELBA frame to APs to re-establish BA session has a > dependency on APs and also some APs may not honour the DELBA frame. That's completely out of spec ... Can you say which AP this was? You could also try sending a BAR that updates the SN. johannes
Re: [PATCH 1/3] cfg80211: Add wiphy flag to trigger STA disconnect after hardware restart
On Tue, 2020-12-15 at 23:00 +0530, Youghandhar Chintala wrote: > > * @WIPHY_FLAG_SUPPORTS_EXT_KEK_KCK: The device supports bigger kek and kck > keys > + * @WIPHY_FLAG_STA_DISCONNECT_ON_HW_RESTART: The device needs a trigger to > + * disconnect STA after target hardware restart. This flag should be > + * exposed by drivers which support target recovery. You're not doing anything with this information in cfg80211, so consequently it doesn't belong there. johannes
Re: Potential invalid ~ operator in net/mac80211/cfg.c
Hi Colin,
> while working through a backlog of older static analysis reports from
> Coverity
So ... yeah. Every time I look at Coverity (not frequently, I must
admit) I see the same thing, and get confused.
> I found an interesting use of the ~ operator that looks
> incorrect to me in function ieee80211_set_bitrate_mask():
>
> for (j = 0; j < IEEE80211_HT_MCS_MASK_LEN; j++) {
> if (~sdata->rc_rateidx_mcs_mask[i][j]) {
> sdata->rc_has_mcs_mask[i] = true;
> break;
> }
> }
>
> for (j = 0; j < NL80211_VHT_NSS_MAX; j++) {
> if (~sdata->rc_rateidx_vht_mcs_mask[i][j]) {
> sdata->rc_has_vht_mcs_mask[i] = true;
> break;
> }
> }
>
> For the ~ operator in both if stanzas, Coverity reports:
>
> Logical vs. bitwise operator (CONSTANT_EXPRESSION_RESULT)
> logical_vs_bitwise:
>
> ~sdata->rc_rateidx_mcs_mask[i][j] is always 1/true regardless of the
> values of its operand. This occurs as the logical operand of if.
> Did you intend to use ! rather than ~?
>
> I've checked the results of this and it does seem that ~ is incorrect
> and always returns true for the if expression. So it probably should be
> !, but I'm not sure if I'm missing something deeper here and wondering
> why this has always worked.
But is it really always true?
I _think_ it was intended to check that it's not 0x or
something?
https://lore.kernel.org/linux-wireless/516c0c7f.3000...@openwrt.org/
But maybe that isn't actually quite right due to integer promotion?
OTOH, that's a u8, so it should do the ~ in u8 space, and then compare
to 0 also?
johannes
Re: [PATCH] wireless: fix typo issue
On Wed, 2021-02-03 at 15:00 +0800, samirweng1979 wrote: > From: wengjianfeng > > change 'iff' to 'if'. > > Signed-off-by: wengjianfeng > --- > net/wireless/chan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/wireless/chan.c b/net/wireless/chan.c > index 285b807..2f17edf 100644 > --- a/net/wireless/chan.c > +++ b/net/wireless/chan.c > @@ -1084,7 +1084,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, > * associated to an AP on the same channel or on the same UNII band > * (assuming that the AP is an authorized master). > * In addition allow operation on a channel on which indoor operation is > - * allowed, iff we are currently operating in an indoor environment. > + * allowed, if we are currently operating in an indoor environment. > */ I suspect that was intentional, as a common abbreviation for "if and only if". johannes
pull-request: mac80211-next 2021-02-02
Hi Jakub, And for mac80211-next, I have only locking fixes right now. I'm happy that it otherwise seems to be fine though, now no new bugs have been reported for a few days. I might send some more things your way another day, but wanted to get the locking fixes out sooner. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit d1f3bdd4eaae1222063c2f309625656108815915: net: dsa: rtl8366rb: standardize init jam tables (2021-01-27 20:21:20 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-net-next-2021-02-02 for you to fetch changes up to 40c575d1ec71f7a61c73ba1603a69650c130559c: cfg80211: fix netdev registration deadlock (2021-02-01 19:30:54 +0100) This time, only RTNL locking reduction fallout. - cfg80211_dev_rename() requires RTNL - cfg80211_change_iface() and cfg80211_set_encryption() require wiphy mutex (was missing in wireless extensions) - cfg80211_destroy_ifaces() requires wiphy mutex - netdev registration can fail due to notifiers, and then notifiers are "unrolled", need to handle this properly ---- Johannes Berg (5): nl80211: call cfg80211_dev_rename() under RTNL wext: call cfg80211_change_iface() with wiphy lock held wext: call cfg80211_set_encryption() with wiphy lock held cfg80211: call cfg80211_destroy_ifaces() with wiphy lock held cfg80211: fix netdev registration deadlock include/net/cfg80211.h | 4 +++- net/wireless/core.c| 7 ++- net/wireless/nl80211.c | 2 +- net/wireless/wext-compat.c | 14 -- 4 files changed, 22 insertions(+), 5 deletions(-)
pull-request: mac80211 2021-02-02
Hi Jakub, So, getting late, but we have two more fixes - the staging one (acked by Greg) is for a recent regression I had through my tree, and the rate tables one is kind of important for (some) drivers to get proper rate scaling going. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit eb4e8fac00d1e01ada5e57c05d24739156086677: neighbour: Prevent a dead entry from updating gc_list (2021-01-30 11:09:07 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2021-02-02 for you to fetch changes up to 50af06d43eab6b09afc37aa7c8bbf69b14a3b2f7: staging: rtl8723bs: Move wiphy setup to after reading the regulatory settings from the chip (2021-02-01 19:26:10 +0100) Two fixes: - station rate tables were not updated correctly after association, leading to bad configuration - rtl8723bs (staging) was initializing data incorrectly after the previous fix and needed to move the init later Felix Fietkau (1): mac80211: fix station rate table updates on assoc Hans de Goede (1): staging: rtl8723bs: Move wiphy setup to after reading the regulatory settings from the chip drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 ++-- net/mac80211/driver-ops.c| 5 - net/mac80211/rate.c | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-)
Re: possible deadlock in cfg80211_netdev_notifier_call
On Mon, 2021-02-01 at 14:37 +0200, Mike Rapoport wrote: > On Mon, Feb 01, 2021 at 01:17:13AM -0800, syzbot wrote: > > Hello, > > > > syzbot found the following issue on: > > > > HEAD commit:b01f250d Add linux-next specific files for 20210129 > > git tree: linux-next > > console output: https://syzkaller.appspot.com/x/log.txt?x=14daa408d0 > > kernel config: https://syzkaller.appspot.com/x/.config?x=725bc96dc234fda7 > > dashboard link: https://syzkaller.appspot.com/bug?extid=2ae0ca9d7737ad1a62b7 > > compiler: gcc (GCC) 10.1.0-syz 20200507 > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1757f2a0d0 > > > > The issue was bisected to: > > > > commit cc9327f3b085ba5be5639a5ec3ce5b08a0f14a7c > > Author: Mike Rapoport > > Date: Thu Jan 28 07:42:40 2021 + > > > > mm: introduce memfd_secret system call to create "secret" memory areas > > > > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1505d28cd0 > > final oops: https://syzkaller.appspot.com/x/report.txt?x=1705d28cd0 > > console output: https://syzkaller.appspot.com/x/log.txt?x=1305d28cd0 > > Sounds really weird to me. At this point the memfd_secret syscall is not > even wired to arch syscall handlers. I cannot see how it can be a reason of > deadlock in wireless... Yeah, forget about it. Usually this is a consequence of the way syzbot creates tests - it might have created something like if (!create_secret_memfd()) return; try_something_on_wireless() and then of course without your patch it cannot get to the wireless bits. Pretty sure I know what's going on here, I'll take a closer look later. johannes
Re: WARNING in sta_info_insert_check
On Sun, 2021-01-31 at 21:26 -0800, syzbot wrote: > Hello, > > syzbot found the following issue on: > > HEAD commit:bec4c296 Merge tag 'ecryptfs-5.11-rc6-setxattr-fix' of git.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=11991778d0 > kernel config: https://syzkaller.appspot.com/x/.config?x=f75d66d6d359ef2f > dashboard link: https://syzkaller.appspot.com/bug?extid=8dcc087eb24227ded47e > userspace arch: arm64 > > Unfortunately, I don't have any reproducer for this issue yet. > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+8dcc087eb24227ded...@syzkaller.appspotmail.com Looks like this is a dup. #syz dup: WARNING in sta_info_insert_rcu Just in this case sta_info_insert_check() didn't get inlined into sta_info_insert_rcu(). johannes
Re: [PATCH] nl80211: ignore the length of hide ssid is zero in scan
On Thu, 2021-01-28 at 19:56 +0800, samirweng1979 wrote: > From: wengjianfeng > > If the length of hide ssid is zero in scan, don't pass > it to driver, which doesn't make any sense. Err, please check again how scanning works. This is quite obviously intentional. johannes
pull-request: mac80211-next 2021-01-27
Hi Jakub, Alright ... let's try again, with two more fixes on top, one for the virt_wifi deadlock and one for a minstrel regression in the earlier patches. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 9e8789c85deee047c5753e22f725d5fc10682468: net: stmmac: dwmac-meson8b: fix the RX delay validation (2021-01-20 22:15:08 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-net-next-2021-01-27 for you to fetch changes up to d3b9b45f7e981bcc6355414c63633fe33d95660c: mac80211: minstrel_ht: fix regression in the max_prob_rate fix (2021-01-27 22:06:38 +0100) More updates: * many minstrel improvements, including removal of the old minstrel in favour of minstrel_ht * speed improvements on FQ * support for RX decapsulation (header conversion) offload * RTNL reduction: limit RTNL usage in the wireless stack mostly to where really needed (regulatory not yet) to reduce contention on it * various other small updates Arend van Spriel (1): cfg80211: add VHT rate entries for MCS-10 and MCS-11 Felix Fietkau (14): net/fq_impl: bulk-free packets from a flow on overmemory net/fq_impl: drop get_default_func, move default flow to fq_tin net/fq_impl: do not maintain a backlog-sorted list of flows mac80211: add rx decapsulation offload support mac80211: minstrel_ht: clean up CCK code mac80211: minstrel_ht: add support for OFDM rates on non-HT clients mac80211: remove legacy minstrel rate control mac80211: minstrel_ht: remove old ewma based rate average code mac80211: minstrel_ht: improve ampdu length estimation mac80211: minstrel_ht: improve sample rate selection mac80211: minstrel_ht: fix max probability rate selection mac80211: minstrel_ht: increase stats update interval mac80211: minstrel_ht: fix rounding error in throughput calculation mac80211: minstrel_ht: fix regression in the max_prob_rate fix Johannes Berg (3): cfg80211: change netdev registration/unregistration semantics cfg80211: avoid holding the RTNL when calling the driver virt_wifi: fix deadlock on RTNL Lorenzo Bianconi (1): mac80211: introduce aql_enable node in debugfs Max Chen (1): cfg80211: Add phyrate conversion support for extended MCS in 60GHz band Philipp Borgers (1): mac80211: add LDPC encoding to ieee80211_parse_tx_radiotap Ramon Fontes (1): mac80211_hwsim: add 6GHz channels Wen Gong (2): mac80211: remove NSS number of 160MHz if not support 160MHz for HE mac80211: reduce peer HE MCS/NSS to own capabilities drivers/net/wireless/ath/ath11k/reg.c | 4 +- drivers/net/wireless/ath/ath6kl/cfg80211.c | 4 +- drivers/net/wireless/ath/ath6kl/core.c | 2 + drivers/net/wireless/ath/ath6kl/init.c | 2 + drivers/net/wireless/ath/wil6210/cfg80211.c| 2 + drivers/net/wireless/ath/wil6210/netdev.c | 11 +- drivers/net/wireless/ath/wil6210/pcie_bus.c| 2 + .../wireless/broadcom/brcm80211/brcmfmac/core.c| 24 +- .../wireless/broadcom/brcm80211/brcmfmac/core.h| 6 +- .../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 12 +- drivers/net/wireless/intel/iwlwifi/mvm/d3.c| 2 +- drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 4 +- drivers/net/wireless/intel/iwlwifi/mvm/nvm.c | 2 +- drivers/net/wireless/mac80211_hwsim.c | 74 ++- drivers/net/wireless/marvell/mwifiex/cfg80211.c| 10 +- drivers/net/wireless/marvell/mwifiex/main.c| 7 + drivers/net/wireless/microchip/wilc1000/cfg80211.c | 2 +- drivers/net/wireless/microchip/wilc1000/mon.c | 4 +- drivers/net/wireless/microchip/wilc1000/netdev.c | 2 +- drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 4 +- drivers/net/wireless/quantenna/qtnfmac/core.c | 5 +- include/net/cfg80211.h | 146 - include/net/fq.h | 11 +- include/net/fq_impl.h | 171 -- include/net/mac80211.h | 26 +- net/mac80211/Makefile | 2 - net/mac80211/debugfs.c | 52 ++ net/mac80211/debugfs_sta.c | 1 + net/mac80211/driver-ops.h | 16 + net/mac80211/he.c | 92 +++ net/mac80211/ieee80211_i.h | 3 +- net/mac80211/iface.c | 40 +- net/mac80211/key.c | 4 +- net/mac80211/main.c| 5 + net/mac80211/pm.c
Re: pull-request: mac80211-next 2021-01-26
On Tue, 2021-01-26 at 22:16 +0100, Johannes Berg wrote: > Hi, > > So here's a pretty big and invasive mac80211-next pull. It has now been > in linux-next for some time, and all the issues that had been reported by > people running that are fixed. I've also thrown hwsim and Intel-internal > tests at it. However, changing the locking is somewhat dangerous, so if I > consider it, I sort of expect some more fallout from that. I did sprinkle > lockdep assertions fairly liberally, so we'll see. I guess I'll withdraw this (and send a new one soon), there was a virt_wifi regression - forgot to update it with the new approach. johannes
Re: [PATCH] ath9k: fix build error with LEDS_CLASS=m
On Wed, 2021-01-27 at 12:35 +0200, Kalle Valo wrote: > Arnd Bergmann writes: > > > On Mon, Jan 25, 2021 at 4:04 PM Krzysztof Kozlowski wrote: > > > On Mon, 25 Jan 2021 at 15:38, Arnd Bergmann wrote: > > > > On Mon, Jan 25, 2021 at 2:27 PM Krzysztof Kozlowski > > > > wrote: > > > > > > I meant that having MAC80211_LEDS selected causes the ath9k driver to > > > toggle on/off the WiFi LED. Every second, regardless whether it's > > > doing something or not. In my setup, I have problems with a WiFi > > > dongle somehow crashing (WiFi disappears, nothing comes from the > > > dongle... maybe it's Atheros FW, maybe some HW problem) and I found > > > this LED on/off slightly increases the chances of this dongle-crash. > > > That was the actual reason behind my commits. > > > > > > Second reason is that I don't want to send USB commands every second > > > when the device is idle. It unnecessarily consumes power on my > > > low-power device. > > > > Ok, I see. > > > > > Of course another solution is to just disable the trigger via sysfs > > > LED API. It would also work but my patch allows entire code to be > > > compiled-out (which was conditional in ath9k already). > > > > > > Therefore the patch I sent allows the ath9k LED option to be fully > > > choosable. Someone wants every-second-LED-blink, sure, enable > > > ATH9K_LEDS and you have it. Someone wants to reduce the kernel size, > > > don't enable ATH9K_LEDS. > > > > Originally, I think this is what CONFIG_MAC80211_LEDS was meant > > for, but it seems that this is not actually practical, since this also > > gets selected by half of the drivers using it, while the other half have > > a dependency on it. Out of the ones that select it, some in turn > > select LEDS_CLASS, while some depend on it. > > > > I think this needs a larger-scale cleanup for consistency between > > (at least) all the wireless drivers using LEDs. > > I agree, this needs cleanup. > > > Either your patch or mine should get applied in the meantime, and I > > don't care much which one in this case, as we still have the remaining > > inconsistency. > > My problem with Krzysztof's patch[1] is that it adds a new Kconfig > option for ath9k, is that really necessary? Like Arnd said, we should > fix drivers to use CONFIG_MAC80211_LEDS instead of having driver > specific options. > > So I would prefer take this Arnd's patch instead and queue it for v5.11. > But as it modifies mac80211 I'll need an ack from Johannes, what do you > think? Sure, that seems fine. Acked-by: Johannes Berg johannes
Re: [PATCH net] iwlwifi: provide gso_type to GSO packets
On Tue, 2021-01-26 at 12:32 -0800, Jakub Kicinski wrote:
> On Mon, 25 Jan 2021 07:09:49 -0800 Eric Dumazet wrote:
> > From: Eric Dumazet
> >
> > net/core/tso.c got recent support for USO, and this broke iwlfifi
> > because the driver implemented a limited form of GSO.
> >
> > Providing ->gso_type allows for skb_is_gso_tcp() to provide
> > a correct result.
> >
> > Fixes: 3d5b459ba0e3 ("net: tso: add UDP segmentation support")
> > Signed-off-by: Eric Dumazet
> > Reported-by: Ben Greear
> > Bisected-by: Ben Greear
> > Tested-by: Ben Greear
> > Cc: Luca Coelho
> > Cc: linux-wirel...@vger.kernel.org
> > Cc: Johannes Berg
>
> Johannes, Eric tagged this for net, are you okay with me taking it?
> No strong preference here.
I guess that really would normally go through Luca's and Kalle's trees,
but yes, please just take it, it's been long and it won't conflict with
anything.
johannes
pull-request: mac80211-next 2021-01-26
Hi, So here's a pretty big and invasive mac80211-next pull. It has now been in linux-next for some time, and all the issues that had been reported by people running that are fixed. I've also thrown hwsim and Intel-internal tests at it. However, changing the locking is somewhat dangerous, so if I consider it, I sort of expect some more fallout from that. I did sprinkle lockdep assertions fairly liberally, so we'll see. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 9e8789c85deee047c5753e22f725d5fc10682468: net: stmmac: dwmac-meson8b: fix the RX delay validation (2021-01-20 22:15:08 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-net-next-2021-01-26 for you to fetch changes up to a05829a7222e9d10c416dd2dbbf3929fe6646b89: cfg80211: avoid holding the RTNL when calling the driver (2021-01-26 11:55:50 +0100) More updates: * many minstrel improvements, including removal of the old minstrel in favour of minstrel_ht * speed improvements on FQ * support for RX decapsulation (header conversion) offload * RTNL reduction: limit RTNL usage in the wireless stack mostly to where really needed (regulatory not yet) to reduce contention on it * various other small updates Arend van Spriel (1): cfg80211: add VHT rate entries for MCS-10 and MCS-11 Felix Fietkau (13): net/fq_impl: bulk-free packets from a flow on overmemory net/fq_impl: drop get_default_func, move default flow to fq_tin net/fq_impl: do not maintain a backlog-sorted list of flows mac80211: add rx decapsulation offload support mac80211: minstrel_ht: clean up CCK code mac80211: minstrel_ht: add support for OFDM rates on non-HT clients mac80211: remove legacy minstrel rate control mac80211: minstrel_ht: remove old ewma based rate average code mac80211: minstrel_ht: improve ampdu length estimation mac80211: minstrel_ht: improve sample rate selection mac80211: minstrel_ht: fix max probability rate selection mac80211: minstrel_ht: increase stats update interval mac80211: minstrel_ht: fix rounding error in throughput calculation Johannes Berg (2): cfg80211: change netdev registration/unregistration semantics cfg80211: avoid holding the RTNL when calling the driver Lorenzo Bianconi (1): mac80211: introduce aql_enable node in debugfs Max Chen (1): cfg80211: Add phyrate conversion support for extended MCS in 60GHz band Philipp Borgers (1): mac80211: add LDPC encoding to ieee80211_parse_tx_radiotap Ramon Fontes (1): mac80211_hwsim: add 6GHz channels Wen Gong (2): mac80211: remove NSS number of 160MHz if not support 160MHz for HE mac80211: reduce peer HE MCS/NSS to own capabilities drivers/net/wireless/ath/ath11k/reg.c | 4 +- drivers/net/wireless/ath/ath6kl/cfg80211.c | 4 +- drivers/net/wireless/ath/ath6kl/core.c | 2 + drivers/net/wireless/ath/ath6kl/init.c | 2 + drivers/net/wireless/ath/wil6210/cfg80211.c| 2 + drivers/net/wireless/ath/wil6210/netdev.c | 11 +- drivers/net/wireless/ath/wil6210/pcie_bus.c| 2 + .../wireless/broadcom/brcm80211/brcmfmac/core.c| 24 +- .../wireless/broadcom/brcm80211/brcmfmac/core.h| 6 +- .../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 12 +- drivers/net/wireless/intel/iwlwifi/mvm/d3.c| 2 +- drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 4 +- drivers/net/wireless/intel/iwlwifi/mvm/nvm.c | 2 +- drivers/net/wireless/mac80211_hwsim.c | 74 ++- drivers/net/wireless/marvell/mwifiex/cfg80211.c| 10 +- drivers/net/wireless/marvell/mwifiex/main.c| 7 + drivers/net/wireless/microchip/wilc1000/cfg80211.c | 2 +- drivers/net/wireless/microchip/wilc1000/mon.c | 4 +- drivers/net/wireless/microchip/wilc1000/netdev.c | 2 +- drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 4 +- drivers/net/wireless/quantenna/qtnfmac/core.c | 5 +- drivers/net/wireless/virt_wifi.c | 8 + include/net/cfg80211.h | 146 - include/net/fq.h | 11 +- include/net/fq_impl.h | 171 -- include/net/mac80211.h | 26 +- net/mac80211/Makefile | 2 - net/mac80211/debugfs.c | 52 ++ net/mac80211/debugfs_sta.c | 1 + net/mac80211/driver-ops.h | 16 + net/mac80211/he.c | 92 +++ net/mac80211/ieee80211_i.h | 3 +-
Re: [PATCH v2] cfg80211: avoid holding the RTNL when calling the driver
Hi Marek,
> I've checked today's linux-next with the updated commit 27bc93583e35
> ("cfg80211: avoid holding the RTNL when calling the driver") and there
> is still an issue there, but at least it doesn't cause a deadlock:
>
> cfg80211: Loading compiled-in X.509 certificates for regulatory database
> Bluetooth: vendor=0x2df, device=0x912a, class=255, fn=2
> cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
> Bluetooth: FW download over, size 533976 bytes
> btmrvl_sdio mmc3:0001:2: sdio device tree data not available
> mwifiex_sdio mmc3:0001:1: WLAN FW already running! Skip FW dnld
> mwifiex_sdio mmc3:0001:1: WLAN FW is active
> mwifiex_sdio mmc3:0001:1: CMD_RESP: cmd 0x242 error, result=0x2
> mwifiex_sdio mmc3:0001:1: mwifiex_process_cmdresp: cmd 0x242 failed
> during initialization
> [ cut here ]
> WARNING: CPU: 0 PID: 5 at net/wireless/core.c:1336
> cfg80211_register_netdevice+0xa4/0x198 [cfg80211]
Yeah, umm, brown paper bag style bug.
I meant to _move_ that line down, but somehow managed to _copy_ it down.
Need to just remove it since rdev is not even initialized at that point.
I've updated my tree to include this:
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 5e8b523dc645..200cd9f5fd5f 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1333,7 +1333,6 @@ int cfg80211_register_netdevice(struct net_device *dev)
int ret;
ASSERT_RTNL();
- lockdep_assert_held(&rdev->wiphy.mtx);
if (WARN_ON(!wdev))
return -EINVAL;
johannes
Re: pull-request: mac80211 2021-01-18.2
On Mon, 2021-01-25 at 13:59 +0100, Hans de Goede wrote: > Yes this fixes things, thank you that saves me from having to debug > the NULL ptr deref. :) > Do you want to submit this to Greg, or shall I (I've already > added it to me local tree as a commit with you as the author) ? > > If you want me to submit it upstream, may I have / add your S-o-b > for this ? I'll send it out, with a note asking where it should go ... could also take it through my tree since it fixes things from there. johannes
pull-request: mac80211 2021-01-26
Hi Jakub, We have a few fixes - note one is for a staging driver, but acked by Greg and fixing the driver for a change that came through my tree. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 1c45ba93d34cd6af75228f34d0675200c81738b5: net: dsa: microchip: Adjust reset release timing to match reference reset circuit (2021-01-20 20:52:28 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2021-01-26 for you to fetch changes up to 81f153faacd04c049e5482d6ff33daddc30ed44e: staging: rtl8723bs: fix wireless regulatory API misuse (2021-01-26 12:21:42 +0100) A couple of fixes: * fix 160 MHz channel switch in mac80211 * fix a staging driver to not deadlock due to some recent cfg80211 changes * fix NULL-ptr deref if cfg80211 returns -EINPROGRESS to wext (syzbot) * pause TX in mac80211 in type change to prevent crashes (syzbot) Johannes Berg (3): wext: fix NULL-ptr-dereference with cfg80211's lack of commit() mac80211: pause TX while changing interface type staging: rtl8723bs: fix wireless regulatory API misuse Shay Bar (1): mac80211: 160MHz with extended NSS BW in CSA drivers/staging/rtl8723bs/include/rtw_wifi_regd.h | 6 +++--- drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 6 +++--- drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 10 +++--- net/mac80211/ieee80211_i.h| 1 + net/mac80211/iface.c | 6 ++ net/mac80211/spectmgmt.c | 10 +++--- net/wireless/wext-core.c | 5 +++-- 7 files changed, 26 insertions(+), 18 deletions(-)
Re: [PATCH] staging: rtl8723bs: fix wireless regulatory API misuse
On Tue, 2021-01-26 at 12:20 +0100, Greg Kroah-Hartman wrote: > > > Greg, can you take this for 5.11 please? Or if you prefer, since the > > patch that exposed this and broke the driver went through my tree, I > > can take it as well. > > Please feel free to take it through yours, as I don't think I'll have > any more staging patches for 5.11-final (or none have been sent to me > yet), so this might be the fastest way in: > > Acked-by: Greg Kroah-Hartman OK, will do, thanks! johannes
[PATCH] staging: rtl8723bs: fix wireless regulatory API misuse
From: Johannes Berg
This code ends up calling wiphy_apply_custom_regulatory(), for which
we document that it should be called before wiphy_register(). This
driver doesn't do that, but calls it from ndo_open() with the RTNL
held, which caused deadlocks.
Since the driver just registers static regdomain data and then the
notifier applies the channel changes if any, there's no reason for
it to call this in ndo_open(), move it earlier to fix the deadlock.
Reported-and-tested-by: Hans de Goede
Fixes: 51d62f2f2c50 ("cfg80211: Save the regulatory domain with a lock")
Signed-off-by: Johannes Berg
---
Greg, can you take this for 5.11 please? Or if you prefer, since the
patch that exposed this and broke the driver went through my tree, I
can take it as well.
---
drivers/staging/rtl8723bs/include/rtw_wifi_regd.h | 6 +++---
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 6 +++---
drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 10 +++---
3 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/rtw_wifi_regd.h
b/drivers/staging/rtl8723bs/include/rtw_wifi_regd.h
index ab5a8627d371..f798b0c744a4 100644
--- a/drivers/staging/rtl8723bs/include/rtw_wifi_regd.h
+++ b/drivers/staging/rtl8723bs/include/rtw_wifi_regd.h
@@ -20,9 +20,9 @@ enum country_code_type_t {
COUNTRY_CODE_MAX
};
-int rtw_regd_init(struct adapter *padapter,
- void (*reg_notifier)(struct wiphy *wiphy,
- struct regulatory_request *request));
+void rtw_regd_init(struct wiphy *wiphy,
+ void (*reg_notifier)(struct wiphy *wiphy,
+ struct regulatory_request *request));
void rtw_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request);
diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index bf1417236161..11032316c53d 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -3211,9 +3211,6 @@ void rtw_cfg80211_init_wiphy(struct adapter *padapter)
rtw_cfg80211_init_ht_capab(&bands->ht_cap,
NL80211_BAND_2GHZ, rf_type);
}
- /* init regulary domain */
- rtw_regd_init(padapter, rtw_reg_notifier);
-
/* copy mac_addr to wiphy */
memcpy(wiphy->perm_addr, padapter->eeprompriv.mac_addr, ETH_ALEN);
@@ -3328,6 +3325,9 @@ int rtw_wdev_alloc(struct adapter *padapter, struct
device *dev)
*((struct adapter **)wiphy_priv(wiphy)) = padapter;
rtw_cfg80211_preinit_wiphy(padapter, wiphy);
+ /* init regulary domain */
+ rtw_regd_init(wiphy, rtw_reg_notifier);
+
ret = wiphy_register(wiphy);
if (ret < 0) {
DBG_8192C("Couldn't register wiphy device\n");
diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 578b9f734231..2833fc6901e6 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -139,15 +139,11 @@ static void _rtw_regd_init_wiphy(struct rtw_regulatory
*reg,
_rtw_reg_apply_flags(wiphy);
}
-int rtw_regd_init(struct adapter *padapter,
- void (*reg_notifier)(struct wiphy *wiphy,
- struct regulatory_request *request))
+void rtw_regd_init(struct wiphy *wiphy,
+ void (*reg_notifier)(struct wiphy *wiphy,
+ struct regulatory_request *request))
{
- struct wiphy *wiphy = padapter->rtw_wdev->wiphy;
-
_rtw_regd_init_wiphy(NULL, wiphy, reg_notifier);
-
- return 0;
}
void rtw_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
--
2.26.2
Re: pull-request: mac80211 2021-01-18.2
Hi, > > I don't have that much sympathy for a staging driver that's clearly > > doing things differently than it was intended (the documentation states > > that the function should be called only before wiphy_register(), not > > during ndo_open). :-) > > I completely understand and I already was worried that this might be > a staging-driver issue, which is why I mentioned this was with a > staging driver in the more detailed bug-report email. I guess I missed that, but no worries. > > But OTOH, that fix to the driver is simple and looks correct to me since > > it only ever has a static regdomain, and the notifier does the work of > > applying it to the channels as well. > > So I've given your fix a quick try and it leads to a NULL pointer deref. Ouch. Oh. I see, that driver is *really* stupid, trying to get to the wiphy from the adapter, but going through the wdev instead ... ouch. Wow are these pointers a mess in that driver ... Something like this, perhaps? https://p.sipsolutions.net/4400d9a3b7b800bb.txt johannes
Re: net: tso: add UDP segmentation support: adds regression for ax200 upload
Hi Eric, On Tue, 2021-01-19 at 11:02 +0100, Eric Dumazet wrote: > > > This does fix the problems reported on iwlwifi, were you planning to > > submit it as a proper patch? > > Sure, I will do, thanks ! Did you do that and I missed it? Or would you prefer we did? Thanks, Johannes
Re: linux-next boot error: WARNING in cfg80211_register_netdevice
On Mon, 2021-01-25 at 01:52 -0800, syzbot wrote: > > [ cut here ] > WARNING: CPU: 0 PID: 1 at net/wireless/core.c:1336 > cfg80211_register_netdevice+0x235/0x330 net/wireless/core.c:1336 > Yes, umm. I accidentally *copied* that line a few lines further down rather than *moving* it down. Ilan told me about it yesterday but I didn't have time to check and fix it up. It's fixed in mac80211-next now. johannes
Re: pull-request: mac80211 2021-01-18.2
Hi, Sorry, apparently we have two threads. > Great, thank you for looking into this. Let me know if you have a patch > which you want me to test on a RTL8723BS adapter. Could you test this instead? https://p.sipsolutions.net/235c352b8ae5db88.txt I don't have that much sympathy for a staging driver that's clearly doing things differently than it was intended (the documentation states that the function should be called only before wiphy_register(), not during ndo_open). :-) But OTOH, that fix to the driver is simple and looks correct to me since it only ever has a static regdomain, and the notifier does the work of applying it to the channels as well. > One thing which I forgot to mention earlier, it is not just lockdep > complaining > this appears to be a real deadlock, the wifi no longer functions, where > as it does function with the patch drops. Right. johannes
Re: pull-request: mac80211 2021-01-18.2
On Sat, 2021-01-23 at 22:31 +0100, Hans de Goede wrote: > > So I'm afraid that I have some bad news about this patch, it fixes > the RCU warning which I reported: > > https://lore.kernel.org/linux-wireless/20210104170713.66956-1-hdego...@redhat.com/ > > But it introduces a deadlock. See: > > https://lore.kernel.org/linux-wireless/d839ab62-e4bc-56f0-d861-f172bf19c...@redhat.com/ > > for details. Note we really should fix this new deadlock before 5.11 > is released. This is worse then the RCU warning which this patch fixes. Ouch. Thanks for the heads-up. I guess I'll revert both patches for now, unless we can quickly figure out a way to get all these paths in order. Thanks, johannes
Re: [PATCH v2] cfg80211: avoid holding the RTNL when calling the driver
On Wed, 2021-01-20 at 19:03 +0100, Johannes Berg wrote: > Could you take a look at these bits to see if that's fine with you? Actually, never mind. Given some bug reports, some rework was necessary, and that means this is no longer needed :-) johannes
Re: [PATCH v2] cfg80211: avoid holding the RTNL when calling the driver
Hi Marek,
> This patch landed in today's (20210122) linux-next as commit
> 791daf8fc49a ("cfg80211: avoid holding the RTNL when calling the
> driver"). Sadly, it causes deadlock with mwifiex driver. I think that
> lockdep report describes it enough:
Yeah, umm, sorry about that. Evidently, I somehow managed to put
"wiphy_lock()" into that part of the code, rather than "wiphy_unlock()"!
I'll fix, thanks!
johannes
Re: pull-request: mac80211 2021-01-18.2
On Wed, 2021-01-20 at 12:37 -0800, Jakub Kicinski wrote: > On Wed, 20 Jan 2021 18:59:21 +0100 Johannes Berg wrote: > > Hi Jakub, > > > > > This pull request was applied to netdev/net.git (refs/heads/master): > > > > Since you pulled this now, question: > > > > I have some pending content for mac80211-next/net-next that either > > conflicts with or requires a fix from here, or such. > > > > Could you pull net into net-next, so I can get it into mac80211-next? Or > > do you prefer another approach here? I could also double-apply the > > single patch, or pull myself but then we'd get a lot of net content into > > net-next only via mac80211-next which seems odd. > > Just merged net -> net-next, you can do your thing :) Ok cool, thanks. > Out of curiosity are you going to rebase mac80211-next or send a PR, > fast forward and then apply the conflicting patches? Normally I'd send a PR for it and then fast-forward. However, it's actually empty at the moment, so I'm just going to fast- forward it to net-next before I apply the next patches :-) johannes
Re: [PATCH v2] cfg80211: avoid holding the RTNL when calling the driver
Hi Oliver,
Could you take a look at these bits to see if that's fine with you? I'd
like to merge it through mac80211-next (pending some logistics with a
conflict)
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 1447da1d5729..47c4c1182ef1 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -1560,6 +1560,8 @@ void usbnet_disconnect (struct usb_interface *intf)
> struct usbnet *dev;
> struct usb_device *xdev;
> struct net_device *net;
> + const struct driver_info *info;
> + void (*unregdev)(struct net_device *);
>
> dev = usb_get_intfdata(intf);
> usb_set_intfdata(intf, NULL);
> @@ -1574,7 +1576,10 @@ void usbnet_disconnect (struct usb_interface *intf)
> dev->driver_info->description);
>
> net = dev->net;
> - unregister_netdev (net);
> +
> + info = dev->driver_info;
> + unregdev = info->unregister_netdev ?: unregister_netdev;
> + unregdev(net);
>
> cancel_work_sync(&dev->kevent);
>
> @@ -1627,6 +1632,7 @@ usbnet_probe (struct usb_interface *udev, const struct
> usb_device_id *prod)
> int status;
> const char *name;
> struct usb_driver *driver = to_usb_driver(udev->dev.driver);
> + int (*regdev)(struct net_device *);
>
> /* usbnet already took usb runtime pm, so have to enable the feature
>* for usb interface, otherwise usb_autopm_get_interface may return
> @@ -1646,6 +1652,8 @@ usbnet_probe (struct usb_interface *udev, const struct
> usb_device_id *prod)
> xdev = interface_to_usbdev (udev);
> interface = udev->cur_altsetting;
>
> + regdev = info->register_netdev ?: register_netdev;
> +
> status = -ENOMEM;
>
> // set up our own records
> @@ -1768,7 +1776,7 @@ usbnet_probe (struct usb_interface *udev, const struct
> usb_device_id *prod)
> }
> }
>
> - status = register_netdev (net);
> + status = regdev(net);
> if (status)
> goto out5;
> netif_info(dev, probe, dev->net,
[...]
> diff --git a/drivers/net/wireless/rndis_wlan.c
> b/drivers/net/wireless/rndis_wlan.c
> index 9fe77556858e..b646d4295cfd 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -3598,6 +3598,8 @@ static const struct driver_info bcm4320b_info = {
> .stop = rndis_wlan_stop,
> .early_init = bcm4320b_early_init,
> .indication = rndis_wlan_indication,
> + .register_netdev = cfg80211_register_netdev,
> + .unregister_netdev = cfg80211_unregister_netdev,
> };
>
> static const struct driver_info bcm4320a_info = {
> @@ -3613,6 +3615,8 @@ static const struct driver_info bcm4320a_info = {
> .stop = rndis_wlan_stop,
> .early_init = bcm4320a_early_init,
> .indication = rndis_wlan_indication,
> + .register_netdev = cfg80211_register_netdev,
> + .unregister_netdev = cfg80211_unregister_netdev,
> };
>
> static const struct driver_info rndis_wlan_info = {
> @@ -3628,6 +3632,8 @@ static const struct driver_info rndis_wlan_info = {
> .stop = rndis_wlan_stop,
> .early_init = unknown_early_init,
> .indication = rndis_wlan_indication,
> + .register_netdev = cfg80211_register_netdev,
> + .unregister_netdev = cfg80211_unregister_netdev,
> };
>
> /*-*/
[...]
> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> index 88a7673894d5..11e57803acf9 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -165,6 +165,12 @@ struct driver_info {
> /* rx mode change (device changes address list filtering) */
> void(*set_rx_mode)(struct usbnet *dev);
>
> + /* register netdev - defaults to register_netdev() */
> + int (*register_netdev)(struct net_device *dev);
> +
> + /* unregister netdev - defaults to unregister_netdev() */
> + void(*unregister_netdev)(struct net_device *dev);
> +
> /* for new devices, use the descriptor-reading code instead */
> int in; /* rx endpoint */
> int out;/* tx endpoint *
Re: pull-request: mac80211 2021-01-18.2
Hi Jakub, > This pull request was applied to netdev/net.git (refs/heads/master): Since you pulled this now, question: I have some pending content for mac80211-next/net-next that either conflicts with or requires a fix from here, or such. Could you pull net into net-next, so I can get it into mac80211-next? Or do you prefer another approach here? I could also double-apply the single patch, or pull myself but then we'd get a lot of net content into net-next only via mac80211-next which seems odd. Thanks, johannes
[PATCH v2] cfg80211: avoid holding the RTNL when calling the driver
From: Johannes Berg Currently, _everything_ in cfg80211 holds the RTNL, and if you have a slow USB device (or a few) you can get some bad lock contention on that. Fix that by re-adding a mutex to each wiphy/rdev as we had at some point, so we have locking for the wireless_dev lists and all the other things in there, and also so that drivers still don't have to worry too much about it (they still won't get parallel calls for a single device). Then, we can restrict the RTNL to a few cases where we add or remove interfaces and really need the added protection. Some of the global list management still also uses the RTNL, since we need to have it anyway for netdev management, but we only hold the RTNL for very short periods of time here. Signed-off-by: Johannes Berg --- v2: - various fixes including suspend/resume, broadcom drivers, etc. - note this applies on top of https://lore.kernel.org/r/iwlwifi.20210105165657.613e9a876829.Ia38d27dbebea28bf9c56d70691d243186ede70e7@changeid Please test/check the drivers ... I cannot test all of them but have tried to convert them all properly. --- drivers/net/usb/usbnet.c | 12 +- drivers/net/wireless/ath/ath11k/reg.c | 4 +- drivers/net/wireless/ath/ath6kl/core.c| 2 + drivers/net/wireless/ath/ath6kl/init.c| 2 + drivers/net/wireless/ath/wil6210/cfg80211.c | 2 + drivers/net/wireless/ath/wil6210/netdev.c | 7 +- drivers/net/wireless/ath/wil6210/pcie_bus.c | 2 + .../broadcom/brcm80211/brcmfmac/core.c| 26 +- .../broadcom/brcm80211/brcmfmac/core.h| 6 +- .../broadcom/brcm80211/brcmfmac/p2p.c | 12 +- drivers/net/wireless/intel/ipw2x00/ipw2100.c | 6 +- drivers/net/wireless/intel/ipw2x00/ipw2200.c | 10 +- drivers/net/wireless/intel/iwlwifi/mvm/d3.c | 2 +- .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 4 +- drivers/net/wireless/intel/iwlwifi/mvm/nvm.c | 2 +- drivers/net/wireless/intersil/orinoco/main.c | 4 +- drivers/net/wireless/marvell/libertas/cfg.c | 2 +- drivers/net/wireless/marvell/libertas/main.c | 2 +- drivers/net/wireless/marvell/libertas/mesh.c | 6 +- .../net/wireless/marvell/mwifiex/cfg80211.c | 6 +- drivers/net/wireless/marvell/mwifiex/main.c | 7 + drivers/net/wireless/microchip/wilc1000/mon.c | 2 +- .../net/wireless/microchip/wilc1000/netdev.c | 4 +- drivers/net/wireless/quantenna/qtnfmac/core.c | 3 +- drivers/net/wireless/rndis_wlan.c | 6 + drivers/net/wireless/virt_wifi.c | 8 + .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 2 +- drivers/staging/rtl8723bs/os_dep/os_intfs.c | 4 +- .../staging/rtl8723bs/os_dep/osdep_service.c | 6 +- drivers/staging/wlan-ng/p80211netdev.c| 4 +- include/linux/usb/usbnet.h| 6 + include/net/cfg80211.h| 112 ++- include/net/mac80211.h| 10 +- net/mac80211/iface.c | 15 +- net/mac80211/key.c| 4 +- net/mac80211/main.c | 5 + net/mac80211/pm.c | 6 +- net/mac80211/tdls.c | 6 +- net/mac80211/util.c | 10 +- net/wireless/chan.c | 5 +- net/wireless/core.c | 67 +- net/wireless/core.h | 2 +- net/wireless/debugfs.c| 4 - net/wireless/ibss.c | 3 +- net/wireless/mlme.c | 6 +- net/wireless/nl80211.c| 644 ++ net/wireless/reg.c| 91 ++- net/wireless/reg.h| 1 - net/wireless/scan.c | 35 +- net/wireless/sme.c| 5 +- net/wireless/sysfs.c | 5 + net/wireless/util.c | 4 +- net/wireless/wext-compat.c| 271 ++-- net/wireless/wext-sme.c | 4 +- 54 files changed, 952 insertions(+), 534 deletions(-) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 1447da1d5729..47c4c1182ef1 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1560,6 +1560,8 @@ void usbnet_disconnect (struct usb_interface *intf) struct usbnet *dev; struct usb_device *xdev; struct net_device *net; + const struct driver_info *info; + void (*unregdev)(struct net_device *); dev = usb_get_intfdata(intf); usb_set_intfdata(intf, NULL); @@ -1574,7 +1576,10 @@ void usbnet_disconnect (struct usb_interface *intf) dev->driver_info->description); net = dev->net; - unregister_netdev (net); + + info = dev->driver_info; + unregdev = i
Re: net: tso: add UDP segmentation support: adds regression for ax200 upload
Hi Eric, all,
Sorry we've been so silent on this.
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
> @@ -773,6 +773,7 @@ iwl_mvm_tx_tso_segment(struct sk_buff *skb,
> unsigned int num_subframes,
>
> next = skb_gso_segment(skb, netdev_flags);
> skb_shinfo(skb)->gso_size = mss;
> + skb_shinfo(skb)->gso_type = ipv4 ? SKB_GSO_TCPV4 : SKB_GSO_TCPV6;
> if (WARN_ON_ONCE(IS_ERR(next)))
> return -EINVAL;
> else if (next)
> @@ -795,6 +796,7 @@ iwl_mvm_tx_tso_segment(struct sk_buff *skb,
> unsigned int num_subframes,
>
> if (tcp_payload_len > mss) {
> skb_shinfo(tmp)->gso_size = mss;
> + skb_shinfo(tmp)->gso_type = ipv4 ?
> SKB_GSO_TCPV4 : SKB_GSO_TCPV6;
> } else {
> if (qos) {
> u8 *qc;
This does fix the problems reported on iwlwifi, were you planning to
submit it as a proper patch?
Thanks,
johannes
pull-request: mac80211 2021-01-18.2
Hi, New try, dropped the 160 MHz CSA patch for now that has the sparse issue since people are waiting for the kernel-doc fixes. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 220efcf9caf755bdf92892afd37484cb6859e0d2: Merge tag 'mlx5-fixes-2021-01-07' of git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux (2021-01-07 19:13:30 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2021-01-18.2 for you to fetch changes up to c13cf5c159660451c8fbdc37efb998b198e1d305: mac80211: check if atf has been disabled in __ieee80211_schedule_txq (2021-01-14 22:27:38 +0100) Various fixes: * kernel-doc parsing fixes * incorrect debugfs string checks * locking fix in regulatory * some encryption-related fixes Felix Fietkau (3): mac80211: fix fast-rx encryption check mac80211: fix encryption key selection for 802.3 xmit mac80211: do not drop tx nulldata packets on encrypted links Ilan Peer (1): cfg80211: Save the regulatory domain with a lock Johannes Berg (1): cfg80211/mac80211: fix kernel-doc for SAR APIs Lorenzo Bianconi (1): mac80211: check if atf has been disabled in __ieee80211_schedule_txq Mauro Carvalho Chehab (1): cfg80211: fix a kerneldoc markup Shayne Chen (1): mac80211: fix incorrect strlen of .write in debugfs include/net/cfg80211.h | 5 - include/net/mac80211.h | 1 + net/mac80211/debugfs.c | 44 net/mac80211/rx.c | 2 ++ net/mac80211/tx.c | 31 +-- net/wireless/reg.c | 11 ++- 6 files changed, 54 insertions(+), 40 deletions(-)
Re: pull-request: mac80211 2021-01-18
On Mon, 2021-01-18 at 10:12 +0100, Johannes Berg wrote: > Hi Jakub, > > Here are some fixes for wireless - probably the thing people > have most been waiting for is the kernel-doc fixes :-) Now that the email finally went through, let me withdraw this, there's a sparse error in one of the patches. johannes
pull-request: mac80211 2021-01-18
Hi Jakub, Here are some fixes for wireless - probably the thing people have most been waiting for is the kernel-doc fixes :-) Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 220efcf9caf755bdf92892afd37484cb6859e0d2: Merge tag 'mlx5-fixes-2021-01-07' of git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux (2021-01-07 19:13:30 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2021-01-18 for you to fetch changes up to 8b194febe111c5cc47595749a766d24ca33dd95a: mac80211: 160MHz with extended NSS BW in CSA (2021-01-18 09:43:41 +0100) Various fixes: * kernel-doc parsing fixes * incorrect debugfs string checks * fix for 160 MHz/extended NSS in CSA * locking fix in regulatory * some encryption-related fixes Felix Fietkau (3): mac80211: fix fast-rx encryption check mac80211: fix encryption key selection for 802.3 xmit mac80211: do not drop tx nulldata packets on encrypted links Ilan Peer (1): cfg80211: Save the regulatory domain with a lock Johannes Berg (1): cfg80211/mac80211: fix kernel-doc for SAR APIs Lorenzo Bianconi (1): mac80211: check if atf has been disabled in __ieee80211_schedule_txq Mauro Carvalho Chehab (1): cfg80211: fix a kerneldoc markup Shay Bar (1): mac80211: 160MHz with extended NSS BW in CSA Shayne Chen (1): mac80211: fix incorrect strlen of .write in debugfs include/net/cfg80211.h | 5 - include/net/mac80211.h | 1 + net/mac80211/debugfs.c | 44 net/mac80211/rx.c| 2 ++ net/mac80211/spectmgmt.c | 9 ++--- net/mac80211/tx.c| 31 +-- net/wireless/reg.c | 11 ++- 7 files changed, 60 insertions(+), 43 deletions(-)
Re: [PATCH 17/18] net: iosm: readme file
Hi Andrew, all, > > +For example, adding a link for a MBIM IP session with SessionId 5: > > + > > + ip link add link wwan0 name wwan0. type vlan id 5 > > So, this is what all the Ethernet nonsense is all about. You have a > session ID you need to somehow represent to user space. And you > decided to use VLANs. But to use VLANs, you need an Ethernet > header. So you added a bogus Ethernet header. So yeah, I don't think anyone likes that. I had half-heartedly started working on a replacement framework (*1), but then things happened and I didn't really have much time, and you also reviewed it and had some comments but when I looked the component framework really didn't seem appropriate, but didn't really have time to do anything on this either. (*1) https://lore.kernel.org/netdev/20200225100053.16385-1-johan...@sipsolutions.net/ In the mean time, the team doing this driver (I'm not directly involved, just helping them out with upstream processes) really needed/wanted to continue on this, and this is what they had already, more or less. Now, the question here at this point of course is they already had it that way. But that's easily explained - that's how it works upstream today, unfortunately, cf. for example drivers/net/usb/cdc_mbim.c. Now, granted, some of the newer ones such as drivers/net/ipa/ _don't_ things that way and come out with ARPHRD_RAWIP, but that requires userspace to actually be aware of this, and know how to create the necessary channels etc. For IPA this is handled by 'rmnet', but rmnet is just Qualcomm's proprietary protocol exposed as an rtnetlink type, so is rather unsuitable for this driver. Hence originally the thought we could come up with a generic framework to handle this all. Unfortunately, I never had the time to follow up on everything there. T be honest I also lost interest when IPA got merged without any thoughts given to unifying this, despite my involvement in the reviews and time spent on trying to make a suitable framework that would serve both IPA and this IOSM driver. > Is any of this VLAN stuff required by MBIM? Yes and no. It's not required to do _VLAN_ stuff, but that's one of the few ways that userspace currently knows of. Note that as far as I can tell Qualcomm (with rmnet/IPA etc.) has basically "reinvented" the world here - requiring the use of either their proprietary modem stack, or libqmi that knows specifically how to drive their modems. This was something we wanted to avoid (unless perhaps we could arrive at a standardised solution, see above) - thus being left with the VLAN method that's used elsewhere in the kernel. > Linux allows you to dynamically create/destroy network > interfaces. So you want to do something like > > ip link add link wwan0 name wwan42 type mbim id 42 > > Which will create a new mbim netdev interface using session id 42 on > top of the device which provides wwan0. I don't actually like this > last bit, but you somehow need to indicate on which MBIM transport you > want to create the new session, since you could have multiple bits of > hardware providing MBIM services. I don't even like the fact that 'wwan0' exists there in the first place (or how it exists in this driver), because it cannot ever actually transport traffic since it's just the root device of sorts. Hence the proposal to have - similar what we do in wifi - a separate abstraction of what a modem device is, and then just allow channels to be created on it, and those channels are exposed as netdevs. In any case - I'm not sure how we resolve this. On the one hand, as a technical person going for the most technically correct solution, I'd say you're completely right and this should expose pure IP netdevs, and have a (custom or not) way to configure channels. That still leaves the "dead" wwan0 interface that can't do anything, but at least it's better for the channel netdevs. Perhaps like with the framework I was trying to do. We could even initially side-step the issue with the component framework and simply not allow that in the framework from the start. However, I'm not sure of the value of this. Qualcomm's newer stuff is already locked in to their custom APIs in rmnet and IPA, with QMI etc. If we're honest with ourselves, older stuff that exists in the kernel today is highly unlikely to be converted since it works now and very few people really care about anything else. Which basically leaves only this driver - either doing some old-fashioned way like it is now, or - doing its own custom way like rmnet/IPA, or - coming with a framework that pretends to be more general than rmnet but really is only used for this driver. The later two choices both require significant investment on the userspace side, so I don't think it's any wonder the first is what the driver chose, especially after my more or less failed attempt at getting traction for the common framework (before IPA got merged, after all.) Also, non-technically speaking, I'm really not
Re: [PATCH v6 12/16] net: tip: fix a couple kernel-doc markups
On Thu, 2021-01-14 at 10:34 -0800, Jakub Kicinski wrote: > On Thu, 14 Jan 2021 10:59:08 -0500 Jon Maloy wrote: > > On 1/14/21 3:04 AM, Mauro Carvalho Chehab wrote: > > > A function has a different name between their prototype > > > and its kernel-doc markup: > > > > > > ../net/tipc/link.c:2551: warning: expecting prototype for > > > link_reset_stats(). Prototype was for tipc_link_reset_stats() instead > > > ../net/tipc/node.c:1678: warning: expecting prototype for is the > > > general link level function for message sending(). Prototype was for > > > tipc_node_xmit() instead > > > > > > Signed-off-by: Mauro Carvalho Chehab > > > > Acked-by: Jon Maloy > > Thanks! Applied this one to net, the cfg80211 one does not apply to > net, so I'll leave it to Johannes. Right, that was diffed against -next, and I've got a fix pending that I didn't send yet. I've applied this now, thanks. johannes
Re: linux-next: build warning after merge of the origin tree
Hi Stephen, > > Right, thanks. I believe I also fixed it in the patch I sent a few days > > ago that fixed the other documentation warning related to SAR that you > > reported. > > I don't think so :-( I did a htmldocs build with your patch ([PATCH > v2] cfg80211/mac80211: fix kernel-doc for SAR APIs) on top of Linus' > tree and still got this warning. That patch did not touch > include/net/mac80211.h ... Umm, I don't know what to say. I even added "cfg80211/mac80211" to the subject, but somehow lost the change to mac80211.h. Sorry about that :( I'll get a v3 out. johannes
Re: linux-next: build warning after merge of the origin tree
Hi Stephen,
On Thu, 2021-01-07 at 09:05 +1100, Stephen Rothwell wrote:
> Hi all,
>
> Building Linus' tree, today's linux-next build (htmldocs) produced
> this warning:
>
> include/net/mac80211.h:4200: warning: Function parameter or member
> 'set_sar_specs' not described in 'ieee80211_ops'
>
> Introduced by commit
>
> c534e093d865 ("mac80211: add ieee80211_set_sar_specs")
>
> Sorry, I missed this earlier.
Right, thanks. I believe I also fixed it in the patch I sent a few days
ago that fixed the other documentation warning related to SAR that you
reported.
Thanks,
johannes
Re: net: tso: add UDP segmentation support: adds regression for ax200 upload
On Fri, 2020-12-18 at 12:16 -0800, Jakub Kicinski wrote: > On Thu, 17 Dec 2020 12:40:26 -0800 Ben Greear wrote: > > On 12/17/20 10:20 AM, Eric Dumazet wrote: > > > On Thu, Dec 17, 2020 at 7:13 PM Ben Greear > > > wrote: > > > > It is the iwlwifi/mvm logic that supports ax200. > > > > > > Let me ask again : > > > > > > I see two different potential call points : > > > > > > drivers/net/wireless/intel/iwlwifi/pcie/tx.c:1529: > > > tso_build_hdr(skb, hdr_page->pos, &tso, data_left, !total_len); > > > drivers/net/wireless/intel/iwlwifi/queue/tx.c:427: > > > tso_build_hdr(skb, hdr_page->pos, &tso, data_left, !total_len); > > > > > > To the best of your knowledge, which one would be used in your case ? > > > > > > Both are horribly complex, I do not want to spend time studying two > > > implementations. > > > > It is the queue/tx.c code that executes on my system, verified with > > printk. > > Not sure why Intel's not on CC here. Heh :) Let's also add linux-wireless. > Luca, is the ax200 TSO performance regression with recent kernel on your > radar? It wasn't on mine for sure, so far. But it's supposed to be Christmas vacation, so haven't checked our bug tracker etc. I see Emmanuel was at least looking at the bug report, but not sure what else happened yet. Off the top of my head, I don't really see the issue. Does anyone have the ability to capture the frames over the air (e.g. with another AX200 in monitor mode, load the driver with amsdu_size=3 module parameter to properly capture A-MSDUs)? johannes
pull-request: mac80211-next 2020-12-11
Hi Dave, Welcome back! I'm a bit late with this, I guess, but I hope you can still pull it into net-next for 5.11. Nothing really stands out, we have some 6 GHz fixes, and various small things all over. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 91163f82143630a9629a8bf0227d49173697c69c: Merge branch 'add-ppp_generic-ioctls-to-bridge-channels' (2020-12-10 13:58:49 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-net-next-2020-12-11 for you to fetch changes up to c534e093d865d926d042e0a3f228d1152627ccab: mac80211: add ieee80211_set_sar_specs (2020-12-11 13:39:59 +0100) A new set of wireless changes: * validate key indices for key deletion * more preamble support in mac80211 * various 6 GHz scan fixes/improvements * a common SAR power limitations API * various small fixes & code improvements Anant Thazhemadam (1): nl80211: validate key indexes for cfg80211_registered_device Avraham Stern (3): nl80211: always accept scan request with the duration set ieee80211: update reduced neighbor report TBTT info length mac80211: support Rx timestamp calculation for all preamble types Ayala Beker (1): cfg80211: scan PSC channels in case of scan with wildcard SSID Carl Huang (2): nl80211: add common API to configure SAR power limitations mac80211: add ieee80211_set_sar_specs Colin Ian King (1): net: wireless: make a const array static, makes object smaller Emmanuel Grumbach (2): rfkill: add a reason to the HW rfkill state mac80211: don't filter out beacons once we start CSA Gustavo A. R. Silva (3): cfg80211: Fix fall-through warnings for Clang mac80211: Fix fall-through warnings for Clang nl80211: Fix fall-through warnings for Clang Ilan Peer (6): cfg80211: Parse SAE H2E only membership selector mac80211: Skip entries with SAE H2E only membership selector cfg80211: Update TSF and TSF BSSID for multi BSS cfg80211: Save the regulatory domain when setting custom regulatory mac80211: Fix calculation of minimal channel width mac80211: Update rate control on channel change Johannes Berg (10): mac80211: support MIC error/replay detected counters driver update mac80211: disallow band-switch during CSA cfg80211: include block-tx flag in channel switch started event cfg80211: remove struct ieee80211_he_bss_color mac80211: use struct assignment for he_obss_pd cfg80211: support immediate reconnect request hint mac80211: support driver-based disconnect with reconnect hint mac80211: don't set set TDLS STA bandwidth wider than possible mac80211: use bitfield helpers for BA session action frames mac80211: ignore country element TX power on 6 GHz Lev Stipakov (1): net: mac80211: use core API for updating TX/RX stats Sami Tolvanen (1): cfg80211: fix callback type mismatches in wext-compat Shaul Triebitz (1): mac80211: he: remove non-bss-conf fields from bss_conf Tom Rix (1): mac80211: remove trailing semicolon in macro definitions Wen Gong (2): mac80211: mlme: save ssid info to ieee80211_bss_conf while assoc mac80211: fix a mistake check for rx_stats update include/linux/ieee80211.h | 9 +- include/linux/rfkill.h| 24 - include/net/cfg80211.h| 75 ++--- include/net/mac80211.h| 35 ++- include/uapi/linux/nl80211.h | 114 +++- include/uapi/linux/rfkill.h | 16 ++- net/mac80211/agg-rx.c | 8 +- net/mac80211/agg-tx.c | 12 +-- net/mac80211/cfg.c| 22 +++- net/mac80211/chan.c | 71 - net/mac80211/debugfs.c| 2 +- net/mac80211/debugfs_key.c| 2 +- net/mac80211/debugfs_netdev.c | 6 +- net/mac80211/debugfs_sta.c| 2 +- net/mac80211/ieee80211_i.h| 14 +-- net/mac80211/key.c| 49 + net/mac80211/mlme.c | 123 +++--- net/mac80211/rx.c | 20 +--- net/mac80211/trace.h | 23 +++- net/mac80211/tx.c | 16 +-- net/mac80211/util.c | 66 +++- net/mac80211/vht.c| 14 ++- net/rfkill/core.c | 41 ++-- net/wireless/core.h | 2 + net/wireless/mlme.c | 26 +++-- net/wireless/nl80211.c| 239 ++ net/wireless/nl80211.h| 8 +- net/wireless/rdev-ops.h | 12 +++ net/wireless/reg.c| 10 +- net/wireless/scan.c | 21 ++-- net/wireless/trace.h | 31 +- net/wireless/util.c | 52 +++-- n
Re: [PATCH net] mac80211: mesh: fix mesh_pathtbl_init() error path
On Fri, 2020-12-04 at 08:24 -0800, Eric Dumazet wrote:
> From: Eric Dumazet
>
> If tbl_mpp can not be allocated, we call mesh_table_free(tbl_path)
> while tbl_path rhashtable has not yet been initialized, which causes
> panics.
>
> Simply factorize the rhashtable_init() call into mesh_table_alloc()
>
> WARNING: CPU: 1 PID: 8474 at kernel/workqueue.c:3040 __flush_work
> kernel/workqueue.c:3040 [inline]
> WARNING: CPU: 1 PID: 8474 at kernel/workqueue.c:3040
> __cancel_work_timer+0x514/0x540 kernel/workqueue.c:3136
> Modules linked in:
> CPU: 1 PID: 8474 Comm: syz-executor663 Not tainted 5.10.0-rc6-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:__flush_work kernel/workqueue.c:3040 [inline]
> RIP: 0010:__cancel_work_timer+0x514/0x540 kernel/workqueue.c:3136
> Code: 5d c3 e8 bf ae 29 00 0f 0b e9 f0 fd ff ff e8 b3 ae 29 00 0f 0b 43 80 3c
> 3e 00 0f 85 31 ff ff ff e9 34 ff ff ff e8 9c ae 29 00 <0f> 0b e9 dc fe ff ff
> 89 e9 80 e1 07 80 c1 03 38 c1 0f 8c 7d fd ff
> RSP: 0018:c9000165f5a0 EFLAGS: 00010293
> RAX: 814b7064 RBX: 0001 RCX: 888021c8
> RDX: RSI: RDI:
> RBP: 888024039ca0 R08: dc00 R09: fbfff1dd3e64
> R10: fbfff1dd3e64 R11: R12: 1920002cbebd
> R13: 888024039c88 R14: 111004807391 R15: dc00
> FS: 01347880() GS:8880b9d0() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 2140 CR3: 2cc0a000 CR4: 001506e0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> Call Trace:
> rhashtable_free_and_destroy+0x25/0x9c0 lib/rhashtable.c:1137
> mesh_table_free net/mac80211/mesh_pathtbl.c:69 [inline]
> mesh_pathtbl_init+0x287/0x2e0 net/mac80211/mesh_pathtbl.c:785
> ieee80211_mesh_init_sdata+0x2ee/0x530 net/mac80211/mesh.c:1591
> ieee80211_setup_sdata+0x733/0xc40 net/mac80211/iface.c:1569
> ieee80211_if_add+0xd5c/0x1cd0 net/mac80211/iface.c:1987
> ieee80211_add_iface+0x59/0x130 net/mac80211/cfg.c:125
> rdev_add_virtual_intf net/wireless/rdev-ops.h:45 [inline]
> nl80211_new_interface+0x563/0xb40 net/wireless/nl80211.c:3855
> genl_family_rcv_msg_doit net/netlink/genetlink.c:739 [inline]
> genl_family_rcv_msg net/netlink/genetlink.c:783 [inline]
> genl_rcv_msg+0xe4e/0x1280 net/netlink/genetlink.c:800
> netlink_rcv_skb+0x190/0x3a0 net/netlink/af_netlink.c:2494
> genl_rcv+0x24/0x40 net/netlink/genetlink.c:811
> netlink_unicast_kernel net/netlink/af_netlink.c:1304 [inline]
> netlink_unicast+0x780/0x930 net/netlink/af_netlink.c:1330
> netlink_sendmsg+0x9a8/0xd40 net/netlink/af_netlink.c:1919
> sock_sendmsg_nosec net/socket.c:651 [inline]
> sock_sendmsg net/socket.c:671 [inline]
> sys_sendmsg+0x519/0x800 net/socket.c:2353
> ___sys_sendmsg net/socket.c:2407 [inline]
> __sys_sendmsg+0x2b1/0x360 net/socket.c:2440
> do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Fixes: 60854fd94573 ("mac80211: mesh: convert path table to rhashtable")
> Signed-off-by: Eric Dumazet
> Reported-by: syzbot
Reviewed-by: Johannes Berg
Jakub, if you want to take it to the net tree I wouldn't mind at all,
since I _just_ sent a pull request a little while ago.
Thanks,
johannes
Re: [PATCH net] mac80211: mesh: fix mesh_pathtbl_init() error path
On Fri, 2020-12-04 at 17:26 +0100, Johannes Berg wrote: > On Fri, 2020-12-04 at 08:24 -0800, Eric Dumazet wrote: > > From: Eric Dumazet > > > > If tbl_mpp can not be allocated, we call mesh_table_free(tbl_path) > > while tbl_path rhashtable has not yet been initialized, which causes > > panics. > > Thanks Eric! > > I was going to ask how you ran into this ... > > > Reported-by: syzbot > > Until I saw this - but doesn't syzbot normally want a > "syzbot+somehashid@..." as the reported-by? > > > > --- a/net/mac80211/mesh_pathtbl.c > > +++ b/net/mac80211/mesh_pathtbl.c > > @@ -60,6 +60,7 @@ static struct mesh_table *mesh_table_alloc(void) > > atomic_set(&newtbl->entries, 0); > > spin_lock_init(&newtbl->gates_lock); > > spin_lock_init(&newtbl->walk_lock); > > + rhashtable_init(&newtbl->rhead, &mesh_rht_params); > > > > return newtbl; > > } > > @@ -773,9 +774,6 @@ int mesh_pathtbl_init(struct ieee80211_sub_if_data > > *sdata) > > goto free_path; > > } > > > > - rhashtable_init(&tbl_path->rhead, &mesh_rht_params); > > - rhashtable_init(&tbl_mpp->rhead, &mesh_rht_params); > > > > Hmm. There were two calls, now there's only one? Is that a bug, or am I > missing something? Umm, never mind. johannes
Re: [PATCH net] mac80211: mesh: fix mesh_pathtbl_init() error path
On Fri, 2020-12-04 at 08:24 -0800, Eric Dumazet wrote: > From: Eric Dumazet > > If tbl_mpp can not be allocated, we call mesh_table_free(tbl_path) > while tbl_path rhashtable has not yet been initialized, which causes > panics. Thanks Eric! I was going to ask how you ran into this ... > Reported-by: syzbot Until I saw this - but doesn't syzbot normally want a "syzbot+somehashid@..." as the reported-by? > --- a/net/mac80211/mesh_pathtbl.c > +++ b/net/mac80211/mesh_pathtbl.c > @@ -60,6 +60,7 @@ static struct mesh_table *mesh_table_alloc(void) > atomic_set(&newtbl->entries, 0); > spin_lock_init(&newtbl->gates_lock); > spin_lock_init(&newtbl->walk_lock); > + rhashtable_init(&newtbl->rhead, &mesh_rht_params); > > return newtbl; > } > @@ -773,9 +774,6 @@ int mesh_pathtbl_init(struct ieee80211_sub_if_data *sdata) > goto free_path; > } > > - rhashtable_init(&tbl_path->rhead, &mesh_rht_params); > - rhashtable_init(&tbl_mpp->rhead, &mesh_rht_params); > Hmm. There were two calls, now there's only one? Is that a bug, or am I missing something? johannes
pull-request: mac80211 2020-12-04
Hi Jakub, We've got a few more fixes for the current cycle, everything else I have pending right now seems likely to go to 5.11 instead. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit bbe2ba04c5a92a49db8a42c850a5a2f6481e47eb: Merge tag 'net-5.10-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2020-12-03 13:10:11 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2020-12-04 for you to fetch changes up to bdeca45a0cc58f864f1eb2e919304203ff5c5f39: mac80211: set SDATA_STATE_RUNNING for monitor interfaces (2020-12-04 12:45:25 +0100) Three small fixes: * initialize some data to avoid using stack garbage * fix 6 GHz channel selection in mac80211 * correctly restart monitor mode interfaces in mac80211 Borwankar, Antara (1): mac80211: set SDATA_STATE_RUNNING for monitor interfaces Sara Sharon (1): cfg80211: initialize rekey_data Wen Gong (1): mac80211: fix return value of ieee80211_chandef_he_6ghz_oper net/mac80211/iface.c | 2 ++ net/mac80211/util.c| 2 +- net/wireless/nl80211.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-)
Re: [PATCH] net: mac80211: cfg: enforce sanity checks for key_index in ieee80211_del_key()
On Tue, 2020-12-01 at 18:15 +0530, Anant Thazhemadam wrote: > > cfg80211_supported_cipher_suite(&rdev->wiphy, params->cipher) returned > false, and thus it worked for the syzbot reproducer. > Would it be a safer idea to enforce the conditions that I initially put (in > ieee80211_del_key()) directly in cfg80211_validate_key_settings() itself - by > updating max_key_index, and checking accordingly? Yes, I think so. But similarly to cfg80211_validate_key_settings() it should look at the device capabilities (beacon protection, etc.) Thanks! johannes
Re: [PATCH] net: mac80211: cfg: enforce sanity checks for key_index in ieee80211_del_key()
On Tue, 2020-12-01 at 17:26 +0530, Anant Thazhemadam wrote: > On 01/12/20 3:30 pm, Johannes Berg wrote: > > On Tue, 2020-12-01 at 15:26 +0530, Anant Thazhemadam wrote: > > > Currently, it is assumed that key_idx values that are passed to > > > ieee80211_del_key() are all valid indexes as is, and no sanity checks > > > are performed for it. > > > However, syzbot was able to trigger an array-index-out-of-bounds bug > > > by passing a key_idx value of 5, when the maximum permissible index > > > value is (NUM_DEFAULT_KEYS - 1). > > > Enforcing sanity checks helps in preventing this bug, or a similar > > > instance in the context of ieee80211_del_key() from occurring. > > I think we should do this more generally in cfg80211, like in > > nl80211_new_key() we do it via cfg80211_validate_key_settings(). > > > > I suppose we cannot use the same function, but still, would be good to > > address this generally in nl80211 for all drivers. > > Hello, > > This gave me the idea of trying to use cfg80211_validate_key_settings() > directly in ieee80211_del_key(). I did try that out, tested it, and this bug > doesn't seem to be getting triggered anymore. > If this is okay, then I can send in a v2 soon. :) > > If there is any reason that I'm missing as to why > cfg80211_validate_key_settings() > cannot be used in this context, please let me know. If it works then I guess that's OK. I thought we didn't have all the right information, e.g. whether a key is pairwise or not? johannes
Re: [PATCH] net: mac80211: cfg: enforce sanity checks for key_index in ieee80211_del_key()
On Tue, 2020-12-01 at 15:26 +0530, Anant Thazhemadam wrote: > Currently, it is assumed that key_idx values that are passed to > ieee80211_del_key() are all valid indexes as is, and no sanity checks > are performed for it. > However, syzbot was able to trigger an array-index-out-of-bounds bug > by passing a key_idx value of 5, when the maximum permissible index > value is (NUM_DEFAULT_KEYS - 1). > Enforcing sanity checks helps in preventing this bug, or a similar > instance in the context of ieee80211_del_key() from occurring. I think we should do this more generally in cfg80211, like in nl80211_new_key() we do it via cfg80211_validate_key_settings(). I suppose we cannot use the same function, but still, would be good to address this generally in nl80211 for all drivers. johannes
Re: [PATCH v5 2/3] net: add kcov handle to skb extensions
On Sat, 2020-11-21 at 12:55 -0800, Jakub Kicinski wrote: > [snip] > Ack, you have to figure out all the places anyway, the question is > whether you put probes there or calls in the source code. > > Shifting the maintenance burden but also BPF is flexibility. Yeah, true. Though I'd argue also visibility - this stuff is pretty simple now, if it gets into lots of lines of BPF code to track it that is maintained "elsewhere", we won't see the bugs in it :-) And it's kinda a thing that we as kernel developers _should_ be the ones looking at since it's testing our code. > Yup, the point is you can feed a raw skb pointer (and all other > possible context you may want) to a BPF prog in kcov_remote_start() > and let BPF/BTF give you the handle it recorded in its maps. Yeah, it's possible. Personally, I don't think it's worth the complexity. > It is more complicated. We can go back to an skb field if this work is > expected to yield results for mac80211. Would you mind sending a patch? I can do that, but I'm not going to be able to do it now/tonight (GMT+1 here), so probably only Monday/Tuesday or so, sorry. johannes
Re: [PATCH v5 2/3] net: add kcov handle to skb extensions
On Sat, 2020-11-21 at 10:35 -0800, Jakub Kicinski wrote: > On Sat, 21 Nov 2020 19:12:21 +0100 Johannes Berg wrote: > > > So I'm leaning towards reverting the whole thing. You can attach > > > kretprobes and record the information you need in BPF maps. > > > > I'm not going to object to reverting it (and perhaps redoing it better > > later), but I will point out that kretprobe isn't going to work, you > > eventually need kcov_remote_start() to be called in strategic points > > before processing the skb after it bounced through the system. > > > > IOW, it's not really about serving userland, it's about enabling (and > > later disabling) coverage collection for the bits of code it cares > > about, mostly because collecting it for _everything_ is going to be too > > slow and will mess up the data since for coverage guided fuzzing you > > really need the reported coverage data to be only about the injected > > fuzz data... > > All you need is make kcov_remote_start_common() be BPF-able, like > the LSM hooks are now, right? And then BPF can return whatever handle > it pleases. Not sure I understand. Are you saying something should call "kcov_remote_start_common()" with, say, the SKB, and leave it to a mass of bpf hooks to figure out where the SKB got cloned or copied or whatnot, track that in a map, and then ... no, wait, I don't really see what you mean, sorry. IIUC, fundamentally, you have this: - at the beginning, a task is tagged with "please collect coverage data for this handle" - this task creates an SKB, etc, and all of the code that this task executes is captured and the coverage data is reported - However, the SKB traverses lots of things, gets copied, cloned, or whatnot, and eventually leaves the annotated task, say for further processing in softirq context or elsewhere. Now since the whole point is to see what chaos this SKB created from beginning (allocation) to end (free), since it was filled with fuzzed data, you now have to figure out where to pick back up when the SKB is processed further. This is what the infrastructure was meant to solve. But note that the SKB might be further cloned etc, so in order to track it you'd have to (out-of-band) figure out all the possible places where it could be reallocated, any time the skb pointer could change. Then, when you know you've got interesting code on your hands, like in mac80211 that was annotated in patch 3 here, you basically say "oohhh, this SKB was annotated before, let's continue capturing coverage data here" (and turn it off again later by the corresponding kcov_remote_stop(). So the only way I could _possibly_ see how to do this would be to * capture all possible places where the skb pointer can change * still call something like skb_get_kcov_handle() but let it call out to a BPF program to query a map or something to figure out if this SKB has a handle attached to it > Or if you don't like BPF or what to KCOV BPF itself in the future you > can roll your own mechanism. The point is - this should be relatively > easily doable out of line... Seems pretty complicated to me though ... johannes
Re: [PATCH v5 2/3] net: add kcov handle to skb extensions
On Sat, 2020-11-21 at 10:06 -0800, Jakub Kicinski wrote: > On Sat, 21 Nov 2020 17:52:27 +0100 Florian Westphal wrote: > > Ido Schimmel wrote: > > > Other suggestions? > > > > Aleksandr, why was this made into an skb extension in the first place? > > > > AFAIU this feature is usually always disabled at build time. > > For debug builds (test farm /debug kernel etc) its always needed. > > > > If thats the case this u64 should be an sk_buff member, not an > > extension. > > Yeah, in hindsight I should have looked at how it's used. Not a great > fit for extensions. We can go back, but... > > In general I'm not very happy at how this is going. First of all just > setting the handle in a couple of allocs seems to not be enough, skbs > get cloned, reused etc. There were also build problems caused by this > patch and Aleksandr & co where nowhere to be found. Now we find out > this causes leaks, how was that not caught by the syzbot it's supposed > to serve?! Heh. > So I'm leaning towards reverting the whole thing. You can attach > kretprobes and record the information you need in BPF maps. I'm not going to object to reverting it (and perhaps redoing it better later), but I will point out that kretprobe isn't going to work, you eventually need kcov_remote_start() to be called in strategic points before processing the skb after it bounced through the system. IOW, it's not really about serving userland, it's about enabling (and later disabling) coverage collection for the bits of code it cares about, mostly because collecting it for _everything_ is going to be too slow and will mess up the data since for coverage guided fuzzing you really need the reported coverage data to be only about the injected fuzz data... johannes
Re: [PATCH v5 2/3] net: add kcov handle to skb extensions
On Sat, 2020-11-21 at 17:52 +0100, Florian Westphal wrote: > > Aleksandr, why was this made into an skb extension in the first place? > > AFAIU this feature is usually always disabled at build time. > For debug builds (test farm /debug kernel etc) its always needed. > > If thats the case this u64 should be an sk_buff member, not an > extension. Because it was requested :-) https://lore.kernel.org/netdev/20201009161558.57792...@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/ johannes
Re: [PATCH net-next] net: don't include ethtool.h from netdevice.h
On Fri, 2020-11-20 at 14:13 -0800, Jakub Kicinski wrote: > linux/netdevice.h is included in very many places, touching any > of its dependecies causes large incremental builds. > > Drop the linux/ethtool.h include, linux/netdevice.h just needs > a forward declaration of struct ethtool_ops. > > Fix all the places which made use of this implicit include. > include/net/cfg80211.h | 1 + Sounds good to me, thanks. Will still cause all wireless drivers to rebuild this way though. Maybe I'll see later if something can be done about that. Acked-by: Johannes Berg Thanks, johannes
Re: [PATCH net] cfg80211: fix callback type mismatches in wext-compat
On Fri, 2020-11-20 at 11:26 -0800, Kees Cook wrote: > On Tue, Nov 17, 2020 at 02:07:43PM -0800, Sami Tolvanen wrote: > > On Tue, Nov 17, 2020 at 1:45 PM Kees Cook wrote: > > > On Tue, Nov 17, 2020 at 12:59:02PM -0800, Sami Tolvanen wrote: > > > > Instead of casting callback functions to type iw_handler, which trips > > > > indirect call checking with Clang's Control-Flow Integrity (CFI), add > > > > stub functions with the correct function type for the callbacks. > > > > > > Oh, wow. iw_handler with union iwreq_data look like really nasty hacks. > > > Aren't those just totally bypassing type checking? Where do the > > > callbacks actually happen? I couldn't find them... > > > > The callbacks to these happen in ioctl_standard_call in wext-core.c. > > Thanks! If this is all the 'old compat' code, this patch looks fine. I > think new stuff should probably encode types in some fashion (rather > than via wrappers). Everything mentioning wext has been deprecated for something like 15 years ... so yeah. But people still use it :( johannes
Re: [PATCH v2 1/3] net: mac80211: use core API for updating TX/RX stats
On Fri, 2020-11-13 at 11:51 -0800, Jakub Kicinski wrote: > On Fri, 13 Nov 2020 14:25:25 +0200 Lev Stipakov wrote: > > > Seems I should take this through my tree, any objections? > > Go for it, you may need to pull net-next first but that should happen > soonish anyway, when I get to your pr. Yeah, I'll fast forward once you have pulled that, and generally I don't apply anything while I have open pull requests (in case I have to rejigger or whatnot), so all should be well. :) Thanks! johannes
Re: [PATCH v2 1/3] net: mac80211: use core API for updating TX/RX stats
On Fri, 2020-11-13 at 10:58 +0200, Lev Stipakov wrote:
> Commits
>
> d3fd65484c781 ("net: core: add dev_sw_netstats_tx_add")
> 451b05f413d3f ("net: netdevice.h: sw_netstats_rx_add helper)
>
> have added API to update net device per-cpu TX/RX stats.
>
> Use core API instead of ieee80211_tx/rx_stats().
>
This looks like a 1/3 but I only ever saw this, not the others.
Seems I should take this through my tree, any objections?
johannes
pull-request: mac80211-next 2020-11-13
Hi Jakub, And here's another set of patches, this one for -next. Nothing much stands out, perhaps apart from the WDS removal, but that was old and pretty much dead code when we turned it off, so it won't be missed. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit c9448e828d113cd7eafe77c414127e877ca88b20: Merge tag 'mlx5-updates-2020-11-03' of git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux (2020-11-05 18:01:31 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-net-next-2020-11-13 for you to fetch changes up to da1e9dd3a11cda85b58dafe64f091734934b2f6c: nl80211: fix kernel-doc warning in the new SAE attribute (2020-11-11 08:39:13 +0100) Some updates: * injection/radiotap updates for new test capabilities * remove WDS support - even years ago when we turned it off by default it was already basically unusable * support for HE (802.11ax) rates for beacons * support for some vendor-specific HE rates * many other small features/cleanups Colin Ian King (1): nl80211/cfg80211: fix potential infinite loop Johannes Berg (9): wireless: remove CONFIG_WIRELESS_WDS ath9k: remove WDS code carl9170: remove WDS code b43: remove WDS code b43legacy: remove WDS code rt2x00: remove WDS code mac80211: remove WDS-related code cfg80211: remove WDS code nl80211: fix kernel-doc warning in the new SAE attribute Julia Lawall (1): mac80211: use semicolons rather than commas to separate statements Kurt Lee (1): ieee80211: Add definition for WFA DPP Mathy Vanhoef (4): mac80211: add radiotap flag to assure frames are not reordered mac80211: adhere to Tx control flag that prevents frame reordering mac80211: don't overwrite QoS TID of injected frames mac80211: assure that certain drivers adhere to DONT_REORDER flag Pradeep Kumar Chitrapu (1): mac80211: save HE oper info in BSS config for mesh Rajkumar Manoharan (2): nl80211: fix beacon tx rate mask validation cfg80211: add support to configure HE MCS for beacon rate Rohan Dutta (1): cfg80211: Add support to configure SAE PWE value to drivers Vamsi Krishna (1): cfg80211: Add support to calculate and report 4096-QAM HE rates drivers/net/wireless/Kconfig | 13 drivers/net/wireless/ath/ath9k/ath9k.h| 1 - drivers/net/wireless/ath/ath9k/debug.c| 4 +- drivers/net/wireless/ath/ath9k/init.c | 19 - drivers/net/wireless/ath/ath9k/main.c | 5 -- drivers/net/wireless/ath/carl9170/mac.c | 4 -- drivers/net/wireless/ath/carl9170/main.c | 1 - drivers/net/wireless/broadcom/b43/main.c | 6 +- drivers/net/wireless/broadcom/b43legacy/main.c| 6 +- drivers/net/wireless/ralink/rt2x00/rt2x00config.c | 1 - drivers/net/wireless/ralink/rt2x00/rt2x00dev.c| 6 +- drivers/net/wireless/ralink/rt2x00/rt2x00mac.c| 3 +- include/linux/ieee80211.h | 3 + include/net/cfg80211.h| 21 -- include/net/ieee80211_radiotap.h | 1 + include/net/mac80211.h| 7 +- include/uapi/linux/nl80211.h | 38 +- net/mac80211/cfg.c| 11 --- net/mac80211/chan.c | 3 +- net/mac80211/debugfs_netdev.c | 11 --- net/mac80211/debugfs_sta.c| 2 +- net/mac80211/ieee80211_i.h| 6 -- net/mac80211/iface.c | 52 ++ net/mac80211/main.c | 8 --- net/mac80211/mesh.c | 30 net/mac80211/pm.c | 15 net/mac80211/rx.c | 5 -- net/mac80211/tx.c | 39 +++ net/mac80211/util.c | 2 +- net/mac80211/wme.c| 18 +++-- net/wireless/chan.c | 6 +- net/wireless/core.c | 8 +-- net/wireless/nl80211.c| 85 --- net/wireless/rdev-ops.h | 10 --- net/wireless/scan.c | 2 +- net/wireless/trace.h | 5 -- net/wireless/util.c | 37 +- net/wireless/wext-compat.c| 51 -- 38 files changed, 198 insertions(+), 347 deletions(-)
pull-request: mac80211 2020-11-13
Hi Jakub, We have a few fixes, most importantly probably the fix for the sleeping-in-atomic syzbot reported half a dozen (or so) times. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 52755b66ddcef2e897778fac5656df18817b59ab: cosa: Add missing kfree in error path of cosa_write (2020-11-11 17:52:01 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2020-11-13 for you to fetch changes up to 7bc40aedf24d31d8bea80e1161e996ef4299fb10: mac80211: free sta in sta_info_insert_finish() on errors (2020-11-13 09:48:32 +0100) A handful of fixes: * a use-after-free fix in rfkill * a memory leak fix in the mac80211 TX status path * some rate scaling fixes * a fix for the often-reported (by syzbot) sleeping in atomic issue with mac80211's station removal Claire Chang (1): rfkill: Fix use-after-free in rfkill_resume() Felix Fietkau (3): mac80211: fix memory leak on filtered powersave frames mac80211: minstrel: remove deferred sampling code mac80211: minstrel: fix tx status processing corner case Johannes Berg (1): mac80211: free sta in sta_info_insert_finish() on errors net/mac80211/rc80211_minstrel.c | 27 +-- net/mac80211/rc80211_minstrel.h | 1 - net/mac80211/sta_info.c | 14 -- net/mac80211/status.c | 18 -- net/rfkill/core.c | 3 +++ 5 files changed, 20 insertions(+), 43 deletions(-)
Re: [PATCH] rfkill: Fix use-after-free in rfkill_resume()
On Wed, 2020-11-11 at 11:23 +0800, Claire Chang wrote: > On Wed, Nov 11, 2020 at 1:35 AM Johannes Berg > wrote: > > On Tue, 2020-11-10 at 16:49 +0800, Claire Chang wrote: > > > If a device is getting removed or reprobed during resume, use-after-free > > > might happen. For example, h5_btrtl_resume()[drivers/bluetooth/hci_h5.c] > > > schedules a work queue for device reprobing. During the reprobing, if > > > rfkill_set_block() in rfkill_resume() is called after the corresponding > > > *_unregister() and kfree() are called, there will be an use-after-free > > > in hci_rfkill_set_block()[net/bluetooth/hci_core.c]. > > > > Not sure I understand. So you're saying > > > > * something (h5_btrtl_resume) schedules a worker > > * said worker run, when it runs, calls rfkill_unregister() > > * somehow rfkill_resume() still gets called after this > > > > But that can't really be right, device_del() removes it from the PM > > lists? > > If device_del() is called right before the device_lock() in > device_resume()[1], > it's possible the rfkill device is unregistered, but rfkill_resume is > still called. OK, I see, thanks for the clarification! I'll try to add that to the commit message. Thanks, johannes
Re: [PATCH] rfkill: Fix use-after-free in rfkill_resume()
On Tue, 2020-11-10 at 16:49 +0800, Claire Chang wrote: > If a device is getting removed or reprobed during resume, use-after-free > might happen. For example, h5_btrtl_resume()[drivers/bluetooth/hci_h5.c] > schedules a work queue for device reprobing. During the reprobing, if > rfkill_set_block() in rfkill_resume() is called after the corresponding > *_unregister() and kfree() are called, there will be an use-after-free > in hci_rfkill_set_block()[net/bluetooth/hci_core.c]. Not sure I understand. So you're saying * something (h5_btrtl_resume) schedules a worker * said worker run, when it runs, calls rfkill_unregister() * somehow rfkill_resume() still gets called after this But that can't really be right, device_del() removes it from the PM lists? johannes
Re: [PATCH net v2 00/21] net: avoid to remove module when its debugfs is being used
On Sat, 2020-11-07 at 11:05 -0800, Jakub Kicinski wrote: > On Sat, 7 Nov 2020 17:21:31 + Taehee Yoo wrote: > > When debugfs file is opened, its module should not be removed until > > it's closed. > > Because debugfs internally uses the module's data. > > So, it could access freed memory. > > > > In order to avoid panic, it just sets .owner to THIS_MODULE. > > So that all modules will be held when its debugfs file is opened. > > Hm, looks like some of the patches need to be revised because > .owner is already set in the ops, and a warning gets generated. > > Also it'd be good to mention why Johannes's approach was abandoned. Well, I had two. One was awful, and worked in all cases. The other was less awful, and didn't work in all cases. I think both gave Al Viro hives ;-) > Patch 1 needs to be split in two. Patches 2 and 3 would go via Johannes. FWIW, I'm happy for you to take patches 2 and 3 as well, but I guess if patch 1 needs to be split there's a resend coming anyway, so then I'll be happy to take the patches 2/3 from a separate set. johannes
Re: [PATCH] mac80211: fix regression where EAPOL frames were sent in plaintext
On Sun, 2020-11-08 at 20:34 +0100, Thomas Deutschmann wrote: > > > Can we please get this applied to linux-5.10 and linux-5.9? It's tagged for that, so once it enters mainline will get picked up. Should be soon now, I assume. johannes
Re: [PATCH net-next 08/11] ath9k: work around false-positive gcc warning
On Mon, 2020-11-02 at 18:26 +0200, Kalle Valo wrote: > Arnd Bergmann writes: > > > From: Arnd Bergmann > > > > gcc-10 shows a false-positive warning with CONFIG_KASAN: > > > > drivers/net/wireless/ath/ath9k/dynack.c: In function > > 'ath_dynack_sample_tx_ts': > > include/linux/etherdevice.h:290:14: warning: writing 4 bytes into a region > > of size 0 [-Wstringop-overflow=] > > 290 | *(u32 *)dst = *(const u32 *)src; > > | ^~~ > > > > Until gcc is fixed, work around this by using memcpy() in place > > of ether_addr_copy(). Hopefully gcc-11 will not have this problem. > > > > Link: https://godbolt.org/z/sab1MK > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97490 > > Signed-off-by: Arnd Bergmann > > --- > > drivers/net/wireless/ath/ath9k/dynack.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/net/wireless/ath/ath9k/dynack.c > > b/drivers/net/wireless/ath/ath9k/dynack.c > > index fbeb4a739d32..e4eb96b26ca4 100644 > > --- a/drivers/net/wireless/ath/ath9k/dynack.c > > +++ b/drivers/net/wireless/ath/ath9k/dynack.c > > @@ -247,8 +247,14 @@ void ath_dynack_sample_tx_ts(struct ath_hw *ah, struct > > sk_buff *skb, > > ridx = ts->ts_rateindex; > > > > da->st_rbf.ts[da->st_rbf.t_rb].tstamp = ts->ts_tstamp; > > +#if defined(CONFIG_KASAN) && (CONFIG_GCC_VERSION >= 10) && > > (CONFIG_GCC_VERSION < 11) > > + /* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97490 */ > > + memcpy(da->st_rbf.addr[da->st_rbf.t_rb].h_dest, hdr->addr1, ETH_ALEN); > > + memcpy(da->st_rbf.addr[da->st_rbf.t_rb].h_src, hdr->addr2, ETH_ALEN); > > +#else > > ether_addr_copy(da->st_rbf.addr[da->st_rbf.t_rb].h_dest, hdr->addr1); > > ether_addr_copy(da->st_rbf.addr[da->st_rbf.t_rb].h_src, hdr->addr2); > > +#endif > > Isn't there a better way to handle this? I really would not want > checking for GCC versions become a common approach in drivers. > > I even think that using memcpy() always is better than the ugly ifdef. If you put memcpy() always somebody will surely go and clean it up to use ether_addr_copy() soon ... That said, if there's a gcc issue with ether_addr_copy() then how come it's specific to this place? johannes
Re: pull-request: mac80211 2020-10-30
On Fri, 2020-10-30 at 13:52 -0700, Jakub Kicinski wrote: > On Fri, 30 Oct 2020 10:43:48 +0100 Johannes Berg wrote: > > Hi Jakub, > > > > Here's a first set of fixes, in particular the nl80211 eapol one > > has people waiting for it. > > > > Please pull and let me know if there's any problem. > > Two patches seem to have your signature twice, do you want to respin? > It's not a big deal. That often happens when I pick up my own patches from the list ... let's leave it. johannes
pull-request: mac80211 2020-10-30
Hi Jakub, Here's a first set of fixes, in particular the nl80211 eapol one has people waiting for it. Please pull and let me know if there's any problem. Thanks, johannes The following changes since commit 07e0887302450a62f51dba72df6afb5fabb23d1c: Merge tag 'fallthrough-fixes-clang-5.10-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux (2020-10-29 13:02:52 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-net-2020-10-30 for you to fetch changes up to c2f46814521113f6699a74e0a0424cbc5b305479: mac80211: don't require VHT elements for HE on 2.4 GHz (2020-10-30 10:22:42 +0100) A couple of fixes, for * HE on 2.4 GHz * a few issues syzbot found, but we have many more reports :-( * a regression in nl80211-transported EAPOL frames which had affected a number of users, from Mathy * kernel-doc markings in mac80211, from Mauro * a format argument in reg.c, from Ye Bin ---- Johannes Berg (4): mac80211: fix use of skb payload instead of header cfg80211: initialize wdev data earlier mac80211: always wind down STA state mac80211: don't require VHT elements for HE on 2.4 GHz Mathy Vanhoef (1): mac80211: fix regression where EAPOL frames were sent in plaintext Mauro Carvalho Chehab (1): mac80211: fix kernel-doc markups Ye Bin (1): cfg80211: regulatory: Fix inconsistent format argument include/net/cfg80211.h | 9 include/net/mac80211.h | 7 +++--- net/mac80211/mlme.c | 3 ++- net/mac80211/sta_info.c | 18 net/mac80211/sta_info.h | 9 +++- net/mac80211/tx.c | 44 -- net/wireless/core.c | 57 +++-- net/wireless/core.h | 5 +++-- net/wireless/nl80211.c | 3 ++- net/wireless/reg.c | 2 +- 10 files changed, 102 insertions(+), 55 deletions(-)
Re: [PATCH][next] nl80211/cfg80211: fix potential infinite loop
On Thu, 2020-10-29 at 22:24 +, Colin King wrote: > From: Colin Ian King > > The for-loop iterates with a u8 loop counter and compares this > with the loop upper limit of request->n_ssids which is an int type. > There is a potential infinite loop if n_ssids is larger than the > u8 loop counter, so fix this by making the loop counter an int. Makes sense, thanks. I'll apply it to next. For the record, it shouldn't be possible for request->n_ssids to be larger than what the driver limit was, and that's 20 by default and doesn't make sense to be really much higher than that, so in practice this won't happen. johannes
Re: [PATCH v5 3/3] mac80211: add KCOV remote annotations to incoming frame processing
On Thu, 2020-10-29 at 17:36 +, Aleksandr Nogikh wrote: > From: Aleksandr Nogikh > > Add KCOV remote annotations to ieee80211_iface_work() and > ieee80211_rx_list(). This will enable coverage-guided fuzzing of > mac80211 code that processes incoming 802.11 frames. I have no idea how we'll get this merged - Jakub, do you want to take the whole series? Or is somebody else responsible for the core kcov part? In any case, Reviewed-by: Johannes Berg johannes
Re: [PATCH, net -> staging, v2] wimax: move out to staging
On Thu, 2020-10-29 at 14:43 +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> There are no known users of this driver as of October 2020, and it will
> be removed unless someone turns out to still need it in future releases.
>
> According to https://en.wikipedia.org/wiki/List_of_WiMAX_networks, there
> have been many public wimax networks, but it appears that many of these
> have migrated to LTE or discontinued their service altogether.
> As most PCs and phones lack WiMAX hardware support, the remaining
> networks tend to use standalone routers. These almost certainly
> run Linux, but not a modern kernel or the mainline wimax driver stack.
>
> NetworkManager appears to have dropped userspace support in 2015
> https://bugzilla.gnome.org/show_bug.cgi?id=747846, the
> www.linuxwimax.org
> site had already shut down earlier.
>
> WiMax is apparently still being deployed on airport campus networks
> ("AeroMACS"), but in a frequency band that was not supported by the old
> Intel 2400m (used in Sandy Bridge laptops and earlier), which is the
> only driver using the kernel's wimax stack.
>
> Move all files into drivers/staging/wimax, including the uapi header
> files and documentation, to make it easier to remove it when it gets
> to that. Only minimal changes are made to the source files, in order
> to make it possible to port patches across the move.
>
> Also remove the MAINTAINERS entry that refers to a broken mailing
> list and website.
>
> Suggested-by: Inaky Perez-Gonzalez
> Signed-off-by: Arnd Bergmann
> ---
> changes in v2:
> - fix a build regression
> - add more information about remaining networks (Dan Carpenter)_
>
> For v1, Greg said he'd appply the patch when he gets an Ack
> from the maintainers.
>
> Inaky, Johannes, Jakub: are you happy with this version?
Sure, looks fine to me.
Acked-by: Johannes Berg
Not that I have much relation to this code other than having fixed up
genetlink stuff over the years :)
johannes
Re: [PATCH v4 3/3] mac80211: add KCOV remote annotations to incoming frame processing
On Wed, 2020-10-28 at 18:20 +, Aleksandr Nogikh wrote:
> From: Aleksandr Nogikh
>
> Add KCOV remote annotations to ieee80211_iface_work and
> ieee80211_rx. This will enable coverage-guided fuzzing of
> mac80211 code that processes incoming 802.11 frames.
>
> Signed-off-by: Aleksandr Nogikh
> ---
> v1 -> v2:
> * The commit now affects ieee80211_rx instead of
> ieee80211_tasklet_handler.
> ---
> include/net/mac80211.h | 2 ++
> net/mac80211/iface.c | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index e8e295dae744..f4c37a1b381e 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -4499,7 +4499,9 @@ void ieee80211_rx_napi(struct ieee80211_hw *hw, struct
> ieee80211_sta *sta,
> */
> static inline void ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb)
> {
> + kcov_remote_start_common(skb_get_kcov_handle(skb));
> ieee80211_rx_napi(hw, NULL, skb, NULL);
> + kcov_remote_stop();
> }
Wouldn't it make more sense to push that a layer down
into ieee80211_rx_napi(), or actually now perhaps even
better ieee80211_rx_list(), so we get it even if the driver called that
API in the first place?
You might only care about hwsim at this point, but perhaps hwsim would
get optimised ..
johannes
Re: [PATCH v3 21/56] mac80211: fix kernel-doc markups
On Fri, 2020-10-23 at 18:33 +0200, Mauro Carvalho Chehab wrote: > Some identifiers have different names between their prototypes > and the kernel-doc markup. > > Others need to be fixed, as kernel-doc markups should use this format: > identifier - description > > In the specific case of __sta_info_flush(), add a documentation > for sta_info_flush(), as this one is the one used outside > sta_info.c. Are you taking the entire series through some tree, or should I pick up this patch? If you're going to take it: Reviewed-by: Johannes Berg johannes
Re: [PATCH net-next 04/11] wimax: fix duplicate initializer warning
On Mon, 2020-10-26 at 22:29 +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > gcc -Wextra points out multiple fields that use the same index '1' > in the wimax_gnl_policy definition: > > net/wimax/stack.c:393:29: warning: initialized field overwritten > [-Woverride-init] > net/wimax/stack.c:397:28: warning: initialized field overwritten > [-Woverride-init] > net/wimax/stack.c:398:26: warning: initialized field overwritten > [-Woverride-init] > > This seems to work since all four use the same NLA_U32 value, but it > still appears to be wrong. In addition, there is no intializer for > WIMAX_GNL_MSG_PIPE_NAME, which uses the same index '2' as > WIMAX_GNL_RFKILL_STATE. That's funny. This means that WIMAX_GNL_MSG_PIPE_NAME was never used, since it is meant to be a string, and that won't (usually) fit into 4 bytes... I suppose that's all an artifact of wimax being completely and utterly dead anyway. We should probably just remove it. > Johannes already changed this twice to improve it, but I don't think > there is a good solution, so try to work around it by using a > numeric index and adding comments. Yeah, though maybe there's a better solution now. Given that we (again and properly) have per-ops policy support, which really was the thing that broke it here (the commit 3b0f31f2b8c9 you mentioned), we could split this up into per-ops policies and do the right thing in the separate policies. OTOH, that really just makes it use more space, for no discernible effect to userspace. So as far as the warning fix is concerned: Acked-by: Johannes Berg Looks like I introduced a bug there with WIMAX_GNL_MSG_PIPE_NAME, but obviously nobody cared. johannes
[PATCH] libnetlink: define __aligned conditionally
On some systems (e.g. current Debian/stable) the inclusion
of utils.h pulled in some other things that may end up
defining __aligned, in a possibly different way than what
we had here.
Use our own definition only if there isn't one already.
Fixes: d5acae244f9d ("libnetlink: add nl_print_policy() helper")
Signed-off-by: Johannes Berg
---
lib/libnetlink.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index a7b60d873afb..c958aa57d0cd 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -30,7 +30,9 @@
#include "libnetlink.h"
#include "utils.h"
+#ifndef __aligned
#define __aligned(x) __attribute__((aligned(x)))
+#endif
#ifndef SOL_NETLINK
#define SOL_NETLINK 270
--
2.26.2
Re: [PATCH net-next v2 00/12] net: add and use function dev_fetch_sw_netstats for fetching pcpu_sw_netstats
On Wed, 2020-10-14 at 09:59 +0200, Heiner Kallweit wrote: > > > Do you have a link? What is the benefit and how can we use it? > > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1873080.html There was also a long discussion a year or so back, starting at http://lore.kernel.org/r/7b73e1b7-cc34-982d-2a9c-acf62b88d...@linuxfoundation.org johannes
Re: [PATCH v6 68/80] nl80211: docs: add a description for s1g_cap parameter
On Tue, 2020-10-13 at 22:41 +0200, Mauro Carvalho Chehab wrote:
> Em Tue, 13 Oct 2020 20:47:47 +0200
> Johannes Berg escreveu:
>
> > Thanks Mauro.
> >
> >
> > On Tue, 2020-10-13 at 13:54 +0200, Mauro Carvalho Chehab wrote:
> > > Changeset df78a0c0b67d ("nl80211: S1G band and channel definitions")
> > > added a new parameter, but didn't add the corresponding kernel-doc
> > > markup, as repoted when doing "make htmldocs":
> > >
> > > ./include/net/cfg80211.h:471: warning: Function parameter or member
> > > 's1g_cap' not described in 'ieee80211_supported_band'
> > >
> > > Add a documentation for it.
> >
> > Should I take this through my tree, or is that part of a larger set
> > that'll go somewhere else?
>
> Whatever works best for you ;-)
>
> If you don't pick it via your tree, I'm planning to send it
> together with the other patches likely on Thursday.
OK, please do, and here's an
Acked-by: Johannes Berg
I don't think I can get it this quickly through net-next at this point,
and there's just no point if you have stuff to send anyway :-)
Thanks!
johannes
Re: [PATCH v6 68/80] nl80211: docs: add a description for s1g_cap parameter
Thanks Mauro.
On Tue, 2020-10-13 at 13:54 +0200, Mauro Carvalho Chehab wrote:
> Changeset df78a0c0b67d ("nl80211: S1G band and channel definitions")
> added a new parameter, but didn't add the corresponding kernel-doc
> markup, as repoted when doing "make htmldocs":
>
> ./include/net/cfg80211.h:471: warning: Function parameter or member
> 's1g_cap' not described in 'ieee80211_supported_band'
>
> Add a documentation for it.
Should I take this through my tree, or is that part of a larger set
that'll go somewhere else?
johannes
Re: [PATCH net-next 1/6] ethtool: Extend link modes settings uAPI with lanes
Hi,
Sorry, somehow didn't see this until now.
> > +/* Lanes, 1, 2, 4 or 8. */
> > +#define ETHTOOL_LANES_11
> > +#define ETHTOOL_LANES_22
> > +#define ETHTOOL_LANES_44
> > +#define ETHTOOL_LANES_88
>
> Not an extremely useful set of defines, not sure Michal would agree.
>
> > +#define ETHTOOL_LANES_UNKNOWN 0
> > struct link_mode_info {
> > int speed;
> > + int lanes;
>
> why signed?
>
> > u8 duplex;
> > };
> > @@ -274,16 +277,17 @@ const struct nla_policy ethnl_linkmodes_set_policy[]
> > = {
> > [ETHTOOL_A_LINKMODES_SPEED] = { .type = NLA_U32 },
> > [ETHTOOL_A_LINKMODES_DUPLEX]= { .type = NLA_U8 },
> > [ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG] = { .type = NLA_U8 },
> > + [ETHTOOL_A_LINKMODES_LANES] = { .type = NLA_U32 },
>
> NLA_POLICY_VALIDATE_FN(), not sure why the types for this
> validation_type are limited.. Johannes, just an abundance of caution?
So let me see if I got this right - you're saying you'd like to use
NLA_POLICY_VALIDATE_FN() for an NLA_U32, to validate against the _LANES
being 1, 2, 4 or 8?
First of all, you _can_, no? I mean, it's limited by
#define NLA_ENSURE_NO_VALIDATION_PTR(tp)\
(__NLA_ENSURE(tp != NLA_BITFIELD32 && \
tp != NLA_REJECT && \
tp != NLA_NESTED && \
tp != NLA_NESTED_ARRAY) + tp)
and the reason is sort of encoded in that - the types listed here
already use the pointer *regardless of the validation_type*, so you
can't have a pointer to the function in the same union.
But not sure I understood :-)
johannes
