Lennert, We appreciate your feedback and are working on a correct solution, thanks for the heads up. Thomas On Fri, Feb 11, 2011 at 4:34 PM, Lennert Buytenhek <buytenh@xxxxxxxxxxxxxx> wrote: > [ adding Nishant Sarmukadam and Sandesh Goel to CC ] > > > On Fri, Feb 11, 2011 at 04:03:51PM -0800, Thomas Pedersen wrote: > >> From: Nishant Sarmukadam <nishants@xxxxxxxxxxx> >> >> Minstrel rate control algorithm is enabled by default by mac80211. >> This is not needed since the firmware does the rate control in our case. >> >> Signed-off-by: Nishant Sarmukadam <nishants@xxxxxxxxxxx> >> Signed-off-by: Thomas Pedersen <thomas@xxxxxxxxxxx> >> --- >> drivers/net/wireless/mwl8k.c | 3 +++ >> 1 files changed, 3 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c >> index f79da1b..840b9e4 100644 >> --- a/drivers/net/wireless/mwl8k.c >> +++ b/drivers/net/wireless/mwl8k.c >> @@ -4765,6 +4765,9 @@ static int mwl8k_firmware_load_success(struct mwl8k_priv *priv) >> >> /* Set rssi values to dBm */ >> hw->flags |= IEEE80211_HW_SIGNAL_DBM; >> + /* AP FW has rate control */ >> + if (priv->ap_fw) >> + hw->flags |= IEEE80211_HW_HAS_RATE_CONTROL; >> hw->vif_data_size = sizeof(struct mwl8k_vif); >> hw->sta_data_size = sizeof(struct mwl8k_sta); > > I got this same patch (almost exactly the same -- STA firmware actually > also has rate control so the added AP check in this version of the patch > is bogus) sent to me a number of times now, the last time from Pradeep > Nemavat <pnemavat@xxxxxxxxxxx> in private email on Feb 26 2010. This > is what I wrote back to him at the time: > > | mac80211.h says: > | > | * @IEEE80211_HW_HAS_RATE_CONTROL: > | * The hardware or firmware includes rate control, and cannot be > | * controlled by the stack. As such, no rate control algorithm > | * should be instantiated, and the TX rate reported to userspace > | * will be taken from the TX status instead of the rate control > | * algorithm. > | * Note that this requires that the driver implement a number of > | * callbacks so it has the correct information, it needs to have > | * the @set_rts_threshold callback and must look at the BSS config > | * @use_cts_prot for G/N protection, @use_short_slot for slot > | * timing in 2.4 GHz and @use_short_preamble for preambles for > | * CCK frames. > | > | In particular, the "need to report TX rate used in the TX status" > | requirement is something that we don't do, which is why I didn't merge > | the IEEE80211_HW_HAS_RATE_CONTROL change upstream yet. (I know that > | it improves performance, but performance is secondary to correctness.) > | > | One case where this is important is when you use tcpdump to look at > | transmitted packets and at which rates they were transmitted for > | debugging purposes -- tcpdump will report rates for each transmitted > | packet just like for received packets, a la: > | > | [...] > | > | In the situation before this patch, transmitted packets would be > | reported at whatever rate minstrel chose for it (while the actual rate > | on the air might be different), but after this patch, they will all > | just be reported at 1 Mb/s, which is hardly better. > | > | If there is no way to find out what rate the firmware decided to > | transmit at, what we should perhaps do is prevent a transmit rate from > | being sent to userspace at all. I.e. in the packet that tcpdump sniffs > | from the packet socket, there should just not be a "Rate" field in the > | Radiotap (www.radiotap.org) header at all. We might be able to do this > | by choosing a special value in the TX status indicating that we don't > | know the TX rate, and then having mac80211 not generate a Rate field > | when it encounters an invalid value -- at least that would be better > | than reporting TX rates we know to be wrong. > > The general strategy of the s/w team on this project at the time > appeared to be to send me patches, to totally ignore the feedback I > would then provide on those patches, and then to submit the exact > same patches again two months later in the hope that I would at some > point just get tired of NAKing them and they would get merged by way > of reviewer exhaustion. > > E.g. it's probably the 3th or 4th time that I get sent this patch now, > and while I freely admit that my reasoning above for not having merged > this patch yet may be faulty, noone has bothered to try to convince me > that it is, all I get is just the exact same patch resubmitted every > N months without any indication that the previous feedback has even > registered. > > I have explained my objections against this patch in detail to Nishant > in the past, and more than once. Why has all that feedback just gone > straight to /dev/null every single time? > > > thanks, > Lennert > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
