DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 001AB20652
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Thu, 25 Oct 2018 16:12:45 -0700
From:   Pradeep Kumar Chitrapu <pradeepc@codeaurora.org>
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     linux-wireless@vger.kernel.org,
        David Spinadel <david.spinadel@intel.com>
Subject: Re: [PATCH] mac80211: support FTM responder configuration/statistics
In-Reply-To: <c832f9007e0a844d3335a0cd9996818f49df2311.camel@sipsolutions.net>
References: <1538623160-25886-1-git-send-email-pradeepc@codeaurora.org>
 (sfid-20181004_051935_494006_E9A4C858)
 <c832f9007e0a844d3335a0cd9996818f49df2311.camel@sipsolutions.net>
Message-ID: <765cbdd596e96b7cf73239a4e04b06a7@codeaurora.org>
User-Agent: Roundcube Webmail/1.2.5
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

> 
> I just realized that this is broken in nl80211_channel_switch() and
> ieee80211_set_csa_beacon(), doing a CSA will always disable FTM unless
> it was actually included in the new configuration.
Hi Johannes

oops..Yes, there is a bug in the patch. The code below, I think, must 
fix this issue.
Please let me know your comments..

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 51622333d460..70d6de29425b 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2934,19 +2934,20 @@ static int 
ieee80211_start_radar_detection(struct wiphy *wiphy,
                 memcpy(pos, beacon->probe_resp, beacon->probe_resp_len);
                 pos += beacon->probe_resp_len;
         }
-       if (beacon->ftm_responder)
+       if (beacon->ftm_responder != -1) {
                 new_beacon->ftm_responder = beacon->ftm_responder;
-       if (beacon->lci) {
-               new_beacon->lci_len = beacon->lci_len;
-               new_beacon->lci = pos;
-               memcpy(pos, beacon->lci, beacon->lci_len);
-               pos += beacon->lci_len;
-       }
-       if (beacon->civicloc) {
-               new_beacon->civicloc_len = beacon->civicloc_len;
-               new_beacon->civicloc = pos;
-               memcpy(pos, beacon->civicloc, beacon->civicloc_len);
-               pos += beacon->civicloc_len;
+               if (beacon->lci) {
+                       new_beacon->lci_len = beacon->lci_len;
+                       new_beacon->lci = pos;
+                       memcpy(pos, beacon->lci, beacon->lci_len);
+                       pos += beacon->lci_len;
+               }
+               if (beacon->civicloc) {
+                       new_beacon->civicloc_len = beacon->civicloc_len;
+                       new_beacon->civicloc = pos;
+                       memcpy(pos, beacon->civicloc, 
beacon->civicloc_len);
+                       pos += beacon->civicloc_len;
+               }
         }

         return new_beacon;


> 
> Doing the trivial thing:
> 
>  	memset(&params, 0, sizeof(params));
> +	params.beacon_after.ftm_responder = -1;
This would not be needed then.
> 
> in nl80211_channel_switch() will not help because then mac80211 will
> lose all the extra configuration, and will actually store -1 into its
> enabled value which is really strange.
> 
> I'd appreciate if you could take a look at this.
> 
> Thanks,
> johannes