* [bug report] iwlwifi: mvm: Add mem debugfs entry
From: Dan Carpenter @ 2016-10-11 10:50 UTC (permalink / raw)
To: ido; +Cc: linux-wireless
Hello Ido Yariv,
The patch 2b55f43f8e47: "iwlwifi: mvm: Add mem debugfs entry" from
Aug 23, 2016, leads to the following static checker warning:
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c:1561 iwl_dbgfs_mem_read()
warn: unsigned 'len' is never less than zero.
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
1521 static ssize_t iwl_dbgfs_mem_read(struct file *file, char __user *user_buf,
1522 size_t count, loff_t *ppos)
1523 {
1524 struct iwl_mvm *mvm = file->private_data;
1525 struct iwl_dbg_mem_access_cmd cmd = {};
1526 struct iwl_dbg_mem_access_rsp *rsp;
1527 struct iwl_host_cmd hcmd = {
1528 .flags = CMD_WANT_SKB | CMD_SEND_IN_RFKILL,
1529 .data = { &cmd, },
1530 .len = { sizeof(cmd) },
1531 };
1532 size_t delta, len;
^^^
Unsigned.
1533 ssize_t ret;
1534
1535 hcmd.id = iwl_cmd_id(*ppos >> 24 ? UMAC_RD_WR : LMAC_RD_WR,
1536 DEBUG_GROUP, 0);
1537 cmd.op = cpu_to_le32(DEBUG_MEM_OP_READ);
1538
1539 /* Take care of alignment of both the position and the length */
1540 delta = *ppos & 0x3;
1541 cmd.addr = cpu_to_le32(*ppos - delta);
1542 cmd.len = cpu_to_le32(min(ALIGN(count + delta, 4) / 4,
1543 (size_t)DEBUG_MEM_MAX_SIZE_DWORDS));
1544
1545 mutex_lock(&mvm->mutex);
1546 ret = iwl_mvm_send_cmd(mvm, &hcmd);
1547 mutex_unlock(&mvm->mutex);
1548
1549 if (ret < 0)
1550 return ret;
1551
1552 rsp = (void *)hcmd.resp_pkt->data;
1553 if (le32_to_cpu(rsp->status) != DEBUG_MEM_STATUS_SUCCESS) {
1554 ret = -ENXIO;
1555 goto out;
1556 }
1557
1558 len = min((size_t)le32_to_cpu(rsp->len) << 2,
1559 iwl_rx_packet_payload_len(hcmd.resp_pkt) - sizeof(*rsp));
1560 len = min(len - delta, count);
1561 if (len < 0) {
^^^^^^^
Unpossible.
1562 ret = -EFAULT;
1563 goto out;
1564 }
1565
1566 ret = len - copy_to_user(user_buf, (void *)rsp->data + delta, len);
1567 *ppos += ret;
1568
1569 out:
1570 iwl_free_resp(&hcmd);
1571 return ret;
1572 }
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
From: Paul Bolle @ 2016-10-11 10:11 UTC (permalink / raw)
To: Luca Coelho, Chris Rorvick, Intel Linux Wireless,
Emmanuel Grumbach, Johannes Berg, Kalle Valo, Oren Givon
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1476108164.5210.11.camel@coelho.fi>
Hi Luca,
On Mon, 2016-10-10 at 17:02 +0300, Luca Coelho wrote:
> On Mon, 2016-10-10 at 02:19 -0500, Chris Rorvick wrote:
> This is not coming from the NIC itself, but from the platform's ACPI
> tables. Can you tell us which platform you are using?
On my machine I'm seeing the same error as Chris. So what exactly do
you mean with "platform" here?
> > Name (SPLX, Package (0x04)
> > {
> > Zero,
> > Package (0x03)
> > {
> > 0,
> > 1200,
> > 1000
> > },
> > Package (0x03)
> > {
> > 0,
> > 1200,
> > 1000
> > },
> > Package (0x03)
> > {
> > 0,
> > 1200,
> > 1000
> > }
> > })
>
> This is not the structure that we are expecting. We expect this:
>
> Name (SPLX, Package (0x02)
> {
> Zero,
> Package (0x03)
> {
> 0x07,
> <value>,
> <value>
> }
> })
>
> ...as you correctly pointed out. The data in the structure you have is
> not for WiFi (actually I don't think 0 is a valid value, but I'll
> double-check).
For what it's worth, on my machine I have twenty (!) SPLX entries, all
reading:
Name (SPLX, Package (0x04)
{
Zero,
Package (0x03)
{
0x80000000,
0x80000000,
0x80000000
},
Package (0x03)
{
0x80000000,
0x80000000,
0x80000000
},
Package (0x03)
{
0x80000000,
0x80000000,
0x80000000
}
})
> There are other things that look a bit inconsistent in this code...
> I'll try to find the official ACPI table definitions for this entries
> to make sure it's correct.
When I looked into this error, some time ago, I searched around a bit
for documentation on this splx stuff. Sadly, commit bcb079a14d75
("iwlwifi: pcie: retrieve and parse ACPI power limitations") provides
very few clues and my searches turned up nothing useful. So a pointer
or two would be really appreciated.
> > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > @@ -540,7 +540,7 @@ static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx)
> > splx->package.count != 2 ||
> > splx->package.elements[0].type != ACPI_TYPE_INTEGER ||
> > splx->package.elements[0].integer.value != 0) {
> > - IWL_ERR(trans, "Unsupported splx structure\n");
> > + IWL_WARN(trans, "Unsupported splx structure, not limiting WiFi power\n");
> > return 0;
> > }
>
> If this is really bothering you, I guess I could apply this patch for
> now. But as I said, this is not solving the actual problem.
Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't
imply one needs to act on this message, while warn does imply that
action is needed.
Thanks,
Paul Bolle
^ permalink raw reply
* Re: [PATCH] mac80211: fix sequence number allocation regression
From: Toke Høiland-Jørgensen @ 2016-10-11 9:39 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, johannes
In-Reply-To: <20161011092831.85565-1-nbd@nbd.name>
Felix Fietkau <nbd@nbd.name> writes:
> The recent commit that moved around TX handlers dropped the sequence
> number allocation at the end of ieee80211_tx_dequeue and calls
> ieee80211_tx_h_sequence instead (for the non-fast-xmit case).
> However, it did not change the fast-xmit sequence allocation condition
> in ieee80211_xmit_fast_finish, which skipped seqno alloc if intermediate
> tx queues are being used.
>
> Drop the now obsolete condition.
>
> Fixes: bb42f2d13ffc ("mac80211: Move reorder-sensitive TX handlers to aft=
er TXQ dequeue")
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
Acked-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
(Think this was introduced in the merging of mac80211-next and net-next?)
-Toke
^ permalink raw reply
* [PATCH] mac80211: fix sequence number allocation regression
From: Felix Fietkau @ 2016-10-11 9:28 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes, toke
The recent commit that moved around TX handlers dropped the sequence
number allocation at the end of ieee80211_tx_dequeue and calls
ieee80211_tx_h_sequence instead (for the non-fast-xmit case).
However, it did not change the fast-xmit sequence allocation condition
in ieee80211_xmit_fast_finish, which skipped seqno alloc if intermediate
tx queues are being used.
Drop the now obsolete condition.
Fixes: bb42f2d13ffc ("mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
net/mac80211/tx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index dc60784..8ff81fd 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3212,7 +3212,6 @@ static void ieee80211_xmit_fast_finish(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb)
{
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
- struct ieee80211_local *local = sdata->local;
struct ieee80211_hdr *hdr = (void *)skb->data;
u8 tid = IEEE80211_NUM_TIDS;
@@ -3224,8 +3223,7 @@ static void ieee80211_xmit_fast_finish(struct ieee80211_sub_if_data *sdata,
if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) {
tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
*ieee80211_get_qos_ctl(hdr) = tid;
- if (!ieee80211_get_txq(local, &sdata->vif, &sta->sta, skb))
- hdr->seq_ctrl = ieee80211_tx_next_seq(sta, tid);
+ hdr->seq_ctrl = ieee80211_tx_next_seq(sta, tid);
} else {
info->flags |= IEEE80211_TX_CTL_ASSIGN_SEQ;
hdr->seq_ctrl = cpu_to_le16(sdata->sequence_number);
--
2.10.1
^ permalink raw reply related
* Re: TCP data throughput for BCM43362
From: Jörg Krause @ 2016-10-11 6:14 UTC (permalink / raw)
To: Arend Van Spriel
Cc: brcm80211-dev-list, Brett Rudley, Franky Lin, Hante Meuleman,
linux-wireless, Franky Lin, Arend van Spriel
In-Reply-To: <CAF7Mx6q+B4RoURNF5XxewjF9aVGCXg==XU0aDD6w+354yXZ70Q@mail.gmail.com>
Hi Arend,
Am 22. September 2016 16:00:36 MESZ, schrieb Arend Van Spriel <arend.vanspriel@broadcom.com>:
>Op 22 sep. 2016 14:52 schreef "Jörg Krause"
><joerg.krause@embedded.rocks>:
>>
>> On Do, 2016-09-22 at 10:09 +0200, Arend Van Spriel wrote:
>> > On 19-9-2016 8:36, Jörg Krause wrote:
>> > >
>> > > Hi Arend,
>> > >
>> > > On Wed, 2016-09-14 at 20:13 +0200, Arend Van Spriel wrote:
>> > > >
>> > > > On 14-9-2016 15:41, Jörg Krause wrote:
>> > > > >
>> > > > >
>> > > > > Hi,
>> > > > >
>> > > > > On Mon, 2016-08-29 at 23:15 +0200, Jörg Krause wrote:
>> > > > > >
>> > > > > >
>> > > > > > On Mi, 2016-08-24 at 20:35 +0200, Arend Van Spriel wrote:
>> > > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > > > On 22-8-2016 15:37, Jörg Krause wrote:
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > Hi all,
>> > > > > > > >
>> > > > > > > > I am back from vacation and I'd like to do more
>> > > > > > > > investigations
>> > > > > > > > about
>> > > > > > > > this issue. Please see my comments below...
>> > > > > > > >
>> > > > > > > > On Sun, 2016-08-07 at 13:41 +0200, Arend van Spriel
>> > > > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > On 06-08-16 16:12, Jörg Krause wrote:
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > Hi all,
>> > > > > > > > >
>> > > > > > > > > A bit weird email format making it a bit hard to
>> > > > > > > > > determine
>> > > > > > > > > where
>> > > > > > > > > your
>> > > > > > > > > last reply starts...
>> > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > On Fr, 2016-08-05 at 17:56 -0700, Franky Lin wrote:
>> > > > > > > > > >
>> > > > > > > > > > On Fri, Aug 5, 2016 at 2:29 PM, Jörg Krause
>> > > > > > > > > > <joerg.krause
>> > > > > > > > > > @emb
>> > > > > > > > > > ed
>> > > > > > > > > > ded.
>> > > > > > > > > > ro
>> > > > > > > > > > cks>
>> > > > > > > > > > wrote:
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > Am 5. August 2016 23:01:10 MESZ, schrieb Arend Van
>> > > > > > > > > > Spriel
>> > > > > > > > > > <
>> > > > > > > > > > arend.vanspriel@broadcom.com>:
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > Op 5 aug. 2016 22:46 schreef "Jörg Krause"
>> > > > > > > > > > <joerg.krause@embedded.rocks>:
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > Hi,
>> > > > > > > > > >
>> > > > > > > > > > I'm using a custom ARM board with an BCM43362 wifi
>> > > > > > > > > > chip
>> > > > > > > > > > from
>> > > > > > > > > >
>> > > > > > > > > > Broadcom.
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > The wifi chip is attached via SDIO to the
>controller
>> > > > > > > > > > with
>> > > > > > > > > > a
>> > > > > > > > > > clock of
>> > > > > > > > > > 48MHz. Linux kernel version is 4.7.
>> > > > > > > > > >
>> > > > > > > > > > When measuring the network bandwidth with iperf3 I
>> > > > > > > > > > get a
>> > > > > > > > > > bandwith of
>> > > > > > > > > > only around 5 Mbps. I found a similar thread at the
>> > > > > > > > > > Broadcom
>> > > > > > > > > >
>> > > > > > > > > > community
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > [1] where the test was done with a M4 CPU +
>BCM43362
>> > > > > > > > > > and
>> > > > > > > > > > an
>> > > > > > > > > > average
>> > > > > > > > > > result of 3.3 Mbps.
>> > > > > > > > > >
>> > > > > > > > > > Interestingly, a BCM43362 Wi-Fi Dev Kit [2] notes a
>> > > > > > > > > > TCP
>> > > > > > > > > > data
>> > > > > > > > > >
>> > > > > > > > > > throughput
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > greater than 20 Mbps.
>> > > > > > > > > >
>> > > > > > > > > > Why is the throughput I measured much lower? Note
>> > > > > > > > > > that I
>> > > > > > > > > > measured
>> > > > > > > > > > several times with almost no neighbor devices or
>> > > > > > > > > > networks.
>> > > > > > > > > >
>> > > > > > > > > > This is a test sample measured with iperf3:
>> > > > > > > > > >
>> > > > > > > > > > $ iperf3 -c 192.168.2.1 -i 1 -t 10
>> > > > > > > > > > Connecting to host 192.168.2.1, port 5201
>> > > > > > > > > > [ 4] local 192.168.2.155 port 36442 connected
>to
>> > > > > > > > > > 192.168.2.1
>> > > > > > > > > >
>> > > > > > > > > > port
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > 5201
>> > > > > > > > > > [ ID]
>> > > > > > > > > > Interval Transfer Bandwidth
>Retr
>> > > > > > > > > > Cwn
>> > > > > > > > > > d
>> > > > > > > > > > [ 4] 0.00-1.00 sec 615 KBytes 5.04
>> > > > > > > > > > Mbits/sec 0 56.6
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 1.00-2.00 sec 622 KBytes 5.10
>> > > > > > > > > > Mbits/sec 0 84.8
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 2.00-3.00 sec 625 KBytes 5.12
>> > > > > > > > > > Mbits/sec 0 113
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 3.00-4.00 sec 571 KBytes 4.68
>> > > > > > > > > > Mbits/sec 0 140
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 4.00-5.00 sec 594 KBytes 4.87
>> > > > > > > > > > Mbits/sec 0 167
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 5.00-6.00 sec 628 KBytes 5.14
>> > > > > > > > > > Mbits/sec 0 195
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 6.00-7.00 sec 619 KBytes 5.07
>> > > > > > > > > > Mbits/sec 0 202
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 7.00-8.00 sec 608 KBytes 4.98
>> > > > > > > > > > Mbits/sec 0 202
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 8.00-9.00 sec 602 KBytes 4.93
>> > > > > > > > > > Mbits/sec 0 202
>> > > > > > > > > > KBytes
>> > > > > > > > > > [ 4] 9.00-10.00 sec 537 KBytes 4.40
>> > > > > > > > > > Mbits/sec 0 202
>> > > > > > > > > > KBytes
>> > > > > > > > > > - - - - - - - - - - - - - - - - - - - - - - - -
>-
>> > > > > > > > > > [ ID]
>> > > > > > > > > > Interval Transfer Bandwidth
>Retr
>> > > > > > > > > > [ 4] 0.00-10.00 sec 5.88 MBytes 4.93
>> > > > > > > > > > Mbits/sec 0 sender
>> > > > > > > > > > [ 4] 0.00-10.00 sec 5.68 MBytes 4.76
>> > > > > > > > > > Mbits/sec receiver
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > Not overly familiar with iperf3. Do these lines
>mean
>> > > > > > > > > > you
>> > > > > > > > > > are
>> > > > > > > > > > doing
>> > > > > > > > > > bidirectional test, ie. upstream and downstream at
>> > > > > > > > > > the
>> > > > > > > > > > same
>> > > > > > > > > > time.
>> > > > > > > > > > Another
>> > > > > > > > > > thing affecting tput could be power-save.
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > No, iperf3 does not support bidrectional test.
>Power-
>> > > > > > > > > > save
>> > > > > > > > > > is
>> > > > > > > > > > turned
>> > > > > > > > > > off.
>> > > > > > > > > >
>> > > > > > > > > > What does iw link say?
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > but I guess it starts here!
>> > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > I compared the results with a Cubietruck I have:
>> > > > > > > > > >
>> > > > > > > > > > # iperf3 -s
>> > > > > > > > > > ---------------------------------------------------
>> > > > > > > > > > ----
>> > > > > > > > > > ----
>> > > > > > > > > > Server listening on 5201
>> > > > > > > > > > ---------------------------------------------------
>> > > > > > > > > > ----
>> > > > > > > > > > ----
>> > > > > > > > > > Accepted connection from 192.168.178.46, port 42906
>> > > > > > > > > > [ 5] local 192.168.178.38 port 5201 connected to
>> > > > > > > > > > 192.168.178.46
>> > > > > > > > > > port
>> > > > > > > > > > 42908
>> > > > > > > > > > [ ID] Interval Transfer Bandwidth
>> > > > > > > > > > [ 5] 0.00-1.00 sec 2.29 MBytes 19.2
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 1.00-2.00 sec 2.21 MBytes 18.5
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 2.00-3.00 sec 2.17 MBytes 18.2
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 3.00-4.00 sec 2.09 MBytes 17.6
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 4.00-5.00 sec 2.20 MBytes 18.5
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 5.00-6.00 sec 2.64 MBytes 22.1
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 6.00-7.00 sec 2.67 MBytes 22.4
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 7.00-8.00 sec 2.62 MBytes 22.0
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 8.00-9.00 sec 2.35 MBytes 19.8
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 9.00-10.00 sec 2.30 MBytes 19.3
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > [ 5] 10.00-10.03 sec 83.4 KBytes 23.5
>> > > > > > > > > > Mbits/sec
>> > > > > > > > > > - - - - - - - - - - - - - - - - - - - - - - - - -
>> > > > > > > > > > [ ID]
>> > > > > > > > > > Interval Transfer Bandwidth
>Retr
>> > > > > > > > > > [ 5] 0.00-10.03 sec 23.9 MBytes 20.0
>> > > > > > > > > > Mbits/sec 0 sender
>> > > > > > > > > > [ 5] 0.00-10.03 sec 23.6 MBytes 19.8
>> > > > > > > > > > Mbits/sec receiver
>> > > > > > > > > >
>> > > > > > > > > > # iw dev wlan0 link
>> > > > > > > > > > Connected to xx:xx:xx:xx:xx (on wlan0)
>> > > > > > > > > > SSID: xxx
>> > > > > > > > > > freq: 2437
>> > > > > > > > > > tx bitrate: 65.0 MBit/s
>> > > > > > > > > >
>> > > > > > > > > > bss flags: short-preamble short-slot-
>> > > > > > > > > > time
>> > > > > > > > > > dtim period: 1
>> > > > > > > > > > beacon int: 100
>> > > > > > > > >
>> > > > > > > > > Too bad RSSI is not in the output above. That may be
>> > > > > > > > > due to
>> > > > > > > > > a
>> > > > > > > > > regression
>> > > > > > > > > in our driver which has been fixed by commit
>> > > > > > > > > 94abd778a7bb
>> > > > > > > > > ("brcmfmac:
>> > > > > > > > > add fallback for devices that do not report per-chain
>> > > > > > > > > values").
>> > > > > > > > > However,
>> > > > > > > > > the tx bitrate seems within the same range as the
>other
>> > > > > > > > > platform.
>> > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > The Cubietruck works also with the brcmfmac driver.
>> > > > > > > > > >
>> > > > > > > > > > May it depend on the NVRAM file?
>> > > > > > > > >
>> > > > > > > > > Not sure. Can you tell me a bit more about the custom
>> > > > > > > > > ARM
>> > > > > > > > > board.
>> > > > > > > > > Does
>> > > > > > > > > it
>> > > > > > > > > use the same wifi module as Cubietruck, ie. the AMPAK
>> > > > > > > > > AP6210?
>> > > > > > > > > If
>> > > > > > > > > you
>> > > > > > > > > can
>> > > > > > > > > make a wireshark sniff we can check the actual
>bitrate
>> > > > > > > > > and
>> > > > > > > > > medium
>> > > > > > > > > density in terms of packets. Another thing to look at
>> > > > > > > > > is
>> > > > > > > > > the
>> > > > > > > > > SDIO
>> > > > > > > > > host
>> > > > > > > > > controller. In brcmf_sdiod_sgtable_alloc() some key
>> > > > > > > > > values
>> > > > > > > > > are
>> > > > > > > > > used
>> > > > > > > > > from
>> > > > > > > > > the host controller. It only logs the number of
>entries
>> > > > > > > > > of
>> > > > > > > > > the
>> > > > > > > > > scatter-gather table, but could you add the other
>> > > > > > > > > values in
>> > > > > > > > > this
>> > > > > > > > > function that are used to determine the number of
>> > > > > > > > > entries.
>> > > > > > > >
>> > > > > > > > My board uses the BCM43362 chip solely (no Bluetooth)
>> > > > > > > > attached to
>> > > > > > > > the
>> > > > > > > > SDIO interface of a NXP i.MX28 processor.
>> > > > > > > >
>> > > > > > > > I added some additional printk() to
>> > > > > > > > brcmf_sdiod_sgtable_alloc().
>> > > > > > > > These
>> > > > > > > > are the values printed after modprobe brcmfmac:
>> > > > > > > >
>> > > > > > > > [ 8.926657] sg_support=1
>> > > > > > > > [ 8.929440] max_blocks=511
>> > > > > > > > [ 8.932213] max_request_size=261632
>> > > > > > > > [ 8.935741] max_segment_count=52
>> > > > > > > > [ 8.939005] max_segment_size=65280
>> > > > > > > > [ 8.946095] nents=35
>> > > > > > >
>> > > > > > > Thanks. That looks good.
>> > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > Additionally I attached a xz compresses wireshark sniff
>> > > > > > > > while
>> > > > > > > > running
>> > > > > > > > iper3 between the BCM43362 running as in AP mode with
>> > > > > > > > iperf3
>> > > > > > > > as a
>> > > > > > > > server and a PC in station mode running iperf3 as a
>> > > > > > > > client.
>> > > > > > >
>> > > > > > > Looking at the sniff it seems you captured on the
>ethernet
>> > > > > > > side.
>> > > > > > > That
>> > > > > > > does not give me any 802.11 specific info. Can you make a
>> > > > > > > wireless
>> > > > > > > capture preferably without encryption.
>> > > > > >
>> > > > > > You,re right! Sorry for this mistake. I did a re-capture on
>> > > > > > the
>> > > > > > wireless side now.
>> > > > >
>> > > > > Anything new about this? Anything I can do to help?
>> > > >
>> > > > I missed your previous email. Was already wondering whether to
>> > > > ping
>> > > > you.
>> > > > Digging around in my email folders I found it so will take a
>look
>> > > > at
>> > > > it.
>> > >
>> > > Did you had some time to look at this?
>> >
>> > Ehm. I still only see TCP stuff. To capture 802.11 management
>frames
>> > you
>> > need preferably a dedicated device using monitor mode [1].
>>
>> Stupid me! Now I used a monitor interface on a desktop to monitor the
>> traffic between the BCM43362 operating in soft-AP mode and a notebook
>> operating in managed mode.
>>
>> The BCM43362 runs the iperf server, the notebook the iperf client.
>
>Thanks.
>
>Week almost through so might next week.
Did you had some time to look at this?
Best regards
Jörg Krause
--
Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.
^ permalink raw reply
* Re: [PATCH v4 1/3] mwifiex: reset card->adapter during device unregister
From: Brian Norris @ 2016-10-11 0:22 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
Xinming Hu, abhishekbh, Dmitry Torokhov
In-Reply-To: <20161010205332.GB11254@localhost>
+ Dmitry
Hi Amit,
On Mon, Oct 10, 2016 at 01:53:32PM -0700, Brian Norris wrote:
> On Thu, Oct 06, 2016 at 11:36:24PM +0530, Amitkumar Karwar wrote:
> > From: Xinming Hu <huxm@marvell.com>
> >
> > card->adapter gets initialized during device registration.
> > As it's not cleared, we may end up accessing invalid memory
> > in some corner cases. This patch fixes the problem.
> >
> > Signed-off-by: Xinming Hu <huxm@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> > v4: Same as v1, v2, v3
> > ---
> > drivers/net/wireless/marvell/mwifiex/pcie.c | 1 +
> > drivers/net/wireless/marvell/mwifiex/sdio.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > index f1eeb73..ba9e068 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > @@ -3042,6 +3042,7 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
> > pci_disable_msi(pdev);
> > }
> > }
> > + card->adapter = NULL;
> > }
> >
> > /* This function initializes the PCI-E host memory space, WCB rings, etc.
> > diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > index 8718950..4cad1c2 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > @@ -2066,6 +2066,7 @@ mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
> > struct sdio_mmc_card *card = adapter->card;
> >
> > if (adapter->card) {
> > + card->adapter = NULL;
> > sdio_claim_host(card->func);
> > sdio_disable_func(card->func);
> > sdio_release_host(card->func);
>
> As discussed on v1, I had qualms about the raciness between reads/writes
> of card->adapter, but I believe we:
> (a) can't have any command activity while writing the ->adapter field
> (either we're just init'ing the device, or we've disabled interrupts and
> are tearing it down) and
> (b) can't have a race between suspend()/resume() and unregister_dev(),
> since unregister_dev() is called from device remove() (which should not
> be concurrent with suspend()).
>
> Also, I thought you had the same problem in usb.c, but in fact, you
> fixed that ages ago here:
>
> 353d2a69ea26 mwifiex: fix issues in driver unload path for USB chipsets
>
> Would be nice if fixes were bettery synchronized across the three
> interface drivers you support. We seem to be discovering unnecessary
> divergence on a few points recently.
>
> At any rate:
>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Brian Norris <briannorris@chromium.org>
Dmitry helped me re-realize my original qualms:
mwifiex_unregister_dev() is called in the failure path for your async FW
request, and so it may race with suspend(). So I retract my Reviewed-by.
Sorry.
I'm going to look into converting to asynchronous device probing, which
might remove the need for async FW request, and would therefore resolve
both patch 1 and 3's races without any additional complicated hacks. But
I'm not sure if that will satisfy all mwifiex users well enough. I'll
have to give it a little more thought. Any thoughts from your side,
Amit?
Brian
^ permalink raw reply
* Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device unregister
From: Brian Norris @ 2016-10-10 23:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Amitkumar Karwar, linux-wireless@vger.kernel.org, Cathy Luo,
Nishant Sarmukadam, rajatja@google.com, Xinming Hu
In-Reply-To: <20161010234708.GA19969@localhost>
(I think Dmitry noticed the same while I wrote this.)
On Mon, Oct 10, 2016 at 04:47:08PM -0700, Brian Norris wrote:
> [*] The other cases are in error handling cases. I guess I should make
> sure those didn't race too...
Ah, well I think I missed one case:
For the async FW request code path, the callback mwifiex_fw_dpc() can
fail to load FW, and so it unwinds with:
if (adapter->if_ops.unregister_dev)
adapter->if_ops.unregister_dev(adapter);
This all happens after probe() is finished, so we can have a race on
card->adapter being read in suspend() and written in the $subject patch.
Can I revoke my Reviewed-by? (Or make it Reviewed-and-found-wanting-by?)
Brian
^ permalink raw reply
* Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device unregister
From: Dmitry Torokhov @ 2016-10-10 23:53 UTC (permalink / raw)
To: Brian Norris
Cc: Amitkumar Karwar, linux-wireless@vger.kernel.org, Cathy Luo,
Nishant Sarmukadam, rajatja@google.com, Xinming Hu
In-Reply-To: <20161010234708.GA19969@localhost>
Hi Brian,
On Mon, Oct 10, 2016 at 4:47 PM, Brian Norris <briannorris@chromium.org> wrote:
> Hi Dmitry,
>
> On Mon, Oct 10, 2016 at 04:43:06PM -0700, Dmitry Torokhov wrote:
>> On Thu, Oct 6, 2016 at 6:03 AM, Amitkumar Karwar <akarwar@marvell.com> wrote:
>> >> From: linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-
>> >> owner@vger.kernel.org] On Behalf Of Brian Norris
>> >> Sent: Wednesday, October 05, 2016 10:00 PM
>> >> To: Amitkumar Karwar
>> >> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
>> >> rajatja@google.com; Xinming Hu
>> >> Subject: Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device
>> >> unregister
>> >>
>> >> On Wed, Oct 05, 2016 at 02:04:53PM +0000, Amitkumar Karwar wrote:
>> >> > > From: Brian Norris [mailto:briannorris@chromium.org]
>> >> > > Sent: Wednesday, October 05, 2016 3:28 AM
>> >> > > To: Amitkumar Karwar
>> >> > > Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
>> >> > > rajatja@google.com; briannorris@google.com; Xinming Hu
>> >> > > Subject: Re: [PATCH v2 1/2] mwifiex: reset card->adapter during
>> >> > > device unregister
>> >> > >
>> >> > > On Tue, Oct 04, 2016 at 10:38:24PM +0530, Amitkumar Karwar wrote:
>> >>
>> >> > > > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
>> >> > > > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
>> >> > > > @@ -3042,6 +3042,7 @@ static void mwifiex_unregister_dev(struct
>> >> > > mwifiex_adapter *adapter)
>> >> > > > pci_disable_msi(pdev);
>> >> > > > }
>> >> > > > }
>> >> > > > + card->adapter = NULL;
>> >> > >
>> >> > > I think you have a similar problem here as in patch 2; there is no
>> >> > > locking to protect fields in struct pcie_service_card or struct
>> >> > > sdio_mmc_card below. That problem kind of already exists, except
>> >> > > that you only write the value of card->adapter once at registration
>> >> > > time, so it's not actually unsafe. But now that you're introducing a
>> >> > > second write, you have a problem.
>> >> > >
>> >> > > Brian
>> >> > >
>> >> >
>> >> > We have a global "add_remove_card_sem" semaphore in our code for
>> >> > synchronizing initialization and teardown threads. Ideally "init +
>> >> > teardown/reboot" should not have a race issue with this logic
>> >> >
>> >> > Later there was a loophole introduced in this after using async
>> >> > firmware download API. During initialization, firmware downloads
>> >> > asynchronously in a separate thread where might have released the
>> >> > semaphore. I am working on a patch to fix this.
>> >> >
>> >> > So "card->adapter" doesn't need to have locking. Even if we have two
>> >> > write operations, those two threads can't run simultaneously due to
>> >> > above mentioned logic.
>> >>
>> >> What about writes racing with reads? You have lots of unsynchronized
>> >> cases that read this, although most of them should be halted by now
>> >> (e.g., cmd processing). I was looking at suspend() in particular, which
>> >> I thought you were looking at in this patch series.
>> >
>> > Please note that "card->adapter" is used only in pcie.c/sdio.c/usb.c files
>> >
>> > Writes won't have race with reads.
>> >
>> > 1) write 1 --- "card->adapter = adapter;" in mwifiex_register_dev()
>> > This place is at the beginning of initialization.
>> > mwifiex_pcie_probe() -> mwifiex_add_card() -> adapter->if_ops.register_dev()
>> > There is no chance that "card->adapter" is read anywhere at this point. FW is not yet downloaded
>> >
>> > 2) write 2 ---- "card->adapter = NULL;" in mwifiex_unregister_dev()
>> > This place the end of teardown phase.
>> > Interrupts are disabled and all cleanup is done. We have "card->adapter" NULL checks at entry point of suspend/remove/resume, if they get called after this.
>>
>> How exactly are you preventing ->suspend() from being called *while*
>> you are running mwifiex_unregister_dev()? I.e. user slamming the lid
>> of a laptop and throwing it in their backpack?
>
> As I already commented, isn't this primarily [*] called from the driver
> remove() callback? remove() doesn't race with suspend(), does it?
Nope, there is request_firmware_nowait() path that is asynchronous
with device registration/unregistration and power management. Maybe
the right way is to move the driver to async probing and switch that
request_firmware_nowait() into plain request_firmware(). I haven't
looked that closely.
The wording "we may end up accessing invalid memory in some corner
cases" which is a synonym for "the code is racy" caught my eye, thus
the response on the patch.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device unregister
From: Brian Norris @ 2016-10-10 23:47 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Amitkumar Karwar, linux-wireless@vger.kernel.org, Cathy Luo,
Nishant Sarmukadam, rajatja@google.com, Xinming Hu
In-Reply-To: <CAKdAkRTKo3JVJPamM44jyG6QuHoQB89FMKj5vo3-z6T+XH1W9Q@mail.gmail.com>
Hi Dmitry,
On Mon, Oct 10, 2016 at 04:43:06PM -0700, Dmitry Torokhov wrote:
> On Thu, Oct 6, 2016 at 6:03 AM, Amitkumar Karwar <akarwar@marvell.com> wrote:
> >> From: linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-
> >> owner@vger.kernel.org] On Behalf Of Brian Norris
> >> Sent: Wednesday, October 05, 2016 10:00 PM
> >> To: Amitkumar Karwar
> >> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> >> rajatja@google.com; Xinming Hu
> >> Subject: Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device
> >> unregister
> >>
> >> On Wed, Oct 05, 2016 at 02:04:53PM +0000, Amitkumar Karwar wrote:
> >> > > From: Brian Norris [mailto:briannorris@chromium.org]
> >> > > Sent: Wednesday, October 05, 2016 3:28 AM
> >> > > To: Amitkumar Karwar
> >> > > Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> >> > > rajatja@google.com; briannorris@google.com; Xinming Hu
> >> > > Subject: Re: [PATCH v2 1/2] mwifiex: reset card->adapter during
> >> > > device unregister
> >> > >
> >> > > On Tue, Oct 04, 2016 at 10:38:24PM +0530, Amitkumar Karwar wrote:
> >>
> >> > > > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> >> > > > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> >> > > > @@ -3042,6 +3042,7 @@ static void mwifiex_unregister_dev(struct
> >> > > mwifiex_adapter *adapter)
> >> > > > pci_disable_msi(pdev);
> >> > > > }
> >> > > > }
> >> > > > + card->adapter = NULL;
> >> > >
> >> > > I think you have a similar problem here as in patch 2; there is no
> >> > > locking to protect fields in struct pcie_service_card or struct
> >> > > sdio_mmc_card below. That problem kind of already exists, except
> >> > > that you only write the value of card->adapter once at registration
> >> > > time, so it's not actually unsafe. But now that you're introducing a
> >> > > second write, you have a problem.
> >> > >
> >> > > Brian
> >> > >
> >> >
> >> > We have a global "add_remove_card_sem" semaphore in our code for
> >> > synchronizing initialization and teardown threads. Ideally "init +
> >> > teardown/reboot" should not have a race issue with this logic
> >> >
> >> > Later there was a loophole introduced in this after using async
> >> > firmware download API. During initialization, firmware downloads
> >> > asynchronously in a separate thread where might have released the
> >> > semaphore. I am working on a patch to fix this.
> >> >
> >> > So "card->adapter" doesn't need to have locking. Even if we have two
> >> > write operations, those two threads can't run simultaneously due to
> >> > above mentioned logic.
> >>
> >> What about writes racing with reads? You have lots of unsynchronized
> >> cases that read this, although most of them should be halted by now
> >> (e.g., cmd processing). I was looking at suspend() in particular, which
> >> I thought you were looking at in this patch series.
> >
> > Please note that "card->adapter" is used only in pcie.c/sdio.c/usb.c files
> >
> > Writes won't have race with reads.
> >
> > 1) write 1 --- "card->adapter = adapter;" in mwifiex_register_dev()
> > This place is at the beginning of initialization.
> > mwifiex_pcie_probe() -> mwifiex_add_card() -> adapter->if_ops.register_dev()
> > There is no chance that "card->adapter" is read anywhere at this point. FW is not yet downloaded
> >
> > 2) write 2 ---- "card->adapter = NULL;" in mwifiex_unregister_dev()
> > This place the end of teardown phase.
> > Interrupts are disabled and all cleanup is done. We have "card->adapter" NULL checks at entry point of suspend/remove/resume, if they get called after this.
>
> How exactly are you preventing ->suspend() from being called *while*
> you are running mwifiex_unregister_dev()? I.e. user slamming the lid
> of a laptop and throwing it in their backpack?
As I already commented, isn't this primarily [*] called from the driver
remove() callback? remove() doesn't race with suspend(), does it?
[*] The other cases are in error handling cases. I guess I should make
sure those didn't race too...
Brian
^ permalink raw reply
* Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device unregister
From: Dmitry Torokhov @ 2016-10-10 23:43 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: Brian Norris, linux-wireless@vger.kernel.org, Cathy Luo,
Nishant Sarmukadam, rajatja@google.com, Xinming Hu
In-Reply-To: <127198026f5149c783bb0f5855dc87b9@SC-EXCH04.marvell.com>
On Thu, Oct 6, 2016 at 6:03 AM, Amitkumar Karwar <akarwar@marvell.com> wrote:
> Hi Brian,
>
>> From: linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-
>> owner@vger.kernel.org] On Behalf Of Brian Norris
>> Sent: Wednesday, October 05, 2016 10:00 PM
>> To: Amitkumar Karwar
>> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
>> rajatja@google.com; Xinming Hu
>> Subject: Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device
>> unregister
>>
>> Hi,
>>
>> On Wed, Oct 05, 2016 at 02:04:53PM +0000, Amitkumar Karwar wrote:
>> > > From: Brian Norris [mailto:briannorris@chromium.org]
>> > > Sent: Wednesday, October 05, 2016 3:28 AM
>> > > To: Amitkumar Karwar
>> > > Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
>> > > rajatja@google.com; briannorris@google.com; Xinming Hu
>> > > Subject: Re: [PATCH v2 1/2] mwifiex: reset card->adapter during
>> > > device unregister
>> > >
>> > > Hi,
>> > >
>> > > On Tue, Oct 04, 2016 at 10:38:24PM +0530, Amitkumar Karwar wrote:
>>
>> > > > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
>> > > > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
>> > > > @@ -3042,6 +3042,7 @@ static void mwifiex_unregister_dev(struct
>> > > mwifiex_adapter *adapter)
>> > > > pci_disable_msi(pdev);
>> > > > }
>> > > > }
>> > > > + card->adapter = NULL;
>> > >
>> > > I think you have a similar problem here as in patch 2; there is no
>> > > locking to protect fields in struct pcie_service_card or struct
>> > > sdio_mmc_card below. That problem kind of already exists, except
>> > > that you only write the value of card->adapter once at registration
>> > > time, so it's not actually unsafe. But now that you're introducing a
>> > > second write, you have a problem.
>> > >
>> > > Brian
>> > >
>> >
>> > We have a global "add_remove_card_sem" semaphore in our code for
>> > synchronizing initialization and teardown threads. Ideally "init +
>> > teardown/reboot" should not have a race issue with this logic
>> >
>> > Later there was a loophole introduced in this after using async
>> > firmware download API. During initialization, firmware downloads
>> > asynchronously in a separate thread where might have released the
>> > semaphore. I am working on a patch to fix this.
>> >
>> > So "card->adapter" doesn't need to have locking. Even if we have two
>> > write operations, those two threads can't run simultaneously due to
>> > above mentioned logic.
>>
>> What about writes racing with reads? You have lots of unsynchronized
>> cases that read this, although most of them should be halted by now
>> (e.g., cmd processing). I was looking at suspend() in particular, which
>> I thought you were looking at in this patch series.
>
> Please note that "card->adapter" is used only in pcie.c/sdio.c/usb.c files
>
> Writes won't have race with reads.
>
> 1) write 1 --- "card->adapter = adapter;" in mwifiex_register_dev()
> This place is at the beginning of initialization.
> mwifiex_pcie_probe() -> mwifiex_add_card() -> adapter->if_ops.register_dev()
> There is no chance that "card->adapter" is read anywhere at this point. FW is not yet downloaded
>
> 2) write 2 ---- "card->adapter = NULL;" in mwifiex_unregister_dev()
> This place the end of teardown phase.
> Interrupts are disabled and all cleanup is done. We have "card->adapter" NULL checks at entry point of suspend/remove/resume, if they get called after this.
How exactly are you preventing ->suspend() from being called *while*
you are running mwifiex_unregister_dev()? I.e. user slamming the lid
of a laptop and throwing it in their backpack?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] rtlwifi: Fix regression caused by commit d86e64768859
From: Larry Finger @ 2016-10-10 23:32 UTC (permalink / raw)
To: kvalo; +Cc: devel, linux-wireless, Larry Finger, Stable [ 4 . 8+ ],
Julia Lawall
In commit d86e64768859 ("rtlwifi: rtl818x: constify local structures"),
the configuration struct for most of the drivers was changed to be
constant. The problem is that five of the modified drivers need to be
able to update the firmware name based on the exact model of the card.
As the file names were stored in one of the members of that struct,
these drivers would fail with a kernel BUG splat when they tried to
update the firmware name.
Rather than reverting the previous commit, I used a suggestion by
Johannes Berg and made the firmware file name pointers be local to
the routines that update the software variables.
The configuration struct of rtl8192cu, which was not touched in the
previous patch, is now constantfied.
Fixes: d86e64768859 ("rtlwifi: rtl818x: constify local structures")
Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable [4.8+] <stable@vger.kernel.org>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
---
Kalle,
My apologies for letting these bugs to get by my review and testing. As
they affect kernel 4.8, please push this patch as soon as possible. To
reiterate, this patch replaces the one entitled 'Revert "rtlwifi: rtl818x:
constify local structures"'
Thanks,
Larry
drivers/net/wireless/realtek/rtlwifi/core.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c | 8 ++++----
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c | 10 +++++-----
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c | 12 ++++++------
drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c | 6 +++---
drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c | 8 ++++----
drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c | 9 +++++----
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c | 10 +++++-----
drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c | 6 +++---
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c | 18 +++++++++---------
drivers/net/wireless/realtek/rtlwifi/wifi.h | 2 --
11 files changed, 45 insertions(+), 46 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c
index f95760c..8e7f23c 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -111,7 +111,7 @@ static void rtl_fw_do_work(const struct firmware *firmware, void *context,
if (!err)
goto found_alt;
}
- pr_err("Firmware %s not available\n", rtlpriv->cfg->fw_name);
+ pr_err("Selected firmware is not available\n");
rtlpriv->max_fw_size = 0;
return;
}
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c
index e7b11b4..f361808 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c
@@ -86,6 +86,7 @@ int rtl88e_init_sw_vars(struct ieee80211_hw *hw)
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
u8 tid;
+ char *fw_name;
rtl8188ee_bt_reg_init(hw);
rtlpriv->dm.dm_initialgain_enable = 1;
@@ -169,10 +170,10 @@ int rtl88e_init_sw_vars(struct ieee80211_hw *hw)
return 1;
}
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8188efw.bin";
+ fw_name = "rtlwifi/rtl8188efw.bin";
rtlpriv->max_fw_size = 0x8000;
- pr_info("Using firmware %s\n", rtlpriv->cfg->fw_name);
- err = request_firmware_nowait(THIS_MODULE, 1, rtlpriv->cfg->fw_name,
+ pr_info("Using firmware %s\n", fw_name);
+ err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl_fw_cb);
if (err) {
@@ -284,7 +285,6 @@ static const struct rtl_hal_cfg rtl88ee_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl88e_pci",
- .fw_name = "rtlwifi/rtl8188efw.bin",
.ops = &rtl8188ee_hal_ops,
.mod_params = &rtl88ee_mod_params,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c
index 5c46a98..25cbd68 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c
@@ -92,6 +92,7 @@ int rtl92c_init_sw_vars(struct ieee80211_hw *hw)
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
+ char *fw_name;
rtl8192ce_bt_reg_init(hw);
@@ -165,13 +166,13 @@ int rtl92c_init_sw_vars(struct ieee80211_hw *hw)
/* request fw */
if (IS_VENDOR_UMC_A_CUT(rtlhal->version) &&
!IS_92C_SERIAL(rtlhal->version))
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8192cfwU.bin";
+ fw_name = "rtlwifi/rtl8192cfwU.bin";
else if (IS_81XXC_VENDOR_UMC_B_CUT(rtlhal->version))
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8192cfwU_B.bin";
+ fw_name = "rtlwifi/rtl8192cfwU_B.bin";
rtlpriv->max_fw_size = 0x4000;
- pr_info("Using firmware %s\n", rtlpriv->cfg->fw_name);
- err = request_firmware_nowait(THIS_MODULE, 1, rtlpriv->cfg->fw_name,
+ pr_info("Using firmware %s\n", fw_name);
+ err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl_fw_cb);
if (err) {
@@ -258,7 +259,6 @@ static const struct rtl_hal_cfg rtl92ce_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl92c_pci",
- .fw_name = "rtlwifi/rtl8192cfw.bin",
.ops = &rtl8192ce_hal_ops,
.mod_params = &rtl92ce_mod_params,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c
index 92588e0..b84e13a 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c
@@ -55,6 +55,7 @@ static int rtl92cu_init_sw_vars(struct ieee80211_hw *hw)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
int err;
+ char *fw_name;
rtlpriv->dm.dm_initialgain_enable = true;
rtlpriv->dm.dm_flag = 0;
@@ -73,18 +74,18 @@ static int rtl92cu_init_sw_vars(struct ieee80211_hw *hw)
}
if (IS_VENDOR_UMC_A_CUT(rtlpriv->rtlhal.version) &&
!IS_92C_SERIAL(rtlpriv->rtlhal.version)) {
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8192cufw_A.bin";
+ fw_name = "rtlwifi/rtl8192cufw_A.bin";
} else if (IS_81XXC_VENDOR_UMC_B_CUT(rtlpriv->rtlhal.version)) {
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8192cufw_B.bin";
+ fw_name = "rtlwifi/rtl8192cufw_B.bin";
} else {
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8192cufw_TMSC.bin";
+ fw_name = "rtlwifi/rtl8192cufw_TMSC.bin";
}
/* provide name of alternative file */
rtlpriv->cfg->alt_fw_name = "rtlwifi/rtl8192cufw.bin";
- pr_info("Loading firmware %s\n", rtlpriv->cfg->fw_name);
+ pr_info("Loading firmware %s\n", fw_name);
rtlpriv->max_fw_size = 0x4000;
err = request_firmware_nowait(THIS_MODULE, 1,
- rtlpriv->cfg->fw_name, rtlpriv->io.dev,
+ fw_name, rtlpriv->io.dev,
GFP_KERNEL, hw, rtl_fw_cb);
return err;
}
@@ -183,7 +184,6 @@ static struct rtl_hal_usbint_cfg rtl92cu_interface_cfg = {
static struct rtl_hal_cfg rtl92cu_hal_cfg = {
.name = "rtl92c_usb",
- .fw_name = "rtlwifi/rtl8192cufw.bin",
.ops = &rtl8192cu_hal_ops,
.mod_params = &rtl92cu_mod_params,
.usb_interface_cfg = &rtl92cu_interface_cfg,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c
index a9c39eb..2d65e40 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c
@@ -88,6 +88,7 @@ static int rtl92d_init_sw_vars(struct ieee80211_hw *hw)
u8 tid;
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
+ char *fw_name = "rtlwifi/rtl8192defw.bin";
rtlpriv->dm.dm_initialgain_enable = true;
rtlpriv->dm.dm_flag = 0;
@@ -177,10 +178,10 @@ static int rtl92d_init_sw_vars(struct ieee80211_hw *hw)
rtlpriv->max_fw_size = 0x8000;
pr_info("Driver for Realtek RTL8192DE WLAN interface\n");
- pr_info("Loading firmware file %s\n", rtlpriv->cfg->fw_name);
+ pr_info("Loading firmware file %s\n", fw_name);
/* request fw */
- err = request_firmware_nowait(THIS_MODULE, 1, rtlpriv->cfg->fw_name,
+ err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl_fw_cb);
if (err) {
@@ -262,7 +263,6 @@ static const struct rtl_hal_cfg rtl92de_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl8192de",
- .fw_name = "rtlwifi/rtl8192defw.bin",
.ops = &rtl8192de_hal_ops,
.mod_params = &rtl92de_mod_params,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c
index ac299cb..46b605de3 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c
@@ -91,6 +91,7 @@ int rtl92ee_init_sw_vars(struct ieee80211_hw *hw)
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
int err = 0;
+ char *fw_name;
rtl92ee_bt_reg_init(hw);
rtlpci->msi_support = rtlpriv->cfg->mod_params->msi_support;
@@ -170,11 +171,11 @@ int rtl92ee_init_sw_vars(struct ieee80211_hw *hw)
}
/* request fw */
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8192eefw.bin";
+ fw_name = "rtlwifi/rtl8192eefw.bin";
rtlpriv->max_fw_size = 0x8000;
- pr_info("Using firmware %s\n", rtlpriv->cfg->fw_name);
- err = request_firmware_nowait(THIS_MODULE, 1, rtlpriv->cfg->fw_name,
+ pr_info("Using firmware %s\n", fw_name);
+ err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl_fw_cb);
if (err) {
@@ -266,7 +267,6 @@ static const struct rtl_hal_cfg rtl92ee_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl92ee_pci",
- .fw_name = "rtlwifi/rtl8192eefw.bin",
.ops = &rtl8192ee_hal_ops,
.mod_params = &rtl92ee_mod_params,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c
index a652d45..998cefb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c
@@ -85,12 +85,13 @@ static void rtl92se_fw_cb(const struct firmware *firmware, void *context)
struct ieee80211_hw *hw = context;
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rt_firmware *pfirmware = NULL;
+ char *fw_name = "rtlwifi/rtl8192sefw.bin";
RT_TRACE(rtlpriv, COMP_ERR, DBG_LOUD,
"Firmware callback routine entered!\n");
complete(&rtlpriv->firmware_loading_complete);
if (!firmware) {
- pr_err("Firmware %s not available\n", rtlpriv->cfg->fw_name);
+ pr_err("Firmware %s not available\n", fw_name);
rtlpriv->max_fw_size = 0;
return;
}
@@ -113,6 +114,7 @@ static int rtl92s_init_sw_vars(struct ieee80211_hw *hw)
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
int err = 0;
u16 earlyrxthreshold = 7;
+ char *fw_name = "rtlwifi/rtl8192sefw.bin";
rtlpriv->dm.dm_initialgain_enable = true;
rtlpriv->dm.dm_flag = 0;
@@ -210,9 +212,9 @@ static int rtl92s_init_sw_vars(struct ieee80211_hw *hw)
rtlpriv->max_fw_size = RTL8190_MAX_FIRMWARE_CODE_SIZE*2 +
sizeof(struct fw_hdr);
pr_info("Driver for Realtek RTL8192SE/RTL8191SE\n"
- "Loading firmware %s\n", rtlpriv->cfg->fw_name);
+ "Loading firmware %s\n", fw_name);
/* request fw */
- err = request_firmware_nowait(THIS_MODULE, 1, rtlpriv->cfg->fw_name,
+ err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl92se_fw_cb);
if (err) {
@@ -306,7 +308,6 @@ static const struct rtl_hal_cfg rtl92se_hal_cfg = {
.bar_id = 1,
.write_readback = false,
.name = "rtl92s_pci",
- .fw_name = "rtlwifi/rtl8192sefw.bin",
.ops = &rtl8192se_hal_ops,
.mod_params = &rtl92se_mod_params,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c
index 89c828a..56ea6be 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c
@@ -94,6 +94,7 @@ int rtl8723e_init_sw_vars(struct ieee80211_hw *hw)
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
int err = 0;
+ char *fw_name;
rtl8723e_bt_reg_init(hw);
@@ -177,13 +178,13 @@ int rtl8723e_init_sw_vars(struct ieee80211_hw *hw)
}
if (IS_VENDOR_8723_A_CUT(rtlhal->version))
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8723fw.bin";
+ fw_name = "rtlwifi/rtl8723fw.bin";
else if (IS_81xxC_VENDOR_UMC_B_CUT(rtlhal->version))
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8723fw_B.bin";
+ fw_name = "rtlwifi/rtl8723fw_B.bin";
rtlpriv->max_fw_size = 0x6000;
- pr_info("Using firmware %s\n", rtlpriv->cfg->fw_name);
- err = request_firmware_nowait(THIS_MODULE, 1, rtlpriv->cfg->fw_name,
+ pr_info("Using firmware %s\n", fw_name);
+ err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl_fw_cb);
if (err) {
@@ -280,7 +281,6 @@ static const struct rtl_hal_cfg rtl8723e_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl8723e_pci",
- .fw_name = "rtlwifi/rtl8723efw.bin",
.ops = &rtl8723e_hal_ops,
.mod_params = &rtl8723e_mod_params,
.maps[SYS_ISO_CTRL] = REG_SYS_ISO_CTRL,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c
index 20b53f0..847644d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c
@@ -91,6 +91,7 @@ int rtl8723be_init_sw_vars(struct ieee80211_hw *hw)
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
struct rtl_mac *mac = rtl_mac(rtl_priv(hw));
+ char *fw_name = "rtlwifi/rtl8723befw.bin";
rtl8723be_bt_reg_init(hw);
rtlpriv->btcoexist.btc_ops = rtl_btc_get_ops_pointer();
@@ -184,8 +185,8 @@ int rtl8723be_init_sw_vars(struct ieee80211_hw *hw)
}
rtlpriv->max_fw_size = 0x8000;
- pr_info("Using firmware %s\n", rtlpriv->cfg->fw_name);
- err = request_firmware_nowait(THIS_MODULE, 1, rtlpriv->cfg->fw_name,
+ pr_info("Using firmware %s\n", fw_name);
+ err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl_fw_cb);
if (err) {
@@ -280,7 +281,6 @@ static const struct rtl_hal_cfg rtl8723be_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl8723be_pci",
- .fw_name = "rtlwifi/rtl8723befw.bin",
.ops = &rtl8723be_hal_ops,
.mod_params = &rtl8723be_mod_params,
.maps[SYS_ISO_CTRL] = REG_SYS_ISO_CTRL,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c
index 22f687b1..297938e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c
@@ -93,6 +93,7 @@ int rtl8821ae_init_sw_vars(struct ieee80211_hw *hw)
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
struct rtl_mac *mac = rtl_mac(rtl_priv(hw));
struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
+ char *fw_name, *wowlan_fw_name;
rtl8821ae_bt_reg_init(hw);
rtlpriv->btcoexist.btc_ops = rtl_btc_get_ops_pointer();
@@ -203,17 +204,17 @@ int rtl8821ae_init_sw_vars(struct ieee80211_hw *hw)
}
if (rtlhal->hw_type == HARDWARE_TYPE_RTL8812AE) {
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8812aefw.bin";
- rtlpriv->cfg->wowlan_fw_name = "rtlwifi/rtl8812aefw_wowlan.bin";
+ fw_name = "rtlwifi/rtl8812aefw.bin";
+ wowlan_fw_name = "rtlwifi/rtl8812aefw_wowlan.bin";
} else {
- rtlpriv->cfg->fw_name = "rtlwifi/rtl8821aefw.bin";
- rtlpriv->cfg->wowlan_fw_name = "rtlwifi/rtl8821aefw_wowlan.bin";
+ fw_name = "rtlwifi/rtl8821aefw.bin";
+ wowlan_fw_name = "rtlwifi/rtl8821aefw_wowlan.bin";
}
rtlpriv->max_fw_size = 0x8000;
/*load normal firmware*/
- pr_info("Using firmware %s\n", rtlpriv->cfg->fw_name);
- err = request_firmware_nowait(THIS_MODULE, 1, rtlpriv->cfg->fw_name,
+ pr_info("Using firmware %s\n", fw_name);
+ err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl_fw_cb);
if (err) {
@@ -222,9 +223,9 @@ int rtl8821ae_init_sw_vars(struct ieee80211_hw *hw)
return 1;
}
/*load wowlan firmware*/
- pr_info("Using firmware %s\n", rtlpriv->cfg->wowlan_fw_name);
+ pr_info("Using firmware %s\n", wowlan_fw_name);
err = request_firmware_nowait(THIS_MODULE, 1,
- rtlpriv->cfg->wowlan_fw_name,
+ wowlan_fw_name,
rtlpriv->io.dev, GFP_KERNEL, hw,
rtl_wowlan_fw_cb);
if (err) {
@@ -320,7 +321,6 @@ static const struct rtl_hal_cfg rtl8821ae_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl8821ae_pci",
- .fw_name = "rtlwifi/rtl8821aefw.bin",
.ops = &rtl8821ae_hal_ops,
.mod_params = &rtl8821ae_mod_params,
.maps[SYS_ISO_CTRL] = REG_SYS_ISO_CTRL,
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index 595f7d5..dafe486 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -2278,9 +2278,7 @@ struct rtl_hal_cfg {
u8 bar_id;
bool write_readback;
char *name;
- char *fw_name;
char *alt_fw_name;
- char *wowlan_fw_name;
struct rtl_hal_ops *ops;
struct rtl_mod_params *mod_params;
struct rtl_hal_usbint_cfg *usb_interface_cfg;
--
2.10.0
^ permalink raw reply related
* Re: [PATCH v4 2/3] mwifiex: remove redundant pdev check in suspend/resume handlers
From: Brian Norris @ 2016-10-10 20:54 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
briannorris
In-Reply-To: <1475777186-20486-2-git-send-email-akarwar@marvell.com>
On Thu, Oct 06, 2016 at 11:36:25PM +0530, Amitkumar Karwar wrote:
> to_pci_dev() would just do struct offset arithmetic on struct
> device to get 'pdev' pointer. We never get NULL pdev pointer
>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> New patch introduced in v3 as per inputs from Brian Norris.
> v4: Same as v3
> ---
> drivers/net/wireless/marvell/mwifiex/pcie.c | 22 ++++++----------------
> 1 file changed, 6 insertions(+), 16 deletions(-)
Reviewed-by: Brian Norris <briannorris@chromium.org>
^ permalink raw reply
* Re: [PATCH v4 1/3] mwifiex: reset card->adapter during device unregister
From: Brian Norris @ 2016-10-10 20:53 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
Xinming Hu, abhishekbh
In-Reply-To: <1475777186-20486-1-git-send-email-akarwar@marvell.com>
Hi Amit,
On Thu, Oct 06, 2016 at 11:36:24PM +0530, Amitkumar Karwar wrote:
> From: Xinming Hu <huxm@marvell.com>
>
> card->adapter gets initialized during device registration.
> As it's not cleared, we may end up accessing invalid memory
> in some corner cases. This patch fixes the problem.
>
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v4: Same as v1, v2, v3
> ---
> drivers/net/wireless/marvell/mwifiex/pcie.c | 1 +
> drivers/net/wireless/marvell/mwifiex/sdio.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index f1eeb73..ba9e068 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -3042,6 +3042,7 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
> pci_disable_msi(pdev);
> }
> }
> + card->adapter = NULL;
> }
>
> /* This function initializes the PCI-E host memory space, WCB rings, etc.
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 8718950..4cad1c2 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -2066,6 +2066,7 @@ mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
> struct sdio_mmc_card *card = adapter->card;
>
> if (adapter->card) {
> + card->adapter = NULL;
> sdio_claim_host(card->func);
> sdio_disable_func(card->func);
> sdio_release_host(card->func);
As discussed on v1, I had qualms about the raciness between reads/writes
of card->adapter, but I believe we:
(a) can't have any command activity while writing the ->adapter field
(either we're just init'ing the device, or we've disabled interrupts and
are tearing it down) and
(b) can't have a race between suspend()/resume() and unregister_dev(),
since unregister_dev() is called from device remove() (which should not
be concurrent with suspend()).
Also, I thought you had the same problem in usb.c, but in fact, you
fixed that ages ago here:
353d2a69ea26 mwifiex: fix issues in driver unload path for USB chipsets
Would be nice if fixes were bettery synchronized across the three
interface drivers you support. We seem to be discovering unnecessary
divergence on a few points recently.
At any rate:
Reviewed-by: Brian Norris <briannorris@chromium.org>
Tested-by: Brian Norris <briannorris@chromium.org>
Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device unregister
From: Brian Norris @ 2016-10-10 20:43 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless@vger.kernel.org, Cathy Luo, Nishant Sarmukadam,
rajatja@google.com, Xinming Hu, abhishekbh
In-Reply-To: <127198026f5149c783bb0f5855dc87b9@SC-EXCH04.marvell.com>
Hi Amit,
On Thu, Oct 06, 2016 at 01:03:02PM +0000, Amitkumar Karwar wrote:
> > From: linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-
> > owner@vger.kernel.org] On Behalf Of Brian Norris
> > Sent: Wednesday, October 05, 2016 10:00 PM
> > To: Amitkumar Karwar
> > Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> > rajatja@google.com; Xinming Hu
> > Subject: Re: [PATCH v2 1/2] mwifiex: reset card->adapter during device
> > unregister
> >
> > On Wed, Oct 05, 2016 at 02:04:53PM +0000, Amitkumar Karwar wrote:
> > > > From: Brian Norris [mailto:briannorris@chromium.org]
> > > > Sent: Wednesday, October 05, 2016 3:28 AM
> > > > To: Amitkumar Karwar
> > > > Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> > > > rajatja@google.com; briannorris@google.com; Xinming Hu
> > > > Subject: Re: [PATCH v2 1/2] mwifiex: reset card->adapter during
> > > > device unregister
> > > >
> > > > On Tue, Oct 04, 2016 at 10:38:24PM +0530, Amitkumar Karwar wrote:
> >
> > > > > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> > > > > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > > > > @@ -3042,6 +3042,7 @@ static void mwifiex_unregister_dev(struct
> > > > mwifiex_adapter *adapter)
> > > > > pci_disable_msi(pdev);
> > > > > }
> > > > > }
> > > > > + card->adapter = NULL;
...
> > What about writes racing with reads? You have lots of unsynchronized
> > cases that read this, although most of them should be halted by now
> > (e.g., cmd processing). I was looking at suspend() in particular, which
> > I thought you were looking at in this patch series.
>
> Please note that "card->adapter" is used only in pcie.c/sdio.c/usb.c files
>
> Writes won't have race with reads.
>
> 1) write 1 --- "card->adapter = adapter;" in mwifiex_register_dev()
> This place is at the beginning of initialization.
> mwifiex_pcie_probe() -> mwifiex_add_card() -> adapter->if_ops.register_dev()
> There is no chance that "card->adapter" is read anywhere at this point. FW is not yet downloaded
Sure.
> 2) write 2 ---- "card->adapter = NULL;" in mwifiex_unregister_dev()
> This place the end of teardown phase.
> Interrupts are disabled and all cleanup is done. We have
> "card->adapter" NULL checks at entry point of
> suspend/remove/resume, if they get called after this.
I guess the question boils down to: can driver suspend() race with
mwifiex_unregister_dev() here, then?
And I guess the answer is "no", because unregistration only happens via
PCIe driver remove() -> mwifiex_remove_card() -> adapter->if_ops.unregister_dev()
and I think the device driver core guarantees that suspend() and
remove() won't race.
In that case:
Reviewed-by: Brian Norris <briannorris@chromium.org>
I'll reply to the latest revision too, since it's identical.
Thanks for the explanations.
Regards,
Brian
^ permalink raw reply
* Re: [PATCH] Revert "rtlwifi: rtl818x: constify local structures"
From: Julia Lawall @ 2016-10-10 20:33 UTC (permalink / raw)
To: Larry Finger; +Cc: Johannes Berg, kvalo, devel, linux-wireless, Stable
In-Reply-To: <90ea968e-dc19-5c2e-8aa4-6296a1d04381@lwfinger.net>
On Mon, 10 Oct 2016, Larry Finger wrote:
> On 10/10/2016 11:56 AM, Johannes Berg wrote:
> > On Mon, 2016-10-10 at 10:25 -0500, Larry Finger wrote:
> > > This reverts commit d86e64768859fca82c78e52877ceeba04e25d27a.
> > >
> > > For drivers rtl8188ee, rtl8192ce, rtl8192ee, rtl8723ae, and
> > > rtl8821ae,
> > > the Coccinelle script missed the fact that the code changes the
> > > firmware
> > > name. When that happens, the kernel issues a BUG splat because
> > > it is unable to overwrite the old name.
> >
> > Hmm. That seems somewhat problematic, for example if you have multiple
> > devices that use the same driver but need different firmware?
> >
> > Not that I really know what's going on, but changing static variables
> > based on runtime seems like it could cause issues in such cases.
>
> I think the situation is OK, but I have created a patch that converts all the
> firmware names into local strings in the routines that initiate firmware
> loading. That way the affected structs can be constified without problem.
Great, thanks :)
julia
>
> @Kalle: Please drop the patch with this subject.
>
> Thanks,
>
> Larry
>
^ permalink raw reply
* Re: [PATCH] Revert "rtlwifi: rtl818x: constify local structures"
From: Larry Finger @ 2016-10-10 20:28 UTC (permalink / raw)
To: Johannes Berg, kvalo; +Cc: devel, linux-wireless, Stable, Julia Lawall
In-Reply-To: <1476118573.7895.13.camel@sipsolutions.net>
On 10/10/2016 11:56 AM, Johannes Berg wrote:
> On Mon, 2016-10-10 at 10:25 -0500, Larry Finger wrote:
>> This reverts commit d86e64768859fca82c78e52877ceeba04e25d27a.
>>
>> For drivers rtl8188ee, rtl8192ce, rtl8192ee, rtl8723ae, and
>> rtl8821ae,
>> the Coccinelle script missed the fact that the code changes the
>> firmware
>> name. When that happens, the kernel issues a BUG splat because
>> it is unable to overwrite the old name.
>
> Hmm. That seems somewhat problematic, for example if you have multiple
> devices that use the same driver but need different firmware?
>
> Not that I really know what's going on, but changing static variables
> based on runtime seems like it could cause issues in such cases.
I think the situation is OK, but I have created a patch that converts all the
firmware names into local strings in the routines that initiate firmware
loading. That way the affected structs can be constified without problem.
@Kalle: Please drop the patch with this subject.
Thanks,
Larry
^ permalink raw reply
* [PATCH v6 2/4] mac80211: filter multicast data packets on AP / AP_VLAN
From: Michael Braun @ 2016-10-10 17:12 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan, netdev
In-Reply-To: <1476119543-24509-1-git-send-email-michael-dev@fami-braun.de>
This patch adds filtering for multicast data packets on AP_VLAN interfaces
that have no authorized station connected and changes filtering on AP
interfaces to not count stations assigned to AP_VLAN interfaces.
This saves airtime and avoids waking up other stations currently authorized
in this BSS. When using WPA, the packets dropped could not be decrypted by
any station.
The behaviour when there are no AP_VLAN interfaces is left unchanged.
When there are AP_VLAN interfaces, this patch
1. adds filtering multicast data packets sent on AP_VLAN interfaces that
have no authorized station connected.
No filtering happens on 4addr AP_VLAN interfaces.
2. makes filtering of multicast data packets sent on AP interfaces depend
on the number of authorized stations in this bss not assigned to an
AP_VLAN interface.
Therefore, a new num_mcast_sta counter is added for AP_VLAN interfaces.
The existing one for AP interfaces is altered to not track stations
assigned to an AP_VLAN interface.
The new counter is exposed in debugfs.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
--
v4:
- update description
v3:
- reuse existing num_mcast_sta
v2:
- use separate function to inc/dec mcast_sta counters
- do not filter in 4addr mode
- change description
- change filtering on AP interface (do not count AP_VLAN sta)
- use new counters regardless of 4addr or not
- simplify cfg.c:change_station
- remove no-op change in __cleanup_single_sta
---
net/mac80211/cfg.c | 20 ++++++--------------
net/mac80211/debugfs_netdev.c | 11 +++++++++++
net/mac80211/ieee80211_i.h | 33 +++++++++++++++++++++++++++++++++
net/mac80211/rx.c | 5 +++--
net/mac80211/sta_info.c | 10 ++--------
net/mac80211/tx.c | 5 ++---
6 files changed, 57 insertions(+), 27 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 24133f5..1edb017 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1357,9 +1357,6 @@ static int ieee80211_change_station(struct wiphy *wiphy,
goto out_err;
if (params->vlan && params->vlan != sta->sdata->dev) {
- bool prev_4addr = false;
- bool new_4addr = false;
-
vlansdata = IEEE80211_DEV_TO_SUB_IF(params->vlan);
if (params->vlan->ieee80211_ptr->use_4addr) {
@@ -1369,26 +1366,21 @@ static int ieee80211_change_station(struct wiphy *wiphy,
}
rcu_assign_pointer(vlansdata->u.vlan.sta, sta);
- new_4addr = true;
__ieee80211_check_fast_rx_iface(vlansdata);
}
if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
- sta->sdata->u.vlan.sta) {
+ sta->sdata->u.vlan.sta)
RCU_INIT_POINTER(sta->sdata->u.vlan.sta, NULL);
- prev_4addr = true;
- }
+
+ if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+ ieee80211_vif_dec_num_mcast(sta->sdata);
sta->sdata = vlansdata;
ieee80211_check_fast_xmit(sta);
- if (sta->sta_state == IEEE80211_STA_AUTHORIZED &&
- prev_4addr != new_4addr) {
- if (new_4addr)
- atomic_dec(&sta->sdata->bss->num_mcast_sta);
- else
- atomic_inc(&sta->sdata->bss->num_mcast_sta);
- }
+ if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+ ieee80211_vif_inc_num_mcast(sta->sdata);
ieee80211_send_layer2_update(sta);
}
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index a5ba739..ed7bff4 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -477,6 +477,7 @@ IEEE80211_IF_FILE_RW(tdls_wider_bw);
IEEE80211_IF_FILE(num_mcast_sta, u.ap.num_mcast_sta, ATOMIC);
IEEE80211_IF_FILE(num_sta_ps, u.ap.ps.num_sta_ps, ATOMIC);
IEEE80211_IF_FILE(dtim_count, u.ap.ps.dtim_count, DEC);
+IEEE80211_IF_FILE(num_mcast_sta_vlan, u.vlan.num_mcast_sta, ATOMIC);
static ssize_t ieee80211_if_fmt_num_buffered_multicast(
const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -643,6 +644,13 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
DEBUGFS_ADD_MODE(tkip_mic_test, 0200);
}
+static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
+{
+ /* add num_mcast_sta_vlan using name num_mcast_sta */
+ debugfs_create_file("num_mcast_sta", 0400, sdata->vif.debugfs_dir,
+ sdata, &num_mcast_sta_vlan_ops);
+}
+
static void add_ibss_files(struct ieee80211_sub_if_data *sdata)
{
DEBUGFS_ADD_MODE(tsf, 0600);
@@ -746,6 +754,9 @@ static void add_files(struct ieee80211_sub_if_data *sdata)
case NL80211_IFTYPE_AP:
add_ap_files(sdata);
break;
+ case NL80211_IFTYPE_AP_VLAN:
+ add_vlan_files(sdata);
+ break;
case NL80211_IFTYPE_WDS:
add_wds_files(sdata);
break;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f56d342..70c0963 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -305,6 +305,7 @@ struct ieee80211_if_vlan {
/* used for all tx if the VLAN is configured to 4-addr mode */
struct sta_info __rcu *sta;
+ atomic_t num_mcast_sta; /* number of stations receiving multicast */
};
struct mesh_stats {
@@ -1496,6 +1497,38 @@ ieee80211_have_rx_timestamp(struct ieee80211_rx_status *status)
return false;
}
+static inline void
+ieee80211_vif_inc_num_mcast(struct ieee80211_sub_if_data *sdata)
+{
+ if (sdata->vif.type == NL80211_IFTYPE_AP)
+ atomic_inc(&sdata->u.ap.num_mcast_sta);
+ else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+ atomic_inc(&sdata->u.vlan.num_mcast_sta);
+}
+
+static inline void
+ieee80211_vif_dec_num_mcast(struct ieee80211_sub_if_data *sdata)
+{
+ if (sdata->vif.type == NL80211_IFTYPE_AP)
+ atomic_dec(&sdata->u.ap.num_mcast_sta);
+ else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+ atomic_dec(&sdata->u.vlan.num_mcast_sta);
+}
+
+/* This function returns the number of multicast stations connected to this
+ * interface. It returns -1 if that number is not tracked, that is for netdevs
+ * not in AP or AP_VLAN mode or when using 4addr.
+ */
+static inline int
+ieee80211_vif_get_num_mcast_if(struct ieee80211_sub_if_data *sdata)
+{
+ if (sdata->vif.type == NL80211_IFTYPE_AP)
+ return atomic_read(&sdata->u.ap.num_mcast_sta);
+ if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN && !sdata->u.vlan.sta)
+ return atomic_read(&sdata->u.vlan.num_mcast_sta);
+ return -1;
+}
+
u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
struct ieee80211_rx_status *status,
unsigned int mpdu_len,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 9617f93..252d922 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2160,7 +2160,8 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
sdata->vif.type == NL80211_IFTYPE_AP_VLAN) &&
!(sdata->flags & IEEE80211_SDATA_DONT_BRIDGE_PACKETS) &&
(sdata->vif.type != NL80211_IFTYPE_AP_VLAN || !sdata->u.vlan.sta)) {
- if (is_multicast_ether_addr(ehdr->h_dest)) {
+ if (is_multicast_ether_addr(ehdr->h_dest) &&
+ ieee80211_vif_get_num_mcast_if(sdata) != 0) {
/*
* send multicast frames both to higher layers in
* local net stack and back to the wireless medium
@@ -2169,7 +2170,7 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
if (!xmit_skb)
net_info_ratelimited("%s: failed to clone multicast frame\n",
dev->name);
- } else {
+ } else if (!is_multicast_ether_addr(ehdr->h_dest)) {
dsta = sta_info_get(sdata, skb->data);
if (dsta) {
/*
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 76b737d..216ef65 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1882,10 +1882,7 @@ int sta_info_move_state(struct sta_info *sta,
if (!sta->sta.support_p2p_ps)
ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
} else if (sta->sta_state == IEEE80211_STA_AUTHORIZED) {
- if (sta->sdata->vif.type == NL80211_IFTYPE_AP ||
- (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
- !sta->sdata->u.vlan.sta))
- atomic_dec(&sta->sdata->bss->num_mcast_sta);
+ ieee80211_vif_dec_num_mcast(sta->sdata);
clear_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
ieee80211_clear_fast_xmit(sta);
ieee80211_clear_fast_rx(sta);
@@ -1893,10 +1890,7 @@ int sta_info_move_state(struct sta_info *sta,
break;
case IEEE80211_STA_AUTHORIZED:
if (sta->sta_state == IEEE80211_STA_ASSOC) {
- if (sta->sdata->vif.type == NL80211_IFTYPE_AP ||
- (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
- !sta->sdata->u.vlan.sta))
- atomic_inc(&sta->sdata->bss->num_mcast_sta);
+ ieee80211_vif_inc_num_mcast(sta->sdata);
set_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
ieee80211_check_fast_xmit(sta);
ieee80211_check_fast_rx(sta);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 5023966..c3ce86e 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -331,9 +331,8 @@ ieee80211_tx_h_check_assoc(struct ieee80211_tx_data *tx)
I802_DEBUG_INC(tx->local->tx_handlers_drop_not_assoc);
return TX_DROP;
}
- } else if (unlikely(tx->sdata->vif.type == NL80211_IFTYPE_AP &&
- ieee80211_is_data(hdr->frame_control) &&
- !atomic_read(&tx->sdata->u.ap.num_mcast_sta))) {
+ } else if (unlikely(ieee80211_is_data(hdr->frame_control) &&
+ ieee80211_vif_get_num_mcast_if(tx->sdata) == 0)) {
/*
* No associated STAs - no need to send multicast
* frames.
--
2.1.4
^ permalink raw reply related
* [PATCH v6 3/4] cfg80211: configure multicast to unicast for AP interfaces
From: Michael Braun @ 2016-10-10 17:12 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan, netdev
In-Reply-To: <1476119543-24509-1-git-send-email-michael-dev@fami-braun.de>
This add a userspace toggle to configure multicast to unicast.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
--
v6:
- clarify documentation
- fix policy for NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED
---
include/net/cfg80211.h | 6 ++++++
include/uapi/linux/nl80211.h | 18 ++++++++++++++++++
net/wireless/nl80211.c | 36 ++++++++++++++++++++++++++++++++++++
net/wireless/rdev-ops.h | 12 ++++++++++++
net/wireless/trace.h | 19 +++++++++++++++++++
5 files changed, 91 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7ce6223..7b0941d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2460,6 +2460,8 @@ struct cfg80211_qos_map {
*
* @set_wds_peer: set the WDS peer for a WDS interface
*
+ * @set_multicast_to_unicast: configure multicast to unicast conversion for BSS
+ *
* @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting
* functions to adjust rfkill hw state
*
@@ -2722,6 +2724,10 @@ struct cfg80211_ops {
int (*set_wds_peer)(struct wiphy *wiphy, struct net_device *dev,
const u8 *addr);
+ int (*set_multicast_to_unicast)(struct wiphy *wiphy,
+ struct net_device *dev,
+ const bool enabled);
+
void (*rfkill_poll)(struct wiphy *wiphy);
#ifdef CONFIG_NL80211_TESTMODE
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 2206941..327bbb8 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -599,6 +599,17 @@
*
* @NL80211_CMD_SET_WDS_PEER: Set the MAC address of the peer on a WDS interface.
*
+ * @NL80211_CMD_SET_MULTICAST_TO_UNICAST: Configure if this AP should perform
+ * multicast to unicast conversion. When enabled, all multicast packets
+ * with ethertype ARP, IPv4 or IPv6 (possibly within an 802.1q header)
+ * will be sent out to each station once with the destination (multicast)
+ * mac address replaced by the stations mac address.
+ * This can only be toggled per BSS. Configure this on an interface of
+ * type %NL80211_IFTYPE_AP. It applies to all its vlans interfaces
+ * (%NL80211_IFTYPE_AP_VLAN), except for those in 4addr (WDS) mode.
+ * If %NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED is not present with this
+ * command, the feature is disabled.
+ *
* @NL80211_CMD_JOIN_MESH: Join a mesh. The mesh ID must be given, and initial
* mesh config parameters may be given.
* @NL80211_CMD_LEAVE_MESH: Leave the mesh network -- no special arguments, the
@@ -1026,6 +1037,8 @@ enum nl80211_commands {
NL80211_CMD_ABORT_SCAN,
+ NL80211_CMD_SET_MULTICAST_TO_UNICAST,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -1867,6 +1880,9 @@ enum nl80211_commands {
* @NL80211_ATTR_MESH_PEER_AID: Association ID for the mesh peer (u16). This is
* used to pull the stored data for mesh peer in power save state.
*
+ * @NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED: Multicast packets should be
+ * send out as unicast to all stations.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2261,6 +2277,8 @@ enum nl80211_attrs {
NL80211_ATTR_MESH_PEER_AID,
+ NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f02653a..3684e28 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -409,6 +409,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
.len = VHT_MUMIMO_GROUPS_DATA_LEN
},
[NL80211_ATTR_MU_MIMO_FOLLOW_MAC_ADDR] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
};
/* policy for the key attributes */
@@ -1538,6 +1539,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
goto nla_put_failure;
}
CMD(set_wds_peer, SET_WDS_PEER);
+ CMD(set_multicast_to_unicast, SET_MULTICAST_TO_UNICAST);
if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS) {
CMD(tdls_mgmt, TDLS_MGMT);
CMD(tdls_oper, TDLS_OPER);
@@ -2164,6 +2166,32 @@ static int nl80211_set_wds_peer(struct sk_buff *skb, struct genl_info *info)
return rdev_set_wds_peer(rdev, dev, bssid);
}
+static int nl80211_set_multicast_to_unicast(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct net_device *dev = info->user_ptr[1];
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ const struct nlattr *nla;
+ bool enabled;
+
+ if (!info->attrs[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED])
+ return -EINVAL;
+
+ if (netif_running(dev))
+ return -EBUSY;
+
+ if (!rdev->ops->set_multicast_to_unicast)
+ return -EOPNOTSUPP;
+
+ if (wdev->iftype != NL80211_IFTYPE_AP)
+ return -EOPNOTSUPP;
+
+ nla = info->attrs[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED];
+ enabled = nla_get_flag(nla);
+ return rdev_set_multicast_to_unicast(rdev, dev, enabled);
+}
+
static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
{
struct cfg80211_registered_device *rdev;
@@ -11574,6 +11602,14 @@ static const struct genl_ops nl80211_ops[] = {
NL80211_FLAG_NEED_RTNL,
},
{
+ .cmd = NL80211_CMD_SET_MULTICAST_TO_UNICAST,
+ .doit = nl80211_set_multicast_to_unicast,
+ .policy = nl80211_policy,
+ .flags = GENL_UNS_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
.cmd = NL80211_CMD_JOIN_MESH,
.doit = nl80211_join_mesh,
.policy = nl80211_policy,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 85ff30b..7d93c3d 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -562,6 +562,18 @@ static inline int rdev_set_wds_peer(struct cfg80211_registered_device *rdev,
return ret;
}
+static inline int
+rdev_set_multicast_to_unicast(struct cfg80211_registered_device *rdev,
+ struct net_device *dev,
+ const bool enabled)
+{
+ int ret;
+ trace_rdev_set_multicast_to_unicast(&rdev->wiphy, dev, enabled);
+ ret = rdev->ops->set_multicast_to_unicast(&rdev->wiphy, dev, enabled);
+ trace_rdev_return_int(&rdev->wiphy, ret);
+ return ret;
+}
+
static inline void rdev_rfkill_poll(struct cfg80211_registered_device *rdev)
{
trace_rdev_rfkill_poll(&rdev->wiphy);
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 72b5255..8c9aa57 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2940,6 +2940,25 @@ DEFINE_EVENT(wiphy_wdev_evt, rdev_abort_scan,
TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
TP_ARGS(wiphy, wdev)
);
+
+TRACE_EVENT(rdev_set_multicast_to_unicast,
+ TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
+ const bool enabled),
+ TP_ARGS(wiphy, netdev, enabled),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ NETDEV_ENTRY
+ __field(bool, enabled)
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ NETDEV_ASSIGN;
+ __entry->enabled = enabled;
+ ),
+ TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", unicast: %s",
+ WIPHY_PR_ARG, NETDEV_PR_ARG,
+ BOOL_TO_STR(__entry->enabled))
+);
#endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */
#undef TRACE_INCLUDE_PATH
--
2.1.4
^ permalink raw reply related
* [PATCH v6 1/4] mac80211: remove unnecessary num_mcast_sta user
From: Michael Braun @ 2016-10-10 17:12 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan, netdev
Checking for num_mcast_sta in __ieee80211_request_smps_ap() is unnecessary,
as sta list will be empty in this case anyway, so list_for_each_entry(sta,
...) will exit immediately.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
net/mac80211/cfg.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 543b1d4..24133f5 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2313,13 +2313,6 @@ int __ieee80211_request_smps_ap(struct ieee80211_sub_if_data *sdata,
smps_mode == IEEE80211_SMPS_AUTOMATIC)
return 0;
- /* If no associated stations, there's no need to do anything */
- if (!atomic_read(&sdata->u.ap.num_mcast_sta)) {
- sdata->smps_mode = smps_mode;
- ieee80211_queue_work(&sdata->local->hw, &sdata->recalc_smps);
- return 0;
- }
-
ht_dbg(sdata,
"SMPS %d requested in AP mode, sending Action frame to %d stations\n",
smps_mode, atomic_read(&sdata->u.ap.num_mcast_sta));
--
2.1.4
^ permalink raw reply related
* [PATCH v6 4/4] mac80211: multicast to unicast conversion
From: Michael Braun @ 2016-10-10 17:12 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan, netdev
In-Reply-To: <1476119543-24509-1-git-send-email-michael-dev@fami-braun.de>
This patch adds support for sending multicast data packets with ARP, IPv4
and IPv6 payload (possible 802.1q tagged) as 802.11 unicast frames to all
stations.
IEEE 802.11 multicast has well known issues, among them:
1. packets are not acked and hence not retransmitted, resulting in
decreased reliablity
2. packets are send at low rate, increasing time required on air
When used with AP_VLAN, there is another disadvantage:
3. all stations in the BSS are woken up, regardsless of their AP_VLAN
assignment.
By doing multicast to unicast conversion, all three issus are solved.
IEEE802.11-2012 proposes directed multicast service (DMS) using A-MSDU
frames and a station initiated control protocol. It has the advantage that
the station can recover the destination multicast mac address, but it is
not backward compatible with non QOS stations and does not enable the
administrator of a BSS to force this mode of operation within a BSS.
Additionally, it would require both the ap and the station to implement
the control protocol, which is optional on both ends. Furthermore, I've
seen a few mobile phone stations locally that indicate qos support but
won't complete DHCP if their broadcasts are encapsulated as A-MSDU. Though
they work fine with this series approach.
This patch therefore does not opt to implement DMS but instead just
replicates the packet and changes the destination address. As this works
fine with ARP, IPv4 and IPv6, it is limited to these protocols and normal
802.11 multicast frames are send out for all other payload protocols.
There is a runtime toggle to enable multicast conversion in a per-bss
fashion.
When there is only a single station assigned to the AP_VLAN interface, no
packet replication will occur. 4addr mode of operation is unchanged.
This change opts for iterating all BSS stations for finding the stations
assigned to this AP/AP_VLAN interface, as there currently is no per
AP_VLAN list to iterate and multicast packets are expected to be few.
If needed, such a list could be added later.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
--
v5:
- rename bss->unicast to bss->multicast_to_unicast
- access sdata->bss only after checking iftype
v4:
- rename MULTICAST_TO_UNICAST to MULTICAST_TO_UNICAST
v3: fix compile error for trace.h
v2: add nl80211 toggle
rename tx_dnat to change_da
change int to bool unicast
---
net/mac80211/cfg.c | 15 +++++++
net/mac80211/debugfs_netdev.c | 3 ++
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/tx.c | 101 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 120 insertions(+)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1edb017..1db4c3e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2242,6 +2242,20 @@ static int ieee80211_set_wds_peer(struct wiphy *wiphy, struct net_device *dev,
return 0;
}
+static int ieee80211_set_multicast_to_unicast(struct wiphy *wiphy,
+ struct net_device *dev,
+ const bool enabled)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+ if (sdata->vif.type != NL80211_IFTYPE_AP)
+ return -1;
+
+ sdata->u.ap.multicast_to_unicast = enabled;
+
+ return 0;
+}
+
static void ieee80211_rfkill_poll(struct wiphy *wiphy)
{
struct ieee80211_local *local = wiphy_priv(wiphy);
@@ -3400,6 +3414,7 @@ const struct cfg80211_ops mac80211_config_ops = {
.set_tx_power = ieee80211_set_tx_power,
.get_tx_power = ieee80211_get_tx_power,
.set_wds_peer = ieee80211_set_wds_peer,
+ .set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
.rfkill_poll = ieee80211_rfkill_poll,
CFG80211_TESTMODE_CMD(ieee80211_testmode_cmd)
CFG80211_TESTMODE_DUMP(ieee80211_testmode_dump)
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index ed7bff4..509c6c3 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -487,6 +487,8 @@ static ssize_t ieee80211_if_fmt_num_buffered_multicast(
}
IEEE80211_IF_FILE_R(num_buffered_multicast);
+IEEE80211_IF_FILE(multicast_to_unicast, u.ap.multicast_to_unicast, HEX);
+
/* IBSS attributes */
static ssize_t ieee80211_if_fmt_tsf(
const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -642,6 +644,7 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
DEBUGFS_ADD(dtim_count);
DEBUGFS_ADD(num_buffered_multicast);
DEBUGFS_ADD_MODE(tkip_mic_test, 0200);
+ DEBUGFS_ADD_MODE(multicast_to_unicast, 0600);
}
static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 70c0963..84374ed 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -293,6 +293,7 @@ struct ieee80211_if_ap {
driver_smps_mode; /* smps mode request */
struct work_struct request_smps_work;
+ bool multicast_to_unicast;
};
struct ieee80211_if_wds {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index c3ce86e..142201d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -16,6 +16,7 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/skbuff.h>
+#include <linux/if_vlan.h>
#include <linux/etherdevice.h>
#include <linux/bitmap.h>
#include <linux/rcupdate.h>
@@ -1770,6 +1771,102 @@ bool ieee80211_tx_prepare_skb(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_tx_prepare_skb);
+/* rewrite destination mac address */
+static int ieee80211_change_da(struct sk_buff *skb, struct sta_info *sta)
+{
+ struct ethhdr *eth;
+ int err;
+
+ err = skb_ensure_writable(skb, ETH_HLEN);
+ if (unlikely(err))
+ return err;
+
+ eth = (void *)skb->data;
+ ether_addr_copy(eth->h_dest, sta->sta.addr);
+
+ return 0;
+}
+
+/* Check if multicast to unicast conversion is needed and do it.
+ * Returns 1 if skb was freed and should not be send out.
+ */
+static int
+ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
+ struct sk_buff *skb, u32 info_flags)
+{
+ struct ieee80211_local *local = sdata->local;
+ const struct ethhdr *eth = (void *)skb->data;
+ const struct vlan_ethhdr *ethvlan = (void *)skb->data;
+ struct sta_info *sta, *prev = NULL;
+ struct sk_buff *cloned_skb;
+ u16 ethertype;
+
+ /* check if this is a multicast frame */
+ if (!is_multicast_ether_addr(eth->h_dest))
+ return 0;
+
+ /* multicast to unicast conversion only for AP interfaces */
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_AP_VLAN:
+ sta = rcu_dereference(sdata->u.vlan.sta);
+ if (sta) /* 4addr */
+ return 0;
+ /* fall through */
+ case NL80211_IFTYPE_AP:
+ /* check runtime toggle for this bss */
+ if (!sdata->bss->multicast_to_unicast)
+ return 0;
+ break;
+ default:
+ return 0;
+ }
+
+ /* info_flags would not get preserved, used only by TLDS */
+ if (info_flags)
+ return 0;
+
+ /* multicast to unicast conversion only for some payload */
+ ethertype = ntohs(eth->h_proto);
+ if (ethertype == ETH_P_8021Q && skb->len >= VLAN_ETH_HLEN)
+ ethertype = ntohs(ethvlan->h_vlan_encapsulated_proto);
+ switch (ethertype) {
+ case ETH_P_ARP:
+ case ETH_P_IP:
+ case ETH_P_IPV6:
+ break;
+ default:
+ return 0;
+ }
+
+ /* clone packets and update destination mac */
+ list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ if (sdata != sta->sdata)
+ continue;
+ if (unlikely(!ether_addr_equal(eth->h_source, sta->sta.addr)))
+ /* do not send back to source */
+ continue;
+ if (!prev) {
+ prev = sta;
+ continue;
+ }
+ cloned_skb = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(ieee80211_change_da(cloned_skb, prev))) {
+ dev_kfree_skb(cloned_skb);
+ continue;
+ }
+ __ieee80211_subif_start_xmit(cloned_skb, cloned_skb->dev, 0);
+ prev = sta;
+ }
+
+ if (likely(prev)) {
+ ieee80211_change_da(skb, prev);
+ return 0;
+ }
+
+ /* no STA connected, drop */
+ return 1;
+}
+
/*
* Returns false if the frame couldn't be transmitted but was queued instead.
*/
@@ -3353,6 +3450,10 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
rcu_read_lock();
+ /* AP multicast to unicast conversion */
+ if (ieee80211_tx_multicast_to_unicast(sdata, skb, info_flags))
+ goto out_free;
+
if (ieee80211_lookup_ra_sta(sdata, skb, &sta))
goto out_free;
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] Revert "rtlwifi: rtl818x: constify local structures"
From: Johannes Berg @ 2016-10-10 16:56 UTC (permalink / raw)
To: Larry Finger, kvalo; +Cc: devel, linux-wireless, Stable, Julia Lawall
In-Reply-To: <20161010152520.9812-1-Larry.Finger@lwfinger.net>
On Mon, 2016-10-10 at 10:25 -0500, Larry Finger wrote:
> This reverts commit d86e64768859fca82c78e52877ceeba04e25d27a.
>
> For drivers rtl8188ee, rtl8192ce, rtl8192ee, rtl8723ae, and
> rtl8821ae,
> the Coccinelle script missed the fact that the code changes the
> firmware
> name. When that happens, the kernel issues a BUG splat because
> it is unable to overwrite the old name.
Hmm. That seems somewhat problematic, for example if you have multiple
devices that use the same driver but need different firmware?
Not that I really know what's going on, but changing static variables
based on runtime seems like it could cause issues in such cases.
johannes
^ permalink raw reply
* [PATCH] mac80211: fix A-MSDU outer SA/DA
From: Michael Braun @ 2016-10-10 16:52 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan
According to IEEE 802.11-2012 section 8.3.2 table 8-19, the outer SA/DA
of A-MSDU frames need to be changed depending on FromDS/ToDS values.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
net/mac80211/tx.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 5023966..acd334c 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3050,7 +3050,7 @@ static bool ieee80211_amsdu_prepare_head(struct ieee80211_sub_if_data *sdata,
int hdr_len = fast_tx->hdr_len - sizeof(rfc1042_header);
int subframe_len = skb->len - hdr_len;
void *data;
- u8 *qc;
+ u8 *qc, *bssid;
if (info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)
return false;
@@ -3062,10 +3062,32 @@ static bool ieee80211_amsdu_prepare_head(struct ieee80211_sub_if_data *sdata,
&subframe_len))
return false;
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_STATION:
+ bssid = sdata->u.mgd.bssid;
+ break;
+ case NL80211_IFTYPE_AP:
+ case NL80211_IFTYPE_AP_VLAN:
+ bssid = sdata->vif.addr;
+ break;
+ default:
+ bssid = NULL;
+ }
+
amsdu_hdr.h_proto = cpu_to_be16(subframe_len);
memcpy(amsdu_hdr.h_source, skb->data + fast_tx->sa_offs, ETH_ALEN);
memcpy(amsdu_hdr.h_dest, skb->data + fast_tx->da_offs, ETH_ALEN);
+ /* according to IEEE 802.11-2012 8.3.2 table 8-19, the outer SA/DA
+ * fields needs to be changed to BSSID for A-MSDU frames depending
+ * on FromDS/ToDS values.
+ */
+ hdr = data;
+ if (bssid && ieee80211_has_fromds(hdr->frame_control))
+ memcpy(amsdu_hdr.h_source, bssid, ETH_ALEN);
+ if (bssid && ieee80211_has_tods(hdr->frame_control))
+ memcpy(amsdu_hdr.h_dest, bssid, ETH_ALEN);
+
data = skb_push(skb, sizeof(amsdu_hdr));
memmove(data, data + sizeof(amsdu_hdr), hdr_len);
memcpy(data + hdr_len, &amsdu_hdr, sizeof(amsdu_hdr));
--
2.1.4
^ permalink raw reply related
* [PATCH v2] mac80211: enable to inject a-msdu frames using monitor interface
From: Michael Braun @ 2016-10-10 16:48 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan
Problem: When injecting an A-MSDU using a PF_PACKET socket, the qos flag
IEEE80211_QOS_CTL_A_MSDU_PRESENT is cleared.
How to reproduce: Inject a frame on a mac80211 hwsim monitor interface and
have tshark sniffing on this monitor interface.
You'll see the packet twice: Once with correct flag and once with flag
cleared. On hwsim0, you'll only see the packet with a cleared flag.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
net/mac80211/wme.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index 9eb0aee..f6a708c 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -248,6 +248,11 @@ void ieee80211_set_qos_hdr(struct ieee80211_sub_if_data *sdata,
/* preserve EOSP bit */
ack_policy = *p & IEEE80211_QOS_CTL_EOSP;
+ /* preserve A-MSDU bit for MONITOR interfaces to allow injecting
+ * A-MSDU frames
+ */
+ ack_policy |= *p & IEEE80211_QOS_CTL_A_MSDU_PRESENT;
+
if (is_multicast_ether_addr(hdr->addr1) ||
sdata->noack_map & BIT(tid)) {
ack_policy |= IEEE80211_QOS_CTL_ACK_POLICY_NOACK;
--
2.1.4
^ permalink raw reply related
* [PATCH v3] ath10k: cache calibration data when the core is stopped
From: Kalle Valo @ 2016-10-10 16:00 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Marty Faltesek
From: Marty Faltesek <mfaltesek@google.com>
Commit 0b8e3c4ca29f ("ath10k: move cal data len to hw_params") broke retrieving
the calibration data from cal_data debugfs file. The length of file was always
zero. The reason is:
static ssize_t ath10k_debug_cal_data_read(struct file *file,
char __user *user_buf,
size_t count, loff_t *ppos)
{
struct ath10k *ar = file->private_data;
void *buf = file->private_data;
This is obviously bogus, private_data cannot contain both struct ath10k and the
buffer. Fix it by caching calibration data to ar->debug.cal_data. This also
allows it to be accessed when the device is not active (interface is down).
The cal_data buffer is fixed size because during the first firmware probe we
don't yet know what will be the lenght of the calibration data. It was simplest
just to use a fixed length. There's a WARN_ON() in
ath10k_debug_cal_data_fetch() if the buffer is too small.
Tested with qca988x and firmware 10.2.4.70.56.
Reported-by: Nikolay Martynov <mar.kolya@gmail.com>
Fixes: 0b8e3c4ca29f ("ath10k: move cal data len to hw_params")
Cc: stable@vger.kernel.org # 4.7+
Signed-off-by: Marty Faltesek <mfaltesek@google.com>
[kvalo@qca.qualcomm.com: improve commit log and minor other changes]
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/debug.c | 79 ++++++++++++++++---------------
2 files changed, 42 insertions(+), 38 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 6e5aa2d09699..b7067cc9328d 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -450,6 +450,7 @@ struct ath10k_debug {
u32 pktlog_filter;
u32 reg_addr;
u32 nf_cal_period;
+ void *cal_data;
struct ath10k_fw_crash_data *fw_crash_data;
};
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 832da6ed9f13..5d508efa0b31 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -30,6 +30,8 @@
/* ms */
#define ATH10K_DEBUG_HTT_STATS_INTERVAL 1000
+#define ATH10K_DEBUG_CAL_DATA_LEN 12064
+
#define ATH10K_FW_CRASH_DUMP_VERSION 1
/**
@@ -1451,75 +1453,68 @@ static const struct file_operations fops_fw_dbglog = {
.llseek = default_llseek,
};
-static int ath10k_debug_cal_data_open(struct inode *inode, struct file *file)
+static int ath10k_debug_cal_data_fetch(struct ath10k *ar)
{
- struct ath10k *ar = inode->i_private;
- void *buf;
u32 hi_addr;
__le32 addr;
int ret;
- mutex_lock(&ar->conf_mutex);
-
- if (ar->state != ATH10K_STATE_ON &&
- ar->state != ATH10K_STATE_UTF) {
- ret = -ENETDOWN;
- goto err;
- }
+ lockdep_assert_held(&ar->conf_mutex);
- buf = vmalloc(ar->hw_params.cal_data_len);
- if (!buf) {
- ret = -ENOMEM;
- goto err;
- }
+ if (WARN_ON(ar->hw_params.cal_data_len > ATH10K_DEBUG_CAL_DATA_LEN))
+ return -EINVAL;
hi_addr = host_interest_item_address(HI_ITEM(hi_board_data));
ret = ath10k_hif_diag_read(ar, hi_addr, &addr, sizeof(addr));
if (ret) {
- ath10k_warn(ar, "failed to read hi_board_data address: %d\n", ret);
- goto err_vfree;
+ ath10k_warn(ar, "failed to read hi_board_data address: %d\n",
+ ret);
+ return ret;
}
- ret = ath10k_hif_diag_read(ar, le32_to_cpu(addr), buf,
+ ret = ath10k_hif_diag_read(ar, le32_to_cpu(addr), ar->debug.cal_data,
ar->hw_params.cal_data_len);
if (ret) {
ath10k_warn(ar, "failed to read calibration data: %d\n", ret);
- goto err_vfree;
+ return ret;
}
- file->private_data = buf;
+ return 0;
+}
- mutex_unlock(&ar->conf_mutex);
+static int ath10k_debug_cal_data_open(struct inode *inode, struct file *file)
+{
+ struct ath10k *ar = inode->i_private;
- return 0;
+ mutex_lock(&ar->conf_mutex);
-err_vfree:
- vfree(buf);
+ if (ar->state == ATH10K_STATE_ON ||
+ ar->state == ATH10K_STATE_UTF) {
+ ath10k_debug_cal_data_fetch(ar);
+ }
-err:
+ file->private_data = ar;
mutex_unlock(&ar->conf_mutex);
- return ret;
+ return 0;
}
static ssize_t ath10k_debug_cal_data_read(struct file *file,
- char __user *user_buf,
- size_t count, loff_t *ppos)
+ char __user *user_buf,
+ size_t count, loff_t *ppos)
{
struct ath10k *ar = file->private_data;
- void *buf = file->private_data;
- return simple_read_from_buffer(user_buf, count, ppos,
- buf, ar->hw_params.cal_data_len);
-}
+ mutex_lock(&ar->conf_mutex);
-static int ath10k_debug_cal_data_release(struct inode *inode,
- struct file *file)
-{
- vfree(file->private_data);
+ count = simple_read_from_buffer(user_buf, count, ppos,
+ ar->debug.cal_data,
+ ar->hw_params.cal_data_len);
- return 0;
+ mutex_unlock(&ar->conf_mutex);
+
+ return count;
}
static ssize_t ath10k_write_ani_enable(struct file *file,
@@ -1580,7 +1575,6 @@ static const struct file_operations fops_ani_enable = {
static const struct file_operations fops_cal_data = {
.open = ath10k_debug_cal_data_open,
.read = ath10k_debug_cal_data_read,
- .release = ath10k_debug_cal_data_release,
.owner = THIS_MODULE,
.llseek = default_llseek,
};
@@ -1932,6 +1926,8 @@ void ath10k_debug_stop(struct ath10k *ar)
{
lockdep_assert_held(&ar->conf_mutex);
+ ath10k_debug_cal_data_fetch(ar);
+
/* Must not use _sync to avoid deadlock, we do that in
* ath10k_debug_destroy(). The check for htt_stats_mask is to avoid
* warning from del_timer(). */
@@ -2344,6 +2340,10 @@ int ath10k_debug_create(struct ath10k *ar)
if (!ar->debug.fw_crash_data)
return -ENOMEM;
+ ar->debug.cal_data = vzalloc(ATH10K_DEBUG_CAL_DATA_LEN);
+ if (!ar->debug.cal_data)
+ return -ENOMEM;
+
INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs);
INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs);
INIT_LIST_HEAD(&ar->debug.fw_stats.peers);
@@ -2357,6 +2357,9 @@ void ath10k_debug_destroy(struct ath10k *ar)
vfree(ar->debug.fw_crash_data);
ar->debug.fw_crash_data = NULL;
+ vfree(ar->debug.cal_data);
+ ar->debug.cal_data = NULL;
+
ath10k_debug_fw_stats_reset(ar);
kfree(ar->debug.tpc_stats);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2] ath10k: cache calibration data when the core is stopped
From: Valo, Kalle @ 2016-10-10 15:59 UTC (permalink / raw)
To: Marty Faltesek; +Cc: ath10k@lists.infradead.org, linux-wireless@vger.kernel.org
In-Reply-To: <CAOiWkA8yX6bnLWHTwMc21EJHh4tQHpdBH+yKL4Y9WWxufBKaKQ@mail.gmail.com>
Marty Faltesek <mfaltesek@google.com> writes:
> ath10k: cache calibration data when the core is stopped
>
> Caching calibration data allows it to be accessed when the
> device is not active.
>
> Signed-off-by: Marty Faltesek <mfaltesek@google.com>
Thanks, I'll now send v3.
--=20
Kalle Valo=
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox