* Re: [PATCH] mwifiex: Use put_unaligned_le32
From: Himanshu Jha @ 2017-10-05 15:22 UTC (permalink / raw)
To: Kalle Valo
Cc: amitkarwar, nishants, gbhat, huxm, linux-wireless, netdev,
linux-kernel
In-Reply-To: <87zi963s1p.fsf@codeaurora.org>
On Thu, Oct 05, 2017 at 11:41:38AM +0300, Kalle Valo wrote:
> Himanshu Jha <himanshujha199640@gmail.com> writes:
>
> >> > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> >> > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> >> > @@ -17,6 +17,7 @@
> >> > * this warranty disclaimer.
> >> > */
> >> >
> >> > +#include <linux/unaligned/access_ok.h>
> >>
> >> I don't think this is correct. Should it be asm/unaligned.h?
> >
> > Would mind explainig me as to why it is incorrect! Also, it defined in
> > both the header files but, why is asm/unaligned.h preferred ?
>
> asm/unaligned.h seems to be the toplevel header file which includes
> header files based on arch configuration. Also grepping the sources
> support that, nobody from drivers/ include access_ok.h directly.
>
Yes, you are correct!
I will send v2 patch with asm/unaligned.h
> But I can't say that I fully understand how the header files work so
> please do correct me if I have mistaken.
>
It is confusing to me as well.
There are various instances where a function used in file say for eg
int func_align (void* a)
is used and it is defined in align.h
But many files don't *directly* include align.h and rather include
any other header which includes align.h
Is compiling the file the only way to check if apppropriate header is
included or is there some other way to check for it.
Thanks
^ permalink raw reply
* Re: converting mac80211 to TXQs entirely
From: Johannes Berg @ 2017-10-05 15:39 UTC (permalink / raw)
To: linux-wireless; +Cc: Toke Høiland-Jørgensen, nbd
In-Reply-To: <1507205618.2387.19.camel@sipsolutions.net>
Part 2 - where can we go from here
Of course - as mentioned in the subject - my goal is to just convert
over to TXQs entirely in mac80211. That would get rid of a LOT of
special case code, like queueing for aggregation setup, powersave
buffering (both unicast and multicast), etc.
I think the following would be appropriate steps to take
1) convert multicast PS buffering
This is a bit strange today - when multicast PS buffering *isn't*
needed, frames go to TXQ drivers via the vif->txq, but when it *is*
done, then frames are tagged with IEEE80211_TX_CTL_SEND_AFTER_DTIM and
are buffered on ps->bc_buf (if HOST_BROADCAST_PS_BUFFERING is set) and
then handed to the driver through the legacy ->tx() path.
This should be reasonably simple to change - get rid of ps->bc_buf, and
keep the frames on the TXQ, making ieee80211_get_buffered_bc() retrieve
them from there instead. We'd have to tell the driver (it needs to know
in the wake callback) that it has sleeping clients and needs to buffer,
so it can decide whether or not to retrieve immediately (basically, for
TXQ drivers, implementing HOST_BROADCAST_PS_BUFFERING in driver logic:
retrieve immediately if it wants to buffer itself, or keep them there
for a later ieee80211_get_buffered_bc() call if it doesn't).
2) use TXQ for offchannel frames
I'm a bit unsure about this - we never really want to queue offchannel
packets, and if they don't go out immediately then we can basically
drop them. Nevertheless, having everything deal with the TXQ API will
be simpler, and so I think this also should. Perhaps for this we should
have a TXQ but only ever use txqi->frags, so that we never really have
to deal with the FQ stuff for these frames. Then the wake would
basically just pull down the packets and send them immediately.
3) handle monitor mode
I thought this was more complicated, but I think the easiest way to
solve this is to actually just use the local->monitor_sdata->vif.txq.
This requires that mac80211 be changed to always allocate local-
>monitor_sdata, even if WANT_MONITOR_VIF isn't set, and ath9k/ath10k do
something special for this TXQ (and perhaps ath9k should set
WANT_MONITOR_VIF), but that seems reasonable.
4) handle non-data frames for stations
This is probably the trickiest part. I have a patch to add a non-data
TID to the STA TXQs, and that's perhaps the right thing to do - though
I guess those frames should again always go onto txqi->frags so they
don't compete with data frames for resources.
For powersave reasons we'll discuss later, this should probably only
apply to bufferable MMPDUs, with others going to the vif->nondata_txq
(next item).
5) handle non-data frames for vifs
Similarly, we need a vif->nondata_txq where we can put probe responses
and the (few) other frames that we send before we have a station entry.
According to my earlier analysis (previous email) after these steps we
have a TXQ for all frames. All of these steps will require certain
adjustments in the drivers currently using TXQs (ath9k & ath10k) since
they'll be relying on some amount of buffering and queue stop/wake in
mac80211 for these frames. We might just have to add some API to ask
"is queue stopped" to make the transition here easy.
After this, we can start thinking about doing internal cleanups in
mac80211.
6) First thing to do is probably to introduce a compat layer, so that
all drivers go through the TXQs, regardless of whether they handle
that. I have the beginnings of a patch that does that, it basically
requires drivers to set the wake_tx_queue method to a new mac80211
function ieee80211_wake_tx_queue() [so the ops can remain const] when
they don't actually want to deal with TXQs themselves.
This method can then check the queue stop reasons etc. and only service
TXQs that don't have their corresponding queue stopped. We'd also hook
into when the queues get re-enabled, and call the servicing function
from there for such drivers.
My current prototype just calls the existing __ieee80211_tx() but I no
longer think that's a good idea - the idea is that this would
eventually allow us to get rid of tx_pending.
So it'd be better to have ieee80211_wake_tx_queue() just check all the
required things, and once a frame is pulled from a TXQ it's committed
to be given to the driver.
For off-channel, a special case will be needed - dropping the frame
when there's no way to transmit it at the time.
7) Remove all the now-dead code
A lot of code is now dead - we'll never have multiple netdev queues,
all IFF_NOQUEUE etc - remove all the code associated with that
8) convert more infrastructure to TXQs
It gets more vague (starting from 6), but eventually I want to
* get rid of tx_pending (why do we even use both TXQ and tx_pending
for aggregation?)
* do all powersave buffering on TXQs
[this may be tricky for filtered frames, perhaps disallow filtered
for TXQ drivers like ath9k/ath10k, and have a separate per-TXQI list
in the private txq data for ieee80211_wake_tx_queue]
Thoughts?
johannes
^ permalink raw reply
* Re: [PATCH] rsi: fix integer overflow warning
From: Joe Perches @ 2017-10-05 16:11 UTC (permalink / raw)
To: David Laight, Arnd Bergmann, Kalle Valo, Prameela Rani Garnepudi,
Amitkumar Karwar
Cc: Pavani Muthyala, Karun Eagalapati, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD008ACEE@AcuExch.aculab.com>
On Thu, 2017-10-05 at 15:12 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 05 October 2017 13:19
> > On Thu, 2017-10-05 at 14:05 +0200, Arnd Bergmann wrote:
> > > gcc produces a harmless warning about a recently introduced
> > > signed integer overflow:
> > >
> > > drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
> > > include/uapi/linux/swab.h:13:15: error: integer overflow in expression [-Werror=overflow]
> > > (((__u16)(x) & (__u16)0x00ffU) << 8) | \
> > > ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> > > include/uapi/linux/swab.h:104:2: note: in expansion of macro '___constant_swab16'
> > > ___constant_swab16(x) : \
> > > ^~~~~~~~~~~~~~~~~~
> > > include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro '__swab16'
> > > #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
> >
> > []
> >
> > > The problem is that the 'mask' value is a signed integer that gets
> > > turned into a negative number when truncated to 16 bits. Making it
> > > an unsigned constant avoids this.
> >
> > I would expect there are more of these.
> >
> > Perhaps this define in include/uapi/linux/swab.h:
> >
> > #define __swab16(x) \
> > (__builtin_constant_p((__u16)(x)) ? \
> > ___constant_swab16(x) : \
> > __fswab16(x))
> >
> > should be
> >
> > #define __swab16(x) \
> > (__builtin_constant_p((__u16)(x)) ? \
> > ___constant_swab16((__u16)(x)) : \
> > __fswab16((__u16)(x)))
>
> You probably don't want the cast in the call to __fswab16() since
> that is likely to generate an explicit and with 0xffff.
> You will likely also get one if the argument is _u16 (not unsigned int).
It would just an explicit vs implicit cast as __fswab16 is
a static inline with a __u16 argument
^ permalink raw reply
* Re: converting mac80211 to TXQs entirely
From: Toke Høiland-Jørgensen @ 2017-10-05 16:28 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: nbd
In-Reply-To: <1507217947.2387.60.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> Part 2 - where can we go from here
>
>
> Of course - as mentioned in the subject - my goal is to just convert
> over to TXQs entirely in mac80211. That would get rid of a LOT of
> special case code, like queueing for aggregation setup, powersave
> buffering (both unicast and multicast), etc.
At first glance this looks like a decent way forward. I'll think about
it some more and comment again later, but for now I just wanted to add
this:
I'm been thinking about how to move the airtime fairness scheduler out
of ath9k and into mac80211 (so more drivers can take advantage of it).
This will require some changes to the TXQ API that drivers speak to, so
wanted to add my thoughts here to make sure it's compatible with your
thinking.
I think the easiest way to have mac80211 handle airtime fairness is to
add a way for ieee80211_tx_dequeue() to return some sort of DEFER signal
which semantically means "there is no packet for this queue right now,
but please keep scheduling it" (which would be the result of a TXQ
belonging to a station that has used its airtime quota but still has
packets pending). This is different from the current meaning of NULL,
which will make the driver stop scheduling that TXQ until it gets a new
wake signal.
The alternative would be to change the API so the driver asks mac80211
which TXQ it should pull from next, instead of doing its own scheduling
as it does now. But I'm not sure that's doable with a sane API.
Now, I believe this is related to this point of yours:
> 5) handle non-data frames for vifs
>
> Similarly, we need a vif->nondata_txq where we can put probe responses
> and the (few) other frames that we send before we have a station entry.
>
> According to my earlier analysis (previous email) after these steps we
> have a TXQ for all frames. All of these steps will require certain
> adjustments in the drivers currently using TXQs (ath9k & ath10k) since
> they'll be relying on some amount of buffering and queue stop/wake in
> mac80211 for these frames. We might just have to add some API to ask
> "is queue stopped" to make the transition here easy.
And I'm wondering if this "is queue stopped" API could be the same one
used for the airtime DEFER case?
Oh, and BTW: I see this is on the list of topics for the wireless summit
in Seoul. How do I go about signing up for that? I'll be at netdev
talking about some of this stuff anyway[1] :)
-Toke
[1] https://www.netdevconf.org/2.2/session.html?jorgensen-wifistack-talk
^ permalink raw reply
* Re: converting mac80211 to TXQs entirely
From: Johannes Berg @ 2017-10-05 16:43 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, linux-wireless; +Cc: nbd
In-Reply-To: <87efqhsgni.fsf@toke.dk>
On Thu, 2017-10-05 at 18:28 +0200, Toke Høiland-Jørgensen wrote:
> I'm been thinking about how to move the airtime fairness scheduler
> out of ath9k and into mac80211 (so more drivers can take advantage of
> it). This will require some changes to the TXQ API that drivers speak
> to, so wanted to add my thoughts here to make sure it's compatible
> with your thinking.
>
> I think the easiest way to have mac80211 handle airtime fairness is
> to add a way for ieee80211_tx_dequeue() to return some sort of DEFER
> signal which semantically means "there is no packet for this queue
> right now, but please keep scheduling it" (which would be the result
> of a TXQ belonging to a station that has used its airtime quota but
> still has packets pending). This is different from the current
> meaning of NULL, which will make the driver stop scheduling that TXQ
> until it gets a new wake signal.
I think that's reasonable. I'm not really sure it's *necessary* though?
Couldn't mac80211 return NULL, and then simply call wake_tx_queue again
when the TXQ becomes eligible for scheduling again? Otherwise the
driver might end up doing a lot of polling for it to become eligible
again?
I've mostly glossed over a mac80211 scheduler, which is obviously
needed as part of a complete conversion, and it'd just have to
integrate with this new return value.
> The alternative would be to change the API so the driver asks
> mac80211 which TXQ it should pull from next, instead of doing its own
> scheduling as it does now. But I'm not sure that's doable with a sane
> API.
>
> Now, I believe this is related to this point of yours:
>
> > 5) handle non-data frames for vifs
> >
> > Similarly, we need a vif->nondata_txq where we can put probe
> > responses and the (few) other frames that we send before we have a
> > station entry.
> >
> > According to my earlier analysis (previous email) after these steps
> > we have a TXQ for all frames. All of these steps will require
> > certain adjustments in the drivers currently using TXQs (ath9k &
> > ath10k) since they'll be relying on some amount of buffering and
> > queue stop/wake in mac80211 for these frames. We might just have to
> > add some API to ask "is queue stopped" to make the transition here
> > easy.
>
> And I'm wondering if this "is queue stopped" API could be the same
> one used for the airtime DEFER case?
I think these are two completely different cases.
The "is queue stopped" I was thinking of would be basically checking
the variable local->queue_stop_reasons, so that the driver could still
use the stop_queue API(s). Yeah, this would be very roundabout, but as
a conversion step I think that'd not be a bad thing.
A more complete conversion likely wouldn't need this, but would instead
have the driver record its own stop reasons and just stop scheduling
those TXQs that belong to a stopped "class" (it's no longer really a
queue).
(and for mac80211 stop reasons, it would just return NULL and re-
schedule the TXQ when it becomes eligible again)
> Oh, and BTW: I see this is on the list of topics for the wireless
> summit in Seoul. How do I go about signing up for that? I'll be at
> netdev talking about some of this stuff anyway[1] :)
Just show up there, or you can add yourself to the list on the wiki
page :)
johannes
^ permalink raw reply
* Re: [PATCH 00/11] SDIO support for ath10k
From: Alagu Sankar @ 2017-10-05 17:24 UTC (permalink / raw)
To: Gary Bisson, silexcommon; +Cc: ath10k, linux-wireless, erik.stromdahl
In-Reply-To: <20171005151205.cymirl4wikuw4f7q@t450s.lan>
Hi Gary,
On 05-10-2017 20:42, Gary Bisson wrote:
> Hi Alagu,
>
> First of all, thank you for sharing your patches, this will be a very
> nice improvement to have SDIO QCA9377 working with ath10k.
>
> I've tried your series with Nitrogen7 [1] platform which is supported in
> mainline already. It uses BD-SDMAC [2] which uses the same module as the
> SX-SDMAC.
>
> Below are some questions/remarks I have after the testing.
>
> On Sat, Sep 30, 2017 at 11:07:37PM +0530, silexcommon at gmail.com wrote:
>> From: Alagu Sankar <alagusankar at silex-india.com>
>>
>> This patchset, generated against master-pending branch, enables a fully
>> functional SDIO interface driver for ath10k. Patches have been verified on
>> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
>> and P2P modes.
>>
>> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
> Quick question on the firmware, is it the one from Kalle's repository?[3]
>
> If so, where does this firmware comes from? Is 00061 the firmware
> version? So far I've only seen up to v0.0.0.60, see qcacld-2.0 output:
> Host SW:4.5.20.037, FW:0.0.0.60, HW:QCA9377_REV1_1
Yes, it is from
https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested.
I have also used custom firmware from QCA/Silex as used in qcacld-2.0
driver without any issue. You need to use the firmware merger tool from
https://github.com/erstrom/linux-ath/wiki/Firmware to combine the
qwlan30.bin and otp30.bin to generate the firmware-sdio.bin.
>> with the board data from respective SDIO card vendors.
> About the board-sdio.bin, is it just a copy of your bdwlan30.bin?
Yes board-sdio.bin is a copy of bdwlan30.bin
>> Receive performance
>> matches the QCA reference driver when used with SDIO3.0 enabled platforms.
>> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
> Nice performances. Unfortunately I don't get better than 30Mbits/s in TX
> and 50Mbits/s in RX:
> # iperf -c 192.168.1.1
> ------------------------------------------------------------
> Client connecting to 192.168.1.1, TCP port 5001
> TCP window size: 43.8 KByte (default)
> ------------------------------------------------------------
> [ 3] local 192.168.1.115 port 41354 connected with 192.168.1.1 port
> 5001
> [ ID] Interval Transfer Bandwidth
> [ 3] 0.0-10.0 sec 34.9 MBytes 29.2 Mbits/sec
> # iperf -s
> ------------------------------------------------------------
> Server listening on TCP port 5001
> TCP window size: 85.3 KByte (default)
> ------------------------------------------------------------
> [ 4] local 192.168.1.115 port 5001 connected with 192.168.1.1 port
> 50646
> [ ID] Interval Transfer Bandwidth
> [ 4] 0.0-10.0 sec 63.1 MBytes 52.7 Mbits/sec
>
> Do you have any idea why? Note that qcacld-2.0 driver on that same
> platform (same OS) gives the performances you advertize (150Mbits/s).
For some reason, if I use the imx_v6_v7_defconfig as is, performance is
very poor. In fact, the firmware download itself will take about 6
seconds. This can also be seen when you up/down the wlan0 interface. For
the i.MX6 SoloX board, I used the configuration of 4.1.15 as provided by
the BSP from NXP/Freescale. This improved the performance quite a bit.
I haven't had a chance to spend time on this to figure out the
difference and reason. Another difference is that the default device
tree for SoloX did not have the correct settings to support SDIO3.x.
Had to modify them, but did not include the device tree patches here as
it is not meant for this group.
>> This patchset differs from the previous high latency patches, specific to SDIO.
>> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the
>> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
>> this flag, the management frames are not sent out by the firmware. Possibility
>> of management frames being sent via WMI and data frames through the reduced Tx
>> completion needs to be probed further.
>>
>> Further improvements can be done on the transmit path by implementing packet
>> bundle. Scatter Gather is another area of improvement for both Transmit and
>> Receive, but may not work on all platforms
>>
>> Known issues: Surprise removal of the card, when the device is in connected
>> state, delays sdio function remove due to delayed WMI command failures.
>> Existing ath10k framework can not differentiate between a kernel module
>> removae and the surprise removal of teh card.
> Here are some questions:
> - Is it normal to see a warning about board-2.bin, shouldn't it look for
> board-sdio.bin only?
> [ 14.696704] ath10k_sdio mmc1:0001:1: Direct firmware load for
> ath10k/QCA9377/hw1.0/board-2.bin failed with error -2
This was only noticed in the latest update. Like the different firmware
versions, the driver also looks for the board-2.bin now. I have only
tested with the board-sdio.bin
> - Did you have pre-cal and/or cal binaries for your testing?
> [ 14.067159] ath10k_sdio mmc1:0001:1: Direct firmware load for
> ath10k/pre-cal-sdio-mmc1:0001:1.bin failed with error -2
> [ 14.149257] ath10k_sdio mmc1:0001:1: Direct firmware load for
> ath10k/cal-sdio-mmc1:0001:1.bin failed with error -2
No. I did not use the pre-cal and cal binaries.
> Also noticed that the "tx bitrate" output of 'iw link' is wrong in my
> case:
> # iw wlan0 link
> Connected to 00:00:00:00:00:b0 (on wlan0)
> SSID: TPLINK_AC_5G
> freq: 5180
> RX: 72072365 bytes (67934 packets)
> TX: 79084128 bytes (73649 packets)
> signal: -35 dBm
> tx bitrate: 6.0 MBit/s
>
> bss flags: short-slot-time
> dtim period: 2
> beacon int: 100
>
> When connecting using qcacld driver it shows 433MBit/s as expected. Is
> it working properly in your case?
Tx rate is not updated properly. I will include it in the list of known
issues.
> As a FYI, I've built Erik's tree[4] for this testing, should I use
> another tree instead?
I use the Kalle's ath10k tree, but when I last looked, they were pretty
much the same, so it should not be a problem.
> Let me know your thoughts.
>
> Regards,
> Gary
>
> [1] https://boundarydevices.com/product/nitrogen7/
> [2] https://boundarydevices.com/product/bd_sdmac_wifi/
> [3] https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested
> [4] https://github.com/erstrom/linux-ath
>
Regards,
Alagu
^ permalink raw reply
* Re: [PATCH] net/mac80211/mesh_plink: Convert timers to use
From: Kees Cook @ 2017-10-05 17:27 UTC (permalink / raw)
To: Johannes Berg
Cc: LKML, David S. Miller, linux-wireless, Network Development,
Thomas Gleixner
In-Reply-To: <1507186052.2387.11.camel@sipsolutions.net>
On Wed, Oct 4, 2017 at 11:47 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2017-10-04 at 17:49 -0700, Kees Cook wrote:
>> In preparation for unconditionally passing the struct timer_list
>> pointer to all timer callbacks, switch to using the new timer_setup()
>> and from_timer() to pass the timer pointer explicitly. This requires
>> adding a pointer back to the sta_info since container_of() can't
>> resolve the sta_info.
>
> The subject seems to be lacking something ... :-)
Oh wonderful, all the subjects are cut off. *sigh* I wonder which
piece of my workflow broke that...
>> This requires commit 686fef928bba ("timer: Prepare to change timer
>> callback argument type") in v4.14-rc3, but should be otherwise
>> stand-alone.
>
> I still can't apply that because that's not in net-next right now.
Okay, I'll see if Dave brings that into net-next and resend after that.
>> static inline void mesh_plink_timer_set(struct sta_info *sta, u32
>> timeout)
>> {
>> sta->mesh->plink_timer.expires = jiffies +
>> msecs_to_jiffies(timeout);
>> - sta->mesh->plink_timer.data = (unsigned long) sta;
>> - sta->mesh->plink_timer.function = mesh_plink_timer;
>> + sta->mesh->plink_sta = sta;
>> + sta->mesh->plink_timer.function =
>> (TIMER_FUNC_TYPE)mesh_plink_timer;
>> sta->mesh->plink_timeout = timeout;
>> add_timer(&sta->mesh->plink_timer);
>
> Wouldn't it be better to convert this to timer_setup() now?
The problem is that plink_timer is used in several places, and it's
originally initialized in net/mac80211/sta_info.c. The call to
mesh_plink_timer_set() does an update of the function field, so it
didn't look like it could get merged with the timer_setup(), but in
looking again, it seems that this is the _only_ update to
plink_timer.function, so it could probably get collapsed into the
timer_setup() call.
> That add_timer() should probably also be mod_timer() anyway?
Agreed. I'd avoided making those changes in most places, but I can do it here.
>> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
>> index 69615016d5bf..5e5de9455e4e 100644
>> --- a/net/mac80211/sta_info.c
>> +++ b/net/mac80211/sta_info.c
>> @@ -332,7 +332,7 @@ struct sta_info *sta_info_alloc(struct
>> ieee80211_sub_if_data *sdata,
>> spin_lock_init(&sta->mesh->plink_lock);
>> if (ieee80211_vif_is_mesh(&sdata->vif) &&
>> !sdata->u.mesh.user_mpm)
>> - init_timer(&sta->mesh->plink_timer);
>> + timer_setup(&sta->mesh->plink_timer, NULL,
>> 0);
>> sta->mesh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
>> }
>
> You just have to make mesh_plink_timer() non-static, put a prototype
> into mesh.h and then you can use the proper timer_setup() here with the
> function?
>
> Also, the sta->mesh->plink_sta assignment should be here I'd say, no
> point rewriting it all the time.
Sounds good. I'll get it cleaned up.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [08/11] ath10k_sdio: common read write
From: Alagu Sankar @ 2017-10-05 17:33 UTC (permalink / raw)
To: Gary Bisson; +Cc: silexcommon, linux-wireless, ath10k
In-Reply-To: <20171005100932.vamkzfulq3dg4iav@t450s.lan>
Hi Gary,
On 2017-10-05 15:39, Gary Bisson wrote:
> Hi Alagu,
>
> On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcommon@gmail.com wrote:
>> From: Alagu Sankar <alagusankar@silex-india.com>
>>
>> convert different read write functions in sdio hif to bring it under a
>> single read-write path. This helps in having a common dma bounce
>> buffer
>> implementation. Also helps in address modification that is required
>> specific to change in certain mbox addresses of sdio_write.
>>
>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>> ---
>> drivers/net/wireless/ath/ath10k/sdio.c | 131
>> ++++++++++++++++-----------------
>> 1 file changed, 64 insertions(+), 67 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c
>> b/drivers/net/wireless/ath/ath10k/sdio.c
>> index 77d4fa4..bb6fa67 100644
>> --- a/drivers/net/wireless/ath/ath10k/sdio.c
>> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
>> @@ -36,6 +36,11 @@
>>
>> #define ATH10K_SDIO_DMA_BUF_SIZE (32 * 1024)
>>
>> +static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
>> + u32 len, bool incr);
>> +static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void
>> *buf,
>> + u32 len, bool incr);
>> +
>
> As mentioned by Kalle, u32 needs to be size_t.
Yes, the compiler I used is probably a step older and did not catch
this.
>
>> /* inlined helper functions */
>>
>> /* Macro to check if DMA buffer is WORD-aligned and DMA-able.
>> @@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar)
>> struct sdio_func *func = ar_sdio->func;
>> unsigned char byte, asyncintdelay = 2;
>> int ret;
>> + u32 addr;
>>
>> ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n");
>>
>> @@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar)
>> CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C |
>> CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D);
>>
>> - ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
>> - CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
>> - byte);
>> + addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
>> + ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte);
>
> Not sure this part is needed.
This is to overcome checkpatch warning. Too big a names for the function
and macro
not helping in there. Will have to move it as a separate patch.
>
>> if (ret) {
>> ath10k_warn(ar, "failed to enable driver strength: %d\n", ret);
>> goto out;
>> @@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar)
>>
>> static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
>> {
>> - struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> - struct sdio_func *func = ar_sdio->func;
>> + __le32 *buf;
>> int ret;
>>
>> - sdio_claim_host(func);
>> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>> + if (!buf)
>> + return -ENOMEM;
>>
>> - sdio_writel(func, val, addr, &ret);
>> + *buf = cpu_to_le32(val);
>> +
>> + ret = ath10k_sdio_write(ar, addr, &val, sizeof(val), true);
>
> Shouldn't we use buf instead of val? buf seems pretty useless
> otherwise.
Yes, thanks for pointing this out. will be corrected in v2.
>
> Regards,
> Gary
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply
* [PATCH v2] net/mac80211/mesh_plink: Convert timers to use timer_setup()
From: Kees Cook @ 2017-10-05 17:39 UTC (permalink / raw)
To: Johannes Berg
Cc: David S. Miller, linux-wireless, netdev, Thomas Gleixner,
linux-kernel
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. This requires adding a pointer back
to the sta_info since container_of() can't resolve the sta_info.
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
v2:
- make mesh_plink_timer non-static and use it in timer_setup() call directly.
---
net/mac80211/mesh.h | 1 +
net/mac80211/mesh_plink.c | 10 ++++------
net/mac80211/sta_info.c | 4 +++-
net/mac80211/sta_info.h | 2 ++
4 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index 7e5f271e3c30..465b7853edc0 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -275,6 +275,7 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
u8 *hw_addr, struct ieee802_11_elems *ie);
bool mesh_peer_accepts_plinks(struct ieee802_11_elems *ie);
u32 mesh_accept_plinks_update(struct ieee80211_sub_if_data *sdata);
+void mesh_plink_timer(struct timer_list *t);
void mesh_plink_broken(struct sta_info *sta);
u32 mesh_plink_deactivate(struct sta_info *sta);
u32 mesh_plink_open(struct sta_info *sta);
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index f69c6c38ca43..e79adb4164f3 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -604,8 +604,9 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
ieee80211_mbss_info_change_notify(sdata, changed);
}
-static void mesh_plink_timer(unsigned long data)
+void mesh_plink_timer(struct timer_list *t)
{
+ struct mesh_sta *mesh = from_timer(mesh, t, plink_timer);
struct sta_info *sta;
u16 reason = 0;
struct ieee80211_sub_if_data *sdata;
@@ -617,7 +618,7 @@ static void mesh_plink_timer(unsigned long data)
* del_timer_sync() this timer after having made sure
* it cannot be readded (by deleting the plink.)
*/
- sta = (struct sta_info *) data;
+ sta = mesh->plink_sta;
if (sta->sdata->local->quiescing)
return;
@@ -697,11 +698,8 @@ static void mesh_plink_timer(unsigned long data)
static inline void mesh_plink_timer_set(struct sta_info *sta, u32 timeout)
{
- sta->mesh->plink_timer.expires = jiffies + msecs_to_jiffies(timeout);
- sta->mesh->plink_timer.data = (unsigned long) sta;
- sta->mesh->plink_timer.function = mesh_plink_timer;
sta->mesh->plink_timeout = timeout;
- add_timer(&sta->mesh->plink_timer);
+ mod_timer(&sta->mesh->plink_timer, jiffies + msecs_to_jiffies(timeout));
}
static bool llid_in_use(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 69615016d5bf..6c254a9d5f11 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -329,10 +329,12 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
sta->mesh = kzalloc(sizeof(*sta->mesh), gfp);
if (!sta->mesh)
goto free;
+ sta->mesh->plink_sta = sta;
spin_lock_init(&sta->mesh->plink_lock);
if (ieee80211_vif_is_mesh(&sdata->vif) &&
!sdata->u.mesh.user_mpm)
- init_timer(&sta->mesh->plink_timer);
+ timer_setup(&sta->mesh->plink_timer, mesh_plink_timer,
+ 0);
sta->mesh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
}
#endif
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 3acbdfa9f649..21d9760ce5c3 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -344,6 +344,7 @@ DECLARE_EWMA(mesh_fail_avg, 20, 8)
* @plink_state: peer link state
* @plink_timeout: timeout of peer link
* @plink_timer: peer link watch timer
+ * @plink_sta: peer link watch timer's sta_info
* @t_offset: timing offset relative to this host
* @t_offset_setpoint: reference timing offset of this sta to be used when
* calculating clockdrift
@@ -356,6 +357,7 @@ DECLARE_EWMA(mesh_fail_avg, 20, 8)
*/
struct mesh_sta {
struct timer_list plink_timer;
+ struct sta_info *plink_sta;
s64 t_offset;
s64 t_offset_setpoint;
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related
* Re: [PATCH 00/18] use ARRAY_SIZE macro
From: J. Bruce Fields @ 2017-10-05 17:57 UTC (permalink / raw)
To: Jérémy Lefaure
Cc: Greg KH, Tobin C. Harding, alsa-devel, nouveau, dri-devel,
dm-devel, brcm80211-dev-list, devel, linux-scsi, linux-rdma,
amd-gfx, Jason Gunthorpe, linux-acpi, linux-video,
intel-wired-lan, linux-media, intel-gfx, ecryptfs, linux-nfs,
linux-raid, openipmi-developer, intel-gvt-dev, devel,
brcm80211-dev-list.pdl, netdev, linux-usb, linux-wireless,
linux-kernel, linux-integrity, jlayton
In-Reply-To: <20171002213312.3f904290@blatinox-laptop.localdomain>
On Mon, Oct 02, 2017 at 09:33:12PM -0400, Jérémy Lefaure wrote:
> On Mon, 2 Oct 2017 15:22:24 -0400
> bfields@fieldses.org (J. Bruce Fields) wrote:
>
> > Mainly I'd just like to know which you're asking for. Do you want me to
> > apply this, or to ACK it so someone else can? If it's sent as a series
> > I tend to assume the latter.
> >
> > But in this case I'm assuming it's the former, so I'll pick up the nfsd
> > one....
> Could you to apply the NFSD patch ("nfsd: use ARRAY_SIZE") with the
> Reviewed-by: Jeff Layton <jlayton@redhat.com>) tag please ?
>
> This patch is an individual patch and it should not have been sent in a
> series (sorry about that).
Applying for 4.15, thanks.--b.
^ permalink raw reply
* Re: [PATCH] mwifiex: Use put_unaligned_le32
From: Brian Norris @ 2017-10-05 18:02 UTC (permalink / raw)
To: Himanshu Jha
Cc: Kalle Valo, amitkarwar, nishants, gbhat, huxm, linux-wireless,
netdev, linux-kernel
In-Reply-To: <20171005152233.GA6250@himanshu-Vostro-3559>
On Thu, Oct 05, 2017 at 08:52:33PM +0530, Himanshu Jha wrote:
> There are various instances where a function used in file say for eg
> int func_align (void* a)
> is used and it is defined in align.h
> But many files don't *directly* include align.h and rather include
> any other header which includes align.h
I believe the general rule is that you should included headers for all
symbols you use, and not rely on implicit includes.
The modification to the general rule is that not all headers are
intended to be included directly, and in such cases there's likely a
parent header that is the more appropriate target.
In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
seems that asm-generic/unaligned.h is set up to include different
headers, based on the expected architecture behavior.
I wonder if include/linux/unaligned/access_ok.h should have a safety
check (e.g., raise an #error if
!CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS?).
> Is compiling the file the only way to check if apppropriate header is
> included or is there some other way to check for it.
I believe it's mostly manual. Implicit includes have been a problem for
anyone who refactors header files.
Brian
^ permalink raw reply
* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Marcelo Ricardo Leitner @ 2017-10-05 19:07 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, luto, baijiaju1990, nhorman, vyasevich, kvalo,
linux-crypto, netdev, linux-sctp, linux-wireless
In-Reply-To: <20171005131631.GA1553@gondor.apana.org.au>
On Thu, Oct 05, 2017 at 09:16:31PM +0800, Herbert Xu wrote:
> On Thu, Oct 05, 2017 at 06:16:20PM +0800, Herbert Xu wrote:
> >
> > That was my point. Functions like sctp_pack_cookie shouldn't be
> > setting the key in the first place. The setkey should happen at
> > the point when the key is generated. That's sctp_endpoint_init
> > which AFAICS only gets called in GFP_KERNEL context.
> >
> > Or is there a code-path where sctp_endpoint_init is called in
> > softirq context?
>
> OK, there are indeed code paths where the key is derived in softirq
> context. Notably sctp_auth_calculate_hmac.
>
> So I think this patch is the correct fix and I will push it upstream
> as well as back to stable.
Okay, thanks.
Marcelo
^ permalink raw reply
* Re: [PATCH] mwifiex: Use put_unaligned_le32
From: Himanshu Jha @ 2017-10-05 19:07 UTC (permalink / raw)
To: Brian Norris
Cc: Kalle Valo, amitkarwar, nishants, gbhat, huxm, linux-wireless,
netdev, linux-kernel
In-Reply-To: <20171005180248.GA94139@google.com>
On Thu, Oct 05, 2017 at 11:02:50AM -0700, Brian Norris wrote:
> On Thu, Oct 05, 2017 at 08:52:33PM +0530, Himanshu Jha wrote:
> > There are various instances where a function used in file say for eg
> > int func_align (void* a)
> > is used and it is defined in align.h
> > But many files don't *directly* include align.h and rather include
> > any other header which includes align.h
>
> I believe the general rule is that you should included headers for all
> symbols you use, and not rely on implicit includes.
>
> The modification to the general rule is that not all headers are
> intended to be included directly, and in such cases there's likely a
> parent header that is the more appropriate target.
>
> In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
> seems that asm-generic/unaligned.h is set up to include different
> headers, based on the expected architecture behavior.
>
Yes, asm-generic/unaligned.h looks more appopriate and is most generic
implementation of unaligned accesses and arc specific.
Let's see what Kalle Valo recommends! And then I will send v2 of the
patch.
Thanks for the information!
Himanshu Jha
> I wonder if include/linux/unaligned/access_ok.h should have a safety
> check (e.g., raise an #error if
> !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS?).
>
> > Is compiling the file the only way to check if apppropriate header is
> > included or is there some other way to check for it.
>
> I believe it's mostly manual. Implicit includes have been a problem for
> anyone who refactors header files.
>
> Brian
^ permalink raw reply
* Re: converting mac80211 to TXQs entirely
From: Toke Høiland-Jørgensen @ 2017-10-05 21:52 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: nbd
In-Reply-To: <1507221836.2387.66.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> On Thu, 2017-10-05 at 18:28 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>
>> I'm been thinking about how to move the airtime fairness scheduler
>> out of ath9k and into mac80211 (so more drivers can take advantage of
>> it). This will require some changes to the TXQ API that drivers speak
>> to, so wanted to add my thoughts here to make sure it's compatible
>> with your thinking.
>>=20
>> I think the easiest way to have mac80211 handle airtime fairness is
>> to add a way for ieee80211_tx_dequeue() to return some sort of DEFER
>> signal which semantically means "there is no packet for this queue
>> right now, but please keep scheduling it" (which would be the result
>> of a TXQ belonging to a station that has used its airtime quota but
>> still has packets pending). This is different from the current
>> meaning of NULL, which will make the driver stop scheduling that TXQ
>> until it gets a new wake signal.
>
> I think that's reasonable. I'm not really sure it's *necessary*
> though? Couldn't mac80211 return NULL, and then simply call
> wake_tx_queue again when the TXQ becomes eligible for scheduling
> again? Otherwise the driver might end up doing a lot of polling for it
> to become eligible again?
Theoretically, but then there would have to be some kind of callback or
another mechanism to figure out when the queue is ready again. The neat
thing about DRR scheduling is that we just use the fact that there was
an attempt to schedule the queue as an opportunity to increase that
queue's deficit by one quantum. This does give a bit of "polling
overhead" as you say, but it has been hidden in the measurement noise in
all my tests.
The obvious alternative is to do a token-based airtime scheduler, where
the airtime used by one station is immediately divided out to all active
stations. But that requires walking over all active stations on every TX
completion, and there's a lot of housekeeping to make sure we don't
accidentally starve everyone. So I'd prefer to stick with the DRR
scheduler :)
>> And I'm wondering if this "is queue stopped" API could be the same
>> one used for the airtime DEFER case?
>
> I think these are two completely different cases.
>
> The "is queue stopped" I was thinking of would be basically checking
> the variable local->queue_stop_reasons, so that the driver could still
> use the stop_queue API(s). Yeah, this would be very roundabout, but as
> a conversion step I think that'd not be a bad thing.
Ah, I see. Yeah, these are different, and the existing packet/NULL
return is probably sufficient, then :)
>> Oh, and BTW: I see this is on the list of topics for the wireless
>> summit in Seoul. How do I go about signing up for that? I'll be at
>> netdev talking about some of this stuff anyway[1] :)
>
> Just show up there, or you can add yourself to the list on the wiki
> page :)
Cool, I did. See you in Seoul :)
-Toke
^ permalink raw reply
* Re: [PATCH] mwifiex: Use put_unaligned_le32
From: Igor Mitsyanko @ 2017-10-05 21:54 UTC (permalink / raw)
To: Himanshu Jha, Brian Norris
Cc: Kalle Valo, amitkarwar, nishants, gbhat, huxm, linux-wireless,
netdev, linux-kernel
In-Reply-To: <20171005190730.GA8043@himanshu-Vostro-3559>
On 10/05/2017 12:07 PM, Himanshu Jha wrote:
>>
>> In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
>> seems that asm-generic/unaligned.h is set up to include different
>> headers, based on the expected architecture behavior.
>>
> Yes, asm-generic/unaligned.h looks more appopriate and is most generic
> implementation of unaligned accesses and arc specific.
>
Probably the idea is that each ARCH knows exactly what is the best way
to do unaligned access, so common code (like drivers) should include
ARCH-specific asm/unaligned.h only. It will then decide how to do that
and will include asm-generic/unaligned.h itself, if required.
^ permalink raw reply
* Re: [RFC v2 1/2] fq: support filtering a given tin
From: Johannes Berg @ 2017-10-06 9:42 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, linux-wireless; +Cc: Johannes Berg
In-Reply-To: <87h8vdslpr.fsf@toke.dk>
On Thu, 2017-10-05 at 16:39 +0200, Toke Høiland-Jørgensen wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
>
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > Add to the FQ API a way to filter a given tin, in order to
> > remove frames that fulfil certain criteria according to a
> > filter function.
> >
> > This will be used by mac80211 to remove frames belonging to
> > an AP VLAN interface that's being removed.
> >
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> Yup, this version looks reasonable to me :)
Do you know if anyone feels responsible for the fq code?
Should I Cc netdev on these patches?
johannes
^ permalink raw reply
* Re: converting mac80211 to TXQs entirely
From: Johannes Berg @ 2017-10-06 9:44 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, linux-wireless; +Cc: nbd, Kalle Valo
In-Reply-To: <878tgps1o7.fsf@toke.dk>
On Thu, 2017-10-05 at 23:52 +0200, Toke Høiland-Jørgensen wrote:
>
> > I think that's reasonable. I'm not really sure it's *necessary*
> > though? Couldn't mac80211 return NULL, and then simply call
> > wake_tx_queue again when the TXQ becomes eligible for scheduling
> > again? Otherwise the driver might end up doing a lot of polling for
> > it to become eligible again?
>
> Theoretically, but then there would have to be some kind of callback
> or another mechanism to figure out when the queue is ready again. The
> neat thing about DRR scheduling is that we just use the fact that
> there was an attempt to schedule the queue as an opportunity to
> increase that queue's deficit by one quantum. This does give a bit of
> "polling overhead" as you say, but it has been hidden in the
> measurement noise in all my tests.
Ok.
> The obvious alternative is to do a token-based airtime scheduler,
> where the airtime used by one station is immediately divided out to
> all active stations. But that requires walking over all active
> stations on every TX completion, and there's a lot of housekeeping to
> make sure we don't accidentally starve everyone. So I'd prefer to
> stick with the DRR scheduler :)
Sure, works for me.
I have no objection to defining a special error code (we can always use
ERR_PTR(-EAGAIN) or something like that, after all) - we just need to
be careful with driver updates.
Given all these discussions, I'd actually like to put a temporary
restriction on merging new drivers with TXQ support - Felix, I think
you were planning to eventually use this for mt7601u? How far along is
that?
johannes
^ permalink raw reply
* Re: [PATCH v2] net/mac80211/mesh_plink: Convert timers to use timer_setup()
From: Johannes Berg @ 2017-10-06 9:49 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, linux-wireless, netdev, Thomas Gleixner,
linux-kernel
In-Reply-To: <20171005173910.GA72335@beast>
On Thu, 2017-10-05 at 10:39 -0700, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list
> pointer to
> all timer callbacks, switch to using the new timer_setup() and
> from_timer()
> to pass the timer pointer explicitly. This requires adding a pointer
> back
> to the sta_info since container_of() can't resolve the sta_info.
>
Applied, thanks. (The change did land in net-next, and I merged that in
to merge this)
johannes
^ permalink raw reply
* [PATCH 2/2] mac80211: only remove AP VLAN frames from TXQ
From: Johannes Berg @ 2017-10-06 9:53 UTC (permalink / raw)
To: linux-wireless
Cc: Toke Høiland-Jørgensen, netdev, Michał Kazior,
Johannes Berg
In-Reply-To: <20171006095333.26335-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
When removing an AP VLAN interface, mac80211 currently purges
the entire TXQ for the AP interface. Fix this by using the FQ
API introduced in the previous patch to filter frames.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/ieee80211_i.h | 2 ++
net/mac80211/iface.c | 25 +++----------------------
net/mac80211/tx.c | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 39 insertions(+), 22 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9675814f64db..68f874e73561 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2009,6 +2009,8 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
struct txq_info *txq, int tid);
void ieee80211_txq_purge(struct ieee80211_local *local,
struct txq_info *txqi);
+void ieee80211_txq_remove_vlan(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata);
void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
u16 transaction, u16 auth_alg, u16 status,
const u8 *extra, size_t extra_len, const u8 *bssid,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2619daa29961..13b16f90e1cf 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -793,9 +793,7 @@ static int ieee80211_open(struct net_device *dev)
static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
bool going_down)
{
- struct ieee80211_sub_if_data *txq_sdata = sdata;
struct ieee80211_local *local = sdata->local;
- struct fq *fq = &local->fq;
unsigned long flags;
struct sk_buff *skb, *tmp;
u32 hw_reconf_flags = 0;
@@ -939,9 +937,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
switch (sdata->vif.type) {
case NL80211_IFTYPE_AP_VLAN:
- txq_sdata = container_of(sdata->bss,
- struct ieee80211_sub_if_data, u.ap);
-
mutex_lock(&local->mtx);
list_del(&sdata->u.vlan.list);
mutex_unlock(&local->mtx);
@@ -998,8 +993,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
skb_queue_purge(&sdata->skb_queue);
}
- sdata->bss = NULL;
-
spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
skb_queue_walk_safe(&local->pending[i], skb, tmp) {
@@ -1012,22 +1005,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
}
spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
- if (txq_sdata->vif.txq) {
- struct txq_info *txqi = to_txq_info(txq_sdata->vif.txq);
+ if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+ ieee80211_txq_remove_vlan(local, sdata);
- /*
- * FIXME FIXME
- *
- * We really shouldn't purge the *entire* txqi since that
- * contains frames for the other AP_VLANs (and possibly
- * the AP itself) as well, but there's no API in FQ now
- * to be able to filter.
- */
-
- spin_lock_bh(&fq->lock);
- ieee80211_txq_purge(local, txqi);
- spin_unlock_bh(&fq->lock);
- }
+ sdata->bss = NULL;
if (local->open_count == 0)
ieee80211_clear_tx_pending(local);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 94826680cf2b..7b8154474b9e 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1396,6 +1396,40 @@ static void ieee80211_txq_enqueue(struct ieee80211_local *local,
fq_flow_get_default_func);
}
+static bool fq_vlan_filter_func(struct fq *fq, struct fq_tin *tin,
+ struct fq_flow *flow, struct sk_buff *skb,
+ void *data)
+{
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+
+ return info->control.vif == data;
+}
+
+void ieee80211_txq_remove_vlan(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata)
+{
+ struct fq *fq = &local->fq;
+ struct txq_info *txqi;
+ struct fq_tin *tin;
+ struct ieee80211_sub_if_data *ap;
+
+ if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_AP_VLAN))
+ return;
+
+ ap = container_of(sdata->bss, struct ieee80211_sub_if_data, u.ap);
+
+ if (!ap->vif.txq)
+ return;
+
+ txqi = to_txq_info(ap->vif.txq);
+ tin = &txqi->tin;
+
+ spin_lock_bh(&fq->lock);
+ fq_tin_filter(fq, tin, fq_vlan_filter_func, &sdata->vif,
+ fq_skb_free_func);
+ spin_unlock_bh(&fq->lock);
+}
+
void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
struct sta_info *sta,
struct txq_info *txqi, int tid)
--
2.14.2
^ permalink raw reply related
* [PATCH 1/2] fq: support filtering a given tin
From: Johannes Berg @ 2017-10-06 9:53 UTC (permalink / raw)
To: linux-wireless
Cc: Toke Høiland-Jørgensen, netdev, Michał Kazior,
Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Add to the FQ API a way to filter a given tin, in order to
remove frames that fulfil certain criteria according to a
filter function.
This will be used by mac80211 to remove frames belonging to
an AP VLAN interface that's being removed.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/net/fq.h | 7 +++++
include/net/fq_impl.h | 72 ++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 69 insertions(+), 10 deletions(-)
diff --git a/include/net/fq.h b/include/net/fq.h
index 6d8521a30c5c..ac944a686840 100644
--- a/include/net/fq.h
+++ b/include/net/fq.h
@@ -90,6 +90,13 @@ typedef void fq_skb_free_t(struct fq *,
struct fq_flow *,
struct sk_buff *);
+/* Return %true to filter (drop) the frame. */
+typedef bool fq_skb_filter_t(struct fq *,
+ struct fq_tin *,
+ struct fq_flow *,
+ struct sk_buff *,
+ void *);
+
typedef struct fq_flow *fq_flow_get_default_t(struct fq *,
struct fq_tin *,
int idx,
diff --git a/include/net/fq_impl.h b/include/net/fq_impl.h
index 4e6131cd3f43..8b237e4afee6 100644
--- a/include/net/fq_impl.h
+++ b/include/net/fq_impl.h
@@ -12,24 +12,22 @@
/* functions that are embedded into includer */
-static struct sk_buff *fq_flow_dequeue(struct fq *fq,
- struct fq_flow *flow)
+static void fq_adjust_removal(struct fq *fq,
+ struct fq_flow *flow,
+ struct sk_buff *skb)
{
struct fq_tin *tin = flow->tin;
- struct fq_flow *i;
- struct sk_buff *skb;
-
- lockdep_assert_held(&fq->lock);
-
- skb = __skb_dequeue(&flow->queue);
- if (!skb)
- return NULL;
tin->backlog_bytes -= skb->len;
tin->backlog_packets--;
flow->backlog -= skb->len;
fq->backlog--;
fq->memory_usage -= skb->truesize;
+}
+
+static void fq_rejigger_backlog(struct fq *fq, struct fq_flow *flow)
+{
+ struct fq_flow *i;
if (flow->backlog == 0) {
list_del_init(&flow->backlogchain);
@@ -43,6 +41,21 @@ static struct sk_buff *fq_flow_dequeue(struct fq *fq,
list_move_tail(&flow->backlogchain,
&i->backlogchain);
}
+}
+
+static struct sk_buff *fq_flow_dequeue(struct fq *fq,
+ struct fq_flow *flow)
+{
+ struct sk_buff *skb;
+
+ lockdep_assert_held(&fq->lock);
+
+ skb = __skb_dequeue(&flow->queue);
+ if (!skb)
+ return NULL;
+
+ fq_adjust_removal(fq, flow, skb);
+ fq_rejigger_backlog(fq, flow);
return skb;
}
@@ -188,6 +201,45 @@ static void fq_tin_enqueue(struct fq *fq,
}
}
+static void fq_flow_filter(struct fq *fq,
+ struct fq_flow *flow,
+ fq_skb_filter_t filter_func,
+ void *filter_data,
+ fq_skb_free_t free_func)
+{
+ struct fq_tin *tin = flow->tin;
+ struct sk_buff *skb, *tmp;
+
+ lockdep_assert_held(&fq->lock);
+
+ skb_queue_walk_safe(&flow->queue, skb, tmp) {
+ if (!filter_func(fq, tin, flow, skb, filter_data))
+ continue;
+
+ __skb_unlink(skb, &flow->queue);
+ fq_adjust_removal(fq, flow, skb);
+ free_func(fq, tin, flow, skb);
+ }
+
+ fq_rejigger_backlog(fq, flow);
+}
+
+static void fq_tin_filter(struct fq *fq,
+ struct fq_tin *tin,
+ fq_skb_filter_t filter_func,
+ void *filter_data,
+ fq_skb_free_t free_func)
+{
+ struct fq_flow *flow;
+
+ lockdep_assert_held(&fq->lock);
+
+ list_for_each_entry(flow, &tin->new_flows, flowchain)
+ fq_flow_filter(fq, flow, filter_func, filter_data, free_func);
+ list_for_each_entry(flow, &tin->old_flows, flowchain)
+ fq_flow_filter(fq, flow, filter_func, filter_data, free_func);
+}
+
static void fq_flow_reset(struct fq *fq,
struct fq_flow *flow,
fq_skb_free_t free_func)
--
2.14.2
^ permalink raw reply related
* Re: converting mac80211 to TXQs entirely
From: Toke Høiland-Jørgensen @ 2017-10-06 10:17 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: nbd, Kalle Valo
In-Reply-To: <1507283082.19300.6.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
>> The obvious alternative is to do a token-based airtime scheduler,
>> where the airtime used by one station is immediately divided out to
>> all active stations. But that requires walking over all active
>> stations on every TX completion, and there's a lot of housekeeping to
>> make sure we don't accidentally starve everyone. So I'd prefer to
>> stick with the DRR scheduler :)
>
> Sure, works for me.
>
> I have no objection to defining a special error code (we can always
> use ERR_PTR(-EAGAIN) or something like that, after all) - we just need
> to be careful with driver updates.
Right. I'll see if I can cook up an RFC.
Do you have any opinion on whether to use ERR_PTR or change the function
to an int return and pass the pointer as an argument? At least ath10k
seems to do the latter (returning -ENOENT when no packet is available).
-Toke
^ permalink raw reply
* Re: converting mac80211 to TXQs entirely
From: Johannes Berg @ 2017-10-06 10:18 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, linux-wireless; +Cc: nbd, Kalle Valo
In-Reply-To: <87376wshqn.fsf@toke.dk>
On Fri, 2017-10-06 at 12:17 +0200, Toke Høiland-Jørgensen wrote:
>
> Do you have any opinion on whether to use ERR_PTR or change the
> function to an int return and pass the pointer as an argument? At
> least ath10k seems to do the latter (returning -ENOENT when no packet
> is available).
I think using an ERR_PTR is nicer, but I have no strong opinions (I
just don't like out parameters all that much and avoid them when
possible)
johannes
^ permalink raw reply
* Re: [PATCH 1/2] fq: support filtering a given tin
From: Toke Høiland-Jørgensen @ 2017-10-06 10:30 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: netdev, Michał Kazior, Johannes Berg
In-Reply-To: <20171006095333.26335-1-johannes@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Add to the FQ API a way to filter a given tin, in order to
> remove frames that fulfil certain criteria according to a
> filter function.
>
> This will be used by mac80211 to remove frames belonging to
> an AP VLAN interface that's being removed.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Acked-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
^ permalink raw reply
* Re: [PATCH 2/2] mac80211: only remove AP VLAN frames from TXQ
From: Toke Høiland-Jørgensen @ 2017-10-06 10:30 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: netdev, Michał Kazior, Johannes Berg
In-Reply-To: <20171006095333.26335-2-johannes@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> From: Johannes Berg <johannes.berg@intel.com>
>
> When removing an AP VLAN interface, mac80211 currently purges
> the entire TXQ for the AP interface. Fix this by using the FQ
> API introduced in the previous patch to filter frames.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Acked-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
^ permalink raw reply
* [PATCH v6 0/3] mt76: add new wireless driver for MediaTek MT76x2 PCIe chips
From: Felix Fietkau @ 2017-10-06 11:02 UTC (permalink / raw)
To: linux-wireless; +Cc: kvalo
Changes since v5:
- Adjust for mac80211 API changes
- EEPROM parsing fixes
- Ad-hoc mode WPA2 fixes
Changes since v4:
- Cleanups suggested by Stanislaw Gruszka
- Device tree fixes suggested by Rob Herring
- EEPROM MAC address parsing fix
Changes since v3:
- DFS fixes
- stability fixes
- use wiphy_read_of_freq_limits
Changes since v2:
- lots of checkpatch cleanups
- various tx path (and other) fixes
- use the new bitfield API
- documented device tree bindings
Felix Fietkau (3):
Documentation: dt: net: add mt76 wireless device binding
mt76: add common code shared between multiple chipsets
mt76: add driver code for MT76x2e
.../bindings/net/wireless/mediatek,mt76.txt | 24 +
drivers/net/wireless/mediatek/Kconfig | 1 +
drivers/net/wireless/mediatek/Makefile | 1 +
drivers/net/wireless/mediatek/mt76/Kconfig | 10 +
drivers/net/wireless/mediatek/mt76/Makefile | 15 +
drivers/net/wireless/mediatek/mt76/debugfs.c | 76 ++
drivers/net/wireless/mediatek/mt76/dma.c | 451 ++++++++++++
drivers/net/wireless/mediatek/mt76/dma.h | 38 +
drivers/net/wireless/mediatek/mt76/eeprom.c | 112 +++
drivers/net/wireless/mediatek/mt76/mac80211.c | 344 +++++++++
drivers/net/wireless/mediatek/mt76/mmio.c | 61 ++
drivers/net/wireless/mediatek/mt76/mt76.h | 355 ++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2.h | 223 ++++++
drivers/net/wireless/mediatek/mt76/mt76x2_core.c | 88 +++
.../net/wireless/mediatek/mt76/mt76x2_debugfs.c | 133 ++++
drivers/net/wireless/mediatek/mt76/mt76x2_dfs.c | 493 +++++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2_dfs.h | 80 +++
drivers/net/wireless/mediatek/mt76/mt76x2_dma.c | 184 +++++
drivers/net/wireless/mediatek/mt76/mt76x2_dma.h | 68 ++
drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.c | 644 +++++++++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.h | 181 +++++
drivers/net/wireless/mediatek/mt76/mt76x2_init.c | 784 +++++++++++++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2_mac.c | 738 +++++++++++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2_mac.h | 189 +++++
drivers/net/wireless/mediatek/mt76/mt76x2_main.c | 534 ++++++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2_mcu.c | 452 ++++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2_mcu.h | 136 ++++
drivers/net/wireless/mediatek/mt76/mt76x2_pci.c | 109 +++
drivers/net/wireless/mediatek/mt76/mt76x2_phy.c | 691 ++++++++++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2_regs.h | 566 +++++++++++++++
drivers/net/wireless/mediatek/mt76/mt76x2_trace.c | 23 +
drivers/net/wireless/mediatek/mt76/mt76x2_trace.h | 144 ++++
drivers/net/wireless/mediatek/mt76/mt76x2_tx.c | 255 +++++++
drivers/net/wireless/mediatek/mt76/trace.c | 23 +
drivers/net/wireless/mediatek/mt76/trace.h | 71 ++
drivers/net/wireless/mediatek/mt76/tx.c | 511 ++++++++++++++
drivers/net/wireless/mediatek/mt76/util.c | 78 ++
drivers/net/wireless/mediatek/mt76/util.h | 44 ++
38 files changed, 8930 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
create mode 100644 drivers/net/wireless/mediatek/mt76/Kconfig
create mode 100644 drivers/net/wireless/mediatek/mt76/Makefile
create mode 100644 drivers/net/wireless/mediatek/mt76/debugfs.c
create mode 100644 drivers/net/wireless/mediatek/mt76/dma.c
create mode 100644 drivers/net/wireless/mediatek/mt76/dma.h
create mode 100644 drivers/net/wireless/mediatek/mt76/eeprom.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mac80211.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mmio.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_core.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_debugfs.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_dfs.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_dfs.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_dma.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_dma.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_init.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_mac.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_mac.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_main.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_mcu.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_mcu.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_pci.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_phy.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_regs.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_trace.c
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_trace.h
create mode 100644 drivers/net/wireless/mediatek/mt76/mt76x2_tx.c
create mode 100644 drivers/net/wireless/mediatek/mt76/trace.c
create mode 100644 drivers/net/wireless/mediatek/mt76/trace.h
create mode 100644 drivers/net/wireless/mediatek/mt76/tx.c
create mode 100644 drivers/net/wireless/mediatek/mt76/util.c
create mode 100644 drivers/net/wireless/mediatek/mt76/util.h
--
2.11.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox