Return-path: Received: from na3sys009aob106.obsmtp.com ([74.125.149.76]:53647 "EHLO na3sys009aog106.obsmtp.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752940Ab2BOQqD convert rfc822-to-8bit (ORCPT ); Wed, 15 Feb 2012 11:46:03 -0500 Received: by mail-ee0-f43.google.com with SMTP id b45so479645eek.2 for ; Wed, 15 Feb 2012 08:46:01 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1327581484-22047-1-git-send-email-victorg@ti.com> <1327581484-22047-6-git-send-email-victorg@ti.com> Date: Wed, 15 Feb 2012 18:46:01 +0200 Message-ID: (sfid-20120215_174642_132093_1D4D1F36) Subject: Re: [RFC 5/9] nl80211/cfg80211: add ap channel switch command/event From: "Goldenshtein, Victor" To: "Luis R. Rodriguez" Cc: linux-wireless@vger.kernel.org, kgiori@qca.qualcomm.com, zefir.kurtisi@neratec.com, adrian.chadd@gmail.com, j@w1.fi, johannes@sipsolutions.net, coelho@ti.com, assaf@ti.com, yoni.divinsky@ti.com, igalc@ti.com, adrian@freebsd.org, nbd@nbd.name Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Fri, Feb 10, 2012 at 12:53 AM, Luis R. Rodriguez wrote: > On Thu, Jan 26, 2012 at 4:38 AM, Victor Goldenshtein wrote: >> Add new NL80211_CMD_AP_CHANNEL_SWITCH command which >> triggers a channel switch process, this command also >> notifies usermode about channel switch complete event. >> >> Signed-off-by: Victor Goldenshtein >> --- >> ?include/linux/nl80211.h | ? 23 +++++++++++++ >> ?include/net/cfg80211.h ?| ? 16 +++++++++ >> ?net/wireless/mlme.c ? ? | ? 11 ++++++ >> ?net/wireless/nl80211.c ?| ? 80 +++++++++++++++++++++++++++++++++++++++++++++++ >> ?net/wireless/nl80211.h ?| ? ?4 ++ >> ?5 files changed, 134 insertions(+), 0 deletions(-) >> >> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h >> index b45ceb1..8a004fc 100644 >> --- a/include/linux/nl80211.h >> +++ b/include/linux/nl80211.h >> @@ -551,6 +551,15 @@ >> ?* ? ? radar interference is detected during this period the dfs master may >> ?* ? ? initiate the tx. >> ?* >> + * @NL80211_CMD_AP_CHANNEL_SWITCH: Perform a channel switch in the driver (for >> + * ? ? AP/GO). >> + * ? ? %NL80211_ATTR_WIPHY_FREQ: new channel frequency. >> + * ? ? %NL80211_ATTR_CH_SWITCH_BLOCK_TX: block tx on the current channel. >> + * ? ? %NL80211_ATTR_CH_SWITCH_POST_BLOCK_TX: block tx on the target channel. > > This is left as optional. It would be good to explain why this is > option and/or why userspace would set this > NL80211_ATTR_CH_SWITCH_POST_BLOCK_TX flag or not. > >> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c >> @@ -4072,6 +4075,39 @@ static int nl80211_dfs_en_tx(struct sk_buff *skb, struct genl_info *info) >> ? ? ? ?return rdev->ops->dfs_en_tx(&rdev->wiphy, dev); >> ?} >> >> +static int nl80211_ap_channel_switch(struct sk_buff *skb, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct genl_info *info) >> +{ >> + ? ? ? struct cfg80211_registered_device *rdev = info->user_ptr[0]; >> + ? ? ? struct net_device *dev = info->user_ptr[1]; >> + ? ? ? u32 freq = 0, count = 0, post_switch_block_tx = 0, block_tx = 0; > > Please use bool for flags. > ok. >> + >> + ? ? ? ASSERT_RDEV_LOCK(rdev); >> + >> + ? ? ? if (!rdev->ops->ap_channel_switch) >> + ? ? ? ? ? ? ? return -EOPNOTSUPP; >> + >> + ? ? ? if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP && >> + ? ? ? ? ? dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO) >> + ? ? ? ? ? ? ? return -EOPNOTSUPP; > > If we want to restrict this command from userspace to only be able to > issue a channel switch for DFS we need to consider some additional > sanity checks here. It may be worth adding a small state machine to > cfg80211 for the device and only allow certain of these commands for > specific DFS states, if these commands are to be used only on DFS > states. This would limit the stupidities that userspace can put > cfg80211 into. > DFS implementation in driver has only one state - DFS_CAC_IN_PROGRESS, so I"m not sure we need here a state machine but some sanity checks will be added in the next RFC. -- Thanks, Victor.