| Message ID | 1395734146-10907-1-git-send-email-yeohchunyeow@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 25 March 2014 08:55, Chun-Yeow Yeoh <yeohchunyeow@gmail.com> wrote: > This patch is intended to fix the problem if we use the > firmware 999.999.0.636 to get peer stats when the number > of peer is more than 3. The WMI_UPDATE_STATS_EVENTID may > trigger more than 1 time if the number of peers is more > than 3. So this patch allows us to do the checking on this > and make sure that we print the peer stats correctly. > > Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow@gmail.com> > --- > drivers/net/wireless/ath/ath10k/debug.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c > index 7be284c..4a4072e 100644 > --- a/drivers/net/wireless/ath/ath10k/debug.c > +++ b/drivers/net/wireless/ath/ath10k/debug.c > @@ -245,10 +245,17 @@ void ath10k_debug_read_target_stats(struct ath10k *ar, > if (num_peer_stats) { > struct wmi_peer_stats_10x *peer_stats; > struct ath10k_peer_stat *s; > + int j = 0; > + > + if (!test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features) && > + !num_pdev_stats) { > + j = 3; > + num_peer_stats += 3; > + } > > stats->peers = num_peer_stats; > > - for (i = 0; i < num_peer_stats; i++) { > + for (i = j; i < num_peer_stats; i++) { > peer_stats = (struct wmi_peer_stats_10x *)tmp; > s = &stats->peer_stat[i]; I suppose there's a limit how much peer entries firmware is able to pack up into a single event. The second event you handle here is still going to have some sort of a limit and you'll end up dropping results again. The same probably goes with vdev stats (which aren't handled yet, but hey). I think a better approach would be to make read_target_stats *append* data to the stats structure and make ath10k wait for more possible events instead of being satisfied with just one. The way you do it now seems like you end up with the extra results being shown upon *next* stat request-response.. i.e. you see old results. Micha? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Michal > I suppose there's a limit how much peer entries firmware is able to > pack up into a single event. The second event you handle here is still > going to have some sort of a limit and you'll end up dropping results > again. The same probably goes with vdev stats (which aren't handled > yet, but hey). vdev stats is not supported by firmware 999.999.0.636, am I right? BTW, I also saw that the same happen to latest AP firmware whenever the peer stats is more than 1. But since the first peer stats is self stats, so it is ok to overwrite it. > I think a better approach would be to make read_target_stats *append* > data to the stats structure and make ath10k wait for more possible > events instead of being satisfied with just one. > > The way you do it now seems like you end up with the extra results > being shown upon *next* stat request-response.. i.e. you see old > results. Will take a look on this. ---- Chun-Yeow -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25 March 2014 09:33, Yeoh Chun-Yeow <yeohchunyeow@gmail.com> wrote: > Hi, Michal > >> I suppose there's a limit how much peer entries firmware is able to >> pack up into a single event. The second event you handle here is still >> going to have some sort of a limit and you'll end up dropping results >> again. The same probably goes with vdev stats (which aren't handled >> yet, but hey). > > vdev stats is not supported by firmware 999.999.0.636, am I right? True, but once it is supported then it will most likely need the same treatment as peer stats. > BTW, I also saw that the same happen to latest AP firmware whenever > the peer stats is more than 1. But since the first peer stats is self > stats, so it is ok to overwrite it. I disagree. Why do you think it is okay to overwrite self peer stats? Btw. 10.1 firmware has more pdev stats which breaks parsing of peer entries... Micha? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> BTW, I also saw that the same happen to latest AP firmware whenever >> the peer stats is more than 1. But since the first peer stats is self >> stats, so it is ok to overwrite it. > > I disagree. Why do you think it is okay to overwrite self peer stats? > Because TxRate, RxRate, RSSI are all no value, just the MAC address. > Btw. 10.1 firmware has more pdev stats which breaks parsing of peer entries... Yes, let me investigate further. ----- Chun-Yeow -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25 March 2014 09:54, Yeoh Chun-Yeow <yeohchunyeow@gmail.com> wrote: >> >>> BTW, I also saw that the same happen to latest AP firmware whenever >>> the peer stats is more than 1. But since the first peer stats is self >>> stats, so it is ok to overwrite it. >> >> I disagree. Why do you think it is okay to overwrite self peer stats? >> > Because TxRate, RxRate, RSSI are all no value, just the MAC address. I don't think that's true for 10.1 firmware and AP interface types. Micha? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> I don't think that's true for 10.1 firmware and AP interface types.
I try to play around with 10.1 firmware as AP (3 connected STAs) and I
get the following:
The first WMI_UPDATE_STATS_EVENTID as follow:
[ 338.290000] ath10k: pdev 0 vdev 0 peer 3
[ 338.300000] ath10k: MAC 04:f0:21:0c:a5:44
[ 338.300000] ath10k: RSSI 60
[ 338.300000] ath10k: Tx 1170000
[ 338.310000] ath10k: Rx 1170000
[ 338.310000] ath10k: MAC 04:f0:21:0c:a5:19
[ 338.320000] ath10k: RSSI 64
[ 338.320000] ath10k: Tx 1300000
[ 338.320000] ath10k: Rx 975000
[ 338.320000] ath10k: MAC 04:f0:21:0c:a5:1c
[ 338.330000] ath10k: RSSI 71
[ 338.330000] ath10k: Tx 975000
[ 338.330000] ath10k: Rx 1300000
I think that above is correct.
The second WMI_UPDATE_STATS_EVENTID as follow:
[ 338.340000] ath10k: pdev 1 vdev 0 peer 1
[ 338.350000] ath10k: MAC a8:02:00:00:00:00
[ 338.350000] ath10k: RSSI 0
[ 338.350000] ath10k: Tx 565
[ 338.360000] ath10k: Rx 0
Although indicating 1 peer, but the data seems to be not the peer
stats, not even self STA stats. Any idea?
---
Chun-Yeow
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/25/2014 01:11 AM, Michal Kazior wrote: > On 25 March 2014 08:55, Chun-Yeow Yeoh <yeohchunyeow@gmail.com> wrote: >> This patch is intended to fix the problem if we use the >> firmware 999.999.0.636 to get peer stats when the number >> of peer is more than 3. The WMI_UPDATE_STATS_EVENTID may >> trigger more than 1 time if the number of peers is more >> than 3. So this patch allows us to do the checking on this >> and make sure that we print the peer stats correctly. >> >> Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow@gmail.com> >> --- >> drivers/net/wireless/ath/ath10k/debug.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c >> index 7be284c..4a4072e 100644 >> --- a/drivers/net/wireless/ath/ath10k/debug.c >> +++ b/drivers/net/wireless/ath/ath10k/debug.c >> @@ -245,10 +245,17 @@ void ath10k_debug_read_target_stats(struct ath10k *ar, >> if (num_peer_stats) { >> struct wmi_peer_stats_10x *peer_stats; >> struct ath10k_peer_stat *s; >> + int j = 0; >> + >> + if (!test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features) && >> + !num_pdev_stats) { >> + j = 3; >> + num_peer_stats += 3; >> + } >> >> stats->peers = num_peer_stats; >> >> - for (i = 0; i < num_peer_stats; i++) { >> + for (i = j; i < num_peer_stats; i++) { >> peer_stats = (struct wmi_peer_stats_10x *)tmp; >> s = &stats->peer_stat[i]; > > I suppose there's a limit how much peer entries firmware is able to > pack up into a single event. The second event you handle here is still > going to have some sort of a limit and you'll end up dropping results > again. The same probably goes with vdev stats (which aren't handled > yet, but hey). At least in my 10.x 389 firmware, the limit is 11 peers. There is some round-robin logic that means next request of stats gets the next set of peer stats, so calling get-stats multiple times would get the entire list of peers. I'm not sure how to deal with this in the driver. In my case, my multiple stations mostly connect to the same AP, so peer-mac is often the same. This makes it difficult for me to know if the round-robin logic is working properly, but I think we might as well assume it does until proven otherwise. I may try to poke a station identifier into the 16 unused bits of the mac-addr struct eventually, but unless upstream firmware does the same, that doesn't help any general issue... Thanks, Ben
Chun-Yeow Yeoh <yeohchunyeow@gmail.com> writes: > This patch is intended to fix the problem if we use the > firmware 999.999.0.636 to get peer stats when the number > of peer is more than 3. The WMI_UPDATE_STATS_EVENTID may > trigger more than 1 time if the number of peers is more > than 3. So this patch allows us to do the checking on this > and make sure that we print the peer stats correctly. > > Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow@gmail.com> Based on Michal's comments I'm going to drop this one, we need to find a better way to solve this.
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index 7be284c..4a4072e 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -245,10 +245,17 @@ void ath10k_debug_read_target_stats(struct ath10k *ar, if (num_peer_stats) { struct wmi_peer_stats_10x *peer_stats; struct ath10k_peer_stat *s; + int j = 0; + + if (!test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features) && + !num_pdev_stats) { + j = 3; + num_peer_stats += 3; + } stats->peers = num_peer_stats; - for (i = 0; i < num_peer_stats; i++) { + for (i = j; i < num_peer_stats; i++) { peer_stats = (struct wmi_peer_stats_10x *)tmp; s = &stats->peer_stat[i];
This patch is intended to fix the problem if we use the firmware 999.999.0.636 to get peer stats when the number of peer is more than 3. The WMI_UPDATE_STATS_EVENTID may trigger more than 1 time if the number of peers is more than 3. So this patch allows us to do the checking on this and make sure that we print the peer stats correctly. Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow@gmail.com> --- drivers/net/wireless/ath/ath10k/debug.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)