Linux wireless drivers development
 help / color / mirror / Atom feed
* [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.
From: Arvind Yadav @ 2016-07-31 11:18 UTC (permalink / raw)
  To: zajec5, leoli
  Cc: qiang.zhao, scottwood, viresh.kumar, akpm, linux-wireless, netdev,
	linuxppc-dev, linux, arnd, davem, David.Laight, Arvind Yadav

IS_ERR_VALUE() assumes that parameter is an unsigned long.
It can not be used to check if 'unsigned int' is passed insted.
Which tends to reflect an error.

In 64bit architectures sizeof (int) == 4 && sizeof (long) == 8.
IS_ERR_VALUE(x) is ((x) >= (unsigned long)-4095).

IS_ERR_VALUE() of 'unsigned int' is always false because the 32bit
value is zero extended to 64 bits.

Value of (unsigned int)-4095 is always less than value of
(unsigned long)-4095.

Now We are taking only first 32 bit for error checking rest of the 32 bit
we ignore such that we get appropriate comparison on 64bit system as well.

First 32bit of Value of (unsigned int)-4095 and (unsigned long)-4095 will
be equal.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 include/linux/err.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/err.h b/include/linux/err.h
index 1e35588..c2a2789 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -18,7 +18,17 @@
 
 #ifndef __ASSEMBLY__
 
-#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
+#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
+
+static inline int is_error_check(unsigned long error)
+{
+	unsigned int first32bit = (error & 0xFFFFFFFF);
+
+	if (first32bit >= (unsigned int)-MAX_ERRNO)
+		return 1;
+	else
+		return 0;
+}
 
 static inline void * __must_check ERR_PTR(long error)
 {
-- 
1.9.1


^ permalink raw reply related

* Re: [RFC v0 7/8] Input: ims-pcu: use firmware_stat instead of completion
From: Bjorn Andersson @ 2016-08-01 17:19 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Arend van Spriel, Daniel Wagner, Dmitry Torokhov, Daniel Wagner,
	Bastien Nocera, Greg Kroah-Hartman, Johannes Berg, Kalle Valo,
	Ohad Ben-Cohen, linux-input, linux-kselftest, linux-wireless,
	linux-kernel
In-Reply-To: <20160730165817.GQ3296@wotan.suse.de>

On Sat 30 Jul 09:58 PDT 2016, Luis R. Rodriguez wrote:

> On Sat, Jul 30, 2016 at 02:42:41PM +0200, Arend van Spriel wrote:
> > + Luis (again) ;-)
> > 
> > On 29-07-16 08:13, Daniel Wagner wrote:
> > > On 07/28/2016 09:01 PM, Bjorn Andersson wrote:
> > >> On Thu 28 Jul 11:33 PDT 2016, Dmitry Torokhov wrote:
> > >>
> > >>> On Thu, Jul 28, 2016 at 09:55:11AM +0200, Daniel Wagner wrote:
> > >>>> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
> > >>>>
> > >> [..]
> > >>>
> > >>> Do not quite like it... I'd rather asynchronous request give out a
> > >>> firmware status pointer that could be used later on.
> > 
> > Excellent. Why not get rid of the callback function as well and have
> > fw_loading_wait() return result (0 = firmware available, < 0 = fail).
> > Just to confirm, you are proposing a new API function next to
> > request_firmware_nowait(), right?
> 
> If proposing new firmware_class patches please bounce / Cc me, I've
> recently asked for me to be added to MAINTAINERS so I get these
> e-mails as I'm working on a new flexible API which would allow us
> to extend the firmware API without having to care about the old
> stupid usermode helper at all.
> 

In the remoteproc world there are several systems where we see 100+MB of
firmware being loaded. It's unfeasible to have these files in an
initramfs, so we need a way to indicate to the kernel that the
second/primary (or a dedicated firmware partition) is mounted.

We're currently loading these files with request_firmware_nowait(), so
either one can either use kernel modules or the user-helper fallback to
delay the loading until the files are available. (I don't like to
enforce the usage of kernel modules)

I'm open to alternative ways of having firmware loading wait on
secondary filesystems to be mounted, but have not yet tried to tackle
this problem. I do believe something that waits and retries the firmware
load as additional file systems gets mounted would be prettier than
forcing user space to tell us it's time to move on.



Due to the size of these firmware files we release the firmware as soon
as we have copied the content into the appropriate memory segments, so
we're not utilizing the caching mechanisms of the current fw loader.

Regards,
Bjorn

^ permalink raw reply

* Re: [PATCH 1/2] cfg80211: Add support to set tx power for a station associated
From: Ben Greear @ 2016-08-01 13:27 UTC (permalink / raw)
  To: Johannes Berg, Ashok Raj Nagarajan
  Cc: Ashok Raj Nagarajan, linux-wireless, ath10k
In-Reply-To: <1470043740.3389.10.camel@sipsolutions.net>



On 08/01/2016 02:29 AM, Johannes Berg wrote:
>
>> Sure.. First use case will be to help with the problem of legacy
>> client devices that roam across multiple APs. It is a classic
>> enterprise Wi-Fi AP problem,  often managed by a "network controller"
>> unit that is connected to all the APs.
>> The problem is how to handle seamless handoff of clients between
>> multiple  APs while maximizing the client throughput and minimizing
>> disruption of IP application services like VoIP calls and video
>> streaming. A legacy client will often  hold onto an AP association,
>> even down to 1 Mbps as it roams away. Instead,  if the AP can
>> recognise that the client RSSI (and therefore throughput) is poor, it
>> can "drop" the Tx power significantly (just to that client) such that
>> it forcesthe client to look for a better, closer, and therefore
>> higher-throughputassociation. It would "give it a kick" without
>> blacklisting it. It just needsto hold the power low for the small
>> amount of time it takes to convince it to go away.
>
> Not sure that *works* since implementations may just compare beacon
> signal strength and hold on to the AP based on that, but it does seem
> like a reasonable use case.

How is that better than just kicking the station deliberately and/or
refusing to send frames to it at all?

Thanks,
Ben

>
> How would this interact with automatic adjustment though?
>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [PATCH 2/3 v6] mac80211: Refactor ieee80211_iface_work
From: Johannes Berg @ 2016-08-01 12:20 UTC (permalink / raw)
  To: Alex Briskin, linux-wireless
In-Reply-To: <1468693140-19483-2-git-send-email-br.shurik@gmail.com>


> Added ieee80211_iface_work_handle_frame_control.

That function name doesn't really make sense - handle_mgmt() perhaps?

johannes

^ permalink raw reply

* Re: [PATCH v3 3/3] mac80211: mesh: fixed HT ies in beacon template
From: Masashi Honma @ 2016-08-01 12:30 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Yaniv Machani, linux-kernel, Meirav Kama, David S. Miller,
	linux-wireless, netdev
In-Reply-To: <1470045822.3389.24.camel@sipsolutions.net>

On 2016年08月01日 19:03, Johannes Berg wrote:
>
> But why is that behaviour *correct*? We still support 40 MHz bandwidth
> things, we just don't use them if we disable HT40.

I could not fully understand your concern...

Do you mean we have 2 bugs about disabling HT40 ?

1) bits in HT capabilities IE
2) HT40 still enabled even if it was disabled by wpa_supplicant or 
hostapd with disable_ht40

And do you mean 1) and 2) should be fixed at one time ?
Indeed, currently on the view point of opposite peer, HT40 was enabled 
even though it was disabled because both 1) and 2) are wrong.
But if only 1) was fixed, this causes unmatch.

Now I do not recognize bug 2).
Do you have any information about 2) ?

^ permalink raw reply

* Re: [RFC v0 7/8] Input: ims-pcu: use firmware_stat instead of completion
From: Daniel Wagner @ 2016-08-01 12:26 UTC (permalink / raw)
  To: Dmitry Torokhov, Luis R. Rodriguez, Arend van Spriel
  Cc: Bjorn Andersson, Daniel Wagner, Bastien Nocera,
	Greg Kroah-Hartman, Johannes Berg, Kalle Valo, Ohad Ben-Cohen,
	linux-input, linux-kselftest, linux-wireless, linux-kernel
In-Reply-To: <C528E404-0CA5-46B4-B551-B1D4B58BC053@gmail.com>

On 07/31/2016 09:23 AM, Dmitry Torokhov wrote:
> On July 30, 2016 9:58:17 AM PDT, "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
>> On Sat, Jul 30, 2016 at 02:42:41PM +0200, Arend van Spriel wrote:
>>> On 29-07-16 08:13, Daniel Wagner wrote:
>>>> On 07/28/2016 09:01 PM, Bjorn Andersson wrote:
>>>>> On Thu 28 Jul 11:33 PDT 2016, Dmitry Torokhov wrote:

>>> + Luis (again) ;-)

That was not on purpose :) My attempt to keep the Cc list a bit shorter 
was a failure.

>>>>>> Do not quite like it... I'd rather asynchronous request give out
>>>>>> firmware status pointer that could be used later on.
>>>
>>> Excellent. Why not get rid of the callback function as well and have
>>> fw_loading_wait() return result (0 = firmware available, < 0 = fail).
>>> Just to confirm, you are proposing a new API function next to
>>> request_firmware_nowait(), right?
>>
>> If proposing new firmware_class patches please bounce / Cc me, I've
>> recently asked for me to be added to MAINTAINERS so I get these
>> e-mails as I'm working on a new flexible API which would allow us
>> to extend the firmware API without having to care about the old
>> stupid usermode helper at all.

These patches here are a first attempt to clean up a bit of the code 
around the completion API. As Dmitry correctly pointed out, it makes 
more sense to go bit further and make the async loading a bit more 
convenient for the drivers.

> I am not sure why we started calling usermode helper "stupid". We
> only  had to implement direct kernel firmware loading because udev/stsremd
> folks had "interesting" ideas how events should be handled; but having
> userspace to feed us data is not stupid.

I was ignorant on all the nasty details around the firmware loading. If 
I parse Luis' patches correctly they introduce an API which calls 
kernel_read_file_from_path() asynchronously:

sysdata_file_request_async(..., &cookie)
   *coookie = async_schedule_domain(request_sysdata_file_work_func(), ..)

   request_sysdata_file_work_fun()
     _sysdata_file_request()
       fw_get_filesystem_firmware()
	kernel_read_file_from_path()

sysdata_synchronize_request(&cookie);

Doesn't look like what your asking for.

> If we want to overhaul firmware loading support we need to figure
> out  how to support case when a driver want to [asynchronously] request
> firmware/config/blob and the rest of the system is not ready. Even if we
> want kernel to do read/load the data we need userspace to tell kernel
> when firmware partition is available, until then the kernel should not
> fail the request.

I gather from Luis' blog post and comments that he is on the quest on 
removing userspace support completely.

Maybe this attempt here could be a step before. Step 1 would be changing 
request_firmware_nowait() to request_firmware_async so drivers don't 
have to come up with their own sync primitives, e.g.

   cookie = request_firmware_async()
   fw_load_wait(cookie)

Step 2 could be something more towards sysdata approach.

cheers,
daniel

^ permalink raw reply

* Re: [PATCH] iw: display 5/10 MHz channel widths
From: Johannes Berg @ 2016-08-01 12:24 UTC (permalink / raw)
  To: Bob Copeland; +Cc: linux-wireless
In-Reply-To: <20160721153948.32171-1-me@bobcopeland.com>

On Thu, 2016-07-21 at 11:39 -0400, Bob Copeland wrote:
> iw was showing 'width: unknown' for channels on OCB interfaces; teach
> it the values for 5/10 MHz so it will show the configured width.
> 
Applied, thanks.

johannes

^ permalink raw reply

* Re: [PATCH 3/3 v6] mac80211: Refactor ieee80211_iface_work
From: Johannes Berg @ 2016-08-01 12:23 UTC (permalink / raw)
  To: Alex Briskin, linux-wireless
In-Reply-To: <1468693140-19483-3-git-send-email-br.shurik@gmail.com>


> +		if (ieee80211_iface_work_handle_pkt_type(skb, sdata) &&
> +		    ieee80211_iface_work_handle_frame_control(skb, sdata)) {
> +			ieee80211_iface_work_handle_vif_type(skb, sdata);
>  		}
> 
This ends up being pretty strange IMHO, using boolean short-circuit
evaluation to avoid doing the next thing...

Not sure how to better do this though.

(You also don't need braces here for a single-statement if branch
contents)

johannes

^ permalink raw reply

* Re: [PATCH 1/3 v6] mac80211: Refactor ieee80211_iface_work
From: Johannes Berg @ 2016-08-01 12:20 UTC (permalink / raw)
  To: Alex Briskin, linux-wireless
In-Reply-To: <1468693140-19483-1-git-send-email-br.shurik@gmail.com>

Please don't submit three patches with identical subject.

> Added ieee80211_iface_work_handle_pkt_type function. Moved part of

Should also use present tense here - "Add ... function, move [...]".

This also applies to the other patches.

The if/else if/... -> switch change should also be part of this patch,
or separate, but not squashed together with a different unrelated
change. Might be worth moving "return 0" into each of the cases and
return an error in the fall-through, but it doesn't really matter.


johannes

^ permalink raw reply

* Re: [PATCH v3 3/3] mac80211: mesh: fixed HT ies in beacon template
From: Johannes Berg @ 2016-08-01 10:03 UTC (permalink / raw)
  To: Masashi Honma, Yaniv Machani, linux-kernel
  Cc: Meirav Kama, David S. Miller, linux-wireless, netdev
In-Reply-To: <40a34537-486e-a466-5a7e-e253f19d81c3@gmail.com>

On Fri, 2016-07-22 at 14:26 +0900, Masashi Honma wrote:
> On 2016年07月14日 05:07, Yaniv Machani wrote:
> > +
> > +	/* if channel width is 20MHz - configure HT capab
> > accordingly*/
> > +	if (sdata->vif.bss_conf.chandef.width ==
> > NL80211_CHAN_WIDTH_20) {
> > +		cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
> > +		cap &= ~IEEE80211_HT_CAP_DSSSCCK40;
> > +	}
> 
> I have tested this part of your patch and this works for me.
> 
> Previouly, "Supported Channel Width Set bit" in HT Capabilities
> element was 1 even though disable_ht40=1 existed in
> wpa_supplicant.conf. After appllication of patch, the bit was 0.
> 

But why is that behaviour *correct*? We still support 40 MHz bandwidth
things, we just don't use them if we disable HT40.

johannes

^ permalink raw reply

* RE: [PATCH 1/1] mwifiex: key_material_v2 remove superfluous condition
From: Amitkumar Karwar @ 2016-08-01 10:46 UTC (permalink / raw)
  To: Heinrich Schuchardt, Nishant Sarmukadam, Kalle Valo
  Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1469967081-3937-1-git-send-email-xypron.glpk@gmx.de>

> From: Heinrich Schuchardt [mailto:xypron.glpk@gmx.de]
> Sent: Sunday, July 31, 2016 5:41 PM
> To: Amitkumar Karwar; Nishant Sarmukadam; Kalle Valo
> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Heinrich Schuchardt
> Subject: [PATCH 1/1] mwifiex: key_material_v2 remove superfluous
> condition
> 
> We are using mac as source address in a memcpy.
> In the lines below we can assume mac is not NULL.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index 128add8..0d4f9fe 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -706,15 +706,10 @@ mwifiex_cmd_802_11_key_material_v2(struct
> mwifiex_private *priv,
>  				    (priv->wep_key_curr_index & KEY_INDEX_MASK))
>  					key_info |= KEY_DEFAULT;
>  			} else {
> -				if (mac) {
> -					if (is_broadcast_ether_addr(mac))
> -						key_info |= KEY_MCAST;
> -					else
> -						key_info |= KEY_UNICAST |
> -							    KEY_DEFAULT;
> -				} else {
> +				if (is_broadcast_ether_addr(mac))
>  					key_info |= KEY_MCAST;
> -				}
> +				else
> +					key_info |= KEY_UNICAST | KEY_DEFAULT;
>  			}
>  		}
>  		km->key_param_set.key_info = cpu_to_le16(key_info);
> --
> 2.8.1

Looks ok to me.

Acked-by: Amitkumar Karwar <akarwar@marvell.com>

Regards,
Amitkumar

^ permalink raw reply

* RE: [PATCH 1/1] mwifiex: remove superfluous condition
From: Amitkumar Karwar @ 2016-08-01 10:46 UTC (permalink / raw)
  To: Heinrich Schuchardt, Nishant Sarmukadam, Kalle Valo
  Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1469961555-2123-1-git-send-email-xypron.glpk@gmx.de>

> From: Heinrich Schuchardt [mailto:xypron.glpk@gmx.de]
> Sent: Sunday, July 31, 2016 4:09 PM
> To: Amitkumar Karwar; Nishant Sarmukadam; Kalle Valo
> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Heinrich Schuchardt
> Subject: [PATCH 1/1] mwifiex: remove superfluous condition
> 
> for_each_property_of_node is only executed if the property prop is not
> NULL.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index 7897037..128add8 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -1482,7 +1482,7 @@ int mwifiex_dnld_dt_cfgdata(struct mwifiex_private
> *priv,
>  			continue;
> 
>  		/* property header is 6 bytes, data must fit in cmd buffer
> */
> -		if (prop && prop->value && prop->length > 6 &&
> +		if (prop->value && prop->length > 6 &&
>  		    prop->length <= MWIFIEX_SIZE_OF_CMD_BUFFER - S_DS_GEN) {
>  			ret = mwifiex_send_cmd(priv, HostCmd_CMD_CFG_DATA,
>  					       HostCmd_ACT_GEN_SET, 0,

Thanks. The patch looks fine to me.

Acked-by: Amitkumar Karwar <akarwar@marvell.com>

Regards,
Amitkumar

^ permalink raw reply

* Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped
From: Johannes Berg @ 2016-08-01  9:38 UTC (permalink / raw)
  To: Machani, Yaniv, linux-kernel@vger.kernel.org, David S . Miller,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org
  Cc: Hahn, Maital
In-Reply-To: <AE1C82FB3D0EC64DB1F752C81CBD1101391FC38C@DFRE01.ent.ti.com>

On Wed, 2016-07-13 at 10:11 +0000, Machani, Yaniv wrote:
> On Wed, Jun 29, 2016 at 10:14:19, Johannes Berg wrote:
> > Cc: Hahn, Maital
> > Subject: Re: [PATCH 1/4] mac80211: mesh: flush stations before
> > beacons 
> > are stopped
> > 
> > On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
> > > From: Maital Hahn <maitalm@ti.com>
> > > 
> > > Some drivers (e.g. wl18xx) expect that the last stage in the 
> > > de-initialization process will be stopping the beacons, similar
> > > to ap.
> > > Update ieee80211_stop_mesh() flow accordingly.
> > > 
> > How well have you tested that with other drivers?
> > 
> 
> Sorry for the delayed response (I've been out) and thanks for your
> comments,
> I have tested it with RT3572 as well, and didn't see any issue.
> I'll update the comment to reflect that.
> 

I'm actually reasonably sure that it *must* work, similiar to the AP
mode change, since it's always valid to remove stations while the the
mesh beacon is still active.

I hoped you'd actually figure out that line of reasoning and put it
into the commit message :)

johannes

^ permalink raw reply

* Re: [PATCH v3] cfg80211: Allow different beacon interval if driver supports
From: Johannes Berg @ 2016-08-01 10:02 UTC (permalink / raw)
  To: Purushottam Kushwaha
  Cc: linux-wireless, jouni, usdutt, ganeshk, mkalikot, amarnath
In-Reply-To: <1469180557-24210-1-git-send-email-pkushwah@qti.qualcomm.com>

On Fri, 2016-07-22 at 15:12 +0530, Purushottam Kushwaha wrote:
> Driver may allow support for different beacon interval on virtual
> interfaces.
> Allow if such support is advertised by driver. This adds new
> ext_feature as
> NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
> 
My earlier comment regarding interface combinations still stands.

johannes

^ permalink raw reply

* Re: [PATCH] ath10k: disable wake_tx_queue for older devices
From: Dave Taht @ 2016-08-01  9:04 UTC (permalink / raw)
  To: Roman Yeryomin
  Cc: Valo, Kalle, linux-wireless@vger.kernel.org,
	michal.kazior@tieto.com, ath10k@lists.infradead.org
In-Reply-To: <CACiydb+yNc-UKVRFR-UxTq-YFBDpEDsa1qWNBGCHpe=_zv8o7g@mail.gmail.com>

On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <leroi.lists@gmail.com> wrote:
> On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> Ideally wake_tx_queue should be used regardless as
>>> it is a requirement for reducing bufferbloat and
>>> implementing airtime fairness in the future.
>>>
>>> However some setups (typically low-end platforms
>>> hosting QCA988X) suffer performance regressions
>>> with the current wake_tx_queue implementation.
>>> Therefore disable it unless it is really
>>> beneficial with current codebase (which is when
>>> firmware supports smart pull-push tx scheduling).
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>
>> I think it's too late to send this to 4.7 anymore (and this due to my
>> vacation). So I'm planning to queue this to 4.8, but if the feedback is
>> positive we can always send this to a 4.7 stable release.
>>
>
> Sorry guys, drowned.
> So, yes, applying this patch does the job. That is gets me to the
> results similar to
> https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html
>
> Going to try latest code on same system...

Can you try increasing the quantum to 1514, and reducing the codel
target to 5ms? (without this patch?)

>
> Regards,
> Roman
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k



-- 
Dave Täht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org

^ permalink raw reply

* Re: [PATCH v2 1/3] mac80211: mesh: flush stations before beacons are stopped
From: Johannes Berg @ 2016-08-01  9:39 UTC (permalink / raw)
  To: Yaniv Machani, linux-kernel
  Cc: Maital Hahn, David S. Miller, linux-wireless, netdev
In-Reply-To: <20160713114449.24665-1-yanivma@ti.com>

On Wed, 2016-07-13 at 14:44 +0300, Yaniv Machani wrote:
> From: Maital Hahn <maitalm@ti.com>
> 
> Some drivers (e.g. wl18xx) expect that the last stage in the
> de-initialization process will be stopping the beacons, similar to AP
> flow.
> Update ieee80211_stop_mesh() flow accordingly.
> As peers can be removed dynamically, this would not impact other
> drivers.
> 
Ah, I see now that you did :)

johannes

^ permalink raw reply

* Re: [PATCH v2 RESEND] qtnfmac: announcement of new FullMAC driver for Quantenna chipsets
From: Johannes Berg @ 2016-08-01  9:37 UTC (permalink / raw)
  To: Igor Mitsyanko, kvalo
  Cc: linux-wireless, Avinash Patil, Dmitrii Lebed, Sergei Maksimenko,
	Sergey Matyukevich, Bindu Therthala, Huizhao Wang, Kamlesh Rath
In-Reply-To: <fd6ee9cd-71fd-352a-e1cd-61d36b296057@quantenna.com>


> > 
> > > +	/* bus private data */
> > > +	char bus_priv[0];
> > You might want to - for future proofing - add some aligned()
> > attribute.
> > Otherwise, if struct mutex doesn't have a size that's a multiple of
> > the
> > pointer size, fields in here will not be aligned right.
> 
> Right, thanks for pointing this out. Probably even better approach
> here would be to take an object-oriented approach and make "bus" a
> base structure with pcie_bus an inheriting structure.

I think both ways to do composition are fine. It depends more on how
your other code is structured. I think in mac80211/cfg80211 we do both,
depending on where/how the allocation is done.

> > > +#define QLINK_HT_MCS_MASK_LEN	10
> > > +#define QLINK_ETH_ALEN		6
> > > +#define QLINK_MAX_SSID_LEN	32
> > These seem a bit strange? Why bother? They are standard values.
> > (not entirely sure what the MCS_MASK_LEN is used for though)
> 
> The idea was to make protocol definition an independent entity, which
> does not depend on actual values which happened to be used in current
> kernel. And so that we can be sure that both sides of protocol 
> communication entities use the same length values.
> 
> We actually had discussions regarding this locally too: it was not
> clear whether we should use existing Linux definitions for standard
> things like MAC length, 802.11 header fields, IEs definitions etc. It
> does make a lot of sense to think that they will never change and can
> not be different on opposite sides of protocol, but we went with
> approach to define everything manually.

Well, I can kinda see the beginning of that argument "we should own
everything to make sure", but it doesn't really work that way. The max
SSID that's passed in, or the ETH_ALEN that's there - you're always
going to rely on the kernel's definition of this in other ways, say
when you alloc_ether_dev() [or whatever it's called], etc.

The end result is that you're never going to be able to change one of
the two and expect the combined system to still work. As a consequence,
I don't see any point in even trying to separate them.

> > > +struct qlink_vht_cap {

> Ok, that makes sense of course. Will discuss this locally.

Same argument applies here too, btw, and you might even have to
translate between this version and the one coming from say cfg80211 -
where the ieee80211.h one is used - and then you can realistically only
do a memcpy(), so you rely on them being the same anyway.

> > > +	memcpy(&bss_cfg->crypto, &sme->crypto, sizeof(bss_cfg-
> > > > crypto));
> > This makes no sense at all - you have to convert the format of this
> > somehow to make it work - at least endianness has to be fixed, even
> > if
> > you copied all of the cfg80211 struct.
> 
> In this particular place we're making a local copy of cfg80211 struct
> to  local data structure (per each virtual interface). Conversion
> before  sending to another side is done in qtnf_cmd_send_connect(),
> it is converted into struct qlink_auth_encr.

Ok. Somehow I thought this (bss_cfg) was sent to the device.


johannes

^ permalink raw reply

* Re: [PATCH 1/2] cfg80211: Add support to set tx power for a station associated
From: Johannes Berg @ 2016-08-01  9:29 UTC (permalink / raw)
  To: Ashok Raj Nagarajan; +Cc: Ashok Raj Nagarajan, linux-wireless, ath10k
In-Reply-To: <1cb3e2cc49b433fe5ed834a33adf64b6@codeaurora.org>


> Sure.. First use case will be to help with the problem of legacy
> client devices that roam across multiple APs. It is a classic
> enterprise Wi-Fi AP problem,  often managed by a "network controller"
> unit that is connected to all the APs. 
> The problem is how to handle seamless handoff of clients between
> multiple  APs while maximizing the client throughput and minimizing
> disruption of IP application services like VoIP calls and video
> streaming. A legacy client will often  hold onto an AP association,
> even down to 1 Mbps as it roams away. Instead,  if the AP can
> recognise that the client RSSI (and therefore throughput) is poor, it
> can "drop" the Tx power significantly (just to that client) such that
> it forcesthe client to look for a better, closer, and therefore
> higher-throughputassociation. It would "give it a kick" without
> blacklisting it. It just needsto hold the power low for the small
> amount of time it takes to convince it to go away.

Not sure that *works* since implementations may just compare beacon
signal strength and hold on to the AP based on that, but it does seem
like a reasonable use case.

How would this interact with automatic adjustment though?

johannes


^ permalink raw reply

* Re: rfkill bound to a switch
From: Johannes Berg @ 2016-08-01  9:16 UTC (permalink / raw)
  To: Kai Hendry, linux-wireless
In-Reply-To: <1469692952.331455.679081257.3C81B1F3@webmail.messagingengine.com>

On Thu, 2016-07-28 at 16:02 +0800, Kai Hendry wrote:
> Hi there,
> 
> Since laptops typically have a toggle button for killing and enabled
> wifi, how does one supposed to bind rfkill like a toggle?
> 
> I.e. knowing which state you are in so you can call {block,unblock}
> accordingly?
> 
> Parsing event or list output seems non-trivial.
> 

Don't use the rfkill tool - just open /dev/rfkill yourself, the API is
really simple.

johannes

^ permalink raw reply

* Re: Dell Venue 8 Pro "Dell Wireless" (Atheros 6k) chipset with SDIO ID 0271:0418 not recognized/supported
From: Kalle Valo @ 2016-08-01  8:55 UTC (permalink / raw)
  To: russianneuromancer@ya.ru; +Cc: linux-wireless, ath6kl
In-Reply-To: <12631469979153@web27h.yandex.ru>

Hi,

please don't reply to me privately. Let's keep the discussion on the
lists so that others can join and learn.

"russianneuromancer@ya.ru" <russianneuromancer@ya.ru> writes:

>> That looks good to me, but please submit it officially as defined in
>> the wiki
>
> Just to be sure I understand you right - I have to just send e-mail with
> attached file "support-Dell-OEM-chipset-found-in-Venue-8-Pro-SDIO-I.patch" - is
> that correct?

You should submit the patch following the general Linux patch submission
rules. If you are not familiar with that I recommend doing thorough
research before your first. And for the actual submission I strongly
recommend to use 'git send-email'. Also remember to make sure that all
legal requirements are fulfilled, especially Signed-off-by tags.

Oh, and you have to use your real legal name. I can't take patches from
pseudonyms.

Here are few pointers:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
https://www.kernel.org/doc/Documentation/SubmittingPatches
https://kernelnewbies.org/FirstKernelPatch

-- 
Kalle Valo

^ permalink raw reply

* Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.
From: Arnd Bergmann @ 2016-08-01  7:02 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Arvind Yadav, zajec5, leoli, qiang.zhao, viresh.kumar,
	linux-wireless, David.Laight, netdev, scottwood, akpm, davem,
	linux
In-Reply-To: <1469963924-8800-1-git-send-email-arvind.yadav.cs@gmail.com>

On Sunday, July 31, 2016 4:48:44 PM CEST Arvind Yadav wrote:
> IS_ERR_VALUE() assumes that parameter is an unsigned long.
> It can not be used to check if 'unsigned int' is passed insted.
> Which tends to reflect an error.
> 
> In 64bit architectures sizeof (int) == 4 && sizeof (long) == 8.
> IS_ERR_VALUE(x) is ((x) >= (unsigned long)-4095).
> 
> IS_ERR_VALUE() of 'unsigned int' is always false because the 32bit
> value is zero extended to 64 bits.
> 
> Value of (unsigned int)-4095 is always less than value of
> (unsigned long)-4095.
> 
> Now We are taking only first 32 bit for error checking rest of the 32 bit
> we ignore such that we get appropriate comparison on 64bit system as well.

This is completely wrong: if you have a valid 64-bit pointer like
0x00001234ffffff00, this will be interpreted as an error now.

> First 32bit of Value of (unsigned int)-4095 and (unsigned long)-4095 will
> be equal.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
>  include/linux/err.h | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/err.h b/include/linux/err.h
> index 1e35588..c2a2789 100644
> --- a/include/linux/err.h
> +++ b/include/linux/err.h
> @@ -18,7 +18,17 @@
>  
>  #ifndef __ASSEMBLY__
>  
> -#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
> +#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
> +
> +static inline int is_error_check(unsigned long error)

Please leave the existing macro alone. I think you were looking for
something specific to the return code of qe_muram_alloc() function,
so please add a helper in that subsystem if you need it, not in
the generic header files.

	Arnd

^ permalink raw reply

* Re: [PATCH] ath10k: disable wake_tx_queue for older devices
From: Roman Yeryomin @ 2016-07-31 23:35 UTC (permalink / raw)
  To: Valo, Kalle
  Cc: michal.kazior@tieto.com, linux-wireless@vger.kernel.org,
	ath10k@lists.infradead.org
In-Reply-To: <87zipt1kco.fsf@kamboji.qca.qualcomm.com>

On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> Ideally wake_tx_queue should be used regardless as
>> it is a requirement for reducing bufferbloat and
>> implementing airtime fairness in the future.
>>
>> However some setups (typically low-end platforms
>> hosting QCA988X) suffer performance regressions
>> with the current wake_tx_queue implementation.
>> Therefore disable it unless it is really
>> beneficial with current codebase (which is when
>> firmware supports smart pull-push tx scheduling).
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> I think it's too late to send this to 4.7 anymore (and this due to my
> vacation). So I'm planning to queue this to 4.8, but if the feedback is
> positive we can always send this to a 4.7 stable release.
>

Sorry guys, drowned.
So, yes, applying this patch does the job. That is gets me to the
results similar to
https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html

Going to try latest code on same system...

Regards,
Roman

^ permalink raw reply

* Re: [PATCH 1/1] rtlwifi: remove superfluous condition
From: Larry Finger @ 2016-07-31 14:14 UTC (permalink / raw)
  To: Heinrich Schuchardt, Chaoming Li, Kalle Valo
  Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1469961015-1888-1-git-send-email-xypron.glpk@gmx.de>

On 07/31/2016 05:30 AM, Heinrich Schuchardt wrote:
> If sta == NULL, the changed line will not be reached.
> So no need to check if stat == NULL here.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  drivers/net/wireless/realtek/rtlwifi/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I agree. If sta were NULL, we would never have reached this statement. There is, 
however, a typo in the last line of the commit message. Once this is fixed, you 
may add the line "Acked-by: Larry Finger <Larry.Finger@lwfinger.net>"

Thanks,

Larry

>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c
> index 41f77f8..7aee5ebb1 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/core.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/core.c
> @@ -1135,7 +1135,7 @@ static void rtl_op_bss_info_changed(struct ieee80211_hw *hw,
>  					mac->mode = WIRELESS_MODE_AC_24G;
>  			}
>
> -			if (vif->type == NL80211_IFTYPE_STATION && sta)
> +			if (vif->type == NL80211_IFTYPE_STATION)
>  				rtlpriv->cfg->ops->update_rate_tbl(hw, sta, 0);
>  			rcu_read_unlock();
>
>


^ permalink raw reply

* [PATCH 1/1] mwifiex: key_material_v2 remove superfluous condition
From: Heinrich Schuchardt @ 2016-07-31 12:11 UTC (permalink / raw)
  To: Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo
  Cc: linux-wireless, netdev, linux-kernel, Heinrich Schuchardt
In-Reply-To: <877fc2816w.fsf@purkki.adurom.net>

We are using mac as source address in a memcpy.
In the lines below we can assume mac is not NULL.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index 128add8..0d4f9fe 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -706,15 +706,10 @@ mwifiex_cmd_802_11_key_material_v2(struct mwifiex_private *priv,
 				    (priv->wep_key_curr_index & KEY_INDEX_MASK))
 					key_info |= KEY_DEFAULT;
 			} else {
-				if (mac) {
-					if (is_broadcast_ether_addr(mac))
-						key_info |= KEY_MCAST;
-					else
-						key_info |= KEY_UNICAST |
-							    KEY_DEFAULT;
-				} else {
+				if (is_broadcast_ether_addr(mac))
 					key_info |= KEY_MCAST;
-				}
+				else
+					key_info |= KEY_UNICAST | KEY_DEFAULT;
 			}
 		}
 		km->key_param_set.key_info = cpu_to_le16(key_info);
-- 
2.8.1


^ permalink raw reply related

* Re: [PATCH 1/1] mwifiex: remove superfluous condition (2)
From: Kalle Valo @ 2016-07-31 12:03 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Amitkumar Karwar, Nishant Sarmukadam, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <1469962177-2432-1-git-send-email-xypron.glpk@gmx.de>

Heinrich Schuchardt <xypron.glpk@gmx.de> writes:

> We are using mac as source address in a memcpy.
> In the lines below we can assume mac is not NULL.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Please try to make the commit titles unique, for example you can use
function names or something else. Adding "(2)" is not the way to do
that.

-- 
Kalle Valo

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox