On 2018-03-27 18:24, Johannes Berg wrote:
On Tue, 2018-03-27 at 14:12 +0530, Vasanthakumar Thiagarajan wrote:

+u16 ieee80211_get_noack_map(struct ieee80211_sub_if_data *sdata, const u8 *mac)
+{
+       struct sta_info *sta;
+       u16 noack_map = 0;
+
+       /* Retrieve per-station noack_map config for the receiver, if any */
+
+       rcu_read_lock();
+
+       sta = sta_info_get(sdata, mac);
+       if (!sta) {
+               rcu_read_unlock();
+               return noack_map;
+       }
+
+       noack_map = sta->noack_map;
+
+       rcu_read_unlock();
+
+       if (!noack_map)
+               noack_map = sdata->noack_map;

So this has an interesting corner case - should it be possible to have a default noack_map that's non-zero, but override it with 0 for a specific
peer? It seems that it should be, which makes this code wrong.

I think 0 as the Noack configuration from user can also be a valid one in the case where user does not want any NoAck policy to be used for a particular station even when a non-zero NoAck configuration is set for ndev level. In this case, the logic may need to be modified so that the default non-zero configuration (something like -1) could be used to determine that the station has been never configured with any NoAck
policy and use ndev level configuration. Does this sound reasonable?

Vasanth

Reply via email to