Linux wireless drivers development
 help / color / mirror / Atom feed
* RE: [PATCH] cfg80211: validate beacon interval for MESH/IBSS
From: Kushwaha, Purushottam @ 2016-08-11  9:56 UTC (permalink / raw)
  To: johannes@sipsolutions.net
  Cc: linux-wireless@vger.kernel.org, Malinen, Jouni,
	Undekari, Sunil Dutt, Kalikot Veetil, Mahesh Kumar,
	Hullur Subramanyam, Amarnath, Kumar, Deepak (QCA)
In-Reply-To: <1470908642-13535-1-git-send-email-pkushwah@qti.qualcomm.com>

Johannes , 
While addressing this change, we could notice that the minimum boundary for beacon interval for the case of IBSS is 1 but for Mesh it is 10. Also this check is not done for AP/P2P GO . 
Any reason why there is a discrepancy among them ? These configurations should be aligned , I guess. 
Shouldn't the minimum value be 10 ? If yes , the boundary check should be moved to cfg80211_validate_beacon_int ? 

Thanks,
Purushottam


-----Original Message-----
From: Kushwaha, Purushottam 
Sent: Thursday, August 11, 2016 3:14 PM
To: johannes@sipsolutions.net
Cc: linux-wireless@vger.kernel.org; Malinen, Jouni <jouni@qca.qualcomm.com>; Undekari, Sunil Dutt <usdutt@qti.qualcomm.com>; Kalikot Veetil, Mahesh Kumar <mkalikot@qca.qualcomm.com>; Hullur Subramanyam, Amarnath <amarnath@qca.qualcomm.com>; Kumar, Deepak (QCA) <djindal@qti.qualcomm.com>; Kushwaha, Purushottam <pkushwah@qti.qualcomm.com>
Subject: [PATCH] cfg80211: validate beacon interval for MESH/IBSS

Beacon interval validation was missing for MESH / IBSS join.
This commit addresses the same.

Signed-off-by: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>
---
 net/wireless/nl80211.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index f02653a..ddb1469 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7780,6 +7780,10 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
 			return -EINVAL;
 	}
 
+	err = cfg80211_validate_beacon_int(rdev, ibss.beacon_interval);
+	if (err)
+		return err;
+
 	if (!rdev->ops->join_ibss)
 		return -EOPNOTSUPP;
 
@@ -9255,6 +9259,10 @@ static int nl80211_join_mesh(struct sk_buff *skb, struct genl_info *info)
 		if (setup.beacon_interval < 10 ||
 		    setup.beacon_interval > 10000)
 			return -EINVAL;
+
+		err = cfg80211_validate_beacon_int(rdev, setup.beacon_interval);
+		if (err)
+			return err;
 	}
 
 	if (info->attrs[NL80211_ATTR_DTIM_PERIOD]) {
--
1.9.1


^ permalink raw reply

* [PATCH] cfg80211: validate beacon interval for MESH/IBSS
From: Purushottam Kushwaha @ 2016-08-11  9:44 UTC (permalink / raw)
  To: johannes
  Cc: linux-wireless, jouni, usdutt, mkalikot, amarnath, djindal,
	pkushwah

Beacon interval validation was missing for MESH / IBSS join.
This commit addresses the same.

Signed-off-by: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>
---
 net/wireless/nl80211.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f02653a..ddb1469 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7780,6 +7780,10 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
 			return -EINVAL;
 	}
 
+	err = cfg80211_validate_beacon_int(rdev, ibss.beacon_interval);
+	if (err)
+		return err;
+
 	if (!rdev->ops->join_ibss)
 		return -EOPNOTSUPP;
 
@@ -9255,6 +9259,10 @@ static int nl80211_join_mesh(struct sk_buff *skb, struct genl_info *info)
 		if (setup.beacon_interval < 10 ||
 		    setup.beacon_interval > 10000)
 			return -EINVAL;
+
+		err = cfg80211_validate_beacon_int(rdev, setup.beacon_interval);
+		if (err)
+			return err;
 	}
 
 	if (info->attrs[NL80211_ATTR_DTIM_PERIOD]) {
-- 
1.9.1


^ permalink raw reply related

* Re: Problems with mwifiex_pcie firmware activation
From: Stanislaw Gruszka @ 2016-08-11  9:01 UTC (permalink / raw)
  To: Amitkumar Karwar; +Cc: Nishant Sarmukadam, linux-wireless@vger.kernel.org
In-Reply-To: <20160623120952.GA11157@redhat.com>

On Thu, Jun 23, 2016 at 02:09:52PM +0200, Stanislaw Gruszka wrote:
> On Thu, Jun 16, 2016 at 01:53:15PM +0200, Stanislaw Gruszka wrote:
> > On Wed, Jun 15, 2016 at 01:47:53PM +0000, Amitkumar Karwar wrote:
> > > Could you please share complete dmesg log for failure and successful cases?
> > 
> > Dmesg from failure case is in attachment. I loose access to system
> > where device initalize, I'll provide missed dmesg when I get back
> > the access.
> 
> Have the access now, dmesg is in attachment.

Any news here ?

Stanislaw

^ permalink raw reply

* Re: [PATCH v3] mac80211: mesh: Add support for HW RC implementation
From: Johannes Berg @ 2016-08-11  8:03 UTC (permalink / raw)
  To: Maxim Altshul, linux-kernel; +Cc: David S. Miller, linux-wireless
In-Reply-To: <20160711141539.30281-1-maxim.altshul@ti.com>

On Mon, 2016-07-11 at 17:15 +0300, Maxim Altshul wrote:
> Mesh HWMP module will be able to rely on the HW
> RC algorithm if it exists, for path metric calculations.
> 
> This allows the metric calculation mechanism to calculate
> a correct metric, based on PER and last TX rate both via
> HW RC algorithm if it exists or via parameters collected
> by the SW.
> 
Applied. I decided to put the fix for the sta->uploaded thing into
mac80211-next, since there's no driver affected by it now, so please
base that patch on this one, if necessary.

johannes

^ permalink raw reply

* Re: pull-request: mac80211 2016-08-05
From: Johannes Berg @ 2016-08-11  7:58 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-wireless
In-Reply-To: <1470400329-23256-1-git-send-email-johannes@sipsolutions.net>

On Fri, 2016-08-05 at 14:32 +0200, Johannes Berg wrote:
> Hi Dave,
> 
> Here's a first set of fixes for the current cycle. See the tag
> message
> for more information.
> 
> I'll probably have a follow-up fix for the real problem in mac80211
> that caused the crash later, but for now we have this patch and it
> makes sense and fixes the crash, even if the behaviour isn't quite
> right (afaict.)
> 

Just FYI, I'm going to "defer" the fix for the wrong behaviour to
mac80211-next since no driver is affected by it.

johannes

^ permalink raw reply

* [PATCH v2] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Wright Feng @ 2016-08-11  5:44 UTC (permalink / raw)
  To: brcm80211-dev-list.pdl, arend.vanspriel, kvalo, franky.lin,
	hante.meuleman, pieterpg, linux-wireless, chln
  Cc: Wright Feng

When stopping hostap on virtual interface, driver will set INFRA and AP
mode that may affect the functionality on primary interface. For example,
if we create and stop hostapd on virtual interface then association
cannot work on primary interface because INFRA mode has been set to IBSS.
Hence we shut down AP and set IBSS mode only on primary interface.

Signed-off-by: Wright Feng <wright.feng@cypress.com>
---
V2:Remove the footnote from the mail.
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e..0687ab9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4716,6 +4716,8 @@ exit:
 
 static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
 {
+	struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
+	struct net_device *primary_ndev = cfg_to_ndev(cfg);
 	struct brcmf_if *ifp = netdev_priv(ndev);
 	s32 err;
 	struct brcmf_fil_bss_enable_le bss_enable;
@@ -4723,7 +4725,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
 
 	brcmf_dbg(TRACE, "Enter\n");
 
-	if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
+	if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
+	    (ndev == primary_ndev)) {
 		/* Due to most likely deauths outstanding we sleep */
 		/* first to make sure they get processed by fw. */
 		msleep(400);
-- 
1.9.1


^ permalink raw reply related

* RE: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Wright Feng @ 2016-08-11  4:53 UTC (permalink / raw)
  To: Arend Van Spriel, brcm80211-dev-list.pdl@broadcom.com,
	franky.lin@broadcom.com, hante.meuleman@broadcom.com,
	pieterpg@broadcom.com, Chi-Hsien Lin,
	linux-wireless@vger.kernel.org, kvalo@codeaurora.org
In-Reply-To: <cab5a204-3487-ebe6-7e82-ca6dbb04ff40@broadcom.com>



On 10-8-2016 12:15, Arend Van Spriel wrote:
> On 10-8-2016 11:44, Wright Feng wrote:
> > Hi Arend,
> >
> > Thanks for the reply.
> >
> > On 10-8-2016 10:26, Arend Van Spriel wrote:
> >> On 10-8-2016 10:01, Wright Feng wrote:
> >>> When stopping hostap on virtual interface, driver will set INFRA and
> >>> AP mode that may affect the functionality on primary interface. For
> >>> example, if we create and stop hostapd on virtual interface then
> >>> association cannot work on primary interface because INFRA mode has
> >> been set to IBSS.
> >>> Hence we shut down AP and set IBSS mode only on primary interface.
> >>
> >> What is actually the use-case here. Can you elaborate? BRCMF_C_DOWN
> >> command turns out to be effectively bring the whole stack down and
> >> not just the supplied interface. I suppose you are hitting that issue here as
> well, right?
> > We want to use AP mode to let client connecting in AP+STA mode with
> 43438 wi-fi chip.
> > After that, the AP mode will be stopped, and wpa_supplicant cannot
> associate to the access point.
> > I describe the steps in detail as below.
> > 1. Create virtual interface and set mode to __ap 2. start
> > wpa_supplicant on primary interface and connect to wireless router.
> > 3. start hostap daemon on virtual interface and let client connecting.
> > 4. stop hostap daemon
> > 5. wpa_supplicant cannot associate to access point normally.
> >
> > Like you said, the issue may be hit by BRCMF_C_DOWN and same as
> > BRCMF_C_SET_INFRA BRCMF_C_SET_INFRA is not just for the supplied
> interface either. The default bss will be changed in firmware and let firmware
> uses IBSS mode to join.
> > About BRCMF_C_DOWN, driver will set BRCMF_C_UP command to bring
> device back up so it can be used again.
> > However, there is no way to set INFRA mode back except starting AP mode
> again.
>
> Ok.
>
> >>
> >> Regards,
> >> Arend
> >>
> >>> Signed-off-by: Wright Feng <wright.feng@cypress.com>
> >>> ---
> >>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5
> >> ++++-
> >>>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git
> >>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >>> index 2628d5e..0687ab9 100644
> >>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >>> @@ -4716,6 +4716,8 @@ exit:
> >>>
> >>>  static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
> >>> net_device *ndev)  {
> >>> +       struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
> >>> +       struct net_device *primary_ndev = cfg_to_ndev(cfg);
> >>>         struct brcmf_if *ifp = netdev_priv(ndev);
> >>>         s32 err;
> >>>         struct brcmf_fil_bss_enable_le bss_enable; @@ -4723,7
> >>> +4725,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy,
> >>> struct net_device *ndev)
> >>>
> >>>         brcmf_dbg(TRACE, "Enter\n");
> >>>
> >>> -       if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
> >>> +       if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
> >>> +           (ndev == primary_ndev)) {
> >>>                 /* Due to most likely deauths outstanding we sleep */
> >>>                 /* first to make sure they get processed by fw. */
> >>>                 msleep(400);
> >>> --
> >>> 1.9.1
> >>>
> >>>
> >>> This message and any attachments may contain Cypress (or its
> >>> subsidiaries)
> >> confidential information. If it has been received in error, please
> >> advise the sender and immediately delete this message.
> >>>
> >
> > This message and any attachments may contain Cypress (or its subsidiaries)
> confidential information. If it has been received in error, please advise the
> sender and immediately delete this message.
>
> Is there any way for you to get rid of this foot note. It may keep Kalle from
> taking this patch. Other option is to take this patch through our internal tree.
No problem. I will remove the footnote from the PATCH v2.
>
> Regards,
> Arend

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: miaoqing @ 2016-08-11  2:54 UTC (permalink / raw)
  To: Jason Cooper
  Cc: Pan, Miaoqing, Stephan Mueller, Herbert Xu, Matt Mackall,
	Valo, Kalle, linux-wireless, ath9k-devel, linux-crypto,
	Sepehrdad, Pouyan
In-Reply-To: <20160810132449.GH2013@io.lakedaemon.net>

Hi Jason,

On 2016-08-10 21:24, Jason Cooper wrote:
> *gentle reminder: others are reading which may not be directly included
> in the conversation.  Including the archives.  Please avoid top 
> posting.
> :)

Thanks:)

> The fact is, barring userspace expectations of /dev/hwrng, hw_random is
> the appropriate place for it.  It's not a devicetree blob, mac address,
> or pci config space.  Which are things we feed in once for the heck of
> it.  This is a *continuous* source or questionable quality.
> 
> I'm seriously considering putting this and timeriomem-rng into a
> subdirectory under hw_random/, maybe environ/.  Anything in there gets
> quality=0 for default, and *doesn't* contribute to /dev/hwrng.
> 
> Regardless which path we take, I think we should include 'adc' in the
> name.  I've heard countless times about "Atheros cards come with an rng
> on board". :-/

If I understand correctly, you want to bind the ADC source to 
/dev/hwrng,
and then change rng-tools to set the entropy to zero in the ioctl call ?
There are two major problems with that approach,

1) We already tried once before to bind our solution to /dev/hwrng, and 
got
so much complaints. The conclusion was that maybe we know that the 
output of
/dev/hwrng does not have perfect entropy, but a normal user does not 
know and
will misuse it. You mentioned in 
https://www.kernel.org/doc/Documentation/hw_random.txt
we have

"This data is NOT CHECKED by any
	fitness tests, and could potentially be bogus (if the
	hardware is faulty or has been tampered with).  Data is only
	output if the hardware "has-data" flag is set, but nevertheless
	a security-conscious person would run fitness tests on the
	data before assuming it is truly random."

But this is not enough to convince upstream to switch to /dev/hwrng. I 
think the
concern of users misusing the solution is a very valid concern.

2) If we set the entropy to zero in rng-tools, we cannot tolerate the 
load.
Rng-tools is not a timer-based solution. Similar to our solution, it is 
based on
/proc/sys/kernel/random/write_wakeup_threshold. If we do not increase 
the entropy
counter, rng-tools keep writing into the pool, and both rng-tools and 
WiFi chip will
be overloaded.


Thanks,
Miaoqing

^ permalink raw reply

* Re: [PATCH v2] RANDOM: ATH9K RNG delivers zero bits of entropy
From: Theodore Ts'o @ 2016-08-10 23:44 UTC (permalink / raw)
  To: Jason Cooper
  Cc: Pan, Miaoqing, Stephan Mueller, Sepehrdad, Pouyan,
	herbert@gondor.apana.org.au, linux-kernel@vger.kernel.org,
	linux-crypto@vger.kernel.org, ath9k-devel,
	linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org,
	Kalle Valo
In-Reply-To: <20160809140444.GB2013@io.lakedaemon.net>

On Tue, Aug 09, 2016 at 02:04:44PM +0000, Jason Cooper wrote:
> 
> iiuc, Ted, you're saying using the hw_random framework would be
> disasterous because despite most drivers having a default quality of 0,
> rngd assumes 1 bit of entropy for every bit read?

Sorry, what I was trying to say (but failed) was that bypassing the
hwrng framework and injecting entropy directly the entropy pool was
disatrous.

> Thankfully, most hw_random drivers don't set the quality.  So unless the
> user sets the default_quality param, it's zero.

The fact that this is "most" and not "all" does scare me a little.

As far as I'm concerned *all* hw_random drivers should set quality to
zero, since it should be up to the system administrator.  Perhaps the
one exception is virtio_rng, since if you don't trust the hypvervisor,
the security of the VM is hopeless.  That being said, I have seen
configurations of KVM which use:

	-object rng-random,filename=/dev/urandom,id=rng0 \
	-device virtio-rng-pci,rng=rng0

Which is somewhat non-ideal.  (Try running od -x /dev/random on such a
guest system....)

					- Ted


^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Jason Cooper @ 2016-08-10 13:24 UTC (permalink / raw)
  To: Pan, Miaoqing
  Cc: Stephan Mueller, Herbert Xu, Matt Mackall,
	miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, Sepehrdad, Pouyan
In-Reply-To: <389c3c1fdde2447aacf31a8b4aadfc08@aptaiexm02f.ap.qualcomm.com>

Hi Miaoqing Pan, Stephen,

*gentle reminder: others are reading which may not be directly included
in the conversation.  Including the archives.  Please avoid top posting.
:)

On Wed, Aug 10, 2016 at 07:43:45AM +0000, Pan, Miaoqing wrote:
> The problem with using the add_device_randomness is that we do not
> know when to call that API, and we have to make our solution either
> timer-based or interrupt based, which is not really the correct way of
> implementing this feature.

Exactly.  I think we're dancing around the problem by discussing the
quality of what the ath9k adc provides.

The fact is, barring userspace expectations of /dev/hwrng, hw_random is
the appropriate place for it.  It's not a devicetree blob, mac address,
or pci config space.  Which are things we feed in once for the heck of
it.  This is a *continuous* source or questionable quality.

I'm seriously considering putting this and timeriomem-rng into a
subdirectory under hw_random/, maybe environ/.  Anything in there gets
quality=0 for default, and *doesn't* contribute to /dev/hwrng.

Regardless which path we take, I think we should include 'adc' in the
name.  I've heard countless times about "Atheros cards come with an rng
on board". :-/

thx,

Jason.

^ permalink raw reply

* RE: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Wright Feng @ 2016-08-10  9:44 UTC (permalink / raw)
  To: Arend Van Spriel, brcm80211-dev-list.pdl@broadcom.com,
	franky.lin@broadcom.com, hante.meuleman@broadcom.com,
	pieterpg@broadcom.com, Chi-Hsien Lin,
	linux-wireless@vger.kernel.org, kvalo@codeaurora.org
In-Reply-To: <4d60ec5e-6462-372a-2d3c-5770493e7e98@broadcom.com>

Hi Arend,

Thanks for the reply.

On 10-8-2016 10:26, Arend Van Spriel wrote:
> On 10-8-2016 10:01, Wright Feng wrote:
> > When stopping hostap on virtual interface, driver will set INFRA and
> > AP mode that may affect the functionality on primary interface. For
> > example, if we create and stop hostapd on virtual interface then
> > association cannot work on primary interface because INFRA mode has
> been set to IBSS.
> > Hence we shut down AP and set IBSS mode only on primary interface.
>
> What is actually the use-case here. Can you elaborate? BRCMF_C_DOWN
> command turns out to be effectively bring the whole stack down and not just
> the supplied interface. I suppose you are hitting that issue here as well, right?
We want to use AP mode to let client connecting in AP+STA mode with 43438 wi-fi chip.
After that, the AP mode will be stopped, and wpa_supplicant cannot associate to the access point.
I describe the steps in detail as below.
1. Create virtual interface and set mode to __ap
2. start wpa_supplicant on primary interface and connect to wireless router.
3. start hostap daemon on virtual interface and let client connecting.
4. stop hostap daemon
5. wpa_supplicant cannot associate to access point normally.

Like you said, the issue may be hit by BRCMF_C_DOWN and same as BRCMF_C_SET_INFRA
BRCMF_C_SET_INFRA is not just for the supplied interface either. The default bss will be changed in firmware and let firmware uses IBSS mode to join.
About BRCMF_C_DOWN, driver will set BRCMF_C_UP command to bring device back up so it can be used again.
However, there is no way to set INFRA mode back except starting AP mode again.
>
> Regards,
> Arend
>
> > Signed-off-by: Wright Feng <wright.feng@cypress.com>
> > ---
> >  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5
> ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > index 2628d5e..0687ab9 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > @@ -4716,6 +4716,8 @@ exit:
> >
> >  static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
> > net_device *ndev)  {
> > +       struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
> > +       struct net_device *primary_ndev = cfg_to_ndev(cfg);
> >         struct brcmf_if *ifp = netdev_priv(ndev);
> >         s32 err;
> >         struct brcmf_fil_bss_enable_le bss_enable; @@ -4723,7 +4725,8
> > @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
> > net_device *ndev)
> >
> >         brcmf_dbg(TRACE, "Enter\n");
> >
> > -       if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
> > +       if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
> > +           (ndev == primary_ndev)) {
> >                 /* Due to most likely deauths outstanding we sleep */
> >                 /* first to make sure they get processed by fw. */
> >                 msleep(400);
> > --
> > 1.9.1
> >
> >
> > This message and any attachments may contain Cypress (or its subsidiaries)
> confidential information. If it has been received in error, please advise the
> sender and immediately delete this message.
> >

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

^ permalink raw reply

* Re: mwifiex: PCIe8997 chip specific handling
From: Steve deRosier @ 2016-08-10 21:08 UTC (permalink / raw)
  To: Amitkumar Karwar
  Cc: Brian Norris, linux-wireless@vger.kernel.org, Cathy Luo,
	Nishant Sarmukadam, linux-kernel@vger.kernel.org, Wei-Ning Huang
In-Reply-To: <9bc980fff61541c2879d5d1dacda7462@SC-EXCH04.marvell.com>

Hi,

On Wed, Aug 10, 2016 at 12:07 AM, Amitkumar Karwar <akarwar@marvell.com> wrote:
> Hi Brian,
>
>> From: Brian Norris [mailto:briannorris@chromium.org]
>> Sent: Wednesday, August 10, 2016 12:14 AM
>> To: Amitkumar Karwar
>> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
>> linux-kernel@vger.kernel.org
>> Subject: Re: mwifiex: PCIe8997 chip specific handling
>>
>> Hi,
>>
>> On Fri, Jul 29, 2016 at 04:08:51PM +0530, Amitkumar Karwar wrote:
>> > The patch corrects the revision id register and uses it along with
>> > magic value and chip version registers to download appropriate
>> > firmware image.
>> >
>> > PCIe8997 Z chipset variant code has been removed, as it won't be used
>> > in production.
>> >
>> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
>> > ---
>> >  drivers/net/wireless/marvell/mwifiex/pcie.c | 35
>> > ++++++++++-------------------
>> > drivers/net/wireless/marvell/mwifiex/pcie.h | 14 +++++-------
>> >  2 files changed, 18 insertions(+), 31 deletions(-)
>>
>> [...]
>>
>> > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h
>> > b/drivers/net/wireless/marvell/mwifiex/pcie.h
>> > index f6992f0..46f99ca 100644
>> > --- a/drivers/net/wireless/marvell/mwifiex/pcie.h
>> > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.h
>> > @@ -32,12 +32,9 @@
>> >  #define PCIE8897_DEFAULT_FW_NAME "mrvl/pcie8897_uapsta.bin"
>> >  #define PCIE8897_A0_FW_NAME "mrvl/pcie8897_uapsta_a0.bin"
>> >  #define PCIE8897_B0_FW_NAME "mrvl/pcie8897_uapsta.bin"
>> > -#define PCIE8997_DEFAULT_FW_NAME "mrvl/pcieusb8997_combo_v2.bin"
>> > -#define PCIEUART8997_FW_NAME_Z "mrvl/pcieuart8997_combo.bin"
>> > -#define PCIEUART8997_FW_NAME_V2 "mrvl/pcieuart8997_combo_v2.bin"
>> > -#define PCIEUSB8997_FW_NAME_Z "mrvl/pcieusb8997_combo.bin"
>> > -#define PCIEUSB8997_FW_NAME_V2 "mrvl/pcieusb8997_combo_v2.bin"
>> > -#define PCIE8997_DEFAULT_WIFIFW_NAME "mrvl/pcie8997_wlan.bin"
>> > +#define PCIEUART8997_FW_NAME_V4 "mrvl/pcieuart8997_combo_v4.bin"
>> > +#define PCIEUSB8997_FW_NAME_V4 "mrvl/pcieusb8997_combo_v4.bin"
>> > +#define PCIE8997_DEFAULT_WIFIFW_NAME "mrvl/pcie8997_wlan_v4.bin"
>>
>> Why do version bumps require firmware renames? Is this just to make sure
>> you don't load the new firmware on old chip revs that you don't plan to
>> support for production (i.e., only early revs like the _Z you're
>> dropping)? This doesn't seems like a good long-term solution, at least
>> once you start getting this silicon out in the wild. At some point, I'd
>> expect to see a stable file name.
>>
>> Brian
>>
>
> We haven't yet submitted any firmware image upstream for 8997 chipset.
> pcieuart8997_combo_v4.bin/pcieusb8997_combo_v4.bin would be our firmware candidate for upstream submission. The filename would remain same hereafter.
>
> pcie*8997_combo_v2.bin had support only for A0 chipset
> pcie*8997_combo_v3.bin was our internal development version which had support for A1 chipset
> pcie*8997_combo_v4.bin has support for both A0 and A1 chipsets and this is the version that shall be released to customers/upstream from now on.
>

Seems to me then it should just be named pcie*8997_wlan.bin.  A
version number shouldn't be part of the file name in this case. Having
to update the driver for a firmware name change is silly. Most
wireless drivers have different names for different hardware/chip revs
and/or an incompatible API change.  Most distributions would typically
only carry a single instance of the firmware for a particular chip.
Speaking for the ones I work with, I usually keep the original
filename intact (with a version number) and make a symlink to it with
the name the driver expects.  eg:

fw-4.bin -> fw_v3.4.0.94.bin
fw_v3.2.0.144.bin
fw_v3.4.0.94.bin

That way I can keep track of the version in my filesystem, but I'm not
hacking the driver every couple of weeks.  And we do issue new
firmware every few weeks. I can't imagine asking our customers to keep
updating the driver for each firmware enhancement.

IMHO changing the driver to rename the firmwares on new versions seems
both inconvenient to people using it, and extra non-useful commit
noise.

- Steve

^ permalink raw reply

* [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Wright Feng @ 2016-08-10  8:01 UTC (permalink / raw)
  To: brcm80211-dev-list.pdl, arend.vanspriel, franky.lin,
	hante.meuleman, pieterpg, chln, linux-wireless, kvalo
  Cc: wright.feng

When stopping hostap on virtual interface, driver will set INFRA and AP
mode that may affect the functionality on primary interface. For example,
if we create and stop hostapd on virtual interface then association
cannot work on primary interface because INFRA mode has been set to IBSS.
Hence we shut down AP and set IBSS mode only on primary interface.

Signed-off-by: Wright Feng <wright.feng@cypress.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e..0687ab9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4716,6 +4716,8 @@ exit:

 static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
 {
+       struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
+       struct net_device *primary_ndev = cfg_to_ndev(cfg);
        struct brcmf_if *ifp = netdev_priv(ndev);
        s32 err;
        struct brcmf_fil_bss_enable_le bss_enable;
@@ -4723,7 +4725,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)

        brcmf_dbg(TRACE, "Enter\n");

-       if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
+       if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
+           (ndev == primary_ndev)) {
                /* Due to most likely deauths outstanding we sleep */
                /* first to make sure they get processed by fw. */
                msleep(400);
--
1.9.1


This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

^ permalink raw reply related

* Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Arend Van Spriel @ 2016-08-10  8:26 UTC (permalink / raw)
  To: Wright Feng, brcm80211-dev-list.pdl, franky.lin, hante.meuleman,
	pieterpg, chln, linux-wireless, kvalo
In-Reply-To: <20160810080100.GA10783@cypress.com>

On 10-8-2016 10:01, Wright Feng wrote:
> When stopping hostap on virtual interface, driver will set INFRA and AP
> mode that may affect the functionality on primary interface. For example,
> if we create and stop hostapd on virtual interface then association
> cannot work on primary interface because INFRA mode has been set to IBSS.
> Hence we shut down AP and set IBSS mode only on primary interface.

What is actually the use-case here. Can you elaborate? BRCMF_C_DOWN
command turns out to be effectively bring the whole stack down and not
just the supplied interface. I suppose you are hitting that issue here
as well, right?

Regards,
Arend

> Signed-off-by: Wright Feng <wright.feng@cypress.com>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 2628d5e..0687ab9 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -4716,6 +4716,8 @@ exit:
> 
>  static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
>  {
> +       struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
> +       struct net_device *primary_ndev = cfg_to_ndev(cfg);
>         struct brcmf_if *ifp = netdev_priv(ndev);
>         s32 err;
>         struct brcmf_fil_bss_enable_le bss_enable;
> @@ -4723,7 +4725,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
> 
>         brcmf_dbg(TRACE, "Enter\n");
> 
> -       if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
> +       if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
> +           (ndev == primary_ndev)) {
>                 /* Due to most likely deauths outstanding we sleep */
>                 /* first to make sure they get processed by fw. */
>                 msleep(400);
> --
> 1.9.1
> 
> 
> This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
> 

^ permalink raw reply

* RE: [PATCH v4] cfg80211: Provision to allow the support for different beacon intervals
From: Undekari, Sunil Dutt @ 2016-08-10 14:53 UTC (permalink / raw)
  To: Johannes Berg, Kushwaha, Purushottam
  Cc: linux-wireless@vger.kernel.org, Malinen, Jouni,
	Kondabattini, Ganesh, Kalikot Veetil, Mahesh Kumar,
	Hullur Subramanyam, Amarnath, Kumar, Deepak (QCA)
In-Reply-To: <1470729393.28531.11.camel@sipsolutions.net>

PiBJIGRvbid0IGtub3cgd2hhdCB5b3VyIGZpcm13YXJlL2hhcmR3YXJlIGNhcGFiaWxpdGllcyBh
cmUgOikNCkkgd2FzIG5vdCByZWZlcnJpbmcgdG8gYW55IHNwZWNpZmljIGFyY2hpdGVjdHVyZSBo
ZXJlLiAgV2l0aCB0aGUgMiBjaG9pY2VzIHlvdSBoYWQgbWVudGlvbmVkIChtZXNoIG11c3QgbWF0
Y2ggZWl0aGVyIG9uZSBvZiB0aGVtIC8gbWFrZSB0aGlzIGFwcGx5IG9uIG1lc2ggYW5kIElCU1Mg
YXMgd2VsbCApIHdlIHNlZSB0aGF0IHRoaXMgbmV3IGZsYWcgYmVpbmcgZGVmaW5lZCBpbiB0aGUg
aW50ZXJmYWNlIGNvbWJpbmF0aW9ucyBjb3VsZCBiZXR0ZXIgYmUgYXBwbGljYWJsZSBmb3IgYWxs
IHRoZSBiZWFjb25pbmcgaW50ZXJmYWNlcy4gDQpJZiB3ZSBoYWQgcmVzdHJpY3RlZCB0aGlzIHRv
IEFQIC8gUDJQIEdPIGFuZCBoYXZlIE1lc2ggbWF0Y2ggZWl0aGVyIG9mIHRoZW0gLCBhIG5ldyBy
ZXF1ZXN0ICggaW4gZnV0dXJlID8gKSB0byBoYXZlIGEgZGlmZmVyZW50IGludGVydmFsIGZvciBN
ZXNoLCBJTU8gIHdvdWxkIGFzayBhIG5lZWQgZm9yIG5ldyBmbGFnICwgaXNuJ3QgPyAgT3VyIGlu
dGVudGlvbiBoZXJlIHdhcyB0byBhdm9pZCB0aGlzIGJ5IGVuc3VyaW5nIHRoYXQgdGhpcyBmbGFn
IGlzIGFwcGxpY2FibGUgZm9yIGFsbCB0aGUgYmVhY29uaW5nIGludGVyZmFjZSBjb21iaW5hdGlv
bnMuIA0KDQo+SSdkIGFyZ3VlIHRoYXQgaW5zdGVhZCBvZiBoYXZpbmcgdGhlIGludGVyZmFjZSBj
b21iaW5hdGlvbnMgZmxhZywgdGhhdA0KPm5sODAyMTEgYXR0cmlidXRlIGNvdWxkIGNhcnJ5IHRo
ZSBHQ0Q/DQpJZiB5b3VyIHNheSBoZXJlIGlzIHRvIGp1c3QgdXNlIHRoZSB3aXBoeSBwYXJhbWV0
ZXIgcmF0aGVyIHRoYW4gaGF2aW5nIHRoZSBmbGFnIGluIHRoZSBpbnRlcmZhY2UgY29tYmluYXRp
b25zLCBzaWduaWZ5aW5nIHRoZSBzdXBwb3J0IGZvciBkaWZmZXJlbnQgYmVhY29uIGludGVydmFs
cyAsIFllcyB3ZSB0b28gYWdyZWUgdG8geW91ciBwb2ludC4gDQpUaGlzIGlzIGNvbnNpZGVyaW5n
IHRoZSBmYWN0IHRoYXQgdGhlIGN1cnJlbnQgaW1wbGVtZW50YXRpb24gcmVzdHJpY3RzIHRoZSB2
YWxpZGF0aW9uIG9mIHRoZSBiZWFjb24gaW50ZXJ2YWwgKGNmZzgwMjExX3ZhbGlkYXRlX2JlYWNv
bl9pbnQgKSB0byBvbmx5IEFQIC8gUDJQIEdPLiANCk5laXRoZXIgdGhlIE1lc2ggLyBJQlNTICgg
am9pbiApIGhhdmUgdGhpcyByZXN0cmljdGlvbiAuIE5vdCBzdXJlIHdoeSB0aGlzIGlzIG5vdCBp
bXBvc2VkIGZvciBNZXNoIC8gSUJTUyB0aG91Z2guIA0KQWZ0ZXIgaW50cm9kdWNpbmcgdGhpcyBm
bGFnIGluIHRoZSBpbnRlcmZhY2UgY29tYmluYXRpb25zLCB3ZSBkbyBmZWVsIHRoYXQgY3VycmVu
dGx5IGl0IGlzIG5vdCBnb2luZyB0byBhcHBseSBmb3IgTWVzaCAvIElCU1MgLCB1bmxlc3MgdGhl
cmUgaXMgYSBmdXR1cmUgbmVlZCB0byBkbyBzbyAsIHdoaWNoIHdlIGFyZSBub3QgYXdhcmUgb2Yg
LiANCldpdGggdGhlIGN1cnJlbnQgcmVzdHJpY3Rpb24gb2YgdmFsaWRhdGluZyB0aGUgYmVhY29u
IGludGVydmFsIG9ubHkgZm9yIEFQIC8gUDJQIEdPICwgd2UgZG8gYWdyZWUgdG8gaGF2ZSBhIHNp
bmdsZSBwYXJhbWV0ZXIgYWR2ZXJ0aXNlZCBieSB0aGUgZHJpdmVycyBpbnRlbmRpbmcgYm90aCAt
IHN1cHBvcnQgdGhlIGRpZmZlcmVudCBiZWFjb24gaW50ZXJ2YWxzIGFuZCBhbHNvIHNpZ25pZnkg
dGhlIEdDRCBhbW9uZyB0aGVtIChhIG5vbi16ZXJvIHZhbHVlIGZvciAibWluX2RpZmZfYmVhY29u
X2ludGVydmFsX211bHRpcGxpZXIiIHNob3VsZCBhZGRyZXNzIHRoaXMgLCBpc24ndCApLiANCg0K
UmVnYXJkcywNClN1bmlsDQoNCg0KDQotLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTog
Sm9oYW5uZXMgQmVyZyBbbWFpbHRvOmpvaGFubmVzQHNpcHNvbHV0aW9ucy5uZXRdIA0KU2VudDog
VHVlc2RheSwgQXVndXN0IDksIDIwMTYgMToyNyBQTQ0KVG86IFVuZGVrYXJpLCBTdW5pbCBEdXR0
IDx1c2R1dHRAcXRpLnF1YWxjb21tLmNvbT47IEt1c2h3YWhhLCBQdXJ1c2hvdHRhbSA8cGt1c2h3
YWhAcXRpLnF1YWxjb21tLmNvbT4NCkNjOiBsaW51eC13aXJlbGVzc0B2Z2VyLmtlcm5lbC5vcmc7
IE1hbGluZW4sIEpvdW5pIDxqb3VuaUBxY2EucXVhbGNvbW0uY29tPjsgS29uZGFiYXR0aW5pLCBH
YW5lc2ggPGdhbmVzaGtAcXRpLnF1YWxjb21tLmNvbT47IEthbGlrb3QgVmVldGlsLCBNYWhlc2gg
S3VtYXIgPG1rYWxpa290QHFjYS5xdWFsY29tbS5jb20+OyBIdWxsdXIgU3VicmFtYW55YW0sIEFt
YXJuYXRoIDxhbWFybmF0aEBxY2EucXVhbGNvbW0uY29tPjsgS3VtYXIsIERlZXBhayAoUUNBKSA8
ZGppbmRhbEBxdGkucXVhbGNvbW0uY29tPg0KU3ViamVjdDogUmU6IFtQQVRDSCB2NF0gY2ZnODAy
MTE6IFByb3Zpc2lvbiB0byBhbGxvdyB0aGUgc3VwcG9ydCBmb3IgZGlmZmVyZW50IGJlYWNvbiBp
bnRlcnZhbHMNCg0KDQo+IEkgZ3Vlc3MgLCB3ZSBjYW4gZXh0ZW5kIHRoaXMgdG8gbWVzaCBhbmQg
SUJTUyBhcyB3ZWxsLg0KDQpJIGRvbid0IGtub3cgd2hhdCB5b3VyIGZpcm13YXJlL2hhcmR3YXJl
IGNhcGFiaWxpdGllcyBhcmUgOikNCg0KPiA+IEl0IHNlZW1zIHRvIG1lIHRoYXQgaWYgSSB3ZXJl
IHRvIHNwZWNpZnkgYmVhY29uIGludGVydmFscyB3aGljaCBoYXZlIA0KPiA+IGEgdmVyeSBzbWFs
bCBHQ0QsIHlvdSdsbCBydW4gaW50byB0cm91YmxlIHdoZW4gYWN0dWFsbHkgc2VuZGluZyANCj4g
PiBiZWFjb25zLg0KPiA+IFBlcmhhcHMgdGhlcmUgc2hvdWxkIGJlIGEgcmVxdWlyZW1lbnQgb24g
dGhlIEdDRD8NCj4gQ2FuIHdlIGhhdmUgdGhpcyBwdWJsaXNoZWQgYnkgdGhlIGhvc3QgZHJpdmVy
cyB0aHJvdWdoIGEgbmV3IHdpcGh5IA0KPiBwYXJhbWV0ZXIgLCBzYXkgIm1pbl9kaWZmX2JlYWNv
bl9pbnRlcnZhbF9tdWx0aXBsaWVyIi4gVGhpcyBzZXQncyB0aGUgDQo+IGV4cGVjdGF0aW9uIHRo
YXQgYW55IGRpZmZlcmVudCBiZWFjb24gaW50ZXJ2YWxzIG9uIHRoZSB3aXBoecKgwqBzaGFsbCBi
ZSANCj4gYSBtdWx0aXBsZSBvZiB0aGlzIHBhcmFtZXRlciB3aGljaCBpcyBhZHZlcnRpc2VkIGJ5
IHRoZSBob3N0IGRyaXZlciAsIA0KPiBpc24ndCA/DQo+IA0KSSdkIGFyZ3VlIHRoYXQgaW5zdGVh
ZCBvZiBoYXZpbmcgdGhlIGludGVyZmFjZSBjb21iaW5hdGlvbnMgZmxhZywgdGhhdA0Kbmw4MDIx
MSBhdHRyaWJ1dGUgY291bGQgY2FycnkgdGhlIEdDRD8NCg0Kam9oYW5uZXMNCg==

^ permalink raw reply

* Re: [PATCH v2 8/8] p54: convert to sysdata API
From: Luis R. Rodriguez @ 2016-08-10 19:40 UTC (permalink / raw)
  To: Arend Van Spriel, Luis R. Rodriguez, Linus Torvalds,
	Bjorn Andersson, Daniel Vetter, Kalle Valo, Ming Lei,
	Greg Kroah-Hartman, Daniel Wagner, Jeff Mahoney, Takashi Iwai,
	Hauke Mehrtens, Vikram Mulukutla, Stephen Boyd,
	Christian Lamparter, Andy Lutomirski, Jonathan Corbet,
	Julia Lawall, Tom Gundersen, Kay Sievers, David Woodhouse,
	Andrew Morton, Tejun Heo, Josh Boyer, Michal Marek, David Howells,
	Mimi Zohar, Johannes Berg, Daniel Vetter, Abhay_Salunke,
	Dmitry Torokhov, Wu Fengguang, Kees Cook, Jiri Slaby,
	Gilles.Muller, Linux Kernel Mailing List, linux-wireless,
	Borislav Petkov, Richard Purdie, nicolas.palix
In-Reply-To: <20160810191708.GO9347@sirena.org.uk>

On Wed, Aug 10, 2016 at 08:17:08PM +0100, Mark Brown wrote:
> On Wed, Aug 10, 2016 at 09:04:38PM +0200, Arend Van Spriel wrote:
> 
> > So why would drivers want the devm solution anyway. Once firmware is
> > loaded in the device it can be freed or do you expect device drivers to
> > keep the firmware/sysdata for suspend/resume scenario as some do because
> > of firmware cache behaviour. Does the "rootfs ready" event cover
> > suspend/resume?
> 
> There are classes of devices that spend a large proportion of their time
> powered off and are only powered up and have firmware loaded when they
> are actively in use.  DSPs used in audio systems are one example of
> this, I'd not be surprised to learn that similar things are done with
> video.  It's too expensive to keep the device powered up and you may be
> swapping between firmwares depending on use case anyway for a lot of
> these devices.

devm use and probe is orthogonal to this use case and suspend/resume.
In any case the proposed new API simply allows the driver caller to
either free the firmware after the consumer callback or to keep it.
It does this without devm, so allows this feature without the API
being used on probe. The point about devm was that it would only
beneficial to us if most users for the API were on probe. Clearly
that's not the case and in fact unless async probe is used that use
is likely buggy as it would delay boot further.

If having a local cache of firmware *beyond* the suspend / resume
case is an optimization that can help we should look into that, but
my preference would be to peg this onto the API through an optional
setting in the request passed.

  Luis

^ permalink raw reply

* Re: [PATCH v2 8/8] p54: convert to sysdata API
From: Luis R. Rodriguez @ 2016-08-10 19:34 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Luis R. Rodriguez, Linus Torvalds, Bjorn Andersson, Daniel Vetter,
	Kalle Valo, Ming Lei, Greg Kroah-Hartman, Daniel Wagner,
	Jeff Mahoney, Takashi Iwai, Hauke Mehrtens, Vikram Mulukutla,
	Stephen Boyd, Christian Lamparter, Andy Lutomirski,
	Jonathan Corbet, Julia Lawall, Tom Gundersen, Kay Sievers,
	David Woodhouse, Andrew Morton, Tejun Heo, Josh Boyer,
	Michal Marek, David Howells, Mimi Zohar, Johannes Berg,
	Daniel Vetter, Abhay_Salunke, Dmitry Torokhov, Wu Fengguang,
	Mark Brown, Kees Cook, Jiri Slaby, Gilles.Muller,
	Linux Kernel Mailing List, linux-wireless, Borislav Petkov,
	Richard Purdie, nicolas.palix
In-Reply-To: <7451c96a-b4f5-e629-a4a7-76d6fe1772e9@broadcom.com>

On Wed, Aug 10, 2016 at 09:04:38PM +0200, Arend Van Spriel wrote:
> On 10-8-2016 20:32, Luis R. Rodriguez wrote:
> > On Fri, Jun 17, 2016 at 08:35:03PM +0200, Luis R. Rodriguez wrote:
> >> On Thu, Jun 16, 2016 at 05:09:30PM -1000, Linus Torvalds wrote:
> >>> On Thu, Jun 16, 2016 at 3:36 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> >>>>
> >>>> Reason this could not wait is folks seem to want to keep extending the API,
> >>>> which is another reason for this, do we want to put an end to an unflexible
> >>>> API now or should we wait ?
> 
> [big snip]
> 
> > 
> > Regarding this -- Dmitry recenlty noted devm only works if used on the probe
> > path, and as we now determined, we don't want firmware loading on probe [3], unless
> > async probe is used, so this would make a devm solution here not ideal for
> > freeing the firmware. Even if you use async probe -- that seems very special
> > use case and adding devm support for the firmware API just for that seems silly.
> 
> So why would drivers want the devm solution anyway.

It depends on the use case, some may want to keep the firmware around,
some may not, but by default the new API ssumes you will not (keep = false)
and we free it. My point above was that using devm will not typically
be the most fruitful solution to free the firmware givne that there are only
a few drivers that should need it upon probe, and drivers using firmware
APIs on probe shoudld be using async probe anyway to delay avoid delaying
boot as processing the firmware can take time.

> Once firmware is
> loaded in the device it can be freed or do you expect device drivers to
> keep the firmware/sysdata for suspend/resume scenario as some do because
> of firmware cache behaviour.

You would think! Upon careful inspection there are tons of odd things drivers
do, some modify the firmware and therefore have their own reasons to keep
it. Whatever the reasons are -- I recall seeing a few well justified uses.

> Does the "rootfs ready" event cover
> suspend/resume?

The "rootfs ready" is orthogonal to suspend/resume case given the firmware_class
cache firmware feature.

The firmware API as-is upstream already has a cache firmware solution added
a long time ago, as reflected by a resent patch set (before this one) I
updated documentation for this given its not clearly well known and people
keep adding their own caching solutions, the firmware API requests firmware
prior to suspend so that upon resume the firmware is present, precisely to
avoid race issues. I will note that this feature is only for non-usermode helper
firmware API, upon suspend we *kill* any pending user mode helper requests
given that this can delay suspend, as such drivers using or relying (only
2) on the user mode helper for firmware have no solution built in for
the cache stuff and I can't say I care given there seems to be no valid
modern use case given as a requirement for it yet. In fact there was a bug
in the firmware_class code that *allows* the cache call to request the
usermode helper, obviously that's buggy if we are trying to kill the
usermode helper upon suspend...  so a pending patch fixes that. That's been
a long standing bug and surprised no one ever picked up on it.

> > As such the current devised solution in the sysdata API is called for, given
> > if you indicated that keep = false, you are explicitly telling the firmware
> > API that your firmware will certainly not be needed after the callback is
> > called.
> > 
> > So for the sync case, a new callback is needed, and that explains the extra
> > bit of code if someone conerts from the old API to the new one.
> > 
> > [3] https://lkml.kernel.org/r/20160803161821.GB32965@dtor-ws
> > 
> >>> or a magical "sysdata_desc" descriptor,
> >>
> >> This is one way to make a flexible and extensible API without affecting drivers
> >> with further collateral evolutions as it gets extended. Its no different than
> >> the "flags" lesson learned from writing system calls, for instance.
> >>
> >> Descriptor seemed, fitting, let me know if you have any other preference.
> > 
> > I haven't heard otherwise so will be sticking to that.
> 
> How about sysdata_req{,uest}_params?

Thanks will go with that.

> >>> and having a new name ("sysdata") that is less descriptive than the old one
> >>> ("firmware")
> >>
> >> Well no, the APIs are used in *a lot* of cases for things that are not firmware
> >> already, and let's recall I originally started working on this to replace CRDA
> >> from userspace to be able to just fetch the signed regulatory database file
> >> from the kernel. Calling it firmware simply makes no sense anymore.
> > 
> > So help me bike shed. This seems to be sticking point and I frankly don't
> > care what we call it. I've asked others for name suggestions and here are
> > a few suggestions:
> > 
> >  o driver_data
> >  o dsd: device specific data
> >  o xfw - eXtensible firmware API
> >  o bbl - binary blob loader
> > 
> > Can someone just pick something? I really, really do not care.
> > 
> > If I don't hear anything concrete I will go with driver_data.
> 
> bit of skin crawling here, but not enough to care.

Bike shedding is so much fun. Not.

> >>> are all in my opinion making the example patch be a
> >>> step _backwards_ rather than an improvement. It does not look like a
> >>> simpler or more natural interface for a driver.
> >>
> >> Hope the above explains the current state. Feedback on desirable changes
> >> welcomed.
> >>
> >> [0] https://lkml.kernel.org/r/1466117661-22075-5-git-send-email-mcgrof@kernel.org
> > 
> > All this said, this series is still justified, the extra code only comes in
> > place when porting the sync requests due to the callback stuff described above
> > and the inability to use devm there. As far as I can tell, just the bike
> > shedding is left.
> > 
> > As it stands then, unless I hear back, I'll roll Daniel Wagner's changes into
> > my series to be applied first, then rename sysdata driver_data, rebase all this
> > and shoot it out again.
> > 
> > Only a few drivers will be converted over as demos. The SmPL grammar can be used
> > by those who do want a change due to the few features added. Long term we'll
> > add more features to the new API:
> > 
> >  o the whole ihex conversion is crap, we should do this within the API and
> >    this can just be specified as a descriptor preference, then drivers
> >    don't have to deal with the ihex crap themselves.
> > 
> >  o firmware singing - this lets us kill CRDA as a requirement
> 
> Strongly suspect a typo here :-p

Indeed, thanks :)

  Luis

^ permalink raw reply

* RE: [PATCH 2/2] ath9k: disable RNG by default
From: Pan, Miaoqing @ 2016-08-10  6:46 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Herbert Xu, Matt Mackall, miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <14565196.xaXq375WQg@tauon.atsec.com>

Hi Stephan,

Would you please provide a recent NIST document which asks the entropy source to pass the NIST randomness tests ?

Thanks,
Miaoqing

-----Original Message-----
From: Stephan Mueller [mailto:smueller@chronox.de] 
Sent: Wednesday, August 10, 2016 2:25 PM
To: Pan, Miaoqing <miaoqing@qti.qualcomm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>; Matt Mackall <mpm@selenic.com>; miaoqing@codeaurora.org; Valo, Kalle <kvalo@qca.qualcomm.com>; linux-wireless@vger.kernel.org; ath9k-devel <ath9k-devel@qca.qualcomm.com>; linux-crypto@vger.kernel.org; jason@lakedaemon.net; Sepehrdad, Pouyan <pouyans@qti.qualcomm.com>
Subject: Re: [PATCH 2/2] ath9k: disable RNG by default

Am Mittwoch, 10. August 2016, 06:04:32 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> FIPS RNG test is supposed to be run on the output of an RNG, and not 
> on the RNG entropy source. It is not surprising that the RNG input 
> fails the entropy tests from NIST. Check the following example.
> 
> Imagine you have a perfectly random sequence, a_1, a_2, .., a_n, where 
> each a_i is a byte. And imagine, this sequence passes all randomness tests.
> 
> Now, let's say I create a new sequence a_1, 0, a_2, 0, a_3, 0, ..., 0, 
> a_n, where each zero is a byte
> 
> If you give this sequence (as an entropy source) to a randomness test, 
> it will fail most of the tests, if not all. This does not mean this 
> sequence is not appropriate as an entropy source, it just means we 
> need twice more bytes to gain the same amount of entropy.

Agreed. But that is a very simplistic view.
> 
> I can give this 2n byte sequence to an RNG as an entropy source and it 
> provides the same amount of security as if I give the n byte stream.

Well, I am working with standards bodies like NIST and BSI on RNG assessments. They all require that the noise source (pre-whitening, of
course) pass statistical tests like the AIS20 tests, SP800-22 and similar. If you fail, you better have a good argument.

And the only argument that is kind of allowed is that you oversample your noise source to seed a DRNG from (i.e. have an entropy to data ratio of significantly below 1). And the argument for the oversampling rate is always a very interesting discussion. You apply 10/32. In private, I am wondering about that ratio, but this should not be discussed here as I hope you have a valid argument for that.

As we are talking about the current rngd, we have to consider that it does
*not* perform an oversampling (yet) as mentioned in the previous emails.

Do not get me wrong on my initial patch: your RNG may provide some entropy. 
But there are quite some folks who want to understand and audit a noise source before using it. Your current implementation simply does not allow switching the noise source off to feed the input_pool with data that increases the entropy estimator (at runtime).

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH v2 8/8] p54: convert to sysdata API
From: Mark Brown @ 2016-08-10 19:17 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Luis R. Rodriguez, Linus Torvalds, Bjorn Andersson, Daniel Vetter,
	Kalle Valo, Ming Lei, Greg Kroah-Hartman, Daniel Wagner,
	Jeff Mahoney, Takashi Iwai, Hauke Mehrtens, Vikram Mulukutla,
	Stephen Boyd, Christian Lamparter, Andy Lutomirski,
	Jonathan Corbet, Julia Lawall, Tom Gundersen, Kay Sievers,
	David Woodhouse, Andrew Morton, Tejun Heo, Josh Boyer,
	Michal Marek, David Howells, Mimi Zohar, Johannes Berg,
	Daniel Vetter, Abhay_Salunke, Dmitry Torokhov, Wu Fengguang,
	Kees Cook, Jiri Slaby, Gilles.Muller, Linux Kernel Mailing List,
	linux-wireless, Borislav Petkov, Richard Purdie, nicolas.palix
In-Reply-To: <7451c96a-b4f5-e629-a4a7-76d6fe1772e9@broadcom.com>

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

On Wed, Aug 10, 2016 at 09:04:38PM +0200, Arend Van Spriel wrote:

> So why would drivers want the devm solution anyway. Once firmware is
> loaded in the device it can be freed or do you expect device drivers to
> keep the firmware/sysdata for suspend/resume scenario as some do because
> of firmware cache behaviour. Does the "rootfs ready" event cover
> suspend/resume?

There are classes of devices that spend a large proportion of their time
powered off and are only powered up and have firmware loaded when they
are actively in use.  DSPs used in audio systems are one example of
this, I'd not be surprised to learn that similar things are done with
video.  It's too expensive to keep the device powered up and you may be
swapping between firmwares depending on use case anyway for a lot of
these devices.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Arend Van Spriel @ 2016-08-10 10:15 UTC (permalink / raw)
  To: Wright Feng, brcm80211-dev-list.pdl@broadcom.com,
	franky.lin@broadcom.com, hante.meuleman@broadcom.com,
	pieterpg@broadcom.com, Chi-Hsien Lin,
	linux-wireless@vger.kernel.org, kvalo@codeaurora.org
In-Reply-To: <DM5PR06MB28258E50D25AE4221558BA28D91D0@DM5PR06MB2825.namprd06.prod.outlook.com>

On 10-8-2016 11:44, Wright Feng wrote:
> Hi Arend,
> 
> Thanks for the reply.
> 
> On 10-8-2016 10:26, Arend Van Spriel wrote:
>> On 10-8-2016 10:01, Wright Feng wrote:
>>> When stopping hostap on virtual interface, driver will set INFRA and
>>> AP mode that may affect the functionality on primary interface. For
>>> example, if we create and stop hostapd on virtual interface then
>>> association cannot work on primary interface because INFRA mode has
>> been set to IBSS.
>>> Hence we shut down AP and set IBSS mode only on primary interface.
>>
>> What is actually the use-case here. Can you elaborate? BRCMF_C_DOWN
>> command turns out to be effectively bring the whole stack down and not just
>> the supplied interface. I suppose you are hitting that issue here as well, right?
> We want to use AP mode to let client connecting in AP+STA mode with 43438 wi-fi chip.
> After that, the AP mode will be stopped, and wpa_supplicant cannot associate to the access point.
> I describe the steps in detail as below.
> 1. Create virtual interface and set mode to __ap
> 2. start wpa_supplicant on primary interface and connect to wireless router.
> 3. start hostap daemon on virtual interface and let client connecting.
> 4. stop hostap daemon
> 5. wpa_supplicant cannot associate to access point normally.
> 
> Like you said, the issue may be hit by BRCMF_C_DOWN and same as BRCMF_C_SET_INFRA
> BRCMF_C_SET_INFRA is not just for the supplied interface either. The default bss will be changed in firmware and let firmware uses IBSS mode to join.
> About BRCMF_C_DOWN, driver will set BRCMF_C_UP command to bring device back up so it can be used again.
> However, there is no way to set INFRA mode back except starting AP mode again.

Ok.

>>
>> Regards,
>> Arend
>>
>>> Signed-off-by: Wright Feng <wright.feng@cypress.com>
>>> ---
>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5
>> ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> index 2628d5e..0687ab9 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> @@ -4716,6 +4716,8 @@ exit:
>>>
>>>  static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
>>> net_device *ndev)  {
>>> +       struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
>>> +       struct net_device *primary_ndev = cfg_to_ndev(cfg);
>>>         struct brcmf_if *ifp = netdev_priv(ndev);
>>>         s32 err;
>>>         struct brcmf_fil_bss_enable_le bss_enable; @@ -4723,7 +4725,8
>>> @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
>>> net_device *ndev)
>>>
>>>         brcmf_dbg(TRACE, "Enter\n");
>>>
>>> -       if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
>>> +       if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
>>> +           (ndev == primary_ndev)) {
>>>                 /* Due to most likely deauths outstanding we sleep */
>>>                 /* first to make sure they get processed by fw. */
>>>                 msleep(400);
>>> --
>>> 1.9.1
>>>
>>>
>>> This message and any attachments may contain Cypress (or its subsidiaries)
>> confidential information. If it has been received in error, please advise the
>> sender and immediately delete this message.
>>>
> 
> This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

Is there any way for you to get rid of this foot note. It may keep Kalle
from taking this patch. Other option is to take this patch through our
internal tree.

Regards,
Arend

^ permalink raw reply

* Re: [PATCH v2 8/8] p54: convert to sysdata API
From: Arend Van Spriel @ 2016-08-10 19:04 UTC (permalink / raw)
  To: Luis R. Rodriguez, Linus Torvalds, Bjorn Andersson, Daniel Vetter,
	Kalle Valo, Ming Lei, Greg Kroah-Hartman, Daniel Wagner,
	Jeff Mahoney, Takashi Iwai
  Cc: Hauke Mehrtens, Vikram Mulukutla, Stephen Boyd,
	Christian Lamparter, Andy Lutomirski, Jonathan Corbet,
	Julia Lawall, Tom Gundersen, Kay Sievers, David Woodhouse,
	Andrew Morton, Tejun Heo, Josh Boyer, Michal Marek, David Howells,
	Mimi Zohar, Johannes Berg, Daniel Vetter, Abhay_Salunke,
	Dmitry Torokhov, Wu Fengguang, Mark Brown, Kees Cook, Jiri Slaby,
	Gilles.Muller, Linux Kernel Mailing List, linux-wireless,
	Borislav Petkov, Richard Purdie, nicolas.palix
In-Reply-To: <20160810183211.GL3296@wotan.suse.de>

On 10-8-2016 20:32, Luis R. Rodriguez wrote:
> On Fri, Jun 17, 2016 at 08:35:03PM +0200, Luis R. Rodriguez wrote:
>> On Thu, Jun 16, 2016 at 05:09:30PM -1000, Linus Torvalds wrote:
>>> On Thu, Jun 16, 2016 at 3:36 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>>>>
>>>> Reason this could not wait is folks seem to want to keep extending the API,
>>>> which is another reason for this, do we want to put an end to an unflexible
>>>> API now or should we wait ?

[big snip]

> 
> Regarding this -- Dmitry recenlty noted devm only works if used on the probe
> path, and as we now determined, we don't want firmware loading on probe [3], unless
> async probe is used, so this would make a devm solution here not ideal for
> freeing the firmware. Even if you use async probe -- that seems very special
> use case and adding devm support for the firmware API just for that seems silly.

So why would drivers want the devm solution anyway. Once firmware is
loaded in the device it can be freed or do you expect device drivers to
keep the firmware/sysdata for suspend/resume scenario as some do because
of firmware cache behaviour. Does the "rootfs ready" event cover
suspend/resume?

> As such the current devised solution in the sysdata API is called for, given
> if you indicated that keep = false, you are explicitly telling the firmware
> API that your firmware will certainly not be needed after the callback is
> called.
> 
> So for the sync case, a new callback is needed, and that explains the extra
> bit of code if someone conerts from the old API to the new one.
> 
> [3] https://lkml.kernel.org/r/20160803161821.GB32965@dtor-ws
> 
>>> or a magical "sysdata_desc" descriptor,
>>
>> This is one way to make a flexible and extensible API without affecting drivers
>> with further collateral evolutions as it gets extended. Its no different than
>> the "flags" lesson learned from writing system calls, for instance.
>>
>> Descriptor seemed, fitting, let me know if you have any other preference.
> 
> I haven't heard otherwise so will be sticking to that.

How about sysdata_req{,uest}_params?

>>> and having a new name ("sysdata") that is less descriptive than the old one
>>> ("firmware")
>>
>> Well no, the APIs are used in *a lot* of cases for things that are not firmware
>> already, and let's recall I originally started working on this to replace CRDA
>> from userspace to be able to just fetch the signed regulatory database file
>> from the kernel. Calling it firmware simply makes no sense anymore.
> 
> So help me bike shed. This seems to be sticking point and I frankly don't
> care what we call it. I've asked others for name suggestions and here are
> a few suggestions:
> 
>  o driver_data
>  o dsd: device specific data
>  o xfw - eXtensible firmware API
>  o bbl - binary blob loader
> 
> Can someone just pick something? I really, really do not care.
> 
> If I don't hear anything concrete I will go with driver_data.

bit of skin crawling here, but not enough to care.

>>> are all in my opinion making the example patch be a
>>> step _backwards_ rather than an improvement. It does not look like a
>>> simpler or more natural interface for a driver.
>>
>> Hope the above explains the current state. Feedback on desirable changes
>> welcomed.
>>
>> [0] https://lkml.kernel.org/r/1466117661-22075-5-git-send-email-mcgrof@kernel.org
> 
> All this said, this series is still justified, the extra code only comes in
> place when porting the sync requests due to the callback stuff described above
> and the inability to use devm there. As far as I can tell, just the bike
> shedding is left.
> 
> As it stands then, unless I hear back, I'll roll Daniel Wagner's changes into
> my series to be applied first, then rename sysdata driver_data, rebase all this
> and shoot it out again.
> 
> Only a few drivers will be converted over as demos. The SmPL grammar can be used
> by those who do want a change due to the few features added. Long term we'll
> add more features to the new API:
> 
>  o the whole ihex conversion is crap, we should do this within the API and
>    this can just be specified as a descriptor preference, then drivers
>    don't have to deal with the ihex crap themselves.
> 
>  o firmware singing - this lets us kill CRDA as a requirement

Strongly suspect a typo here :-p

Regards,
Arend

^ permalink raw reply

* [ANNOUNCE] Netdev 1.2 updates
From: Hajime Tazaki @ 2016-08-10 15:06 UTC (permalink / raw)
  To: netdev, netfilter-devel, linux-wireless, netdev-conference


Hello folks,

The deadline for the early bird registration of netdev 1.2
(Tokyo) is coming very soon (August 15, next Monday).
Please don't forget to register with the discount price.
Your early registration will be definitely helpful to
prepare the conference.

http://netdevconf.org/1.2/registration.html


Here is the accepted session list of netdev 1.2.  We will
announce more sessions once we got confirmed.

http://netdevconf.org/1.2/accepted-sessions.html

* Workshop
1. XDP workshop: Introduction, experience, and future
development (Tom Herbert)

2. TC extended workshop proposal (Jamal Hadi Salim)

* Talk
3. Encapsulation Offloads LCO, GSO_PARTIAL, TSO_MANGLEID, and
Why Less is More (Alexander Duyck)

4. User Space TCP - Getting LKL ready for the Prime Time Use
(Jerry Chu, Yuan Liu)

* Tutorial
5. Practical Guide to Run an IEEE 802.15.4 Network with
6LoWPAN under Linux (Stefan Schmidt)


The call for proposals is still opened so we'd be waiting
for your great talk proposals too.


Our sponsors:
- Platinum
Verizon, Facebook, Cumulus Networks (new)
- Gold
Mojatatu Networks, VMWare, Google, NTT (new)
- Silver
NetApp, IIJ, Netronome, SolarFlare, Mellanox

Twitter: https://twitter.com/netdev01
Web: http://netdevconf.org/1.2/

-- Hajime



^ permalink raw reply

* Re: [PATCH v4] cfg80211: Provision to allow the support for different beacon intervals
From: Johannes Berg @ 2016-08-10 15:18 UTC (permalink / raw)
  To: Undekari, Sunil Dutt, Kushwaha, Purushottam
  Cc: linux-wireless@vger.kernel.org, Malinen, Jouni,
	Kondabattini, Ganesh, Kalikot Veetil, Mahesh Kumar,
	Hullur Subramanyam, Amarnath, Kumar, Deepak (QCA)
In-Reply-To: <33789175707446c89435264aa54f7474@APSANEXR01F.ap.qualcomm.com>

On Wed, 2016-08-10 at 14:53 +0000, Undekari, Sunil Dutt wrote:
> > 
> > I don't know what your firmware/hardware capabilities are :)
> I was not referring to any specific architecture here.  With the 2
> choices you had mentioned (mesh must match either one of them / make
> this apply on mesh and IBSS as well ) we see that this new flag being
> defined in the interface combinations could better be applicable for
> all the beaconing interfaces. 

Yes, I agree. If that's what you can implement, that's clearly the
better choice, and once you have multiple BIs for APs it seems
relatively straight-forward to also add for other interface types.

> If we had restricted this to AP / P2P GO and have Mesh match either
> of them , a new request ( in future ? ) to have a different interval
> for Mesh, IMO  would ask a need for new flag , isn't ?  Our intention
> here was to avoid this by ensuring that this flag is applicable for
> all the beaconing interface combinations. 

Makes sense.

Just need to update the documentation then, I guess?

> > I'd argue that instead of having the interface combinations flag,
> > that nl80211 attribute could carry the GCD?
> If your say here is to just use the wiphy parameter rather than
> having the flag in the interface combinations, signifying the support
> for different beacon intervals , Yes we too agree to your point. 

Well, I think it would still make sense to have this on the interface
combinations, if only for flexibility. It's practically equally
complicated (or cheap) to do that way, but gives a bit more
flexibility.

I was just thinking that instead of adding the *flag* on interface
combinations, which you did in this patch:
> +	bool supp_diff_beacon_int;
...
> + * @NL80211_IFACE_COMB_DIFF_BEACON_INTERVAL: flag attribute specifying that

it would make sense to make that a

u8 diff_beacon_int_gcd;
...
NL80211_IFACE_COMB_DIFF_BEACON_GCD: u32 attribute specifying the GCD
	of different beacon interfaces (not present if all beacon
	intervals must match)


> This is considering the fact that the current implementation
> restricts the validation of the beacon interval
> (cfg80211_validate_beacon_int ) to only AP / P2P GO. 
> Neither the Mesh / IBSS ( join ) have this restriction . Not sure why
> this is not imposed for Mesh / IBSS though. 

Interesting, that seems like a bug that we should fix. I suspect the
reason is that IBSS used to not be supported in combinations, and so
nobody thought of adding it there, and mesh was more or less copied
from IBSS. Do you want to submit a (separate) patch to fix this?
Otherwise I can do that.

> After introducing this flag in the interface combinations, we do feel
> that currently it is not going to apply for Mesh / IBSS , unless
> there is a future need to do so , which we are not aware of . 

Well, you're right given the circumstances, but I'd rather fix that
first, forcing MBSS/IBSS to match, and then let it be relaxed with the
new capability. I'm pretty sure current drivers wouldn't do something
useful when mesh/AP combinations have different BIs.

johannes

^ permalink raw reply

* RE: [PATCH 2/2] ath9k: disable RNG by default
From: Pan, Miaoqing @ 2016-08-10  6:04 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Herbert Xu, Matt Mackall, miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <1543667.vXsZDTRgbm@positron.chronox.de>

Hi Stephan,

FIPS RNG test is supposed to be run on the output of an RNG, and not on the RNG entropy source. It is not surprising that the RNG input fails the entropy tests from NIST. Check the following example.

Imagine you have a perfectly random sequence, a_1, a_2, .., a_n, where each a_i is a byte. And imagine, this sequence passes all randomness tests.

Now, let's say I create a new sequence a_1, 0, a_2, 0, a_3, 0, ..., 0, a_n, where each zero is a byte

If you give this sequence (as an entropy source) to a randomness test, it will fail most of the tests, if not all. This does not mean this sequence is not appropriate as an entropy source, it just means we need twice more bytes to gain the same amount of entropy.

I can give this 2n byte sequence to an RNG as an entropy source and it provides the same amount of security as if I give the n byte stream.

Thanks,
Miaoqing

-----Original Message-----
From: Stephan Mueller [mailto:smueller@chronox.de] 
Sent: Wednesday, August 10, 2016 1:29 PM
To: Pan, Miaoqing <miaoqing@qti.qualcomm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>; Matt Mackall <mpm@selenic.com>; miaoqing@codeaurora.org; Valo, Kalle <kvalo@qca.qualcomm.com>; linux-wireless@vger.kernel.org; ath9k-devel <ath9k-devel@qca.qualcomm.com>; linux-crypto@vger.kernel.org; jason@lakedaemon.net; Sepehrdad, Pouyan <pouyans@qti.qualcomm.com>
Subject: Re: [PATCH 2/2] ath9k: disable RNG by default

Am Mittwoch, 10. August 2016, 02:35:04 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> For those less perfect noise source, can't pass the FIPS test.
> 
> static int update_kernel_random(int random_step,
>         unsigned char *buf, fips_ctx_t *fipsctx_in) {
>         unsigned char *p;
>         int fips;
> 
>         fips = fips_run_rng_test(fipsctx_in, buf);
>         if (fips)
>                 return 1;
> 
>         for (p = buf; p + random_step <= &buf[FIPS_RNG_BUFFER_SIZE];
>                  p += random_step) {
>                 random_add_entropy(p, random_step);
>                 random_sleep();
>         }
>         return 0;
> }

Not even the poor cheap AIS20 statistical tests from rngd pass?

I guess the only sensible solution is what Ted suggested to use add_device_randomness.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Stephan Mueller @ 2016-08-10  6:51 UTC (permalink / raw)
  To: Pan, Miaoqing
  Cc: Herbert Xu, Matt Mackall, miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <82844e4b56c9475fabf22b3fe12b53bb@aptaiexm02f.ap.qualcomm.com>

Am Mittwoch, 10. August 2016, 06:46:31 CEST schrieb Pan, Miaoqing:

Hi Miaoqing,

> Hi Stephan,
> 
> Would you please provide a recent NIST document which asks the entropy
> source to pass the NIST randomness tests ?

See FIPS 140-2 IG 7.15 which explicitly references SP800-22.

Ciao
Stephan

^ 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