From patchwork Fri Nov 22 14:05:33 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karl Beldan X-Patchwork-Id: 3222611 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id E12E8C045B for ; Fri, 22 Nov 2013 14:06:20 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B4155207A6 for ; Fri, 22 Nov 2013 14:06:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 11154207AA for ; Fri, 22 Nov 2013 14:06:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755682Ab3KVOGO (ORCPT ); Fri, 22 Nov 2013 09:06:14 -0500 Received: from mail-we0-f180.google.com ([74.125.82.180]:57075 "EHLO mail-we0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755670Ab3KVOGO (ORCPT ); Fri, 22 Nov 2013 09:06:14 -0500 Received: by mail-we0-f180.google.com with SMTP id u56so1158599wes.25 for ; Fri, 22 Nov 2013 06:06:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=/X7oSHkf1mITOsYZ+CiEYcdjvg0M+v3S/FBckgpR+uc=; b=L2TczDoHbs3XS7MQIJi5WLrkaxHPH97Ja267EeOeQ4MikKSWL5PkJszrdbx8YV0M26 Tgd4lJzg1jtFbVYde4kKMIxrbLel2wepxwNYZUxUuoG1wIyZSM442+W6tiOO10Jk05ZA viZR6omxqjOf6blj/6OSGJDZMt36YISsGs/jli916TWCAZz01NYMQmlPRVWq+UEesTIE MYIJlyWwqV+jjtvCkOwB9CZnv9BLCb92LM69oASPrAuW1MP8TEkc/7jzsuzWDG0jG+sT dikZzo3ngiAaTNDzleK/UzYV76vFDusFhhutsDa3T7gDNF+WdF+wtCa0uDMnUhrK7ZZn 2kAQ== X-Received: by 10.180.221.38 with SMTP id qb6mr2823182wic.8.1385129172914; Fri, 22 Nov 2013 06:06:12 -0800 (PST) Received: from magnum.frso.rivierawaves.com (vpn.rivierawaves.com. [91.151.119.162]) by mx.google.com with ESMTPSA id f15sm16429317wik.6.2013.11.22.06.06.12 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 22 Nov 2013 06:06:12 -0800 (PST) Date: Fri, 22 Nov 2013 15:05:33 +0100 From: Karl Beldan To: Simon Wunderlich Cc: Johannes Berg , linux-wireless Subject: Re: [PATCH v3] mac80211_hwsim: claim CSA support for AP Message-ID: <20131122140533.GA24796@magnum.frso.rivierawaves.com> References: <1385111186-19551-1-git-send-email-karl.beldan@gmail.com> <20131122104527.GD8527@magnum.frso.rivierawaves.com> <201311221308.11994.sw@simonwunderlich.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <201311221308.11994.sw@simonwunderlich.de> X-Location: France-Nice User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Nov 22, 2013 at 01:08:11PM +0100, Simon Wunderlich wrote: > Hello Karl, > > > > > Hmm, I thought the CSA code would make ieee80211_beacon_get_tim return > > NULL (or do something alike) after the last ieee80211_beacon_get_tim > > returned a beacon with a null CSA count until the config is done - but > > it seems it doesn't - in that case this change would be race prone. > > Did I miss something ? > > No, you have to do the check yourself (as you appearently did). In ath9k I'm > checking if the CSA finished before scheduling the next beacon. Can you do the > same for hwsim? Where do you see the race? > Hi Simon, I would see a race in such scenario: {{{ cmd: stack/channel_switch(count=1); bcn-intr: driver/beacon_get(); vif->csa_active == true ieee80211_csa_is_complete(); // == true ieee80211_csa_finish(); // schedules csa work bcn-intr: driver/beacon_get(); // beacon but on the old channel vif->csa_active still true ieee80211_csa_is_complete(); // still true csa worker: csa_finalize_work(); ----> too late change_channel(); ----> too late }}} To prevent this, I thought that there was something like : {{{ .. Of course there are other means to achieve this, just an example. However I might have overlooked some things in the CSA code so .. > Simon > > P.S.: Please use my new e-mail address, the old one will go out of service by > end of the year. Sure Karl --- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 3e2dfcb..97b0382 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -2405,7 +2405,7 @@ void ieee80211_csa_finish(struct ieee80211_vif *vif) } EXPORT_SYMBOL(ieee80211_csa_finish); -static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata, +static bool ieee80211_update_csa(struct ieee80211_sub_if_data *sdata, struct beacon_data *beacon) { struct probe_resp *resp; @@ -2428,14 +2428,14 @@ static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata, beacon_data_len = beacon->head_len; break; default: - return; + return false; } if (WARN_ON(counter_offset_beacon >= beacon_data_len)) - return; + return false; /* warn if the driver did not check for/react to csa completeness */ if (WARN_ON(beacon_data[counter_offset_beacon] == 0)) - return; + return false; beacon_data[counter_offset_beacon]--; @@ -2446,11 +2446,13 @@ static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata, /* if nl80211 accepted the offset, this should not happen. */ if (WARN_ON(!resp)) { rcu_read_unlock(); - return; + return true; } resp->data[counter_offset_presp]--; rcu_read_unlock(); } + + return true; } bool ieee80211_csa_is_complete(struct ieee80211_vif *vif) @@ -2540,7 +2542,8 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw, if (beacon) { if (sdata->vif.csa_active) - ieee80211_update_csa(sdata, beacon); + if (!ieee80211_update_csa(sdata, beacon)) + goto out; ... blah blah for mesh .. ibss .. }}}