On Sun, Feb 10, 2019 at 8:31 PM Tony Chuang <yhchuang@xxxxxxxxxxx> wrote: > To fix `iw wlan0 station dump` display, I think I can just restore one line > in pci.c. That is, restore IEEE80211_TX_STAT_ACK flag line: > > + continue; > + } > ieee80211_tx_info_clear_status(info); > - info->flags |= IEEE80211_TX_STAT_ACK; > ieee80211_tx_status_irqsafe(hw, skb) > > And with some modifications, such as IEEE80211_TX_CTL_NO_ACK check. > Then we can better reporting ACK status for data frames without > IEEE80211_TX_CTL_REQ_TX_STATUS. This way we can also ensure the > connection monitor can work. (but it will be no loss) This seems like a small improvement. It means you will almost never actually report a "drop", but that's still probably better than *always* reporting drops I think. I also see that there are handful of older (likely poorly-maintained? or perhaps similarly-constrained) drivers that have a similar default behavior -- they report ACK by default, apparently without any particular notice that the packet was actually ACKed by the receiver. But I'm not really a mac80211 expert, so someone else may have better ideas here. > It's not buggy I think, if firmware is not reporting status, something must > go wrong. And after some test I know why you feel it's unreliable. > > For WOW implementation, we modified a lot in fw.c functions. > And correct some driver-firmware interface behaviors. To make sure the > firmware is running as expected. But the patches are still holding in my hand. > I can attach them in this patch set, and apparently I should. I will separate > them out of WOW patch set and resend again. Ah, well I don't know at what point we should cut things off, but I'll stick to my stated opinion from previously: all bugfixes will help someone like me. It's tough to differentiate firmware bugs from driver bugs, especially when I have absolutely no firmware documentation :) BTW, while we're at it: is this still a reasonable firmware to reference? https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/?id=338684a0c7760644031483311464c7cf5b3aac94 rtw88: Add firmware file for driver rtw88 > I think we should keep this feature. Because we actually can report status, > despite not for every packet. The only problem is when we use `iw wlan0 > station dump` we could get *no* packet loss (like I've mentioned above, > report TX_STAT_ACK for every other packets not have > IEEE80211_TX_CTL_REQ_TX_STATUS). We cannot accurately report > everything by firmware report, it takes too many tx bandwidth, and > the performance will degrade severely. If we really cannot accept reporting Yeah, it does seem like it would be pretty heavyweight to do this C2H reporting for every frame. > tx status this way, we need to find another way to solve it. That means I > need some time to investigate and test connect monitor system and get > a better report logic if we drop the REPORTS_TX_ACK_STATUS feature. > Or if you have a point of view, we can discuss about it. > Thanks! Regards, Brian
