[PATCHv2 0/5] mac80211/cfg80211/hwsim: Basic rate and TXQ params

[PATCHv2 0/5] mac80211/cfg80211/hwsim: Basic rate and TXQ params

[Jouni Malinen]

These patches add cfg80211/nl80211 attributes for configuring basic
rate set and TX queue parameters for AP mode. The current hostapd
development branch has matching userspace code for using the new
attributes. In addition, mac80211_hwsim is extended to show more
detailed debug information on BSS and TXQ changes.

v2 addresses the comments from Johannes and adds a patch to fix
kernel panics in certain rmmod mac80211_hwsim cases.

-- 
Jouni Malinen                                            PGP id EFC895FA

[PATCHv2 1/5] mac80211_hwsim: Debug info for BSS config changes

[Jouni Malinen]

Provide detailed information on BSS configuration changes to make it
easier to debug mac80211 functionality.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>

Index: wireless-testing/drivers/net/wireless/mac80211_hwsim.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/mac80211_hwsim.c	2008-10-28 19:50:54.000000000 +0200
+++ wireless-testing/drivers/net/wireless/mac80211_hwsim.c	2008-10-30 16:09:28.000000000 +0200
@@ -415,6 +415,43 @@
 					    u32 changed)
 {
 	hwsim_check_magic(vif);
+
+	printk(KERN_DEBUG "%s:%s(changed=0x%x)\n",
+	       wiphy_name(hw->wiphy), __func__, changed);
+
+	if (changed & BSS_CHANGED_ASSOC) {
+		printk(KERN_DEBUG "  %s: ASSOC: assoc=%d aid=%d\n",
+		       wiphy_name(hw->wiphy), info->assoc, info->aid);
+	}
+
+	if (changed & BSS_CHANGED_ERP_CTS_PROT) {
+		printk(KERN_DEBUG "  %s: ERP_CTS_PROT: %d\n",
+		       wiphy_name(hw->wiphy), info->use_cts_prot);
+	}
+
+	if (changed & BSS_CHANGED_ERP_PREAMBLE) {
+		printk(KERN_DEBUG "  %s: ERP_PREAMBLE: %d\n",
+		       wiphy_name(hw->wiphy), info->use_short_preamble);
+	}
+
+	if (changed & BSS_CHANGED_ERP_SLOT) {
+		printk(KERN_DEBUG "  %s: ERP_SLOT: %d\n",
+		       wiphy_name(hw->wiphy), info->use_short_slot);
+	}
+
+	if (changed & BSS_CHANGED_HT) {
+		printk(KERN_DEBUG "  %s: HT: sec_ch_offs=%d width_40_ok=%d "
+		       "op_mode=%d\n",
+		       wiphy_name(hw->wiphy),
+		       info->ht.secondary_channel_offset,
+		       info->ht.width_40_ok, info->ht.operation_mode);
+	}
+
+	if (changed & BSS_CHANGED_BASIC_RATES) {
+		printk(KERN_DEBUG "  %s: BASIC_RATES: 0x%llx\n",
+		       wiphy_name(hw->wiphy),
+		       (unsigned long long) info->basic_rates);
+	}
 }
 
 static void mac80211_hwsim_sta_notify(struct ieee80211_hw *hw,

--

-- 
Jouni Malinen                                            PGP id EFC895FA

[PATCHv2 2/5] nl80211: Add basic rate configuration for AP mode

[Jouni Malinen]

Add a new attribute, NL80211_ATTR_BSS_BASIC_RATES, that can be used with
NL80211_CMD_SET_BSS for userspace (e.g., hostapd) to set which rates are
in the basic rate set.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>

Index: wireless-testing/include/linux/nl80211.h
===================================================================
--- wireless-testing.orig/include/linux/nl80211.h	2008-10-28 19:50:57.000000000 +0200
+++ wireless-testing/include/linux/nl80211.h	2008-10-28 19:57:07.000000000 +0200
@@ -243,6 +243,9 @@
  *	(u8, 0 or 1)
  * @NL80211_ATTR_BSS_SHORT_SLOT_TIME: whether short slot time enabled
  *	(u8, 0 or 1)
+ * @NL80211_ATTR_BSS_BASIC_RATES: basic rates, array of basic
+ *	rates in format defined by IEEE 802.11 7.3.2.2 but without the length
+ *	restriction (at most %NL80211_MAX_SUPP_RATES).
  *
  * @NL80211_ATTR_HT_CAPABILITY: HT Capability information element (from
  *	association request when used with NL80211_CMD_NEW_STATION)
@@ -307,6 +310,8 @@
 
 	NL80211_ATTR_MESH_PARAMS,
 
+	NL80211_ATTR_BSS_BASIC_RATES,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -318,6 +323,7 @@
  * here
  */
 #define NL80211_ATTR_HT_CAPABILITY NL80211_ATTR_HT_CAPABILITY
+#define NL80211_ATTR_BSS_BASIC_RATES NL80211_ATTR_BSS_BASIC_RATES
 
 #define NL80211_MAX_SUPP_RATES			32
 #define NL80211_MAX_SUPP_REG_RULES		32
Index: wireless-testing/include/net/cfg80211.h
===================================================================
--- wireless-testing.orig/include/net/cfg80211.h	2008-10-28 19:59:34.000000000 +0200
+++ wireless-testing/include/net/cfg80211.h	2008-10-28 20:00:43.000000000 +0200
@@ -280,11 +280,16 @@
  *	(0 = no, 1 = yes, -1 = do not change)
  * @use_short_slot_time: Whether the use of short slot time is allowed
  *	(0 = no, 1 = yes, -1 = do not change)
+ * @basic_rates: basic rates in IEEE 802.11 format
+ *	(or NULL for no change)
+ * @basic_rates_len: number of basic rates
  */
 struct bss_parameters {
 	int use_cts_prot;
 	int use_short_preamble;
 	int use_short_slot_time;
+	u8 *basic_rates;
+	u8 basic_rates_len;
 };
 
 /**
Index: wireless-testing/net/mac80211/cfg.c
===================================================================
--- wireless-testing.orig/net/mac80211/cfg.c	2008-10-28 20:01:20.000000000 +0200
+++ wireless-testing/net/mac80211/cfg.c	2008-10-28 20:20:42.000000000 +0200
@@ -1048,6 +1048,24 @@
 		changed |= BSS_CHANGED_ERP_SLOT;
 	}
 
+	if (params->basic_rates) {
+		int i, j;
+		u32 rates = 0;
+		struct ieee80211_local *local = wiphy_priv(wiphy);
+		struct ieee80211_supported_band *sband =
+			wiphy->bands[local->oper_channel->band];
+
+		for (i = 0; i < params->basic_rates_len; i++) {
+			int rate = (params->basic_rates[i] & 0x7f) * 5;
+			for (j = 0; j < sband->n_bitrates; j++) {
+				if (sband->bitrates[j].bitrate == rate)
+					rates |= BIT(j);
+			}
+		}
+		sdata->vif.bss_conf.basic_rates = rates;
+		changed |= BSS_CHANGED_BASIC_RATES;
+	}
+
 	ieee80211_bss_info_change_notify(sdata, changed);
 
 	return 0;
Index: wireless-testing/net/wireless/nl80211.c
===================================================================
--- wireless-testing.orig/net/wireless/nl80211.c	2008-10-28 19:56:53.000000000 +0200
+++ wireless-testing/net/wireless/nl80211.c	2008-10-28 19:59:00.000000000 +0200
@@ -95,6 +95,8 @@
 	[NL80211_ATTR_BSS_CTS_PROT] = { .type = NLA_U8 },
 	[NL80211_ATTR_BSS_SHORT_PREAMBLE] = { .type = NLA_U8 },
 	[NL80211_ATTR_BSS_SHORT_SLOT_TIME] = { .type = NLA_U8 },
+	[NL80211_ATTR_BSS_BASIC_RATES] = { .type = NLA_BINARY,
+					   .len = NL80211_MAX_SUPP_RATES },
 
 	[NL80211_ATTR_MESH_PARAMS] = { .type = NLA_NESTED },
 
@@ -1613,6 +1615,12 @@
 	if (info->attrs[NL80211_ATTR_BSS_SHORT_SLOT_TIME])
 		params.use_short_slot_time =
 		    nla_get_u8(info->attrs[NL80211_ATTR_BSS_SHORT_SLOT_TIME]);
+	if (info->attrs[NL80211_ATTR_BSS_BASIC_RATES]) {
+		params.basic_rates =
+			nla_data(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
+		params.basic_rates_len =
+			nla_len(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
+	}
 
 	err = get_drv_dev_by_info_ifindex(info->attrs, &drv, &dev);
 	if (err)

--

-- 
Jouni Malinen                                            PGP id EFC895FA

[PATCHv2 3/5] mac80211_hwsim: Debug info for TX queue parameters

[Jouni Malinen]

Provide detailed information on TX queue parameter changes to make it
easier to debug mac80211 functionality.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>

Index: wireless-testing/drivers/net/wireless/mac80211_hwsim.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/mac80211_hwsim.c	2008-10-30 16:09:28.000000000 +0200
+++ wireless-testing/drivers/net/wireless/mac80211_hwsim.c	2008-10-30 16:11:33.000000000 +0200
@@ -478,6 +478,17 @@
 	return 0;
 }
 
+static int mac80211_hwsim_conf_tx(
+	struct ieee80211_hw *hw, u16 queue,
+	const struct ieee80211_tx_queue_params *params)
+{
+	printk(KERN_DEBUG "%s:%s (queue=%d txop=%d cw_min=%d cw_max=%d "
+	       "aifs=%d)\n",
+	       wiphy_name(hw->wiphy), __func__, queue,
+	       params->txop, params->cw_min, params->cw_max, params->aifs);
+	return 0;
+}
+
 static const struct ieee80211_ops mac80211_hwsim_ops =
 {
 	.tx = mac80211_hwsim_tx,
@@ -491,6 +502,7 @@
 	.bss_info_changed = mac80211_hwsim_bss_info_changed,
 	.sta_notify = mac80211_hwsim_sta_notify,
 	.set_tim = mac80211_hwsim_set_tim,
+	.conf_tx = mac80211_hwsim_conf_tx,
 };
 
 

--

-- 
Jouni Malinen                                            PGP id EFC895FA

[PATCHv2 4/5] nl80211: Add TX queue parameter configuration

[Jouni Malinen]

Add a new attribute, NL80211_ATTR_WIPHY_TXQ_PARAMS, that can be used with
NL80211_CMD_SET_WIPHY for userspace (e.g., hostapd) to set TX queue
parameters (txop, cwmin, cwmax, aifs).

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>

Index: wireless-testing/include/linux/nl80211.h
===================================================================
--- wireless-testing.orig/include/linux/nl80211.h	2008-10-30 16:11:27.000000000 +0200
+++ wireless-testing/include/linux/nl80211.h	2008-10-30 16:13:37.000000000 +0200
@@ -25,8 +25,9 @@
  *
  * @NL80211_CMD_GET_WIPHY: request information about a wiphy or dump request
  *	to get a list of all present wiphys.
- * @NL80211_CMD_SET_WIPHY: set wiphy name, needs %NL80211_ATTR_WIPHY and
- *	%NL80211_ATTR_WIPHY_NAME.
+ * @NL80211_CMD_SET_WIPHY: set wiphy parameters, needs %NL80211_ATTR_WIPHY or
+ *	%NL80211_ATTR_IFINDEX; can be used to set %NL80211_ATTR_WIPHY_NAME
+ *	and/or %NL80211_ATTR_WIPHY_TXQ_PARAMS.
  * @NL80211_CMD_NEW_WIPHY: Newly created wiphy, response to get request
  *	or rename notification. Has attributes %NL80211_ATTR_WIPHY and
  *	%NL80211_ATTR_WIPHY_NAME.
@@ -178,6 +179,7 @@
  * @NL80211_ATTR_WIPHY: index of wiphy to operate on, cf.
  *	/sys/class/ieee80211/<phyname>/index
  * @NL80211_ATTR_WIPHY_NAME: wiphy name (used for renaming)
+ * @NL80211_ATTR_WIPHY_TXQ_PARAMS: a nested array of TX queue parameters
  *
  * @NL80211_ATTR_IFINDEX: network interface index of the device to operate on
  * @NL80211_ATTR_IFNAME: network interface name
@@ -312,6 +314,8 @@
 
 	NL80211_ATTR_BSS_BASIC_RATES,
 
+	NL80211_ATTR_WIPHY_TXQ_PARAMS,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -324,6 +328,7 @@
  */
 #define NL80211_ATTR_HT_CAPABILITY NL80211_ATTR_HT_CAPABILITY
 #define NL80211_ATTR_BSS_BASIC_RATES NL80211_ATTR_BSS_BASIC_RATES
+#define NL80211_ATTR_WIPHY_TXQ_PARAMS NL80211_ATTR_WIPHY_TXQ_PARAMS
 
 #define NL80211_MAX_SUPP_RATES			32
 #define NL80211_MAX_SUPP_REG_RULES		32
@@ -698,4 +703,38 @@
 	NL80211_MESHCONF_ATTR_MAX = __NL80211_MESHCONF_ATTR_AFTER_LAST - 1
 };
 
+/**
+ * enum nl80211_txq_attr - TX queue parameter attributes
+ * @__NL80211_TXQ_ATTR_INVALID: Attribute number 0 is reserved
+ * @NL80211_TXQ_ATTR_QUEUE: TX queue identifier (NL80211_TXQ_Q_*)
+ * @NL80211_TXQ_ATTR_TXOP: Maximum burst time in units of 32 usecs, 0 meaning
+ *	disabled
+ * @NL80211_TXQ_ATTR_CWMIN: Minimum contention window [a value of the form
+ *	2^n-1 in the range 1..32767]
+ * @NL80211_TXQ_ATTR_CWMAX: Maximum contention window [a value of the form
+ *	2^n-1 in the range 1..32767]
+ * @NL80211_TXQ_ATTR_AIFS: Arbitration interframe space [0..255]
+ * @__NL80211_TXQ_ATTR_AFTER_LAST: Internal
+ * @NL80211_TXQ_ATTR_MAX: Maximum TXQ attribute number
+ */
+enum nl80211_txq_attr {
+	__NL80211_TXQ_ATTR_INVALID,
+	NL80211_TXQ_ATTR_QUEUE,
+	NL80211_TXQ_ATTR_TXOP,
+	NL80211_TXQ_ATTR_CWMIN,
+	NL80211_TXQ_ATTR_CWMAX,
+	NL80211_TXQ_ATTR_AIFS,
+
+	/* keep last */
+	__NL80211_TXQ_ATTR_AFTER_LAST,
+	NL80211_TXQ_ATTR_MAX = __NL80211_TXQ_ATTR_AFTER_LAST - 1
+};
+
+enum nl80211_txq_q {
+	NL80211_TXQ_Q_VO,
+	NL80211_TXQ_Q_VI,
+	NL80211_TXQ_Q_BE,
+	NL80211_TXQ_Q_BK
+};
+
 #endif /* __LINUX_NL80211_H */
Index: wireless-testing/include/net/cfg80211.h
===================================================================
--- wireless-testing.orig/include/net/cfg80211.h	2008-10-30 16:11:27.000000000 +0200
+++ wireless-testing/include/net/cfg80211.h	2008-10-30 16:31:21.000000000 +0200
@@ -371,6 +371,24 @@
 	u16 dot11MeshHWMPnetDiameterTraversalTime;
 };
 
+/**
+ * struct ieee80211_txq_params - TX queue parameters
+ * @queue: TX queue identifier (NL80211_TXQ_Q_*)
+ * @txop: Maximum burst time in units of 32 usecs, 0 meaning disabled
+ * @cwmin: Minimum contention window [a value of the form 2^n-1 in the range
+ *	1..32767]
+ * @cwmax: Maximum contention window [a value of the form 2^n-1 in the range
+ *	1..32767]
+ * @aifs: Arbitration interframe space [0..255]
+ */
+struct ieee80211_txq_params {
+	enum nl80211_txq_q queue;
+	u16 txop;
+	u16 cwmin;
+	u16 cwmax;
+	u8 aifs;
+};
+
 /* from net/wireless.h */
 struct wiphy;
 
@@ -430,6 +448,8 @@
  * @set_mesh_cfg: set mesh parameters (by now, just mesh id)
  *
  * @change_bss: Modify parameters for a given BSS.
+ *
+ * @set_txq_params: Set TX queue parameters
  */
 struct cfg80211_ops {
 	int	(*add_virtual_intf)(struct wiphy *wiphy, char *name,
@@ -490,6 +510,9 @@
 				const struct mesh_config *nconf, u32 mask);
 	int	(*change_bss)(struct wiphy *wiphy, struct net_device *dev,
 			      struct bss_parameters *params);
+
+	int	(*set_txq_params)(struct wiphy *wiphy,
+				  struct ieee80211_txq_params *params);
 };
 
 #endif /* __NET_CFG80211_H */
Index: wireless-testing/net/mac80211/cfg.c
===================================================================
--- wireless-testing.orig/net/mac80211/cfg.c	2008-10-30 16:11:27.000000000 +0200
+++ wireless-testing/net/mac80211/cfg.c	2008-10-30 16:11:40.000000000 +0200
@@ -1071,6 +1071,30 @@
 	return 0;
 }
 
+static int ieee80211_set_txq_params(struct wiphy *wiphy,
+				    struct ieee80211_txq_params *params)
+{
+	struct ieee80211_local *local = wiphy_priv(wiphy);
+	struct ieee80211_tx_queue_params p;
+
+	if (!local->ops->conf_tx)
+		return -EOPNOTSUPP;
+
+	memset(&p, 0, sizeof(p));
+	p.aifs = params->aifs;
+	p.cw_max = params->cwmax;
+	p.cw_min = params->cwmin;
+	p.txop = params->txop;
+	if (local->ops->conf_tx(local_to_hw(local), params->queue, &p)) {
+		printk(KERN_DEBUG "%s: failed to set TX queue "
+		       "parameters for queue %d\n", local->mdev->name,
+		       params->queue);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 struct cfg80211_ops mac80211_config_ops = {
 	.add_virtual_intf = ieee80211_add_iface,
 	.del_virtual_intf = ieee80211_del_iface,
@@ -1097,4 +1121,5 @@
 	.get_mesh_params = ieee80211_get_mesh_params,
 #endif
 	.change_bss = ieee80211_change_bss,
+	.set_txq_params = ieee80211_set_txq_params,
 };
Index: wireless-testing/net/wireless/nl80211.c
===================================================================
--- wireless-testing.orig/net/wireless/nl80211.c	2008-10-30 16:11:27.000000000 +0200
+++ wireless-testing/net/wireless/nl80211.c	2008-10-30 16:14:54.000000000 +0200
@@ -58,6 +58,7 @@
 	[NL80211_ATTR_WIPHY] = { .type = NLA_U32 },
 	[NL80211_ATTR_WIPHY_NAME] = { .type = NLA_NUL_STRING,
 				      .len = BUS_ID_SIZE-1 },
+	[NL80211_ATTR_WIPHY_TXQ_PARAMS] = { .type = NLA_NESTED },
 
 	[NL80211_ATTR_IFTYPE] = { .type = NLA_U32 },
 	[NL80211_ATTR_IFINDEX] = { .type = NLA_U32 },
@@ -286,20 +287,76 @@
 	return -ENOBUFS;
 }
 
+static const struct nla_policy txq_params_policy[NL80211_TXQ_ATTR_MAX + 1] = {
+	[NL80211_TXQ_ATTR_QUEUE]		= { .type = NLA_U8 },
+	[NL80211_TXQ_ATTR_TXOP]			= { .type = NLA_U16 },
+	[NL80211_TXQ_ATTR_CWMIN]		= { .type = NLA_U16 },
+	[NL80211_TXQ_ATTR_CWMAX]		= { .type = NLA_U16 },
+	[NL80211_TXQ_ATTR_AIFS]			= { .type = NLA_U8 },
+};
+
+static int parse_txq_params(struct nlattr *tb[],
+			    struct ieee80211_txq_params *txq_params)
+{
+	if (!tb[NL80211_TXQ_ATTR_QUEUE] || !tb[NL80211_TXQ_ATTR_TXOP] ||
+	    !tb[NL80211_TXQ_ATTR_CWMIN] || !tb[NL80211_TXQ_ATTR_CWMAX] ||
+	    !tb[NL80211_TXQ_ATTR_AIFS])
+		return -EINVAL;
+
+	txq_params->queue = nla_get_u8(tb[NL80211_TXQ_ATTR_QUEUE]);
+	txq_params->txop = nla_get_u16(tb[NL80211_TXQ_ATTR_TXOP]);
+	txq_params->cwmin = nla_get_u16(tb[NL80211_TXQ_ATTR_CWMIN]);
+	txq_params->cwmax = nla_get_u16(tb[NL80211_TXQ_ATTR_CWMAX]);
+	txq_params->aifs = nla_get_u8(tb[NL80211_TXQ_ATTR_AIFS]);
+
+	return 0;
+}
+
 static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev;
-	int result;
-
-	if (!info->attrs[NL80211_ATTR_WIPHY_NAME])
-		return -EINVAL;
+	int result = 0, rem_txq_params = 0;
+	struct nlattr *nl_txq_params;
 
 	rdev = cfg80211_get_dev_from_info(info);
 	if (IS_ERR(rdev))
 		return PTR_ERR(rdev);
 
-	result = cfg80211_dev_rename(rdev, nla_data(info->attrs[NL80211_ATTR_WIPHY_NAME]));
+	if (info->attrs[NL80211_ATTR_WIPHY_NAME]) {
+		result = cfg80211_dev_rename(
+			rdev, nla_data(info->attrs[NL80211_ATTR_WIPHY_NAME]));
+		if (result)
+			goto bad_res;
+	}
+
+	if (info->attrs[NL80211_ATTR_WIPHY_TXQ_PARAMS]) {
+		struct ieee80211_txq_params txq_params;
+		struct nlattr *tb[NL80211_TXQ_ATTR_MAX + 1];
+
+		if (!rdev->ops->set_txq_params) {
+			result = -EOPNOTSUPP;
+			goto bad_res;
+		}
+
+		nla_for_each_nested(nl_txq_params,
+				    info->attrs[NL80211_ATTR_WIPHY_TXQ_PARAMS],
+				    rem_txq_params) {
+			nla_parse(tb, NL80211_TXQ_ATTR_MAX,
+				  nla_data(nl_txq_params),
+				  nla_len(nl_txq_params),
+				  txq_params_policy);
+			result = parse_txq_params(tb, &txq_params);
+			if (result)
+				goto bad_res;
+
+			result = rdev->ops->set_txq_params(&rdev->wiphy,
+							   &txq_params);
+			if (result)
+				goto bad_res;
+		}
+	}
 
+bad_res:
 	cfg80211_put_dev(rdev);
 	return result;
 }

--

-- 
Jouni Malinen                                            PGP id EFC895FA

[PATCHv2 5/5] mac80211_hwsim: Make sure beacon_timer gets deleted

[Jouni Malinen]

It was possible to trigger a kernel panic because beacon_timer may not
have been deleted in all cases when the kernel module was removed while
hostapd was still running.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>

Index: wireless-testing/drivers/net/wireless/mac80211_hwsim.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/mac80211_hwsim.c	2008-10-30 16:11:33.000000000 +0200
+++ wireless-testing/drivers/net/wireless/mac80211_hwsim.c	2008-10-30 16:26:49.000000000 +0200
@@ -290,6 +290,7 @@
 {
 	struct mac80211_hwsim_data *data = hw->priv;
 	data->started = 0;
+	del_timer(&data->beacon_timer);
 	printk(KERN_DEBUG "%s:%s\n", wiphy_name(hw->wiphy), __func__);
 }
 

--

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCHv2 5/5] mac80211_hwsim: Make sure beacon_timer gets deleted

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

On Thu, 2008-10-30 at 16:59 +0200, Jouni Malinen wrote:
> plain text document attachment (hwsim_del_timer.patch)
> It was possible to trigger a kernel panic because beacon_timer may not
> have been deleted in all cases when the kernel module was removed while
> hostapd was still running.
> 
> Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>
> 
> Index: wireless-testing/drivers/net/wireless/mac80211_hwsim.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/mac80211_hwsim.c	2008-10-30 16:11:33.000000000 +0200
> +++ wireless-testing/drivers/net/wireless/mac80211_hwsim.c	2008-10-30 16:26:49.000000000 +0200
> @@ -290,6 +290,7 @@
>  {
>  	struct mac80211_hwsim_data *data = hw->priv;
>  	data->started = 0;
> +	del_timer(&data->beacon_timer);

should that be del_timer_sync, just in case? Not sure how that works
with the rearming thing, I never really understood rearming timers.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCHv2 0/5] mac80211/cfg80211/hwsim: Basic rate and TXQ params

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On Thu, 2008-10-30 at 16:59 +0200, Jouni Malinen wrote:
> These patches add cfg80211/nl80211 attributes for configuring basic
> rate set and TX queue parameters for AP mode. The current hostapd
> development branch has matching userspace code for using the new
> attributes. In addition, mac80211_hwsim is extended to show more
> detailed debug information on BSS and TXQ changes.
> 
> v2 addresses the comments from Johannes and adds a patch to fix
> kernel panics in certain rmmod mac80211_hwsim cases.

Looks good to me, thanks.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCHv2 5/5] mac80211_hwsim: Make sure beacon_timer gets deleted

[Jouni Malinen]

On Thu, Oct 30, 2008 at 04:08:40PM +0100, Johannes Berg wrote:
> On Thu, 2008-10-30 at 16:59 +0200, Jouni Malinen wrote:
> > Index: wireless-testing/drivers/net/wireless/mac80211_hwsim.c
> > @@ -290,6 +290,7 @@
> >  {
> >  	struct mac80211_hwsim_data *data = hw->priv;
> >  	data->started = 0;
> > +	del_timer(&data->beacon_timer);
> 
> should that be del_timer_sync, just in case? Not sure how that works
> with the rearming thing, I never really understood rearming timers.

No idea.. I have to admit I don't care that much in this particular
case (hwsim is a test driver), but if someone can explain how this is
supposed to be done properly, I would be happy to fix this ;-).

That started=0 will prevent future timer handlers from using add_timer,
but a currently running handler may miss that and as such,
del_timer_sync() might not be enough. I would assume calling
del_timer_sync() here twice would take care of that corner case, but
that looks somewhat funny.

John, can you at least apply the first four patches from this series? I
would be fine with getting the fifth one in as-is, too, since it does at
least make it quite a bit less likely to hit the kernel panic. We can
then update this if someone can tell what the recommended way of killing
this type of timer is.

-- 
Jouni Malinen                                            PGP id EFC895FA