Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 23 Dec 2019 13:52:35 +0300
From:   Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
To:     Orr Mazor <orr.mazor@tandemg.com>
CC:     Johannes Berg <johannes@sipsolutions.net>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH] subsystem: Fix radar event during another phy CAC
Message-ID: <20191223105234.lgsupxfapbmxuvc5@bars>
Mail-Followup-To: Orr Mazor <orr.mazor@tandemg.com>,
        Johannes Berg <johannes@sipsolutions.net>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
References: <20191222145449.15792-1-Orr.Mazor@tandemg.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191222145449.15792-1-Orr.Mazor@tandemg.com>
User-Agent: NeoMutt/20170113 (1.7.2)
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Received-SPF: None (protection.outlook.com: quantenna.com does not designate
 permitted sender hosts)
X-MS-Exchange-CrossTenant-Network-Message-Id: 524d7bf2-9a78-49ae-5ae7-08d787963b4a
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: a355dbce-62b4-4789-9446-c1d5582180ff
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: kiN6PxjgcihCHxPTgFBwZps9uZuAVFZVCgFFH8w56uEMC+YK/04AVCjoMeR47ADVyBu9+xD+mk11ePLDMCRg48RCwAV3l6tcUk5Vy38nVvrl9owVTjRRe+oR5ErFo65NKY2YmmRR69FJ/BwUWKNpYRac30pJhYmOdSiqKATTNSlGoj8olN8Dwuboiych66KPEl6yv+F7HLuWePt+FmmF3hKtpSj6FjqDCU7Ep+ZjIwk=
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Dec 2019 10:53:14.5223
 (UTC)
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR0501MB3911
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,18.0.572
 definitions=2019-12-23_05:2019-12-23,2019-12-23 signatures=0
X-Proofpoint-Spam-Reason: safe
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

Hi Orr,

> In case a radar event of CAC_FINISHED or RADAR_DETECTED
> happens during another phy is during CAC we might need
> to cancel that CAC.
> If we got a radar in a channel that another phy is now
> doing CAC on then the CAC should be canceled.
> If, for example, 2 phys doing CAC on the same channels,
> or on comptable channels, once on of them will finish his CAC
> the other might need to cancel his CAC, since it is no
> longer relevant.
>
> To fix that the commit adds an callback and implement it in mac80211
> to end CAC.
> This commit also adds a call to said callback if after a radar
> event we see the cac is no longer relevant

>  net/mac80211/cfg.c      | 23 +++++++++++++++++++++++
>  net/wireless/rdev-ops.h | 10 ++++++++++
>  net/wireless/reg.c      | 24 +++++++++++++++++++++++-
>  net/wireless/trace.h    |  5 +++++
>  5 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 4ab2c49423dc..68782ba8b6e8 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -3537,6 +3537,9 @@ struct cfg80211_update_owe_info {
>   *
>   * @start_radar_detection: Start radar detection in the driver.
>   *
> + * @end_cac: End running CAC, probably because a related CAC
> + *   was finished on another phy.
> + *

Maybe it makes sense to follow existing naming convention here
and to use something like 'stop_radar_detection' ?

>   * @update_ft_ies: Provide updated Fast BSS Transition information to th=
e
>   *   driver. If the SME is in the driver/firmware, this information can =
be
>   *   used in building Authentication and Reassociation Request frames.
> @@ -3863,6 +3866,8 @@ struct cfg80211_ops {
>                                        struct net_device *dev,
>                                        struct cfg80211_chan_def *chandef,
>                                        u32 cac_time_ms);
> +     void    (*end_cac)(struct wiphy *wiphy,
> +                             struct net_device *dev);

...

> +static void cfg80211_check_and_end_cac(struct cfg80211_registered_device=
 *rdev)
> +{
> +     struct wireless_dev *wdev;
> +     /* If we finished CAC or received radar, we should end any
> +      * CAC running on the same channels.
> +      * the check !cfg80211_chandef_dfs_usable contain 2 options:
> +      * either all channels are available - those the CAC_FINISHED
> +      * event has effected another wdev state, or there is a channel
> +      * in unavailable state in wdev chandef - those the RADAR_DETECTED
> +      * event has effected another wdev state.
> +      * In both cases we should end the CAC on the wdev.
> +      *
> +      */
> +     list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
> +             if (wdev->cac_started &&
> +                 !cfg80211_chandef_dfs_usable(&rdev->wiphy, &wdev->chand=
ef))
> +                     rdev_end_cac(rdev, wdev->netdev);
> +     }
> +}
> +

IIUC, this code does not match your commit message. You are stopping CAC
on all the virtual wireless interfaces on the same PHY, but not CACs on
different PHYs. Meanwhile CAC does not need to be started on multiple
virtual interfaces. For instance, in multiple BSSID configuration,
hostapd performs CAC only on primary interface.

Could you please clarify the use-case which requires this functionality ?

Regards,
Sergey


This email, including its contents and any attachment(s), may contain confi=
dential information of ON Semiconductor and is solely for the intended reci=
pient(s). If you may have received this in error, please contact the sender=
 and permanently delete this email, its contents and any attachment(s).