[PATCH 0/8] qtnfmac: add more features to driver

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/8] qtnfmac: add more features to driver
@ 2017-06-20 19:55 Sergey Matyukevich
  2017-06-20 19:55 ` [PATCH 1/8] qtnfmac: updates and fixes for regulatory support Sergey Matyukevich
                   ` (7 more replies)
  0 siblings, 8 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil

This patch series adds more features to qtnfmac driver including
the following major pieces:
 - updates for regulatory support
 - AP/VLAN support
 - dump_survey handler
 - channel_switch handler

bus.h        |    1 
 cfg80211.c   |  560 +++++++++++++++++++++++++++++++++++++++++++++++++----------
 cfg80211.h   |    4 
 commands.c   |  515 ++++++++++++++++++++++++++++++++++++++++++++++++------
 commands.h   |    5 
 core.c       |   83 ++++++++
 core.h       |   62 +++++-
 event.c      |   67 ++++++-
 pearl/pcie.c |    5 
 qlink.h      |  201 +++++++++++++++++++--
 qlink_util.c |   26 +-
 qlink_util.h |    2 
 util.c       |   74 +++++--
 util.h       |   34 ++-
 14 files changed, 1422 insertions(+), 217 deletions(-)

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/8] qtnfmac: updates and fixes for regulatory support
  2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
@ 2017-06-20 19:55 ` Sergey Matyukevich
  2017-06-27 17:20   ` Kalle Valo
  2017-06-20 19:55 ` [PATCH 2/8] qtnfmac: cleanup and fixes preparing AP_VLAN support Sergey Matyukevich
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich

This patch introduces several changes for regulatory support
in qtnfmac driver:

* Introduce support for setting regdomain from host
New command is implemented to notify firmware about regdomain changes
performed by host. Firmware is supposed to check requested regdomain
changes. On success firmware enables updated hardware channel
configuration and then host driver updates channel info for each band.

* Introduce support of custom regulatory rules
Obtain custom regulatory rules from the firmware and
enable them during wiphy registration

* Regulatory for self-managed setup
Regdomain information needs to be registered with cfg80211
for devices with REGULATORY_WIPHY_SELF_MANAGED flag set.

* Misc fixes in regulatory code paths
  - add missing lock in qtnf_cmd_get_mac_chan_info
  - do not free band channel structure if channel count is the same
  - free allocated regdomain hw_info.rd structure on detach

Signed-off-by: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>
Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c |  83 +++----
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 286 ++++++++++++++++++----
 drivers/net/wireless/quantenna/qtnfmac/commands.h |   1 +
 drivers/net/wireless/quantenna/qtnfmac/core.c     |   3 +
 drivers/net/wireless/quantenna/qtnfmac/core.h     |   7 +-
 drivers/net/wireless/quantenna/qtnfmac/qlink.h    | 126 +++++++++-
 6 files changed, 403 insertions(+), 103 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index e3c090008125..825a6334fbfe 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -700,67 +700,45 @@ static struct cfg80211_ops qtn_cfg80211_ops = {
 	.disconnect		= qtnf_disconnect
 };
 
-static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy,
+static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy_in,
 				       struct regulatory_request *req)
 {
-	struct qtnf_wmac *mac = wiphy_priv(wiphy);
-	struct qtnf_bus *bus;
-	struct qtnf_vif *vif;
-	struct qtnf_wmac *chan_mac;
-	int i;
+	struct qtnf_wmac *mac = wiphy_priv(wiphy_in);
+	struct wiphy *wiphy;
+	struct qtnf_bus *bus = mac->bus;
+	unsigned int mac_idx;
 	enum nl80211_band band;
-
-	bus = mac->bus;
+	int ret;
 
 	pr_debug("MAC%u: initiator=%d alpha=%c%c\n", mac->macid, req->initiator,
 		 req->alpha2[0], req->alpha2[1]);
 
-	vif = qtnf_mac_get_base_vif(mac);
-	if (!vif) {
-		pr_err("MAC%u: primary VIF is not configured\n", mac->macid);
-		return;
-	}
-
-	/* ignore non-ISO3166 country codes */
-	for (i = 0; i < sizeof(req->alpha2); i++) {
-		if (req->alpha2[i] < 'A' || req->alpha2[i] > 'Z') {
-			pr_err("MAC%u: not an ISO3166 code\n", mac->macid);
-			return;
-		}
-	}
-	if (!strncasecmp(req->alpha2, bus->hw_info.alpha2_code,
-			 sizeof(req->alpha2))) {
-		pr_warn("MAC%u: unchanged country code\n", mac->macid);
-		return;
-	}
-
-	if (qtnf_cmd_send_regulatory_config(mac, req->alpha2)) {
-		pr_err("MAC%u: failed to configure regulatory\n", mac->macid);
+	ret = qtnf_cmd_reg_notify(bus, req);
+	if (ret) {
+		if (ret != -EOPNOTSUPP && ret != -EALREADY)
+			pr_err("failed to update reg domain to %c%c\n",
+			       req->alpha2[0], req->alpha2[1]);
 		return;
 	}
 
-	for (i = 0; i < bus->hw_info.num_mac; i++) {
-		chan_mac = bus->mac[i];
-
-		if (!chan_mac)
+	for (mac_idx = 0; mac_idx < QTNF_MAX_MAC; ++mac_idx) {
+		if (!(bus->hw_info.mac_bitmap & (1 << mac_idx)))
 			continue;
 
-		if (!(bus->hw_info.mac_bitmap & BIT(i)))
-			continue;
+		mac = bus->mac[mac_idx];
+		wiphy = priv_to_wiphy(mac);
 
 		for (band = 0; band < NUM_NL80211_BANDS; ++band) {
 			if (!wiphy->bands[band])
 				continue;
 
-			if (qtnf_cmd_get_mac_chan_info(chan_mac,
-						       wiphy->bands[band])) {
-				pr_err("MAC%u: can't get channel info\n",
-				       chan_mac->macid);
-				qtnf_core_detach(bus);
-
-				return;
-			}
+			ret = qtnf_cmd_get_mac_chan_info(mac,
+							 wiphy->bands[band]);
+			if (ret)
+				pr_err("failed to get chan info for mac %u band %u\n",
+				       mac_idx, band);
 		}
+
 	}
 }
 
@@ -889,21 +867,26 @@ int qtnf_wiphy_register(struct qtnf_hw_info *hw_info, struct qtnf_wmac *mac)
 	ether_addr_copy(wiphy->perm_addr, mac->macaddr);
 
 	if (hw_info->hw_capab & QLINK_HW_SUPPORTS_REG_UPDATE) {
-		pr_debug("device supports REG_UPDATE\n");
+		wiphy->regulatory_flags |= REGULATORY_STRICT_REG |
+			REGULATORY_CUSTOM_REG;
 		wiphy->reg_notifier = qtnf_cfg80211_reg_notifier;
-		pr_debug("hint regulatory about EP region: %c%c\n",
-			 hw_info->alpha2_code[0],
-			 hw_info->alpha2_code[1]);
-		regulatory_hint(wiphy, hw_info->alpha2_code);
+		wiphy_apply_custom_regulatory(wiphy, hw_info->rd);
 	} else {
-		pr_debug("device doesn't support REG_UPDATE\n");
 		wiphy->regulatory_flags |= REGULATORY_WIPHY_SELF_MANAGED;
 	}
 
 	ret = wiphy_register(wiphy);
+	if (ret < 0)
+		goto out;
+
+	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+		ret = regulatory_set_wiphy_regd(wiphy, hw_info->rd);
+	else if (isalpha(hw_info->rd->alpha2[0]) &&
+		 isalpha(hw_info->rd->alpha2[1]))
+		ret = regulatory_hint(wiphy, hw_info->rd->alpha2);
 
 out:
-	if (ret < 0) {
+	if (ret) {
 		kfree(iface_comb);
 		return ret;
 	}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index cce62f39edaf..2ee007ecb236 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -181,38 +181,6 @@ int qtnf_cmd_send_start_ap(struct qtnf_vif *vif)
 	return ret;
 }
 
-int qtnf_cmd_send_regulatory_config(struct qtnf_wmac *mac, const char *alpha2)
-{
-	struct sk_buff *cmd_skb;
-	u16 res_code = QLINK_CMD_RESULT_OK;
-	int ret;
-
-	cmd_skb = qtnf_cmd_alloc_new_cmdskb(mac->macid, QLINK_VIFID_RSVD,
-					    QLINK_CMD_REG_REGION,
-					    sizeof(struct qlink_cmd));
-	if (unlikely(!cmd_skb))
-		return -ENOMEM;
-
-	qtnf_cmd_skb_put_tlv_arr(cmd_skb, WLAN_EID_COUNTRY, alpha2,
-				 QTNF_MAX_ALPHA_LEN);
-
-	ret = qtnf_cmd_send(mac->bus, cmd_skb, &res_code);
-
-	if (unlikely(ret))
-		goto out;
-
-	if (unlikely(res_code != QLINK_CMD_RESULT_OK)) {
-		pr_err("MAC%u: CMD failed: %u\n", mac->macid, res_code);
-		ret = -EFAULT;
-		goto out;
-	}
-
-	memcpy(mac->bus->hw_info.alpha2_code, alpha2,
-	       sizeof(mac->bus->hw_info.alpha2_code));
-out:
-	return ret;
-}
-
 int qtnf_cmd_send_config_ap(struct qtnf_vif *vif)
 {
 	struct sk_buff *cmd_skb;
@@ -850,28 +818,175 @@ int qtnf_cmd_send_del_intf(struct qtnf_vif *vif)
 	return ret;
 }
 
+static u32 qtnf_cmd_resp_reg_rule_flags_parse(u32 qflags)
+{
+	u32 flags = 0;
+
+	if (qflags & QLINK_RRF_NO_OFDM)
+		flags |= NL80211_RRF_NO_OFDM;
+
+	if (qflags & QLINK_RRF_NO_CCK)
+		flags |= NL80211_RRF_NO_CCK;
+
+	if (qflags & QLINK_RRF_NO_INDOOR)
+		flags |= NL80211_RRF_NO_INDOOR;
+
+	if (qflags & QLINK_RRF_NO_OUTDOOR)
+		flags |= NL80211_RRF_NO_OUTDOOR;
+
+	if (qflags & QLINK_RRF_DFS)
+		flags |= NL80211_RRF_DFS;
+
+	if (qflags & QLINK_RRF_PTP_ONLY)
+		flags |= NL80211_RRF_PTP_ONLY;
+
+	if (qflags & QLINK_RRF_PTMP_ONLY)
+		flags |= NL80211_RRF_PTMP_ONLY;
+
+	if (qflags & QLINK_RRF_NO_IR)
+		flags |= NL80211_RRF_NO_IR;
+
+	if (qflags & QLINK_RRF_AUTO_BW)
+		flags |= NL80211_RRF_AUTO_BW;
+
+	if (qflags & QLINK_RRF_IR_CONCURRENT)
+		flags |= NL80211_RRF_IR_CONCURRENT;
+
+	if (qflags & QLINK_RRF_NO_HT40MINUS)
+		flags |= NL80211_RRF_NO_HT40MINUS;
+
+	if (qflags & QLINK_RRF_NO_HT40PLUS)
+		flags |= NL80211_RRF_NO_HT40PLUS;
+
+	if (qflags & QLINK_RRF_NO_80MHZ)
+		flags |= NL80211_RRF_NO_80MHZ;
+
+	if (qflags & QLINK_RRF_NO_160MHZ)
+		flags |= NL80211_RRF_NO_160MHZ;
+
+	return flags;
+}
+
 static int
 qtnf_cmd_resp_proc_hw_info(struct qtnf_bus *bus,
-			   const struct qlink_resp_get_hw_info *resp)
+			   const struct qlink_resp_get_hw_info *resp,
+			   size_t info_len)
 {
 	struct qtnf_hw_info *hwinfo = &bus->hw_info;
+	const struct qlink_tlv_hdr *tlv;
+	const struct qlink_tlv_reg_rule *tlv_rule;
+	struct ieee80211_reg_rule *rule;
+	u16 tlv_type;
+	u16 tlv_value_len;
+	unsigned int rule_idx = 0;
+
+	if (WARN_ON(resp->n_reg_rules > NL80211_MAX_SUPP_REG_RULES))
+		return -E2BIG;
+
+	hwinfo->rd = kzalloc(sizeof(*hwinfo->rd)
+			     + sizeof(struct ieee80211_reg_rule)
+			     * resp->n_reg_rules, GFP_KERNEL);
+
+	if (!hwinfo->rd)
+		return -ENOMEM;
 
 	hwinfo->num_mac = resp->num_mac;
 	hwinfo->mac_bitmap = resp->mac_bitmap;
 	hwinfo->fw_ver = le32_to_cpu(resp->fw_ver);
 	hwinfo->ql_proto_ver = le16_to_cpu(resp->ql_proto_ver);
-	memcpy(hwinfo->alpha2_code, resp->alpha2_code,
-	       sizeof(hwinfo->alpha2_code));
 	hwinfo->total_tx_chain = resp->total_tx_chain;
 	hwinfo->total_rx_chain = resp->total_rx_chain;
 	hwinfo->hw_capab = le32_to_cpu(resp->hw_capab);
+	hwinfo->rd->n_reg_rules = resp->n_reg_rules;
+	hwinfo->rd->alpha2[0] = resp->alpha2[0];
+	hwinfo->rd->alpha2[1] = resp->alpha2[1];
+
+	switch (resp->dfs_region) {
+	case QLINK_DFS_FCC:
+		hwinfo->rd->dfs_region = NL80211_DFS_FCC;
+		break;
+	case QLINK_DFS_ETSI:
+		hwinfo->rd->dfs_region = NL80211_DFS_ETSI;
+		break;
+	case QLINK_DFS_JP:
+		hwinfo->rd->dfs_region = NL80211_DFS_JP;
+		break;
+	case QLINK_DFS_UNSET:
+	default:
+		hwinfo->rd->dfs_region = NL80211_DFS_UNSET;
+		break;
+	}
+
+	tlv = (const struct qlink_tlv_hdr *)resp->info;
+
+	while (info_len >= sizeof(*tlv)) {
+		tlv_type = le16_to_cpu(tlv->type);
+		tlv_value_len = le16_to_cpu(tlv->len);
+
+		if (tlv_value_len + sizeof(*tlv) > info_len) {
+			pr_warn("malformed TLV 0x%.2X; LEN: %u\n",
+				tlv_type, tlv_value_len);
+			goto err;
+		}
+
+		switch (tlv_type) {
+		case QTN_TLV_ID_REG_RULE:
+			if (rule_idx >= resp->n_reg_rules) {
+				pr_warn("unexpected number of rules: %u\n",
+					resp->n_reg_rules);
+				goto err;
+			}
+
+			if (tlv_value_len != sizeof(*tlv_rule) - sizeof(*tlv)) {
+				pr_warn("malformed TLV 0x%.2X; LEN: %u\n",
+					tlv_type, tlv_value_len);
+				goto err;
+			}
+
+			tlv_rule = (const struct qlink_tlv_reg_rule *)tlv;
+			rule = &hwinfo->rd->reg_rules[rule_idx++];
+
+			rule->freq_range.start_freq_khz =
+				le32_to_cpu(tlv_rule->start_freq_khz);
+			rule->freq_range.end_freq_khz =
+				le32_to_cpu(tlv_rule->end_freq_khz);
+			rule->freq_range.max_bandwidth_khz =
+				le32_to_cpu(tlv_rule->max_bandwidth_khz);
+			rule->power_rule.max_antenna_gain =
+				le32_to_cpu(tlv_rule->max_antenna_gain);
+			rule->power_rule.max_eirp =
+				le32_to_cpu(tlv_rule->max_eirp);
+			rule->dfs_cac_ms =
+				le32_to_cpu(tlv_rule->dfs_cac_ms);
+			rule->flags = qtnf_cmd_resp_reg_rule_flags_parse(
+					le32_to_cpu(tlv_rule->flags));
+			break;
+		default:
+			break;
+		}
+
+		info_len -= tlv_value_len + sizeof(*tlv);
+		tlv = (struct qlink_tlv_hdr *)(tlv->val + tlv_value_len);
+	}
+
+	if (rule_idx != resp->n_reg_rules) {
+		pr_warn("unexpected number of rules: expected %u got %u\n",
+			resp->n_reg_rules, rule_idx);
+		goto err;
+	}
 
 	pr_info("fw_version=%d, MACs map %#x, alpha2=\"%c%c\", chains Tx=%u Rx=%u\n",
 		hwinfo->fw_ver, hwinfo->mac_bitmap,
-		hwinfo->alpha2_code[0], hwinfo->alpha2_code[1],
+		hwinfo->rd->alpha2[0], hwinfo->rd->alpha2[1],
 		hwinfo->total_tx_chain, hwinfo->total_rx_chain);
 
 	return 0;
+
+err:
+	kfree(hwinfo->rd);
+	hwinfo->rd = NULL;
+
+	return -EINVAL;
 }
 
 static int qtnf_parse_variable_mac_info(struct qtnf_wmac *mac,
@@ -1015,14 +1130,24 @@ qtnf_cmd_resp_fill_channels_info(struct ieee80211_supported_band *band,
 	unsigned int chidx = 0;
 	u32 qflags;
 
-	kfree(band->channels);
-	band->channels = NULL;
+	if (band->channels) {
+		if (band->n_channels == resp->num_chans) {
+			memset(band->channels, 0,
+			       sizeof(*band->channels) * band->n_channels);
+		} else {
+			kfree(band->channels);
+			band->n_channels = 0;
+			band->channels = NULL;
+		}
+	}
 
 	band->n_channels = resp->num_chans;
 	if (band->n_channels == 0)
 		return 0;
 
-	band->channels = kcalloc(band->n_channels, sizeof(*chan), GFP_KERNEL);
+	if (!band->channels)
+		band->channels = kcalloc(band->n_channels, sizeof(*chan),
+					 GFP_KERNEL);
 	if (!band->channels) {
 		band->n_channels = 0;
 		return -ENOMEM;
@@ -1258,6 +1383,7 @@ int qtnf_cmd_get_hw_info(struct qtnf_bus *bus)
 	const struct qlink_resp_get_hw_info *resp;
 	u16 res_code = QLINK_CMD_RESULT_OK;
 	int ret = 0;
+	size_t info_len;
 
 	cmd_skb = qtnf_cmd_alloc_new_cmdskb(QLINK_MACID_RSVD, QLINK_VIFID_RSVD,
 					    QLINK_CMD_GET_HW_INFO,
@@ -1268,7 +1394,7 @@ int qtnf_cmd_get_hw_info(struct qtnf_bus *bus)
 	qtnf_bus_lock(bus);
 
 	ret = qtnf_cmd_send_with_reply(bus, cmd_skb, &resp_skb, &res_code,
-				       sizeof(*resp), NULL);
+				       sizeof(*resp), &info_len);
 
 	if (unlikely(ret))
 		goto out;
@@ -1280,7 +1406,7 @@ int qtnf_cmd_get_hw_info(struct qtnf_bus *bus)
 	}
 
 	resp = (const struct qlink_resp_get_hw_info *)resp_skb->data;
-	ret = qtnf_cmd_resp_proc_hw_info(bus, resp);
+	ret = qtnf_cmd_resp_proc_hw_info(bus, resp, info_len);
 
 out:
 	qtnf_bus_unlock(bus);
@@ -1322,6 +1448,9 @@ int qtnf_cmd_get_mac_chan_info(struct qtnf_wmac *mac,
 
 	cmd = (struct qlink_cmd_chans_info_get *)cmd_skb->data;
 	cmd->band = qband;
+
+	qtnf_bus_lock(mac->bus);
+
 	ret = qtnf_cmd_send_with_reply(mac->bus, cmd_skb, &resp_skb, &res_code,
 				       sizeof(*resp), &info_len);
 
@@ -1345,6 +1474,7 @@ int qtnf_cmd_get_mac_chan_info(struct qtnf_wmac *mac,
 	ret = qtnf_cmd_resp_fill_channels_info(band, resp, info_len);
 
 out:
+	qtnf_bus_unlock(mac->bus);
 	consume_skb(resp_skb);
 
 	return ret;
@@ -1980,3 +2110,77 @@ int qtnf_cmd_send_updown_intf(struct qtnf_vif *vif, bool up)
 	qtnf_bus_unlock(vif->mac->bus);
 	return ret;
 }
+
+int qtnf_cmd_reg_notify(struct qtnf_bus *bus, struct regulatory_request *req)
+{
+	struct sk_buff *cmd_skb;
+	int ret;
+	u16 res_code;
+	struct qlink_cmd_reg_notify *cmd;
+
+	cmd_skb = qtnf_cmd_alloc_new_cmdskb(QLINK_MACID_RSVD, QLINK_VIFID_RSVD,
+					    QLINK_CMD_REG_NOTIFY,
+					    sizeof(*cmd));
+	if (!cmd_skb)
+		return -ENOMEM;
+
+	cmd = (struct qlink_cmd_reg_notify *)cmd_skb->data;
+	cmd->alpha2[0] = req->alpha2[0];
+	cmd->alpha2[1] = req->alpha2[1];
+
+	switch (req->initiator) {
+	case NL80211_REGDOM_SET_BY_CORE:
+		cmd->initiator = QLINK_REGDOM_SET_BY_CORE;
+		break;
+	case NL80211_REGDOM_SET_BY_USER:
+		cmd->initiator = QLINK_REGDOM_SET_BY_USER;
+		break;
+	case NL80211_REGDOM_SET_BY_DRIVER:
+		cmd->initiator = QLINK_REGDOM_SET_BY_DRIVER;
+		break;
+	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
+		cmd->initiator = QLINK_REGDOM_SET_BY_COUNTRY_IE;
+		break;
+	}
+
+	switch (req->user_reg_hint_type) {
+	case NL80211_USER_REG_HINT_USER:
+		cmd->user_reg_hint_type = QLINK_USER_REG_HINT_USER;
+		break;
+	case NL80211_USER_REG_HINT_CELL_BASE:
+		cmd->user_reg_hint_type = QLINK_USER_REG_HINT_CELL_BASE;
+		break;
+	case NL80211_USER_REG_HINT_INDOOR:
+		cmd->user_reg_hint_type = QLINK_USER_REG_HINT_INDOOR;
+		break;
+	}
+
+	qtnf_bus_lock(bus);
+
+	ret = qtnf_cmd_send(bus, cmd_skb, &res_code);
+	if (ret)
+		goto out;
+
+	switch (res_code) {
+	case QLINK_CMD_RESULT_ENOTSUPP:
+		pr_warn("reg update not supported\n");
+		ret = -EOPNOTSUPP;
+		break;
+	case QLINK_CMD_RESULT_EALREADY:
+		pr_info("regulatory domain is already set to %c%c",
+			req->alpha2[0], req->alpha2[1]);
+		ret = -EALREADY;
+		break;
+	case QLINK_CMD_RESULT_OK:
+		ret = 0;
+		break;
+	default:
+		ret = -EFAULT;
+		break;
+	}
+
+out:
+	qtnf_bus_unlock(bus);
+
+	return ret;
+}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.h b/drivers/net/wireless/quantenna/qtnfmac/commands.h
index 6c51854ef5e7..155b265d42bf 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.h
@@ -70,5 +70,6 @@ int qtnf_cmd_send_disconnect(struct qtnf_vif *vif,
 			     u16 reason_code);
 int qtnf_cmd_send_updown_intf(struct qtnf_vif *vif,
 			      bool up);
+int qtnf_cmd_reg_notify(struct qtnf_bus *bus, struct regulatory_request *req);
 
 #endif /* QLINK_COMMANDS_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c
index c5ac252464f4..21cbe6dcf6fa 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -549,6 +549,9 @@ void qtnf_core_detach(struct qtnf_bus *bus)
 		destroy_workqueue(bus->workqueue);
 	}
 
+	kfree(bus->hw_info.rd);
+	bus->hw_info.rd = NULL;
+
 	qtnf_trans_free(bus);
 }
 EXPORT_SYMBOL_GPL(qtnf_core_detach);
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index a616434281cf..31b7ec2bfd3e 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -42,7 +42,6 @@
 
 #define QTNF_MAX_SSID_LIST_LENGTH	2
 #define QTNF_MAX_VSIE_LEN		255
-#define QTNF_MAX_ALPHA_LEN		2
 #define QTNF_MAX_INTF			8
 #define QTNF_MAX_EVENT_QUEUE_LEN	255
 #define QTNF_DEFAULT_BG_SCAN_PERIOD	300
@@ -136,14 +135,14 @@ struct qtnf_wmac {
 };
 
 struct qtnf_hw_info {
+	u16 ql_proto_ver;
 	u8 num_mac;
 	u8 mac_bitmap;
-	u8 alpha2_code[QTNF_MAX_ALPHA_LEN];
 	u32 fw_ver;
-	u16 ql_proto_ver;
+	u32 hw_capab;
+	struct ieee80211_regdomain *rd;
 	u8 total_tx_chain;
 	u8 total_rx_chain;
-	u32 hw_capab;
 };
 
 struct qtnf_vif *qtnf_mac_get_free_vif(struct qtnf_wmac *mac);
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 6eafc15e0065..e27833b78940 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -19,7 +19,7 @@
 
 #include <linux/ieee80211.h>
 
-#define QLINK_PROTO_VER		3
+#define QLINK_PROTO_VER		4
 
 #define QLINK_MACID_RSVD		0xFF
 #define QLINK_VIFID_RSVD		0xFF
@@ -133,6 +133,9 @@ enum qlink_channel_width {
  *	number of operational channels and information on each of the channel.
  *	This command is generic to a specified MAC, interface index must be set
  *	to QLINK_VIFID_RSVD in command header.
+ * @QLINK_CMD_REG_NOTIFY: notify device about regulatory domain change. This
+ *	command is supported only if device reports QLINK_HW_SUPPORTS_REG_UPDATE
+ *	capability.
  */
 enum qlink_cmd_type {
 	QLINK_CMD_FW_INIT		= 0x0001,
@@ -148,7 +151,7 @@ enum qlink_cmd_type {
 	QLINK_CMD_DEL_INTF		= 0x0016,
 	QLINK_CMD_CHANGE_INTF		= 0x0017,
 	QLINK_CMD_UPDOWN_INTF		= 0x0018,
-	QLINK_CMD_REG_REGION		= 0x0019,
+	QLINK_CMD_REG_NOTIFY		= 0x0019,
 	QLINK_CMD_CHANS_INFO_GET	= 0x001A,
 	QLINK_CMD_CONFIG_AP		= 0x0020,
 	QLINK_CMD_START_AP		= 0x0021,
@@ -430,6 +433,44 @@ struct qlink_cmd_chans_info_get {
 	u8 band;
 } __packed;
 
+/**
+ * enum qlink_reg_initiator - Indicates the initiator of a reg domain request
+ *
+ * See &enum nl80211_reg_initiator for more info.
+ */
+enum qlink_reg_initiator {
+	QLINK_REGDOM_SET_BY_CORE,
+	QLINK_REGDOM_SET_BY_USER,
+	QLINK_REGDOM_SET_BY_DRIVER,
+	QLINK_REGDOM_SET_BY_COUNTRY_IE,
+};
+
+/**
+ * enum qlink_user_reg_hint_type - type of user regulatory hint
+ *
+ * See &enum nl80211_user_reg_hint_type for more info.
+ */
+enum qlink_user_reg_hint_type {
+	QLINK_USER_REG_HINT_USER	= 0,
+	QLINK_USER_REG_HINT_CELL_BASE	= 1,
+	QLINK_USER_REG_HINT_INDOOR	= 2,
+};
+
+/**
+ * struct qlink_cmd_reg_notify - data for QLINK_CMD_REG_NOTIFY command
+ *
+ * @alpha2: the ISO / IEC 3166 alpha2 country code.
+ * @initiator: which entity sent the request, one of &enum qlink_reg_initiator.
+ * @user_reg_hint_type: type of hint for QLINK_REGDOM_SET_BY_USER request, one
+ *	of &enum qlink_user_reg_hint_type.
+ */
+struct qlink_cmd_reg_notify {
+	struct qlink_cmd chdr;
+	u8 alpha2[2];
+	u8 initiator;
+	u8 user_reg_hint_type;
+} __packed;
+
 /* QLINK Command Responses messages related definitions
  */
 
@@ -438,6 +479,7 @@ enum qlink_cmd_result {
 	QLINK_CMD_RESULT_INVALID,
 	QLINK_CMD_RESULT_ENOTSUPP,
 	QLINK_CMD_RESULT_ENOTFOUND,
+	QLINK_CMD_RESULT_EALREADY,
 };
 
 /**
@@ -497,6 +539,18 @@ struct qlink_resp_get_mac_info {
 } __packed;
 
 /**
+ * enum qlink_dfs_regions - regulatory DFS regions
+ *
+ * Corresponds to &enum nl80211_dfs_regions.
+ */
+enum qlink_dfs_regions {
+	QLINK_DFS_UNSET	= 0,
+	QLINK_DFS_FCC	= 1,
+	QLINK_DFS_ETSI	= 2,
+	QLINK_DFS_JP	= 3,
+};
+
+/**
  * struct qlink_resp_get_hw_info - response for QLINK_CMD_GET_HW_INFO command
  *
  * Description of wireless hardware capabilities and features.
@@ -504,22 +558,29 @@ struct qlink_resp_get_mac_info {
  * @fw_ver: wireless hardware firmware version.
  * @hw_capab: Bitmap of capabilities supported by firmware.
  * @ql_proto_ver: Version of QLINK protocol used by firmware.
- * @country_code: country code ID firmware is configured to.
  * @num_mac: Number of separate physical radio devices provided by hardware.
  * @mac_bitmap: Bitmap of MAC IDs that are active and can be used in firmware.
  * @total_tx_chains: total number of transmit chains used by device.
  * @total_rx_chains: total number of receive chains.
+ * @alpha2: country code ID firmware is configured to.
+ * @n_reg_rules: number of regulatory rules TLVs in variable portion of the
+ *	message.
+ * @dfs_region: regulatory DFS region, one of @enum qlink_dfs_region.
+ * @info: variable-length HW info, can contain QTN_TLV_ID_REG_RULE.
  */
 struct qlink_resp_get_hw_info {
 	struct qlink_resp rhdr;
 	__le32 fw_ver;
 	__le32 hw_capab;
 	__le16 ql_proto_ver;
-	u8 alpha2_code[2];
 	u8 num_mac;
 	u8 mac_bitmap;
 	u8 total_tx_chain;
 	u8 total_rx_chain;
+	u8 alpha2[2];
+	u8 n_reg_rules;
+	u8 dfs_region;
+	u8 info[0];
 } __packed;
 
 /**
@@ -741,6 +802,7 @@ enum qlink_tlv_id {
 	QTN_TLV_ID_LRETRY_LIMIT		= 0x0204,
 	QTN_TLV_ID_BCN_PERIOD		= 0x0205,
 	QTN_TLV_ID_DTIM			= 0x0206,
+	QTN_TLV_ID_REG_RULE		= 0x0207,
 	QTN_TLV_ID_CHANNEL		= 0x020F,
 	QTN_TLV_ID_COVERAGE_CLASS	= 0x0213,
 	QTN_TLV_ID_IFACE_LIMIT		= 0x0214,
@@ -844,12 +906,54 @@ struct qlink_tlv_cclass {
 	u8 cclass;
 } __packed;
 
-enum qlink_dfs_state {
-	QLINK_DFS_USABLE,
-	QLINK_DFS_UNAVAILABLE,
-	QLINK_DFS_AVAILABLE,
+/**
+ * enum qlink_reg_rule_flags - regulatory rule flags
+ *
+ * See description of &enum nl80211_reg_rule_flags
+ */
+enum qlink_reg_rule_flags {
+	QLINK_RRF_NO_OFDM	= BIT(0),
+	QLINK_RRF_NO_CCK	= BIT(1),
+	QLINK_RRF_NO_INDOOR	= BIT(2),
+	QLINK_RRF_NO_OUTDOOR	= BIT(3),
+	QLINK_RRF_DFS		= BIT(4),
+	QLINK_RRF_PTP_ONLY	= BIT(5),
+	QLINK_RRF_PTMP_ONLY	= BIT(6),
+	QLINK_RRF_NO_IR		= BIT(7),
+	QLINK_RRF_AUTO_BW	= BIT(8),
+	QLINK_RRF_IR_CONCURRENT	= BIT(9),
+	QLINK_RRF_NO_HT40MINUS	= BIT(10),
+	QLINK_RRF_NO_HT40PLUS	= BIT(11),
+	QLINK_RRF_NO_80MHZ	= BIT(12),
+	QLINK_RRF_NO_160MHZ	= BIT(13),
 };
 
+/**
+ * struct qlink_tlv_reg_rule - data for QTN_TLV_ID_REG_RULE TLV
+ *
+ * Regulatory rule description.
+ *
+ * @start_freq_khz: start frequency of the range the rule is attributed to.
+ * @end_freq_khz: end frequency of the range the rule is attributed to.
+ * @max_bandwidth_khz: max bandwidth that channels in specified range can be
+ *	configured to.
+ * @max_antenna_gain: max antenna gain that can be used in the specified
+ *	frequency range, dBi.
+ * @max_eirp: maximum EIRP.
+ * @flags: regulatory rule flags in &enum qlink_reg_rule_flags.
+ * @dfs_cac_ms: DFS CAC period.
+ */
+struct qlink_tlv_reg_rule {
+	struct qlink_tlv_hdr hdr;
+	__le32 start_freq_khz;
+	__le32 end_freq_khz;
+	__le32 max_bandwidth_khz;
+	__le32 max_antenna_gain;
+	__le32 max_eirp;
+	__le32 flags;
+	__le32 dfs_cac_ms;
+} __packed;
+
 enum qlink_channel_flags {
 	QLINK_CHAN_DISABLED		= BIT(0),
 	QLINK_CHAN_NO_IR		= BIT(1),
@@ -865,6 +969,12 @@ enum qlink_channel_flags {
 	QLINK_CHAN_NO_10MHZ		= BIT(12),
 };
 
+enum qlink_dfs_state {
+	QLINK_DFS_USABLE,
+	QLINK_DFS_UNAVAILABLE,
+	QLINK_DFS_AVAILABLE,
+};
+
 struct qlink_tlv_channel {
 	struct qlink_tlv_hdr hdr;
 	__le16 hw_value;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/8] qtnfmac: updates and fixes for regulatory support
  2017-06-20 19:55 ` [PATCH 1/8] qtnfmac: updates and fixes for regulatory support Sergey Matyukevich
@ 2017-06-27 17:20   ` Kalle Valo
  2017-06-29 16:43     ` Sergey Matyukevich
  0 siblings, 1 reply; 31+ messages in thread
From: Kalle Valo @ 2017-06-27 17:20 UTC (permalink / raw)
  To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> This patch introduces several changes for regulatory support
> in qtnfmac driver:
>
> * Introduce support for setting regdomain from host
> New command is implemented to notify firmware about regdomain changes
> performed by host. Firmware is supposed to check requested regdomain
> changes. On success firmware enables updated hardware channel
> configuration and then host driver updates channel info for each band.
>
> * Introduce support of custom regulatory rules
> Obtain custom regulatory rules from the firmware and
> enable them during wiphy registration
>
> * Regulatory for self-managed setup
> Regdomain information needs to be registered with cfg80211
> for devices with REGULATORY_WIPHY_SELF_MANAGED flag set.
>
> * Misc fixes in regulatory code paths
>   - add missing lock in qtnf_cmd_get_mac_chan_info
>   - do not free band channel structure if channel count is the same
>   - free allocated regdomain hw_info.rd structure on detach
>
> Signed-off-by: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

Each patch should contain one logical change, but here you are listing
multiple changes in the commit log which is a clear indication that's
not the case here. Is there a specific reason why you did it like this?

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/8] qtnfmac: updates and fixes for regulatory support
  2017-06-27 17:20   ` Kalle Valo
@ 2017-06-29 16:43     ` Sergey Matyukevich
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-29 16:43 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

> 
> Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:
> 
> > This patch introduces several changes for regulatory support
> > in qtnfmac driver:
> >
> > * Introduce support for setting regdomain from host
> > New command is implemented to notify firmware about regdomain changes
> > performed by host. Firmware is supposed to check requested regdomain
> > changes. On success firmware enables updated hardware channel
> > configuration and then host driver updates channel info for each band.
> >
> > * Introduce support of custom regulatory rules
> > Obtain custom regulatory rules from the firmware and
> > enable them during wiphy registration
> >
> > * Regulatory for self-managed setup
> > Regdomain information needs to be registered with cfg80211
> > for devices with REGULATORY_WIPHY_SELF_MANAGED flag set.
> >
> > * Misc fixes in regulatory code paths
> >   - add missing lock in qtnf_cmd_get_mac_chan_info
> >   - do not free band channel structure if channel count is the same
> >   - free allocated regdomain hw_info.rd structure on detach
> >
> > Signed-off-by: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>
> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> 
> Each patch should contain one logical change, but here you are listing
> multiple changes in the commit log which is a clear indication that's
> not the case here. Is there a specific reason why you did it like this?

There is no specific reason. I grouped changes into patches based on features.
For instance, this particular patch includes all the regulatory changes.
Though I agree that this particular patch can be splitted at least into
3 different self-contained patches. Will do for the v2.

Regards,
Sergey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 2/8] qtnfmac: cleanup and fixes preparing AP_VLAN support
  2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
  2017-06-20 19:55 ` [PATCH 1/8] qtnfmac: updates and fixes for regulatory support Sergey Matyukevich
@ 2017-06-20 19:55 ` Sergey Matyukevich
  2017-06-27 17:21   ` Kalle Valo
  2017-06-20 19:55 ` [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support Sergey Matyukevich
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich

This patch implements various cleanups and fixes aimed to
simplify adding AP_VLAN support to qtnfmac driver:

- Remove unused flag field from qlink_intf_info
- Add interface type to qlink_cmd_change_sta
- Modify qtnf_cmd_send_change_sta: add interface type to command
- Fix handling of iftype mask reported by firmware:
  Firmware sends supported interface type rather than mask. As a result,
  types field of ieee80211_iface_limit structure may end up having
  multiple iftype bits set. This leads to WARN_ON from
  wiphy_verify_combinations.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/commands.c  | 35 +++++++++++++++++-----
 drivers/net/wireless/quantenna/qtnfmac/qlink.h     |  4 +--
 .../net/wireless/quantenna/qtnfmac/qlink_util.c    | 23 +++++++-------
 .../net/wireless/quantenna/qtnfmac/qlink_util.h    |  2 +-
 4 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 2ee007ecb236..5a6caf5b685b 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -995,7 +995,7 @@ static int qtnf_parse_variable_mac_info(struct qtnf_wmac *mac,
 	struct ieee80211_iface_limit *limits = NULL;
 	const struct qlink_iface_limit *limit_record;
 	size_t record_count = 0, rec = 0;
-	u16 tlv_type, tlv_value_len, mask;
+	u16 tlv_type, tlv_value_len;
 	struct qlink_iface_comb_num *comb;
 	size_t tlv_full_len;
 	const struct qlink_tlv_hdr *tlv;
@@ -1048,9 +1048,10 @@ static int qtnf_parse_variable_mac_info(struct qtnf_wmac *mac,
 
 			limit_record = (void *)tlv->val;
 			limits[rec].max = le16_to_cpu(limit_record->max_num);
-			mask = le16_to_cpu(limit_record->type_mask);
-			limits[rec].types = qlink_iface_type_mask_to_nl(mask);
-			/* only AP and STA modes are supported */
+			limits[rec].types = qlink_iface_type_to_nl_mask(
+				le16_to_cpu(limit_record->type));
+
+			/* supported modes: STA, AP */
 			limits[rec].types &= BIT(NL80211_IFTYPE_AP) |
 					     BIT(NL80211_IFTYPE_STATION);
 
@@ -1063,6 +1064,7 @@ static int qtnf_parse_variable_mac_info(struct qtnf_wmac *mac,
 		default:
 			break;
 		}
+
 		tlv_buf_size -= tlv_full_len;
 		tlv = (struct qlink_tlv_hdr *)(tlv->val + tlv_value_len);
 	}
@@ -1808,10 +1810,27 @@ int qtnf_cmd_send_change_sta(struct qtnf_vif *vif, const u8 *mac,
 
 	cmd = (struct qlink_cmd_change_sta *)cmd_skb->data;
 	ether_addr_copy(cmd->sta_addr, mac);
-	cmd->sta_flags_mask = cpu_to_le32(qtnf_encode_sta_flags(
-					  params->sta_flags_mask));
-	cmd->sta_flags_set = cpu_to_le32(qtnf_encode_sta_flags(
-					 params->sta_flags_set));
+
+	switch (vif->wdev.iftype) {
+	case NL80211_IFTYPE_AP:
+		cmd->if_type = cpu_to_le16(QLINK_IFTYPE_AP);
+		cmd->sta_flags_mask = cpu_to_le32(qtnf_encode_sta_flags(
+						  params->sta_flags_mask));
+		cmd->sta_flags_set = cpu_to_le32(qtnf_encode_sta_flags(
+						 params->sta_flags_set));
+		break;
+	case NL80211_IFTYPE_STATION:
+		cmd->if_type = cpu_to_le16(QLINK_IFTYPE_STATION);
+		cmd->sta_flags_mask = cpu_to_le32(qtnf_encode_sta_flags(
+						  params->sta_flags_mask));
+		cmd->sta_flags_set = cpu_to_le32(qtnf_encode_sta_flags(
+						 params->sta_flags_set));
+		break;
+	default:
+		pr_err("unsupported iftype %d\n", vif->wdev.iftype);
+		ret = -EINVAL;
+		goto out;
+	}
 
 	ret = qtnf_cmd_send(vif->mac->bus, cmd_skb, &res_code);
 	if (unlikely(ret))
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index e27833b78940..8bcd8a55ad11 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -90,7 +90,6 @@ enum qlink_iface_type {
  */
 struct qlink_intf_info {
 	__le16 if_type;
-	__le16 flags;
 	u8 mac_addr[ETH_ALEN];
 	u8 rsvd[2];
 } __packed;
@@ -350,6 +349,7 @@ struct qlink_cmd_change_sta {
 	struct qlink_cmd chdr;
 	__le32 sta_flags_mask;
 	__le32 sta_flags_set;
+	__le16 if_type;
 	u8 sta_addr[ETH_ALEN];
 } __packed;
 
@@ -823,7 +823,7 @@ struct qlink_tlv_hdr {
 
 struct qlink_iface_limit {
 	__le16 max_num;
-	__le16 type_mask;
+	__le16 type;
 } __packed;
 
 struct qlink_iface_comb_num {
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink_util.c b/drivers/net/wireless/quantenna/qtnfmac/qlink_util.c
index 49ae652ad9a3..22fa631d692d 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink_util.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink_util.c
@@ -17,24 +17,27 @@
 
 #include "qlink_util.h"
 
-u16 qlink_iface_type_mask_to_nl(u16 qlink_mask)
+u16 qlink_iface_type_to_nl_mask(u16 qlink_type)
 {
 	u16 result = 0;
 
-	if (qlink_mask & QLINK_IFTYPE_AP)
+	switch (qlink_type) {
+	case QLINK_IFTYPE_AP:
 		result |= BIT(NL80211_IFTYPE_AP);
-
-	if (qlink_mask & QLINK_IFTYPE_STATION)
+		break;
+	case QLINK_IFTYPE_STATION:
 		result |= BIT(NL80211_IFTYPE_STATION);
-
-	if (qlink_mask & QLINK_IFTYPE_ADHOC)
+		break;
+	case QLINK_IFTYPE_ADHOC:
 		result |= BIT(NL80211_IFTYPE_ADHOC);
-
-	if (qlink_mask & QLINK_IFTYPE_MONITOR)
+		break;
+	case QLINK_IFTYPE_MONITOR:
 		result |= BIT(NL80211_IFTYPE_MONITOR);
-
-	if (qlink_mask & QLINK_IFTYPE_WDS)
+		break;
+	case QLINK_IFTYPE_WDS:
 		result |= BIT(NL80211_IFTYPE_WDS);
+		break;
+	}
 
 	return result;
 }
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink_util.h b/drivers/net/wireless/quantenna/qtnfmac/qlink_util.h
index d8de484b5995..18d0d679a649 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink_util.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink_util.h
@@ -74,7 +74,7 @@ static inline void qtnf_cmd_skb_put_tlv_u16(struct sk_buff *skb,
 	memcpy(hdr->val, &tmp, sizeof(tmp));
 }
 
-u16 qlink_iface_type_mask_to_nl(u16 qlink_mask);
+u16 qlink_iface_type_to_nl_mask(u16 qlink_type);
 u8 qlink_chan_width_mask_to_nl(u16 qlink_mask);
 
 #endif /* _QTN_FMAC_QLINK_UTIL_H_ */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/8] qtnfmac: cleanup and fixes preparing AP_VLAN support
  2017-06-20 19:55 ` [PATCH 2/8] qtnfmac: cleanup and fixes preparing AP_VLAN support Sergey Matyukevich
@ 2017-06-27 17:21   ` Kalle Valo
  2017-06-29 16:46     ` Sergey Matyukevich
  0 siblings, 1 reply; 31+ messages in thread
From: Kalle Valo @ 2017-06-27 17:21 UTC (permalink / raw)
  To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> This patch implements various cleanups and fixes aimed to
> simplify adding AP_VLAN support to qtnfmac driver:
>
> - Remove unused flag field from qlink_intf_info
> - Add interface type to qlink_cmd_change_sta
> - Modify qtnf_cmd_send_change_sta: add interface type to command
> - Fix handling of iftype mask reported by firmware:
>   Firmware sends supported interface type rather than mask. As a result,
>   types field of ieee80211_iface_limit structure may end up having
>   multiple iftype bits set. This leads to WARN_ON from
>   wiphy_verify_combinations.
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

Again multiple changes in one patch.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/8] qtnfmac: cleanup and fixes preparing AP_VLAN support
  2017-06-27 17:21   ` Kalle Valo
@ 2017-06-29 16:46     ` Sergey Matyukevich
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-29 16:46 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Igor Mitsyanko

On Tue, Jun 27, 2017 at 08:21:55PM +0300, Kalle Valo wrote:
> 
> External Email
> 
> 
> Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:
> 
> > This patch implements various cleanups and fixes aimed to
> > simplify adding AP_VLAN support to qtnfmac driver:
> >
> > - Remove unused flag field from qlink_intf_info
> > - Add interface type to qlink_cmd_change_sta
> > - Modify qtnf_cmd_send_change_sta: add interface type to command
> > - Fix handling of iftype mask reported by firmware:
> >   Firmware sends supported interface type rather than mask. As a result,
> >   types field of ieee80211_iface_limit structure may end up having
> >   multiple iftype bits set. This leads to WARN_ON from
> >   wiphy_verify_combinations.
> >
> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> 
> Again multiple changes in one patch.

Ok, I will split this patch into more reasonable chunks in v2.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support
  2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
  2017-06-20 19:55 ` [PATCH 1/8] qtnfmac: updates and fixes for regulatory support Sergey Matyukevich
  2017-06-20 19:55 ` [PATCH 2/8] qtnfmac: cleanup and fixes preparing AP_VLAN support Sergey Matyukevich
@ 2017-06-20 19:55 ` Sergey Matyukevich
  2017-06-27 17:35   ` Kalle Valo
  2017-09-05 13:45   ` Johannes Berg
  2017-06-20 19:55 ` [PATCH 4/8] qtnfmac: implement cfg80211 dump_survey handler Sergey Matyukevich
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich

This patch implements AP_VLAN interface type support enabling
the use of dynamic VLAN mode in hostapd.

Implementation notes.

1. Passing dynamic VLAN tag to firmware
Currently there is no established way to pass VLAN tag assigned to STA
by Radius server to wireless driver. It is assumed that hostapd is able
to setup all the bridging and tagging on its own. However qtnf firmware
needs to know the assigned dynamic VLAN tags in order to perform various
internal tasks including group key management, filtering, acceleration,
and more.

Current implementation makes use of the following workaround.
Driver obtains dynamic VLAN tags assigned by Radius server
from AP/VLAN interface names:
- for primary interfaces: wlanX.Z
  where X and Z are macid and vlan tag respectively
- for MBSS virtual interfaces: wlanX_Y.Z
  where X, Y, Z are macid, vifid, and vlan tag respectively

Such a naming convention can be configured using
hostapd vlan_file configuration file.

2. Packet routing to/from AP/VLAN interfaces
Firmware operates with tagged packets after dynamic VLAN mode is
configured. In particular, packets destined to STAs should be
properly tagged before they can be passed to firmware. Packets
received from STAs are properly tagged by firmware and then
passed to wireless driver. As a result, packet routing to AP/VLAN
interfaces is straightforward: it is enough to check VLAN tags.

Normally hostapd expects untagged packets from AP/VLAN interfaces.
Meanwhile firmware performs tagging using h/w acceleration. That
is why it makes sense to avoid untagging packets in driver if
they are supposed to by tagged again on host. To enable this
behavior a new module parameter 'dyn_vlan_tagged' has been
introduced:

- dyn_vlan_tagged = 0 (default)
In this case untagged packets are sent to and expected from AP/VLAN interfaces.
Driver tags/untags packets going to/from firmware. This behaviour is expected
by hostapd which is able to create bridges and VLAN interfaces automatically
when hostapd is built with CONFIG_FULL_DYNAMIC_VLAN option enabled.

- dyn_vlan_tagged = 1
In this case tagged packets are sent to and expected from AP/VLAN interfaces.
Hostapd build option CONFIG_FULL_DYNAMIC_VLAN should be disabled. Setup of
networking topology on host is left up to the implementers.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/bus.h       |   1 +
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c  | 247 ++++++++++++++++++---
 drivers/net/wireless/quantenna/qtnfmac/commands.c  |  28 ++-
 drivers/net/wireless/quantenna/qtnfmac/core.c      |  78 ++++++-
 drivers/net/wireless/quantenna/qtnfmac/core.h      |  25 ++-
 .../net/wireless/quantenna/qtnfmac/pearl/pcie.c    |   5 +
 drivers/net/wireless/quantenna/qtnfmac/qlink.h     |  10 +-
 .../net/wireless/quantenna/qtnfmac/qlink_util.c    |   3 +
 drivers/net/wireless/quantenna/qtnfmac/util.c      |  74 ++++--
 drivers/net/wireless/quantenna/qtnfmac/util.h      |  34 ++-
 10 files changed, 428 insertions(+), 77 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/bus.h b/drivers/net/wireless/quantenna/qtnfmac/bus.h
index dda05003d522..819ba3ba0f05 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/bus.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/bus.h
@@ -52,6 +52,7 @@ struct qtnf_bus {
 	enum qtnf_fw_state fw_state;
 	u32 chip;
 	u32 chiprev;
+	u8 dyn_vlan_tagged;
 	const struct qtnf_bus_ops *bus_ops;
 	struct qtnf_wmac *mac[QTNF_MAX_MAC];
 	struct qtnf_qlink_transport trans;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 825a6334fbfe..e222e8d038d3 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -77,6 +77,35 @@ qtnf_mgmt_stypes[NUM_NL80211_IFTYPES] = {
 	},
 };
 
+static int qtnf_vlan_vif_exists(struct qtnf_vif *vif, u16 vlanid)
+{
+	struct qtnf_vif *vlan_vif;
+
+	vlan_vif = qtnf_vlan_list_lookup(&vif->vlan_list, vlanid);
+	if (vlan_vif)
+		return 1;
+
+	return 0;
+}
+
+static struct qtnf_vif *qtnf_add_vlan_vif(struct qtnf_vif *vif, u16 vlanid)
+{
+	struct qtnf_vif *vlan_vif;
+
+	vlan_vif = qtnf_vlan_list_add(&vif->vlan_list, vlanid);
+	if (vlan_vif) {
+		vlan_vif->u.vlan.parent = vif;
+		vlan_vif->u.vlan.vlanid = vlanid;
+	}
+
+	return vlan_vif;
+}
+
+static int qtnf_del_vlan_vif(struct qtnf_vif *vif, u16 vlanid)
+{
+	return qtnf_vlan_list_del(&vif->vlan_list, vlanid);
+}
+
 static int
 qtnf_change_virtual_intf(struct wiphy *wiphy,
 			 struct net_device *dev,
@@ -92,7 +121,15 @@ qtnf_change_virtual_intf(struct wiphy *wiphy,
 	else
 		mac_addr = NULL;
 
-	qtnf_scan_done(vif->mac, true);
+	switch (type) {
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_AP:
+		qtnf_scan_done(vif->mac, true);
+		break;
+	default:
+		pr_err("unsupported virtual interface type (%d)\n", type);
+		return -ENOTSUPP;
+	}
 
 	ret = qtnf_cmd_send_change_intf_type(vif, type, mac_addr);
 	if (ret) {
@@ -108,29 +145,57 @@ qtnf_change_virtual_intf(struct wiphy *wiphy,
 int qtnf_del_virtual_intf(struct wiphy *wiphy, struct wireless_dev *wdev)
 {
 	struct net_device *netdev =  wdev->netdev;
+	struct qtnf_vif *parent;
 	struct qtnf_vif *vif;
+	u16 vlanid;
 
 	if (WARN_ON(!netdev))
 		return -EFAULT;
 
+	netif_tx_stop_all_queues(netdev);
+	if (netif_carrier_ok(netdev))
+		netif_carrier_off(netdev);
+
 	vif = qtnf_netdev_get_priv(wdev->netdev);
 
+	switch (wdev->iftype) {
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_AP:
+		qtnf_virtual_intf_cleanup(vif);
+		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		break;
+	default:
+		pr_err("unsupported virtual interface type (%d)\n",
+		       wdev->iftype);
+		return -ENOTSUPP;
+	}
+
 	if (qtnf_cmd_send_del_intf(vif))
 		pr_err("VIF%u.%u: failed to delete VIF\n", vif->mac->macid,
 		       vif->vifid);
 
-	/* Stop data */
-	netif_tx_stop_all_queues(netdev);
-	if (netif_carrier_ok(netdev))
-		netif_carrier_off(netdev);
-
 	if (netdev->reg_state == NETREG_REGISTERED)
 		unregister_netdevice(netdev);
 
-	vif->netdev->ieee80211_ptr = NULL;
-	vif->netdev = NULL;
-	vif->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
-	eth_zero_addr(vif->mac_addr);
+	switch (wdev->iftype) {
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_AP:
+		vif->netdev->ieee80211_ptr = NULL;
+		vif->netdev = NULL;
+		vif->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
+		eth_zero_addr(vif->mac_addr);
+		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		parent = vif->u.vlan.parent;
+		vlanid = vif->u.vlan.vlanid;
+		if (!qtnf_del_vlan_vif(parent, vlanid))
+			pr_warn("failed to delete AP_VLAN for VLAN tag %u",
+				vlanid);
+		break;
+	default:
+		break;
+	}
 
 	return 0;
 }
@@ -143,7 +208,12 @@ static struct wireless_dev *qtnf_add_virtual_intf(struct wiphy *wiphy,
 {
 	struct qtnf_wmac *mac;
 	struct qtnf_vif *vif;
+	struct qtnf_vif *parent_vif;
 	u8 *mac_addr = NULL;
+	u32 vlanid = 0;
+	u32 macid;
+	u32 vifid;
+	int ret;
 
 	mac = wiphy_priv(wiphy);
 
@@ -164,24 +234,86 @@ static struct wireless_dev *qtnf_add_virtual_intf(struct wiphy *wiphy,
 		vif->wdev.wiphy = wiphy;
 		vif->wdev.iftype = type;
 		vif->sta_state = QTNF_STA_DISCONNECTED;
+
+		if (params)
+			mac_addr = params->macaddr;
+
+		if (qtnf_cmd_send_add_intf(vif, type, mac_addr)) {
+			pr_err("VIF%u.%u: failed to add VIF\n", mac->macid,
+			       vif->vifid);
+			goto err_cmd;
+		}
+
+		if (!is_valid_ether_addr(vif->mac_addr)) {
+			pr_err("VIF%u.%u: FW reported bad MAC: %pM\n",
+			       mac->macid, vif->vifid, vif->mac_addr);
+			goto err_mac;
+		}
+
 		break;
-	default:
-		pr_err("MAC%u: unsupported IF type %d\n", mac->macid, type);
-		return ERR_PTR(-ENOTSUPP);
-	}
+	case NL80211_IFTYPE_AP_VLAN:
+		/* Note: expect two valid inputs:
+		 *   wlan%d.%d -> macid/vlanid
+		 *   wlan%d_%d.%d -> macid/vifid/vlanid
+		 * Note: don't care about macid since wiphy is known
+		 */
+		ret = sscanf(name, "wlan%u_%u.%u", &macid, &vifid, &vlanid);
+		if (ret != 3) {
+			ret = sscanf(name, "wlan%u.%u", &macid, &vlanid);
+			if (ret != 2) {
+				pr_err("unsupported AP_VLAN interface naming");
+				return ERR_PTR(-ENOTSUPP);
+			}
+
+			vifid = 0;
+		}
 
-	if (params)
-		mac_addr = params->macaddr;
+		if (vifid >= QTNF_MAX_INTF) {
+			pr_err("VIF index %d is out of range\n", vifid);
+			return ERR_PTR(-EINVAL);
+		}
 
-	if (qtnf_cmd_send_add_intf(vif, type, mac_addr)) {
-		pr_err("VIF%u.%u: failed to add VIF\n", mac->macid, vif->vifid);
-		goto err_cmd;
-	}
+		macid = mac->macid;
+
+		pr_debug("add AP_VLAN with tag %u to mac/vif %u/%u\n",
+			 vlanid, macid, vifid);
+
+		parent_vif = &mac->iflist[vifid];
+		if (parent_vif->wdev.iftype != NL80211_IFTYPE_AP) {
+			pr_err("only AP supports VLAN virtual iterface\n");
+			return ERR_PTR(-ENOTSUPP);
+		}
 
-	if (!is_valid_ether_addr(vif->mac_addr)) {
-		pr_err("VIF%u.%u: FW reported bad MAC: %pM\n",
-		       mac->macid, vif->vifid, vif->mac_addr);
-		goto err_mac;
+		if (qtnf_vlan_vif_exists(parent_vif, vlanid)) {
+			pr_err("AP_VLAN for tag %u already exists for %s\n",
+			       vlanid, name);
+			return ERR_PTR(-EEXIST);
+		}
+
+		vif = qtnf_add_vlan_vif(parent_vif, vlanid);
+		if (!vif) {
+			pr_err("couldn't create VLAN vif for tag %u\n", vlanid);
+			return ERR_PTR(-EFAULT);
+		}
+
+		vif->mac = mac;
+		vif->vifid = vifid;
+
+		eth_zero_addr(vif->mac_addr);
+		vif->bss_priority = QTNF_DEF_BSS_PRIORITY;
+		vif->wdev.wiphy = wiphy;
+		vif->wdev.iftype = type;
+		vif->sta_state = QTNF_STA_DISCONNECTED;
+
+		if (qtnf_cmd_send_add_intf(vif, type, NULL)) {
+			pr_err("failed to send add_intf command\n");
+			goto err_cmd;
+		}
+
+		break;
+	default:
+		pr_err("MAC%u: unsupported IF type %d\n", mac->macid, type);
+		return ERR_PTR(-ENOTSUPP);
 	}
 
 	if (qtnf_core_net_attach(mac, vif, name, name_assign_t, type)) {
@@ -198,7 +330,17 @@ static struct wireless_dev *qtnf_add_virtual_intf(struct wiphy *wiphy,
 err_mac:
 	qtnf_cmd_send_del_intf(vif);
 err_cmd:
-	vif->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
+	switch (type) {
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_AP:
+		vif->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
+		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		qtnf_del_vlan_vif(vif->u.vlan.parent, vlanid);
+		break;
+	default:
+		break;
+	}
 
 	return ERR_PTR(-EFAULT);
 }
@@ -541,8 +683,28 @@ qtnf_change_station(struct wiphy *wiphy, struct net_device *dev,
 		    const u8 *mac, struct station_parameters *params)
 {
 	struct qtnf_vif *vif = qtnf_netdev_get_priv(dev);
+	struct qtnf_sta_node *sta_node;
 	int ret;
 
+	if (mac && params && params->vlan) {
+		sta_node = qtnf_sta_list_lookup(&vif->sta_list, mac);
+		if (unlikely(!sta_node)) {
+			pr_err("VIF%u.%u: STA %pM does not exist\n",
+			       vif->mac->macid, vif->vifid, mac);
+			return -ENOENT;
+		}
+
+		vif = qtnf_netdev_get_priv(params->vlan);
+		if (vif->wdev.iftype != NL80211_IFTYPE_AP_VLAN) {
+			pr_err("VIF%u.%u: interface %s: unexpected type %d\n",
+			       vif->mac->macid, vif->vifid, params->vlan->name,
+			       vif->wdev.iftype);
+			return -EINVAL;
+		}
+
+		sta_node->ndev = params->vlan;
+	}
+
 	ret = qtnf_cmd_send_change_sta(vif, mac, params);
 	if (ret)
 		pr_err("VIF%u.%u: failed to change STA %pM\n",
@@ -898,14 +1060,26 @@ void qtnf_netdev_updown(struct net_device *ndev, bool up)
 {
 	struct qtnf_vif *vif = qtnf_netdev_get_priv(ndev);
 
-	if (qtnf_cmd_send_updown_intf(vif, up))
-		pr_err("failed to send up/down command to FW\n");
+	switch (vif->wdev.iftype) {
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_AP:
+		if (qtnf_cmd_send_updown_intf(vif, up))
+			pr_err("failed to send up/down command to FW\n");
+		break;
+	default:
+		pr_err("unsupported virtual interface type (%d)\n",
+		       vif->wdev.iftype);
+		break;
+	}
 }
 
-void qtnf_virtual_intf_cleanup(struct net_device *ndev)
+void qtnf_virtual_intf_cleanup(struct qtnf_vif *vif)
 {
-	struct qtnf_vif *vif = qtnf_netdev_get_priv(ndev);
 	struct qtnf_wmac *mac = wiphy_priv(vif->wdev.wiphy);
+	struct qtnf_sta_node *sta;
+	struct qtnf_vif *vlan;
+	struct qtnf_vif *tmp;
+	struct qtnf_list *list;
 
 	if (vif->wdev.iftype == NL80211_IFTYPE_STATION) {
 		switch (vif->sta_state) {
@@ -917,20 +1091,31 @@ void qtnf_virtual_intf_cleanup(struct net_device *ndev)
 						NULL, 0,
 						WLAN_STATUS_UNSPECIFIED_FAILURE,
 						GFP_KERNEL);
-			qtnf_disconnect(vif->wdev.wiphy, ndev,
+			qtnf_disconnect(vif->wdev.wiphy, vif->netdev,
 					WLAN_REASON_DEAUTH_LEAVING);
 			break;
 		case QTNF_STA_CONNECTED:
 			cfg80211_disconnected(vif->netdev,
 					      WLAN_REASON_DEAUTH_LEAVING,
 					      NULL, 0, 1, GFP_KERNEL);
-			qtnf_disconnect(vif->wdev.wiphy, ndev,
+			qtnf_disconnect(vif->wdev.wiphy, vif->netdev,
 					WLAN_REASON_DEAUTH_LEAVING);
 			break;
 		}
 
 		vif->sta_state = QTNF_STA_DISCONNECTED;
 		qtnf_scan_done(mac, true);
+	} else if (vif->wdev.iftype == NL80211_IFTYPE_AP) {
+		if (qtnf_list_empty(&vif->vlan_list))
+			return;
+
+		list = &vif->vlan_list;
+		list_for_each_entry_safe(vlan, tmp, &list->head, u.vlan.list)
+			qtnf_del_virtual_intf(vlan->wdev.wiphy, &vlan->wdev);
+
+		list = &vif->sta_list;
+		list_for_each_entry(sta, &list->head, list)
+			sta->ndev = NULL;
 	}
 }
 
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 5a6caf5b685b..221804f8c43a 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -719,6 +719,10 @@ static int qtnf_cmd_send_add_change_intf(struct qtnf_vif *vif,
 	case NL80211_IFTYPE_STATION:
 		cmd->intf_info.if_type = cpu_to_le16(QLINK_IFTYPE_STATION);
 		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		cmd->intf_info.if_type = cpu_to_le16(QLINK_IFTYPE_AP_VLAN);
+		cmd->intf_info.vlanid = cpu_to_le16(vif->u.vlan.vlanid);
+		break;
 	default:
 		pr_err("VIF%u.%u: unsupported type %d\n", vif->mac->macid,
 		       vif->vifid, iftype);
@@ -745,7 +749,15 @@ static int qtnf_cmd_send_add_change_intf(struct qtnf_vif *vif,
 	}
 
 	resp = (const struct qlink_resp_manage_intf *)resp_skb->data;
-	ether_addr_copy(vif->mac_addr, resp->intf_info.mac_addr);
+
+	switch (iftype) {
+	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_STATION:
+		ether_addr_copy(vif->mac_addr, resp->intf_info.mac_addr);
+		break;
+	default:
+		break;
+	}
 
 out:
 	qtnf_bus_unlock(vif->mac->bus);
@@ -792,6 +804,10 @@ int qtnf_cmd_send_del_intf(struct qtnf_vif *vif)
 	case NL80211_IFTYPE_STATION:
 		cmd->intf_info.if_type = cpu_to_le16(QLINK_IFTYPE_STATION);
 		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		cmd->intf_info.if_type = cpu_to_le16(QLINK_IFTYPE_AP_VLAN);
+		cmd->intf_info.vlanid = cpu_to_le16(vif->u.vlan.vlanid);
+		break;
 	default:
 		pr_warn("VIF%u.%u: unsupported iftype %d\n", vif->mac->macid,
 			vif->vifid, vif->wdev.iftype);
@@ -1051,8 +1067,9 @@ static int qtnf_parse_variable_mac_info(struct qtnf_wmac *mac,
 			limits[rec].types = qlink_iface_type_to_nl_mask(
 				le16_to_cpu(limit_record->type));
 
-			/* supported modes: STA, AP */
+			/* supported modes: STA, AP, AP_VLAN */
 			limits[rec].types &= BIT(NL80211_IFTYPE_AP) |
+					     BIT(NL80211_IFTYPE_AP_VLAN) |
 					     BIT(NL80211_IFTYPE_STATION);
 
 			pr_debug("MAC%u: MAX: %u; TYPES: %.4X\n", mac->macid,
@@ -1645,6 +1662,9 @@ int qtnf_cmd_send_add_key(struct qtnf_vif *vif, u8 key_index, bool pairwise,
 					 params->seq,
 					 params->seq_len);
 
+	if (vif->wdev.iftype == NL80211_IFTYPE_AP_VLAN)
+		cmd->vlanid = cpu_to_le16(vif->u.vlan.vlanid);
+
 	ret = qtnf_cmd_send(vif->mac->bus, cmd_skb, &res_code);
 	if (unlikely(ret))
 		goto out;
@@ -1826,6 +1846,10 @@ int qtnf_cmd_send_change_sta(struct qtnf_vif *vif, const u8 *mac,
 		cmd->sta_flags_set = cpu_to_le32(qtnf_encode_sta_flags(
 						 params->sta_flags_set));
 		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		cmd->if_type = cpu_to_le16(QLINK_IFTYPE_AP_VLAN);
+		cmd->vlanid = cpu_to_le16(vif->u.vlan.vlanid);
+		break;
 	default:
 		pr_err("unsupported iftype %d\n", vif->wdev.iftype);
 		ret = -EINVAL;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c
index 21cbe6dcf6fa..3d9b217790ed 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -16,6 +16,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/if_vlan.h>
 #include <linux/if_ether.h>
 
 #include "core.h"
@@ -59,6 +60,18 @@ struct qtnf_wmac *qtnf_core_get_mac(const struct qtnf_bus *bus, u8 macid)
  */
 static int qtnf_netdev_open(struct net_device *ndev)
 {
+	struct qtnf_vif *vif = qtnf_netdev_get_priv(ndev);
+
+	if (unlikely(!vif || !vif->mac || !vif->mac->bus)) {
+		pr_warn("invalid network device\n");
+		return -ENODEV;
+	}
+
+	if (vif->wdev.iftype == NL80211_IFTYPE_AP_VLAN) {
+		netif_carrier_on(ndev);
+		return 0;
+	}
+
 	netif_carrier_off(ndev);
 	qtnf_netdev_updown(ndev, 1);
 	return 0;
@@ -68,8 +81,14 @@ static int qtnf_netdev_open(struct net_device *ndev)
  */
 static int qtnf_netdev_close(struct net_device *ndev)
 {
+	struct qtnf_vif *vif = qtnf_netdev_get_priv(ndev);
+
 	netif_carrier_off(ndev);
-	qtnf_virtual_intf_cleanup(ndev);
+
+	if (vif->wdev.iftype == NL80211_IFTYPE_AP_VLAN)
+		return 0;
+
+	qtnf_virtual_intf_cleanup(vif);
 	qtnf_netdev_updown(ndev, 0);
 	return 0;
 }
@@ -111,6 +130,19 @@ qtnf_netdev_hard_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		return 0;
 	}
 
+	if (vif->wdev.iftype == NL80211_IFTYPE_AP_VLAN) {
+		if (!mac->bus->dyn_vlan_tagged) {
+			skb = vlan_insert_tag(skb, htons(ETH_P_8021Q),
+					      vif->u.vlan.vlanid);
+			if (unlikely(!skb)) {
+				pr_err_ratelimited("failed to insert VLAN %d\n",
+						   vif->u.vlan.vlanid);
+				ndev->stats.tx_dropped++;
+				return 0;
+			}
+		}
+	}
+
 	/* tx path is enabled: reset vif timeout */
 	vif->cons_tx_timeout_cnt = 0;
 
@@ -150,13 +182,29 @@ static void qtnf_netdev_tx_timeout(struct net_device *ndev)
 	}
 }
 
+static int qtnf_netdev_set_mac_address(struct net_device *ndev, void *addr)
+{
+	struct qtnf_vif *vif = qtnf_netdev_get_priv(ndev);
+	struct sockaddr *sa = addr;
+	int ret;
+
+	if (vif->wdev.iftype == NL80211_IFTYPE_AP_VLAN) {
+		ret = eth_mac_addr(ndev, sa);
+		if (ret == 0)
+			ether_addr_copy(vif->mac_addr, sa->sa_data);
+	}
+
+	return 0;
+}
+
 /* Network device ops handlers */
-const struct net_device_ops qtnf_netdev_ops = {
+const struct net_device_ops qtnf_bss_netdev_ops = {
 	.ndo_open = qtnf_netdev_open,
 	.ndo_stop = qtnf_netdev_close,
 	.ndo_start_xmit = qtnf_netdev_hard_start_xmit,
 	.ndo_tx_timeout = qtnf_netdev_tx_timeout,
 	.ndo_get_stats = qtnf_netdev_get_stats,
+	.ndo_set_mac_address = qtnf_netdev_set_mac_address,
 };
 
 static int qtnf_mac_init_single_band(struct wiphy *wiphy,
@@ -180,7 +228,6 @@ static int qtnf_mac_init_single_band(struct wiphy *wiphy,
 
 	qtnf_band_init_rates(wiphy->bands[band]);
 	qtnf_band_setup_htvht_caps(&mac->macinfo, wiphy->bands[band]);
-
 	return 0;
 }
 
@@ -287,7 +334,8 @@ static struct qtnf_wmac *qtnf_core_mac_alloc(struct qtnf_bus *bus,
 		mac->iflist[i].wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
 		mac->iflist[i].mac = mac;
 		mac->iflist[i].vifid = i;
-		qtnf_sta_list_init(&mac->iflist[i].sta_list);
+		qtnf_list_init(&mac->iflist[i].sta_list);
+		qtnf_list_init(&mac->iflist[i].vlan_list);
 	}
 
 	qtnf_mac_init_primary_intf(mac);
@@ -314,8 +362,8 @@ int qtnf_core_net_attach(struct qtnf_wmac *mac, struct qtnf_vif *vif,
 	}
 
 	vif->netdev = dev;
+	dev->netdev_ops = &qtnf_bss_netdev_ops;
 
-	dev->netdev_ops = &qtnf_netdev_ops;
 	dev->destructor = free_netdev;
 	dev_net_set(dev, wiphy_net(wiphy));
 	dev->ieee80211_ptr = &vif->wdev;
@@ -360,7 +408,7 @@ static void qtnf_core_mac_detach(struct qtnf_bus *bus, unsigned int macid)
 		rtnl_lock();
 		if (vif->netdev &&
 		    vif->wdev.iftype != NL80211_IFTYPE_UNSPECIFIED) {
-			qtnf_virtual_intf_cleanup(vif->netdev);
+			qtnf_virtual_intf_cleanup(vif);
 			qtnf_del_virtual_intf(wiphy, &vif->wdev);
 		}
 		rtnl_unlock();
@@ -567,6 +615,8 @@ struct net_device *qtnf_classify_skb(struct qtnf_bus *bus, struct sk_buff *skb)
 	struct net_device *ndev = NULL;
 	struct qtnf_wmac *mac;
 	struct qtnf_vif *vif;
+	struct qtnf_vif *vlan_vif;
+	u16 vlanid;
 
 	meta = (struct qtnf_frame_meta_info *)
 		(skb_tail_pointer(skb) - sizeof(*meta));
@@ -603,6 +653,22 @@ struct net_device *qtnf_classify_skb(struct qtnf_bus *bus, struct sk_buff *skb)
 
 	ndev = vif->netdev;
 
+	if (!qtnf_list_empty(&vif->vlan_list)) {
+		if (__vlan_get_tag(skb, &vlanid) == 0) {
+			vlan_vif = qtnf_vlan_list_lookup(
+				&vif->vlan_list, vlanid & VLAN_VID_MASK);
+			if (vlan_vif) {
+				ndev = vlan_vif->netdev;
+				if (!mac->bus->dyn_vlan_tagged) {
+					/* remove tag */
+					memmove(skb->data + VLAN_HLEN,
+						skb->data, ETH_ALEN * 2);
+					skb_pull(skb, VLAN_HLEN);
+				}
+			}
+		}
+	}
+
 	if (unlikely(!ndev)) {
 		pr_err_ratelimited("netdev for wlan%u.%u does not exists\n",
 				   meta->macid, meta->ifidx);
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index 31b7ec2bfd3e..0d06ec932caf 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -54,7 +54,8 @@
 #define QTNF_STATE_AP_CONFIG		BIT(2)
 #define QTNF_STATE_AP_START		BIT(1)
 
-extern const struct net_device_ops qtnf_netdev_ops;
+extern const struct net_device_ops qtnf_bss_netdev_ops;
+
 struct qtnf_bus;
 struct qtnf_vif;
 
@@ -76,9 +77,10 @@ struct qtnf_bss_config {
 struct qtnf_sta_node {
 	struct list_head list;
 	u8 mac_addr[ETH_ALEN];
+	struct net_device *ndev;
 };
 
-struct qtnf_sta_list {
+struct qtnf_list {
 	struct list_head head;
 	atomic_t size;
 };
@@ -89,6 +91,12 @@ enum qtnf_sta_state {
 	QTNF_STA_CONNECTED
 };
 
+struct qtnf_vif_vlan {
+	struct qtnf_vif *parent;
+	struct list_head list;
+	u16 vlanid;
+};
+
 struct qtnf_vif {
 	struct wireless_dev wdev;
 	u8 vifid;
@@ -101,8 +109,14 @@ struct qtnf_vif {
 	u8 mac_addr[ETH_ALEN];
 	struct work_struct reset_work;
 	struct qtnf_bss_config bss_cfg;
-	struct qtnf_sta_list sta_list;
 	unsigned long cons_tx_timeout_cnt;
+
+	struct qtnf_list sta_list;
+	struct qtnf_list vlan_list;
+
+	union {
+		struct qtnf_vif_vlan vlan;
+	} u;
 };
 
 struct qtnf_mac_info {
@@ -151,16 +165,15 @@ struct wiphy *qtnf_wiphy_allocate(struct qtnf_bus *bus);
 int qtnf_core_net_attach(struct qtnf_wmac *mac, struct qtnf_vif *priv,
 			 const char *name, unsigned char name_assign_type,
 			 enum nl80211_iftype iftype);
+
 void qtnf_main_work_queue(struct work_struct *work);
 int qtnf_cmd_send_update_phy_params(struct qtnf_wmac *mac, u32 changed);
 int qtnf_cmd_send_get_phy_params(struct qtnf_wmac *mac);
 
 struct qtnf_wmac *qtnf_core_get_mac(const struct qtnf_bus *bus, u8 macid);
 struct net_device *qtnf_classify_skb(struct qtnf_bus *bus, struct sk_buff *skb);
-struct net_device *qtnf_classify_skb_no_mbss(struct qtnf_bus *bus,
-					     struct sk_buff *skb);
 
-void qtnf_virtual_intf_cleanup(struct net_device *ndev);
+void qtnf_virtual_intf_cleanup(struct qtnf_vif *vif);
 
 void qtnf_netdev_updown(struct net_device *ndev, bool up);
 
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index 4814d90c8040..82bde70b1f96 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -52,6 +52,10 @@ static u8 flashboot = 1;
 module_param(flashboot, byte, 0644);
 MODULE_PARM_DESC(flashboot, "set to 0 to use FW binary file on FS");
 
+static u8 dyn_vlan_tagged;
+module_param(dyn_vlan_tagged, byte, 0444);
+MODULE_PARM_DESC(dyn_vlan_tagged, "enable tagged traffic on AP_VLAN ports");
+
 #define DRV_NAME	"qtnfmac_pearl_pcie"
 
 static inline void qtnf_non_posted_write(u32 val, void __iomem *basereg)
@@ -1151,6 +1155,7 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	bus->bus_ops = &qtnf_pcie_bus_ops;
 	bus->dev = &pdev->dev;
 	bus->fw_state = QTNF_FW_STATE_RESET;
+	bus->dyn_vlan_tagged = dyn_vlan_tagged;
 	pcie_priv->pdev = pdev;
 
 	strcpy(bus->fwname, QTN_PCI_PEARL_FW_NAME);
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 8bcd8a55ad11..892752599109 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -19,7 +19,7 @@
 
 #include <linux/ieee80211.h>
 
-#define QLINK_PROTO_VER		4
+#define QLINK_PROTO_VER		5
 
 #define QLINK_MACID_RSVD		0xFF
 #define QLINK_VIFID_RSVD		0xFF
@@ -77,6 +77,7 @@ enum qlink_iface_type {
 	QLINK_IFTYPE_ADHOC	= 3,
 	QLINK_IFTYPE_MONITOR	= 4,
 	QLINK_IFTYPE_WDS	= 5,
+	QLINK_IFTYPE_AP_VLAN	= 6,
 };
 
 /**
@@ -85,11 +86,12 @@ enum qlink_iface_type {
  * Data describing a single virtual interface.
  *
  * @if_type: Mode of interface operation, one of &enum qlink_iface_type
- * @flags: interface flagsmap.
+ * @vlanid: VLAN ID for AP_VLAN interface type
  * @mac_addr: MAC address of virtual interface.
  */
 struct qlink_intf_info {
 	__le16 if_type;
+	__le16 vlanid;
 	u8 mac_addr[ETH_ALEN];
 	u8 rsvd[2];
 } __packed;
@@ -297,6 +299,7 @@ struct qlink_cmd_add_key {
 	u8 pairwise;
 	u8 addr[ETH_ALEN];
 	__le32 cipher;
+	__le16 vlanid;
 	u8 key_data[0];
 } __packed;
 
@@ -343,6 +346,8 @@ struct qlink_cmd_set_def_mgmt_key {
  *
  * @sta_flags_mask: STA flags mask, bitmap of &enum qlink_sta_flags
  * @sta_flags_set: STA flags values, bitmap of &enum qlink_sta_flags
+ * @if_type: Mode of interface operation, one of &enum qlink_iface_type
+ * @vlanid: VLAN ID to assign to specific STA
  * @sta_addr: address of the STA for which parameters are set.
  */
 struct qlink_cmd_change_sta {
@@ -350,6 +355,7 @@ struct qlink_cmd_change_sta {
 	__le32 sta_flags_mask;
 	__le32 sta_flags_set;
 	__le16 if_type;
+	__le16 vlanid;
 	u8 sta_addr[ETH_ALEN];
 } __packed;
 
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink_util.c b/drivers/net/wireless/quantenna/qtnfmac/qlink_util.c
index 22fa631d692d..cf024c995fd6 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink_util.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink_util.c
@@ -37,6 +37,9 @@ u16 qlink_iface_type_to_nl_mask(u16 qlink_type)
 	case QLINK_IFTYPE_WDS:
 		result |= BIT(NL80211_IFTYPE_WDS);
 		break;
+	case QLINK_IFTYPE_AP_VLAN:
+		result |= BIT(NL80211_IFTYPE_AP_VLAN);
+		break;
 	}
 
 	return result;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/util.c b/drivers/net/wireless/quantenna/qtnfmac/util.c
index ed38e87471bf..8de6f2fc0600 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/util.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/util.c
@@ -16,16 +16,7 @@
 
 #include "util.h"
 
-void qtnf_sta_list_init(struct qtnf_sta_list *list)
-{
-	if (unlikely(!list))
-		return;
-
-	INIT_LIST_HEAD(&list->head);
-	atomic_set(&list->size, 0);
-}
-
-struct qtnf_sta_node *qtnf_sta_list_lookup(struct qtnf_sta_list *list,
+struct qtnf_sta_node *qtnf_sta_list_lookup(struct qtnf_list *list,
 					   const u8 *mac)
 {
 	struct qtnf_sta_node *node;
@@ -41,12 +32,12 @@ struct qtnf_sta_node *qtnf_sta_list_lookup(struct qtnf_sta_list *list,
 	return NULL;
 }
 
-struct qtnf_sta_node *qtnf_sta_list_lookup_index(struct qtnf_sta_list *list,
+struct qtnf_sta_node *qtnf_sta_list_lookup_index(struct qtnf_list *list,
 						 size_t index)
 {
 	struct qtnf_sta_node *node;
 
-	if (qtnf_sta_list_size(list) <= index)
+	if (qtnf_list_size(list) <= index)
 		return NULL;
 
 	list_for_each_entry(node, &list->head, list) {
@@ -57,8 +48,7 @@ struct qtnf_sta_node *qtnf_sta_list_lookup_index(struct qtnf_sta_list *list,
 	return NULL;
 }
 
-struct qtnf_sta_node *qtnf_sta_list_add(struct qtnf_sta_list *list,
-					const u8 *mac)
+struct qtnf_sta_node *qtnf_sta_list_add(struct qtnf_list *list, const u8 *mac)
 {
 	struct qtnf_sta_node *node;
 
@@ -82,10 +72,10 @@ struct qtnf_sta_node *qtnf_sta_list_add(struct qtnf_sta_list *list,
 	return node;
 }
 
-bool qtnf_sta_list_del(struct qtnf_sta_list *list, const u8 *mac)
+int qtnf_sta_list_del(struct qtnf_list *list, const u8 *mac)
 {
 	struct qtnf_sta_node *node;
-	bool ret = false;
+	int ret = 0;
 
 	node = qtnf_sta_list_lookup(list, mac);
 
@@ -93,13 +83,13 @@ bool qtnf_sta_list_del(struct qtnf_sta_list *list, const u8 *mac)
 		list_del(&node->list);
 		atomic_dec(&list->size);
 		kfree(node);
-		ret = true;
+		ret = 1;
 	}
 
 	return ret;
 }
 
-void qtnf_sta_list_free(struct qtnf_sta_list *list)
+void qtnf_sta_list_free(struct qtnf_list *list)
 {
 	struct qtnf_sta_node *node, *tmp;
 
@@ -112,3 +102,51 @@ void qtnf_sta_list_free(struct qtnf_sta_list *list)
 
 	INIT_LIST_HEAD(&list->head);
 }
+
+struct qtnf_vif *qtnf_vlan_list_lookup(struct qtnf_list *list, const u16 vlanid)
+{
+	struct qtnf_vif *node;
+
+	list_for_each_entry(node, &list->head, u.vlan.list) {
+		if (node->u.vlan.vlanid ==  vlanid)
+			return node;
+	}
+
+	return NULL;
+}
+
+struct qtnf_vif *qtnf_vlan_list_add(struct qtnf_list *list, const u16 vlanid)
+{
+	struct qtnf_vif *node;
+
+	/* don't use existing vlan vif */
+	node = qtnf_vlan_list_lookup(list, vlanid);
+	if (node)
+		return NULL;
+
+	node = kzalloc(sizeof(*node), GFP_KERNEL);
+	if (unlikely(!node))
+		return NULL;
+
+	list_add_tail(&node->u.vlan.list, &list->head);
+	atomic_inc(&list->size);
+
+	return node;
+}
+
+int qtnf_vlan_list_del(struct qtnf_list *list, const u16 vlanid)
+{
+	struct qtnf_vif *node;
+	int ret = 0;
+
+	node = qtnf_vlan_list_lookup(list, vlanid);
+
+	if (node) {
+		list_del(&node->u.vlan.list);
+		atomic_dec(&list->size);
+		kfree(node);
+		ret = 1;
+	}
+
+	return ret;
+}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/util.h b/drivers/net/wireless/quantenna/qtnfmac/util.h
index 0359eae8c24b..85144b07a2ce 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/util.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/util.h
@@ -20,26 +20,36 @@
 #include <linux/kernel.h>
 #include "core.h"
 
-void qtnf_sta_list_init(struct qtnf_sta_list *list);
-
-struct qtnf_sta_node *qtnf_sta_list_lookup(struct qtnf_sta_list *list,
-					   const u8 *mac);
-struct qtnf_sta_node *qtnf_sta_list_lookup_index(struct qtnf_sta_list *list,
-						 size_t index);
-struct qtnf_sta_node *qtnf_sta_list_add(struct qtnf_sta_list *list,
-					const u8 *mac);
-bool qtnf_sta_list_del(struct qtnf_sta_list *list, const u8 *mac);
+static inline void qtnf_list_init(struct qtnf_list *list)
+{
+	if (unlikely(!list))
+		return;
 
-void qtnf_sta_list_free(struct qtnf_sta_list *list);
+	INIT_LIST_HEAD(&list->head);
+	atomic_set(&list->size, 0);
+}
 
-static inline size_t qtnf_sta_list_size(const struct qtnf_sta_list *list)
+static inline size_t qtnf_list_size(const struct qtnf_list *list)
 {
 	return atomic_read(&list->size);
 }
 
-static inline bool qtnf_sta_list_empty(const struct qtnf_sta_list *list)
+static inline int qtnf_list_empty(const struct qtnf_list *list)
 {
 	return list_empty(&list->head);
 }
 
+struct qtnf_sta_node *qtnf_sta_list_lookup(struct qtnf_list *list,
+					   const u8 *mac);
+struct qtnf_sta_node *qtnf_sta_list_lookup_index(struct qtnf_list *list,
+						 size_t index);
+struct qtnf_sta_node *qtnf_sta_list_add(struct qtnf_list *list, const u8 *mac);
+int qtnf_sta_list_del(struct qtnf_list *list, const u8 *mac);
+void qtnf_sta_list_free(struct qtnf_list *list);
+
+struct qtnf_vif *qtnf_vlan_list_lookup(struct qtnf_list *list,
+				       const u16 vlanid);
+struct qtnf_vif *qtnf_vlan_list_add(struct qtnf_list *list, const u16 vlanid);
+int qtnf_vlan_list_del(struct qtnf_list *list, const u16 vlanid);
+
 #endif /* QTNFMAC_UTIL_H */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support
  2017-06-20 19:55 ` [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support Sergey Matyukevich
@ 2017-06-27 17:35   ` Kalle Valo
  2017-07-01  8:45     ` Sergey Matyukevich
  2017-09-05 13:45   ` Johannes Berg
  1 sibling, 1 reply; 31+ messages in thread
From: Kalle Valo @ 2017-06-27 17:35 UTC (permalink / raw)
  To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> This patch implements AP_VLAN interface type support enabling
> the use of dynamic VLAN mode in hostapd.
>
> Implementation notes.
>
> 1. Passing dynamic VLAN tag to firmware
> Currently there is no established way to pass VLAN tag assigned to STA
> by Radius server to wireless driver. It is assumed that hostapd is able
> to setup all the bridging and tagging on its own. However qtnf firmware
> needs to know the assigned dynamic VLAN tags in order to perform various
> internal tasks including group key management, filtering, acceleration,
> and more.
>
> Current implementation makes use of the following workaround.
> Driver obtains dynamic VLAN tags assigned by Radius server
> from AP/VLAN interface names:
> - for primary interfaces: wlanX.Z
>   where X and Z are macid and vlan tag respectively
> - for MBSS virtual interfaces: wlanX_Y.Z
>   where X, Y, Z are macid, vifid, and vlan tag respectively
>
> Such a naming convention can be configured using
> hostapd vlan_file configuration file.
>
> 2. Packet routing to/from AP/VLAN interfaces
> Firmware operates with tagged packets after dynamic VLAN mode is
> configured. In particular, packets destined to STAs should be
> properly tagged before they can be passed to firmware. Packets
> received from STAs are properly tagged by firmware and then
> passed to wireless driver. As a result, packet routing to AP/VLAN
> interfaces is straightforward: it is enough to check VLAN tags.
>
> Normally hostapd expects untagged packets from AP/VLAN interfaces.
> Meanwhile firmware performs tagging using h/w acceleration. That
> is why it makes sense to avoid untagging packets in driver if
> they are supposed to by tagged again on host. To enable this
> behavior a new module parameter 'dyn_vlan_tagged' has been
> introduced:
>
> - dyn_vlan_tagged = 0 (default)
> In this case untagged packets are sent to and expected from AP/VLAN interfaces.
> Driver tags/untags packets going to/from firmware. This behaviour is expected
> by hostapd which is able to create bridges and VLAN interfaces automatically
> when hostapd is built with CONFIG_FULL_DYNAMIC_VLAN option enabled.
>
> - dyn_vlan_tagged = 1
> In this case tagged packets are sent to and expected from AP/VLAN interfaces.
> Hostapd build option CONFIG_FULL_DYNAMIC_VLAN should be disabled. Setup of
> networking topology on host is left up to the implementers.
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

This looks somewhat controversial to me but I'm not very familiar with
VLANs. Do other wireless drivers do something similar or how is this
usually implemented? Or not at all?

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support
  2017-06-27 17:35   ` Kalle Valo
@ 2017-07-01  8:45     ` Sergey Matyukevich
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-07-01  8:45 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Igor Mitsyanko

> Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:
> 
> > This patch implements AP_VLAN interface type support enabling
> > the use of dynamic VLAN mode in hostapd.
> >
> > Implementation notes.

[...]

> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> 
> This looks somewhat controversial to me but I'm not very familiar with
> VLANs. Do other wireless drivers do something similar or how is this
> usually implemented? Or not at all?
> 
> --
> Kalle Valo

Well, IIUC this feature is not supported by other full-mac drivers at the
moment, so no good example to follow. I don't think it is too controversial.
Probably the restrictions on hostapd AP_VLAN interface names can be
frowned upon. But at the moment this information is not passed by hostapd
to kernel. It needs some additional work both in hostapd and cfg80211
to get this info directly in the driver.

Anyways, other patches in this series are more straighforward. So I will split
submission in v2: AP_VLAN support will be posted separately as RFC in order to
some feedback regarding the overall approach.

Regards,
Sergey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support
  2017-06-20 19:55 ` [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support Sergey Matyukevich
  2017-06-27 17:35   ` Kalle Valo
@ 2017-09-05 13:45   ` Johannes Berg
  2017-09-05 14:20       ` Johannes Berg
  2017-09-06 16:22     ` [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support Sergey Matyukevich
  1 sibling, 2 replies; 31+ messages in thread
From: Johannes Berg @ 2017-09-05 13:45 UTC (permalink / raw)
  To: Sergey Matyukevich, linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil

Hi Sergey, all,

On Tue, 2017-06-20 at 22:55 +0300, Sergey Matyukevich wrote:
> This patch implements AP_VLAN interface type support enabling
> the use of dynamic VLAN mode in hostapd.

My first thought here is that this is completely wrong.

AP_VLAN interfaces have no relation to 802.1Q, but it seems that you're
trying to use them as such.

I'll comment a bit below, but I'd like to ask you to actually explain
what you're trying to achieve, because then I think I can comment
better on what you need to be doing.

> Implementation notes.
> 
> 1. Passing dynamic VLAN tag to firmware

There's no such thing as a "VLAN tag" with AP_VLAN interfaces. You want
802.1Q, which can be done on top of them if needed, but it's an
unrelated concept.

> Currently there is no established way to pass VLAN tag assigned to
> STA by Radius server to wireless driver. It is assumed that hostapd
> is able to setup all the bridging and tagging on its own. 

I don't even know if this is true? Does hostapd in fact set up 802.1Q
tagging in any way? Can you say how to configure this?

This might actually be your problem - what you *want* is doing 802.1Q
tagging which hostapd doesn't support, and so you're actually doing
client isolation which hostapd *does* support, instead?

As far as I can tell hostapd only has the ability to put different
types of clients into different AP_VLANs to achieve *over the air*
isolation. It has no isolation over the backend (yet) - that's
typically set up by putting the AP_VLAN interfaces into different
bridges or whatever.

> However qtnf firmware needs to know the assigned dynamic VLAN tags in
> order to perform various internal tasks including group key
> management, filtering, acceleration, and more.

This doesn't make any sense - what does it need the *tag* for? I can
understand it needing *a* tag, but not *the* tag that the RADIUS server
gave it?

IOW - I can understand it needing an identifier of the VLAN or
something like that, but then why can't you just number the AP_VLAN
interfaces 1,2,3,4,...?

> Current implementation makes use of the following workaround.
[snip]

This really shouldn't be done.

> 2. Packet routing to/from AP/VLAN interfaces
> Firmware operates with tagged packets after dynamic VLAN mode is
> configured. In particular, packets destined to STAs should be
> properly tagged before they can be passed to firmware. Packets
> received from STAs are properly tagged by firmware and then
> passed to wireless driver. As a result, packet routing to AP/VLAN
> interfaces is straightforward: it is enough to check VLAN tags.

Ok, so here the whole tagging comes - a bit - into play. Technically
you could, however, do the following:

 * assign "fake" tags as I suggested above
 * pack/unpack the 802.1Q header from the firmware (or put it into
   metadata) in the driver and just tx/rx untagged packets into the
   right interface
 * if the AP_VLAN has a real 802.1Q on top, then it's re-packed again
   by the ethernet driver when the data goes out

> Normally hostapd expects untagged packets from AP/VLAN interfaces.

hostapd doesn't really expect anything there, does it?

> Meanwhile firmware performs tagging using h/w acceleration. That
> is why it makes sense to avoid untagging packets in driver if
> they are supposed to by tagged again on host. To enable this
> behavior a new module parameter 'dyn_vlan_tagged' has been
> introduced:
> 
> - dyn_vlan_tagged = 0 (default)
> In this case untagged packets are sent to and expected from AP/VLAN
> interfaces.
> Driver tags/untags packets going to/from firmware. This behaviour is
> expected
> by hostapd which is able to create bridges and VLAN interfaces
> automatically
> when hostapd is built with CONFIG_FULL_DYNAMIC_VLAN option enabled.
> 
> - dyn_vlan_tagged = 1
> In this case tagged packets are sent to and expected from AP/VLAN
> interfaces.
> Hostapd build option CONFIG_FULL_DYNAMIC_VLAN should be disabled.
> Setup of
> networking topology on host is left up to the implementers.

This is all very hacky, I really don't think we can accept that.

Firewalling and similar things will likely not work correctly if
there's an 802.1Q packet coming in that they don't expect, for example.
We don't want to go around all the Linux infrastructure for something
like this.

In a way this feature seems mis-designed - you never have 802.1Q tags
over the air, but you're inserting them on RX and stripping them on TX,
probably in order to make bridging to ethernet easier and not have to
have 802.1Q acceleration on the ethernet port, or - well - in order to
have an ability to do this with an ethernet card that only has a single
CPU port.

In a way this is more like a switch or bridge feature, but I don't
think we have any abstraction for this in Linux. I think this is
something we should discuss over on netdev and perhaps in person at the
wireless workshop/netdev conference?

johannes

^ permalink raw reply	[flat|nested] 31+ messages in thread

* VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)
@ 2017-09-05 14:20       ` Johannes Berg
  0 siblings, 0 replies; 31+ messages in thread
From: Johannes Berg @ 2017-09-05 14:20 UTC (permalink / raw)
  To: Sergey Matyukevich, linux-wireless, netdev; +Cc: Igor Mitsyanko, Avinash Patil

+netdev

On Tue, 2017-09-05 at 15:45 +0200, Johannes Berg wrote:
> 
> In a way this feature seems mis-designed - you never have 802.1Q tags
> over the air, but you're inserting them on RX and stripping them on
> TX, probably in order to make bridging to ethernet easier and not
> have to have 802.1Q acceleration on the ethernet port, or - well - in
> order to have an ability to do this with an ethernet card that only
> has a single CPU port.

Ok this isn't really right either - it's only for saving the 802.1Q
acceleration on the Ethernet port, really - and saving the extra
bridges.

To clarify, I think what you - conceptually - want is the following
topology:

        +--- eth0.1  ---  br.1  ---  wlan0.1
        |
eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
        | 
        +--- eth0.3  ---  br.3  ---  wlan0.3

where eth0.N is just "ip link add link eth0 name eth0.N type vlan id N"
and br.N is obviously a bridge for each, and the wlan0.N are AP_VLAN
type interfaces that isolate the clients against each other as far as
wifi is concerned.

Is this correct? As far as I understand, that's the baseline topology
that you're trying to achieve, expressed in terms of Linux networking.

Now, you seem to want to compress this to

                  +---  wlan0.1
                  |
eth0  ---  br  ---+---  wlan0.2
                  |
                  +---  wlan0.3

and have the 802.1Q tag insertion/removal that's normally configured to
happen in eth0.N already be handled in wlan0.N.

Also correct?

We clearly don't have APIs for this, and I don't think it makes sense
in the Linux space - the bridge and wlan0.N suddenly have tagged
traffic rather than untagged, and the VLAN tagging is completely hidden
from the management view.

johannes

^ permalink raw reply	[flat|nested] 31+ messages in thread

* VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)
@ 2017-09-05 14:20       ` Johannes Berg
  0 siblings, 0 replies; 31+ messages in thread
From: Johannes Berg @ 2017-09-05 14:20 UTC (permalink / raw)
  To: Sergey Matyukevich, linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
  Cc: Igor Mitsyanko, Avinash Patil

+netdev

On Tue, 2017-09-05 at 15:45 +0200, Johannes Berg wrote:
> 
> In a way this feature seems mis-designed - you never have 802.1Q tags
> over the air, but you're inserting them on RX and stripping them on
> TX, probably in order to make bridging to ethernet easier and not
> have to have 802.1Q acceleration on the ethernet port, or - well - in
> order to have an ability to do this with an ethernet card that only
> has a single CPU port.

Ok this isn't really right either - it's only for saving the 802.1Q
acceleration on the Ethernet port, really - and saving the extra
bridges.

To clarify, I think what you - conceptually - want is the following
topology:

        +--- eth0.1  ---  br.1  ---  wlan0.1
        |
eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
        | 
        +--- eth0.3  ---  br.3  ---  wlan0.3

where eth0.N is just "ip link add link eth0 name eth0.N type vlan id N"
and br.N is obviously a bridge for each, and the wlan0.N are AP_VLAN
type interfaces that isolate the clients against each other as far as
wifi is concerned.

Is this correct? As far as I understand, that's the baseline topology
that you're trying to achieve, expressed in terms of Linux networking.

Now, you seem to want to compress this to

                  +---  wlan0.1
                  |
eth0  ---  br  ---+---  wlan0.2
                  |
                  +---  wlan0.3

and have the 802.1Q tag insertion/removal that's normally configured to
happen in eth0.N already be handled in wlan0.N.

Also correct?

We clearly don't have APIs for this, and I don't think it makes sense
in the Linux space - the bridge and wlan0.N suddenly have tagged
traffic rather than untagged, and the VLAN tagging is completely hidden
from the management view.

johannes

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)
@ 2017-09-06 15:45         ` Sergey Matyukevich
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-09-06 15:45 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, netdev, Igor Mitsyanko, Avinash Patil


Hi Johannes and all,

> > In a way this feature seems mis-designed - you never have 802.1Q tags
> > over the air, but you're inserting them on RX and stripping them on
> > TX, probably in order to make bridging to ethernet easier and not
> > have to have 802.1Q acceleration on the ethernet port, or - well - in
> > order to have an ability to do this with an ethernet card that only
> > has a single CPU port.
> 
> Ok this isn't really right either - it's only for saving the 802.1Q
> acceleration on the Ethernet port, really - and saving the extra
> bridges.
> 
> To clarify, I think what you - conceptually - want is the following
> topology:
> 
>         +--- eth0.1  ---  br.1  ---  wlan0.1
>         |
> eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
>         |
>         +--- eth0.3  ---  br.3  ---  wlan0.3
> 
> where eth0.N is just "ip link add link eth0 name eth0.N type vlan id N"
> and br.N is obviously a bridge for each, and the wlan0.N are AP_VLAN
> type interfaces that isolate the clients against each other as far as
> wifi is concerned.
> 
> Is this correct? As far as I understand, that's the baseline topology
> that you're trying to achieve, expressed in terms of Linux networking.

That's right. In fact, hostapd is able to create this kind of network
bridge infrastructure automatically when it is built
with CONFIG_FULL_DYNAMIC_VLAN option enabled.

> Now, you seem to want to compress this to
> 
>                   +---  wlan0.1
>                   |
> eth0  ---  br  ---+---  wlan0.2
>                   |
>                   +---  wlan0.3
> 
> and have the 802.1Q tag insertion/removal that's normally configured to
> happen in eth0.N already be handled in wlan0.N.
> 
> Also correct?

Exactly. And yes, the only purpose of this 'non-conventional' mode was
to have 802.1Q acceleration on the ethernet port.

> 
> 
> We clearly don't have APIs for this, and I don't think it makes sense
> in the Linux space - the bridge and wlan0.N suddenly have tagged
> traffic rather than untagged, and the VLAN tagging is completely hidden
> from the management view.
> 
> johannes

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)
@ 2017-09-06 15:45         ` Sergey Matyukevich
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-09-06 15:45 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev, Igor Mitsyanko,
	Avinash Patil


Hi Johannes and all,

> > In a way this feature seems mis-designed - you never have 802.1Q tags
> > over the air, but you're inserting them on RX and stripping them on
> > TX, probably in order to make bridging to ethernet easier and not
> > have to have 802.1Q acceleration on the ethernet port, or - well - in
> > order to have an ability to do this with an ethernet card that only
> > has a single CPU port.
> 
> Ok this isn't really right either - it's only for saving the 802.1Q
> acceleration on the Ethernet port, really - and saving the extra
> bridges.
> 
> To clarify, I think what you - conceptually - want is the following
> topology:
> 
>         +--- eth0.1  ---  br.1  ---  wlan0.1
>         |
> eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
>         |
>         +--- eth0.3  ---  br.3  ---  wlan0.3
> 
> where eth0.N is just "ip link add link eth0 name eth0.N type vlan id N"
> and br.N is obviously a bridge for each, and the wlan0.N are AP_VLAN
> type interfaces that isolate the clients against each other as far as
> wifi is concerned.
> 
> Is this correct? As far as I understand, that's the baseline topology
> that you're trying to achieve, expressed in terms of Linux networking.

That's right. In fact, hostapd is able to create this kind of network
bridge infrastructure automatically when it is built
with CONFIG_FULL_DYNAMIC_VLAN option enabled.

> Now, you seem to want to compress this to
> 
>                   +---  wlan0.1
>                   |
> eth0  ---  br  ---+---  wlan0.2
>                   |
>                   +---  wlan0.3
> 
> and have the 802.1Q tag insertion/removal that's normally configured to
> happen in eth0.N already be handled in wlan0.N.
> 
> Also correct?

Exactly. And yes, the only purpose of this 'non-conventional' mode was
to have 802.1Q acceleration on the ethernet port.

> 
> 
> We clearly don't have APIs for this, and I don't think it makes sense
> in the Linux space - the bridge and wlan0.N suddenly have tagged
> traffic rather than untagged, and the VLAN tagging is completely hidden
> from the management view.
> 
> johannes

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)
  2017-09-06 15:45         ` Sergey Matyukevich
  (?)
@ 2017-09-07  6:45         ` Johannes Berg
  -1 siblings, 0 replies; 31+ messages in thread
From: Johannes Berg @ 2017-09-07  6:45 UTC (permalink / raw)
  To: Sergey Matyukevich; +Cc: linux-wireless, netdev, Igor Mitsyanko, Avinash Patil

Hi,

> > To clarify, I think what you - conceptually - want is the following
> > topology:
> > 
> >         +--- eth0.1  ---  br.1  ---  wlan0.1
> >         |
> > eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
> >         |
> >         +--- eth0.3  ---  br.3  ---  wlan0.3
[...]
> That's right. In fact, hostapd is able to create this kind of network
> bridge infrastructure automatically when it is built
> with CONFIG_FULL_DYNAMIC_VLAN option enabled.

Cool, I was unaware of the exact functionality of this build-time
option. :)

> > Now, you seem to want to compress this to
> > 
> >                   +---  wlan0.1
> >                   |
> > eth0  ---  br  ---+---  wlan0.2
> >                   |
> >                   +---  wlan0.3

[...]

> Exactly. And yes, the only purpose of this 'non-conventional' mode
> was to have 802.1Q acceleration on the ethernet port.

Right. You can still have acceleration in the top picture by placing
the feature into the Ethernet hardware, so that tagging/untagging
doesn't have to touch the packet data but just touches (skb) metadata.
But obviously that's something that happens on the other side and you
don't have control over it.

Anyway, I'm happy we cleared up what was going on and also that you
decided to leave it for now and work with the regular Linux topology
model.

Thanks,
johannes

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support
  2017-09-05 13:45   ` Johannes Berg
  2017-09-05 14:20       ` Johannes Berg
@ 2017-09-06 16:22     ` Sergey Matyukevich
  1 sibling, 0 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-09-06 16:22 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

Hi Johannes and all,

> > 1. Passing dynamic VLAN tag to firmware
> 
> There's no such thing as a "VLAN tag" with AP_VLAN interfaces. You want
> 802.1Q, which can be done on top of them if needed, but it's an
> unrelated concept.
> 
> > Currently there is no established way to pass VLAN tag assigned to
> > STA by Radius server to wireless driver. It is assumed that hostapd
> > is able to setup all the bridging and tagging on its own.
> 
> I don't even know if this is true? Does hostapd in fact set up 802.1Q
> tagging in any way? Can you say how to configure this?

Yes, hostapd supports this feature when it is built with option
CONFIG_FULL_DYNAMIC_VLAN enabled. There is a set of hostapd configuration
options and additional files that make it possible to specify 'WAN'
interface, naming conventions for those dynamic bridges, and more.

However I agree that using 802.1Q is only one of possible ways to isolate
stations attached to different AP_VLAN intefaces.

> > 2. Packet routing to/from AP/VLAN interfaces
> > Firmware operates with tagged packets after dynamic VLAN mode is
> > configured. In particular, packets destined to STAs should be
> > properly tagged before they can be passed to firmware. Packets
> > received from STAs are properly tagged by firmware and then
> > passed to wireless driver. As a result, packet routing to AP/VLAN
> > interfaces is straightforward: it is enough to check VLAN tags.
> 
> Ok, so here the whole tagging comes - a bit - into play. Technically
> you could, however, do the following:
> 
>  * assign "fake" tags as I suggested above
>  * pack/unpack the 802.1Q header from the firmware (or put it into
>    metadata) in the driver and just tx/rx untagged packets into the
>    right interface
>  * if the AP_VLAN has a real 802.1Q on top, then it's re-packed again
>    by the ethernet driver when the data goes out

Yes, this is approach with 'fake' tags is how it can be done provided
we do not want to use any hacks with naming conventions for AP_VLAN
interfaces enforced by hostapd.

In fact, the 802.1Q approach has been chosen since in our case it is
possible to make use of 802.1Q acceleration in firmware. So we could
save some cycles on host CPU, tagging/untagging packets on the card.
But again, I agree that using 802.1Q is only one of the possible ways
to isolate stations attached to different AP_VLAN. Thus there is no
point to complicate/drop other usecases only because we are able
to support 802.1Q case better due to acceleration in firmware.

> > Normally hostapd expects untagged packets from AP/VLAN interfaces.
> 
> hostapd doesn't really expect anything there, does it?

In fact, hostapd does not expect 802.1Q tagged packets. IIRC when
CONFIG_FULL_DYNAMIC_VLAN enabled, then hostapd creates bridges
and VLAN interfaces dynamically, connecting them to 'WAN' network
interface specified by vlan_tagged_interface configuration option.

> > Meanwhile firmware performs tagging using h/w acceleration. That
> > is why it makes sense to avoid untagging packets in driver if
> > they are supposed to by tagged again on host. To enable this
> > behavior a new module parameter 'dyn_vlan_tagged' has been
> > introduced:
> >
> > - dyn_vlan_tagged = 0 (default)
> > In this case untagged packets are sent to and expected from AP/VLAN
> > interfaces.
> > Driver tags/untags packets going to/from firmware. This behaviour is
> > expected
> > by hostapd which is able to create bridges and VLAN interfaces
> > automatically
> > when hostapd is built with CONFIG_FULL_DYNAMIC_VLAN option enabled.
> >
> > - dyn_vlan_tagged = 1
> > In this case tagged packets are sent to and expected from AP/VLAN
> > interfaces.
> > Hostapd build option CONFIG_FULL_DYNAMIC_VLAN should be disabled.
> > Setup of
> > networking topology on host is left up to the implementers.
> 
> This is all very hacky, I really don't think we can accept that.

Fair enough. I didn't like it that much anyway :) I will rework the whole
patch using opaque tags for 802.1Q headers from the firmware, leaving AP_VLAN
interface management to hostapd or any other userspace tool.

Regards,
Sergey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 4/8] qtnfmac: implement cfg80211 dump_survey handler
  2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
                   ` (2 preceding siblings ...)
  2017-06-20 19:55 ` [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support Sergey Matyukevich
@ 2017-06-20 19:55 ` Sergey Matyukevich
  2017-06-20 19:55 ` [PATCH 5/8] qtnfmac: enable reporting the current operating channel Sergey Matyukevich
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich

This patch implements cfg80211 dump_survey handler enabling
per-channel survey data reports.

Signed-off-by: Avinash Patil <avinashp@quantenna.com>
Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c |  69 +++++++++++++-
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 107 ++++++++++++++++++++++
 drivers/net/wireless/quantenna/qtnfmac/commands.h |   2 +
 drivers/net/wireless/quantenna/qtnfmac/core.h     |   9 ++
 drivers/net/wireless/quantenna/qtnfmac/qlink.h    |  31 +++++++
 5 files changed, 217 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index e222e8d038d3..eb648f02aa40 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -839,6 +839,72 @@ qtnf_disconnect(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
+static int
+qtnf_dump_survey(struct wiphy *wiphy, struct net_device *dev,
+		 int idx, struct survey_info *survey)
+{
+	struct qtnf_wmac *mac = wiphy_priv(wiphy);
+	struct ieee80211_supported_band *sband;
+	struct ieee80211_channel *chan;
+	struct qtnf_chan_stats stats;
+	int ret;
+
+	sband = wiphy->bands[NL80211_BAND_2GHZ];
+	if (sband && idx >= sband->n_channels) {
+		idx -= sband->n_channels;
+		sband = NULL;
+	}
+
+	if (!sband)
+		sband = wiphy->bands[NL80211_BAND_5GHZ];
+
+	if (!sband || idx >= sband->n_channels)
+		return -ENOENT;
+
+	chan = &sband->channels[idx];
+	memset(&stats, 0, sizeof(stats));
+
+	survey->channel = chan;
+	survey->filled = 0x0;
+
+	ret = qtnf_cmd_get_chan_stats(mac, chan->hw_value, &stats);
+	switch (ret) {
+	case 0:
+		if (unlikely(stats.chan_num != chan->hw_value)) {
+			pr_err("received stats for channel %d instead of %d\n",
+			       stats.chan_num, chan->hw_value);
+			ret = -EINVAL;
+			break;
+		}
+
+		survey->filled = SURVEY_INFO_TIME |
+				 SURVEY_INFO_TIME_SCAN |
+				 SURVEY_INFO_TIME_BUSY |
+				 SURVEY_INFO_TIME_RX |
+				 SURVEY_INFO_TIME_TX |
+				 SURVEY_INFO_NOISE_DBM;
+
+		survey->time_scan = stats.cca_try;
+		survey->time = stats.cca_try;
+		survey->time_tx = stats.cca_tx;
+		survey->time_rx = stats.cca_rx;
+		survey->time_busy = stats.cca_busy;
+		survey->noise = stats.chan_noise;
+		break;
+	case -ENOENT:
+		pr_debug("no stats for channel %u\n", chan->hw_value);
+		ret = 0;
+		break;
+	default:
+		pr_debug("failed to get chan(%d) stats from card\n",
+			 chan->hw_value);
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
 static struct cfg80211_ops qtn_cfg80211_ops = {
 	.add_virtual_intf	= qtnf_add_virtual_intf,
 	.change_virtual_intf	= qtnf_change_virtual_intf,
@@ -859,7 +925,8 @@ static struct cfg80211_ops qtn_cfg80211_ops = {
 	.set_default_mgmt_key	= qtnf_set_default_mgmt_key,
 	.scan			= qtnf_scan,
 	.connect		= qtnf_connect,
-	.disconnect		= qtnf_disconnect
+	.disconnect		= qtnf_disconnect,
+	.dump_survey		= qtnf_dump_survey
 };
 
 static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy_in,
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 221804f8c43a..22741bf6f4ac 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -1358,6 +1358,62 @@ static int qtnf_cmd_resp_proc_phy_params(struct qtnf_wmac *mac,
 	return 0;
 }
 
+static int
+qtnf_cmd_resp_proc_chan_stat_info(struct qtnf_chan_stats *stats,
+				  const u8 *payload, size_t payload_len)
+{
+	struct qlink_chan_stats *qlink_stats;
+	const struct qlink_tlv_hdr *tlv;
+	size_t tlv_full_len;
+	u16 tlv_value_len;
+	u16 tlv_type;
+
+	tlv = (struct qlink_tlv_hdr *)payload;
+	while (payload_len >= sizeof(struct qlink_tlv_hdr)) {
+		tlv_type = le16_to_cpu(tlv->type);
+		tlv_value_len = le16_to_cpu(tlv->len);
+		tlv_full_len = tlv_value_len + sizeof(struct qlink_tlv_hdr);
+		if (tlv_full_len > payload_len) {
+			pr_warn("malformed TLV 0x%.2X; LEN: %u\n",
+				tlv_type, tlv_value_len);
+			return -EINVAL;
+		}
+		switch (tlv_type) {
+		case QTN_TLV_ID_CHANNEL_STATS:
+			if (unlikely(tlv_value_len != sizeof(*qlink_stats))) {
+				pr_err("invalid CHANNEL_STATS entry size\n");
+				return -EINVAL;
+			}
+
+			qlink_stats = (void *)tlv->val;
+
+			stats->chan_num = le32_to_cpu(qlink_stats->chan_num);
+			stats->cca_tx = le32_to_cpu(qlink_stats->cca_tx);
+			stats->cca_rx = le32_to_cpu(qlink_stats->cca_rx);
+			stats->cca_busy = le32_to_cpu(qlink_stats->cca_busy);
+			stats->cca_try = le32_to_cpu(qlink_stats->cca_try);
+			stats->chan_noise = qlink_stats->chan_noise;
+
+			pr_debug("chan(%u) try(%u) busy(%u) noise(%d)\n",
+				 stats->chan_num, stats->cca_try,
+				 stats->cca_busy, stats->chan_noise);
+			break;
+		default:
+			pr_warn("Unknown TLV type: %#x\n",
+				le16_to_cpu(tlv->type));
+		}
+		payload_len -= tlv_full_len;
+		tlv = (struct qlink_tlv_hdr *)(tlv->val + tlv_value_len);
+	}
+
+	if (payload_len) {
+		pr_warn("malformed TLV buf; bytes left: %zu\n", payload_len);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int qtnf_cmd_get_mac_info(struct qtnf_wmac *mac)
 {
 	struct sk_buff *cmd_skb, *resp_skb = NULL;
@@ -2227,3 +2283,54 @@ int qtnf_cmd_reg_notify(struct qtnf_bus *bus, struct regulatory_request *req)
 
 	return ret;
 }
+
+int qtnf_cmd_get_chan_stats(struct qtnf_wmac *mac, u16 channel,
+			    struct qtnf_chan_stats *stats)
+{
+	struct sk_buff *cmd_skb, *resp_skb = NULL;
+	struct qlink_cmd_get_chan_stats *cmd;
+	struct qlink_resp_get_chan_stats *resp;
+	size_t var_data_len;
+	u16 res_code = QLINK_CMD_RESULT_OK;
+	int ret = 0;
+
+	cmd_skb = qtnf_cmd_alloc_new_cmdskb(mac->macid, QLINK_VIFID_RSVD,
+					    QLINK_CMD_CHAN_STATS,
+					    sizeof(*cmd));
+	if (!cmd_skb)
+		return -ENOMEM;
+
+	qtnf_bus_lock(mac->bus);
+
+	cmd = (struct qlink_cmd_get_chan_stats *)cmd_skb->data;
+	cmd->channel = cpu_to_le16(channel);
+
+	ret = qtnf_cmd_send_with_reply(mac->bus, cmd_skb, &resp_skb, &res_code,
+				       sizeof(*resp), &var_data_len);
+	if (unlikely(ret)) {
+		qtnf_bus_unlock(mac->bus);
+		return ret;
+	}
+
+	if (unlikely(res_code != QLINK_CMD_RESULT_OK)) {
+		switch (res_code) {
+		case QLINK_CMD_RESULT_ENOTFOUND:
+			ret = -ENOENT;
+			break;
+		default:
+			pr_err("cmd exec failed: 0x%.4X\n", res_code);
+			ret = -EFAULT;
+			break;
+		}
+		goto out;
+	}
+
+	resp = (struct qlink_resp_get_chan_stats *)resp_skb->data;
+	ret = qtnf_cmd_resp_proc_chan_stat_info(stats, resp->info,
+						var_data_len);
+
+out:
+	qtnf_bus_unlock(mac->bus);
+	consume_skb(resp_skb);
+	return ret;
+}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.h b/drivers/net/wireless/quantenna/qtnfmac/commands.h
index 155b265d42bf..41e2d50988b7 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.h
@@ -71,5 +71,7 @@ int qtnf_cmd_send_disconnect(struct qtnf_vif *vif,
 int qtnf_cmd_send_updown_intf(struct qtnf_vif *vif,
 			      bool up);
 int qtnf_cmd_reg_notify(struct qtnf_bus *bus, struct regulatory_request *req);
+int qtnf_cmd_get_chan_stats(struct qtnf_wmac *mac, u16 channel,
+			    struct qtnf_chan_stats *stats);
 
 #endif /* QLINK_COMMANDS_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index 0d06ec932caf..8824e3d0b5bc 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -138,6 +138,15 @@ struct qtnf_mac_info {
 	size_t n_limits;
 };
 
+struct qtnf_chan_stats {
+	u32 chan_num;
+	u32 cca_tx;
+	u32 cca_rx;
+	u32 cca_busy;
+	u32 cca_try;
+	s8 chan_noise;
+};
+
 struct qtnf_wmac {
 	u8 macid;
 	u8 wiphy_registered;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 892752599109..847c80f40a9a 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -165,6 +165,7 @@ enum qlink_cmd_type {
 	QLINK_CMD_CHANGE_STA		= 0x0051,
 	QLINK_CMD_DEL_STA		= 0x0052,
 	QLINK_CMD_SCAN			= 0x0053,
+	QLINK_CMD_CHAN_STATS		= 0x0054,
 	QLINK_CMD_CONNECT		= 0x0060,
 	QLINK_CMD_DISCONNECT		= 0x0061,
 };
@@ -440,6 +441,16 @@ struct qlink_cmd_chans_info_get {
 } __packed;
 
 /**
+ * struct qlink_cmd_get_chan_stats - data for QLINK_CMD_CHAN_STATS command
+ *
+ * @channel: channel number according to 802.11 17.3.8.3.2 and Annex J
+ */
+struct qlink_cmd_get_chan_stats {
+	struct qlink_cmd chdr;
+	__le16 channel;
+} __packed;
+
+/**
  * enum qlink_reg_initiator - Indicates the initiator of a reg domain request
  *
  * See &enum nl80211_reg_initiator for more info.
@@ -641,6 +652,16 @@ struct qlink_resp_phy_params {
 	u8 info[0];
 } __packed;
 
+/**
+ * struct qlink_resp_get_chan_stats - response for QLINK_CMD_CHAN_STATS cmd
+ *
+ * @info: variable-length channel info.
+ */
+struct qlink_resp_get_chan_stats {
+	struct qlink_cmd rhdr;
+	u8 info[0];
+} __packed;
+
 /* QLINK Events messages related definitions
  */
 
@@ -813,6 +834,7 @@ enum qlink_tlv_id {
 	QTN_TLV_ID_COVERAGE_CLASS	= 0x0213,
 	QTN_TLV_ID_IFACE_LIMIT		= 0x0214,
 	QTN_TLV_ID_NUM_IFACE_COMB	= 0x0215,
+	QTN_TLV_ID_CHANNEL_STATS	= 0x0216,
 	QTN_TLV_ID_STA_BASIC_COUNTERS	= 0x0300,
 	QTN_TLV_ID_STA_GENERIC_INFO	= 0x0301,
 	QTN_TLV_ID_KEY			= 0x0302,
@@ -1014,4 +1036,13 @@ struct qlink_auth_encr {
 	u8 control_port_no_encrypt;
 } __packed;
 
+struct qlink_chan_stats {
+	__le32 chan_num;
+	__le32 cca_tx;
+	__le32 cca_rx;
+	__le32 cca_busy;
+	__le32 cca_try;
+	s8 chan_noise;
+} __packed;
+
 #endif /* _QTN_QLINK_H_ */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 5/8] qtnfmac: enable reporting the current operating channel
  2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
                   ` (3 preceding siblings ...)
  2017-06-20 19:55 ` [PATCH 4/8] qtnfmac: implement cfg80211 dump_survey handler Sergey Matyukevich
@ 2017-06-20 19:55 ` Sergey Matyukevich
  2017-06-20 19:59   ` Johannes Berg
  2017-06-27 17:30   ` Kalle Valo
  2017-06-20 19:55 ` [PATCH 6/8] qtnfmac: move current channel info from vif to mac Sergey Matyukevich
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich

This patch implements all the necessary features needed to keep
operating channel information in sync between firmware and driver:

- implement cfg80211 qtnf_get_channel handler
- implement QLINK_EVENT_FREQ_CHANGE event handling
  using this event firmware notifies host when operating channel is changed
- update qtnf_dump_survey
  specify current channel in use in survey report
- update qtnf_connect
  pass to firmware channel number rather than frequency

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 66 ++++++++++++++++++++++-
 drivers/net/wireless/quantenna/qtnfmac/commands.c |  2 +-
 drivers/net/wireless/quantenna/qtnfmac/event.c    | 60 ++++++++++++++++++++-
 drivers/net/wireless/quantenna/qtnfmac/qlink.h    | 15 +++++-
 4 files changed, 136 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index eb648f02aa40..77afc0fc8c03 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -755,6 +755,7 @@ qtnf_connect(struct wiphy *wiphy, struct net_device *dev,
 	     struct cfg80211_connect_params *sme)
 {
 	struct qtnf_vif *vif = qtnf_netdev_get_priv(dev);
+	struct cfg80211_chan_def chandef;
 	struct qtnf_bss_config *bss_cfg;
 	int ret;
 
@@ -767,9 +768,20 @@ qtnf_connect(struct wiphy *wiphy, struct net_device *dev,
 	bss_cfg = &vif->bss_cfg;
 	memset(bss_cfg, 0, sizeof(*bss_cfg));
 
+	if (sme->channel) {
+		/* FIXME: need to set proper nl80211_channel_type value */
+		cfg80211_chandef_create(&chandef, sme->channel,
+					NL80211_CHAN_HT20);
+		/* fall-back to minimal safe chandef description */
+		if (!cfg80211_chandef_valid(&chandef))
+			cfg80211_chandef_create(&chandef, sme->channel,
+						NL80211_CHAN_HT20);
+
+		memcpy(&bss_cfg->chandef, &chandef, sizeof(bss_cfg->chandef));
+	}
+
 	bss_cfg->ssid_len = sme->ssid_len;
 	memcpy(&bss_cfg->ssid, sme->ssid, bss_cfg->ssid_len);
-	bss_cfg->chandef.chan = sme->channel;
 	bss_cfg->auth_type = sme->auth_type;
 	bss_cfg->privacy = sme->privacy;
 	bss_cfg->mfp = sme->mfp;
@@ -845,10 +857,15 @@ qtnf_dump_survey(struct wiphy *wiphy, struct net_device *dev,
 {
 	struct qtnf_wmac *mac = wiphy_priv(wiphy);
 	struct ieee80211_supported_band *sband;
+	struct cfg80211_chan_def *bss_chandef;
 	struct ieee80211_channel *chan;
 	struct qtnf_chan_stats stats;
+	struct qtnf_vif *vif;
 	int ret;
 
+	vif = qtnf_netdev_get_priv(dev);
+	bss_chandef = &vif->bss_cfg.chandef;
+
 	sband = wiphy->bands[NL80211_BAND_2GHZ];
 	if (sband && idx >= sband->n_channels) {
 		idx -= sband->n_channels;
@@ -867,6 +884,10 @@ qtnf_dump_survey(struct wiphy *wiphy, struct net_device *dev,
 	survey->channel = chan;
 	survey->filled = 0x0;
 
+	if (bss_chandef->chan)
+		if (chan->hw_value == bss_chandef->chan->hw_value)
+			survey->filled |= SURVEY_INFO_IN_USE;
+
 	ret = qtnf_cmd_get_chan_stats(mac, chan->hw_value, &stats);
 	switch (ret) {
 	case 0:
@@ -905,6 +926,46 @@ qtnf_dump_survey(struct wiphy *wiphy, struct net_device *dev,
 	return ret;
 }
 
+static int
+qtnf_get_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
+		 struct cfg80211_chan_def *chandef)
+{
+	struct net_device *ndev = wdev->netdev;
+	struct qtnf_bss_config *bss_cfg;
+	struct qtnf_vif *vif;
+
+	if (!ndev)
+		return -ENODEV;
+
+	vif = qtnf_netdev_get_priv(wdev->netdev);
+	bss_cfg = &vif->bss_cfg;
+
+	switch (vif->wdev.iftype) {
+	case NL80211_IFTYPE_STATION:
+		if (vif->sta_state == QTNF_STA_DISCONNECTED) {
+			pr_warn("%s: STA disconnected\n", ndev->name);
+			return -ENODATA;
+		}
+		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		/* AP_VLAN: get primary vif and pass through to AP */
+		vif = vif->u.vlan.parent;
+		bss_cfg = &vif->bss_cfg;
+	case NL80211_IFTYPE_AP:
+		if (!(vif->bss_status & QTNF_STATE_AP_START)) {
+			pr_warn("%s: AP not started\n", ndev->name);
+			return -ENODATA;
+		}
+		break;
+	default:
+		pr_err("unsupported vif type (%d)\n", vif->wdev.iftype);
+		return -ENODATA;
+	}
+
+	memcpy(chandef, &bss_cfg->chandef, sizeof(*chandef));
+	return 0;
+}
+
 static struct cfg80211_ops qtn_cfg80211_ops = {
 	.add_virtual_intf	= qtnf_add_virtual_intf,
 	.change_virtual_intf	= qtnf_change_virtual_intf,
@@ -926,7 +987,8 @@ static struct cfg80211_ops qtn_cfg80211_ops = {
 	.scan			= qtnf_scan,
 	.connect		= qtnf_connect,
 	.disconnect		= qtnf_disconnect,
-	.dump_survey		= qtnf_dump_survey
+	.dump_survey		= qtnf_dump_survey,
+	.get_channel		= qtnf_get_channel
 };
 
 static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy_in,
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 22741bf6f4ac..2e7034a560aa 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -2087,7 +2087,7 @@ int qtnf_cmd_send_connect(struct qtnf_vif *vif,
 	ether_addr_copy(cmd->bssid, bss_cfg->bssid);
 
 	if (bss_cfg->chandef.chan)
-		cmd->freq = cpu_to_le16(bss_cfg->chandef.chan->center_freq);
+		cmd->channel = cpu_to_le16(bss_cfg->chandef.chan->hw_value);
 
 	cmd->bg_scan_period = cpu_to_le16(bss_cfg->bg_scan_period);
 
diff --git a/drivers/net/wireless/quantenna/qtnfmac/event.c b/drivers/net/wireless/quantenna/qtnfmac/event.c
index 9b61e9a83670..6e1e93ccc40e 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/event.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/event.c
@@ -211,8 +211,8 @@ qtnf_event_handle_bss_leave(struct qtnf_vif *vif,
 
 	pr_debug("VIF%u.%u: disconnected\n", vif->mac->macid, vif->vifid);
 
-	cfg80211_disconnected(vif->netdev, leave_info->reason, NULL, 0, 0,
-			      GFP_KERNEL);
+	cfg80211_disconnected(vif->netdev, le16_to_cpu(leave_info->reason),
+			      NULL, 0, 0, GFP_KERNEL);
 
 	vif->sta_state = QTNF_STA_DISCONNECTED;
 	netif_carrier_off(vif->netdev);
@@ -350,6 +350,58 @@ qtnf_event_handle_scan_complete(struct qtnf_wmac *mac,
 	return 0;
 }
 
+static int
+qtnf_event_handle_freq_change(struct qtnf_wmac *mac,
+			      const struct qlink_event_freq_change *data,
+			      u16 len)
+{
+	struct wiphy *wiphy = priv_to_wiphy(mac);
+	struct cfg80211_chan_def chandef;
+	struct ieee80211_channel *chan;
+	struct qtnf_bss_config *bss_cfg;
+	struct qtnf_vif *vif;
+	int freq;
+	int i;
+
+	if (len < sizeof(*data)) {
+		pr_err("payload is too short\n");
+		return -EINVAL;
+	}
+
+	freq = le32_to_cpu(data->freq);
+	chan = ieee80211_get_channel(wiphy, freq);
+	if (!chan) {
+		pr_err("channel at %d MHz not found\n", freq);
+		return -EINVAL;
+	}
+
+	pr_info("MAC%d switch to new channel %u MHz\n", mac->macid, freq);
+
+	/* FIXME: need to figure out proper nl80211_channel_type value */
+	cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_HT20);
+	/* fall-back to minimal safe chandef description */
+	if (!cfg80211_chandef_valid(&chandef))
+		cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_HT20);
+
+	for (i = 0; i < QTNF_MAX_INTF; i++) {
+		vif = &mac->iflist[i];
+		if (vif->wdev.iftype == NL80211_IFTYPE_UNSPECIFIED)
+			continue;
+
+		if (vif->netdev) {
+			bss_cfg = &vif->bss_cfg;
+			memcpy(&bss_cfg->chandef, &chandef,
+			       sizeof(bss_cfg->chandef));
+
+			mutex_lock(&vif->wdev.mtx);
+			cfg80211_ch_switch_notify(vif->netdev, &chandef);
+			mutex_unlock(&vif->wdev.mtx);
+		}
+	}
+
+	return 0;
+}
+
 static int qtnf_event_parse(struct qtnf_wmac *mac,
 			    const struct sk_buff *event_skb)
 {
@@ -400,6 +452,10 @@ static int qtnf_event_parse(struct qtnf_wmac *mac,
 		ret = qtnf_event_handle_bss_leave(vif, (const void *)event,
 						  event_len);
 		break;
+	case QLINK_EVENT_FREQ_CHANGE:
+		ret = qtnf_event_handle_freq_change(mac, (const void *)event,
+						    event_len);
+		break;
 	default:
 		pr_warn("unknown event type: %x\n", event_id);
 		break;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 847c80f40a9a..5859ef624058 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -390,7 +390,7 @@ enum qlink_sta_connect_flags {
 struct qlink_cmd_connect {
 	struct qlink_cmd chdr;
 	__le32 flags;
-	__le16 freq;
+	__le16 channel;
 	__le16 bg_scan_period;
 	u8 bssid[ETH_ALEN];
 	u8 payload[0];
@@ -673,6 +673,7 @@ enum qlink_event_type {
 	QLINK_EVENT_SCAN_COMPLETE	= 0x0025,
 	QLINK_EVENT_BSS_JOIN		= 0x0026,
 	QLINK_EVENT_BSS_LEAVE		= 0x0027,
+	QLINK_EVENT_FREQ_CHANGE		= 0x0028,
 };
 
 /**
@@ -739,7 +740,17 @@ struct qlink_event_bss_join {
  */
 struct qlink_event_bss_leave {
 	struct qlink_event ehdr;
-	u16 reason;
+	__le16 reason;
+} __packed;
+
+/**
+ * struct qlink_event_freq_change - data for QLINK_EVENT_FREQ_CHANGE event
+ *
+ * @freq: new operating frequency in MHz
+ */
+struct qlink_event_freq_change {
+	struct qlink_event ehdr;
+	__le32 freq;
 } __packed;
 
 enum qlink_rxmgmt_flags {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/8] qtnfmac: enable reporting the current operating channel
  2017-06-20 19:55 ` [PATCH 5/8] qtnfmac: enable reporting the current operating channel Sergey Matyukevich
@ 2017-06-20 19:59   ` Johannes Berg
  2017-06-20 20:09     ` Sergey Matyukevich
  2017-06-27 17:30   ` Kalle Valo
  1 sibling, 1 reply; 31+ messages in thread
From: Johannes Berg @ 2017-06-20 19:59 UTC (permalink / raw)
  To: Sergey Matyukevich, linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil

On Tue, 2017-06-20 at 22:55 +0300, Sergey Matyukevich wrote:

> +	if (sme->channel) {
> +		/* FIXME: need to set proper nl80211_channel_type
> value */
> +		cfg80211_chandef_create(&chandef, sme->channel,
> +					NL80211_CHAN_HT20);
> +		/* fall-back to minimal safe chandef description */
> +		if (!cfg80211_chandef_valid(&chandef))
> +			cfg80211_chandef_create(&chandef, sme-
> >channel,
> +						NL80211_CHAN_HT20);
> 
This seems odd since you just do the same thing over again? Not that I
could see how it would be invalid anyway.

johannes

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/8] qtnfmac: enable reporting the current operating channel
  2017-06-20 19:59   ` Johannes Berg
@ 2017-06-20 20:09     ` Sergey Matyukevich
  2017-06-20 20:17       ` Johannes Berg
  0 siblings, 1 reply; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 20:09 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

> 
> > +     if (sme->channel) {
> > +             /* FIXME: need to set proper nl80211_channel_type
> > value */
> > +             cfg80211_chandef_create(&chandef, sme->channel,
> > +                                     NL80211_CHAN_HT20);
> > +             /* fall-back to minimal safe chandef description */
> > +             if (!cfg80211_chandef_valid(&chandef))
> > +                     cfg80211_chandef_create(&chandef, sme-
> > >channel,
> > +                                             NL80211_CHAN_HT20);
> >
> This seems odd since you just do the same thing over again? Not that I
> could see how it would be invalid anyway.

The first call of cfg80211_chandef_create will be replaced by proper chandef
calculation based on current h/w channel settings. This piece is still in work.
NL80211_CHAN_HT20 is going to be used as a safe fallback when channel info
turns out to be inconsistent.

Regards,
Sergey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/8] qtnfmac: enable reporting the current operating channel
  2017-06-20 20:09     ` Sergey Matyukevich
@ 2017-06-20 20:17       ` Johannes Berg
  0 siblings, 0 replies; 31+ messages in thread
From: Johannes Berg @ 2017-06-20 20:17 UTC (permalink / raw)
  To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

On Tue, 2017-06-20 at 23:09 +0300, Sergey Matyukevich wrote:
> > 
> > > +     if (sme->channel) {
> > > +             /* FIXME: need to set proper nl80211_channel_type
> > > value */
> > > +             cfg80211_chandef_create(&chandef, sme->channel,
> > > +                                     NL80211_CHAN_HT20);
> > > +             /* fall-back to minimal safe chandef description */
> > > +             if (!cfg80211_chandef_valid(&chandef))
> > > +                     cfg80211_chandef_create(&chandef, sme-
> > > > channel,
> > > 
> > > +                                             NL80211_CHAN_HT20);
> > > 
> > 
> > This seems odd since you just do the same thing over again? Not
> > that I
> > could see how it would be invalid anyway.
> 
> The first call of cfg80211_chandef_create will be replaced by proper
> chandef calculation based on current h/w channel settings. This piece
> is still in work. NL80211_CHAN_HT20 is going to be used as a safe
> fallback when channel info turns out to be inconsistent.

Yeah, ok. I guess I'd advocate doing that when the code is actually
there, but I suppose it doesn't really matter much.

johannes

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/8] qtnfmac: enable reporting the current operating channel
  2017-06-20 19:55 ` [PATCH 5/8] qtnfmac: enable reporting the current operating channel Sergey Matyukevich
  2017-06-20 19:59   ` Johannes Berg
@ 2017-06-27 17:30   ` Kalle Valo
  1 sibling, 0 replies; 31+ messages in thread
From: Kalle Valo @ 2017-06-27 17:30 UTC (permalink / raw)
  To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> This patch implements all the necessary features needed to keep
> operating channel information in sync between firmware and driver:
>
> - implement cfg80211 qtnf_get_channel handler
> - implement QLINK_EVENT_FREQ_CHANGE event handling
>   using this event firmware notifies host when operating channel is changed
> - update qtnf_dump_survey
>   specify current channel in use in survey report
> - update qtnf_connect
>   pass to firmware channel number rather than frequency
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

[...]

> +static int
> +qtnf_event_handle_freq_change(struct qtnf_wmac *mac,
> +			      const struct qlink_event_freq_change *data,
> +			      u16 len)
> +{
> +	struct wiphy *wiphy = priv_to_wiphy(mac);
> +	struct cfg80211_chan_def chandef;
> +	struct ieee80211_channel *chan;
> +	struct qtnf_bss_config *bss_cfg;
> +	struct qtnf_vif *vif;
> +	int freq;
> +	int i;
> +
> +	if (len < sizeof(*data)) {
> +		pr_err("payload is too short\n");
> +		return -EINVAL;
> +	}
> +
> +	freq = le32_to_cpu(data->freq);
> +	chan = ieee80211_get_channel(wiphy, freq);
> +	if (!chan) {
> +		pr_err("channel at %d MHz not found\n", freq);
> +		return -EINVAL;
> +	}
> +
> +	pr_info("MAC%d switch to new channel %u MHz\n", mac->macid, freq);

Should this be a debug message? I don't see why the driver would need to
spam this every time.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 6/8] qtnfmac: move current channel info from vif to mac
  2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
                   ` (4 preceding siblings ...)
  2017-06-20 19:55 ` [PATCH 5/8] qtnfmac: enable reporting the current operating channel Sergey Matyukevich
@ 2017-06-20 19:55 ` Sergey Matyukevich
  2017-06-20 19:55 ` [PATCH 7/8] qtnfmac: implement cfg80211 channel_switch handler Sergey Matyukevich
  2017-06-20 19:55 ` [PATCH 8/8] qtnfmac: implement scan timeout Sergey Matyukevich
  7 siblings, 0 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich

Wireless cfg80211 core supplies channel settings in cfg80211_ap_settings
structure for each BSS in multiple BSS configuration. On the other hand
all the virtual interfaces on one radio are using the same PHY settings
including channel.

Move chandef structure from vif to mac structure in order to mantain
the only instance of cfg80211_chan_def structure in qtnf_wmac
rather than its multiple copies in qtnf_vif.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 39 +++++++++++++++--------
 drivers/net/wireless/quantenna/qtnfmac/commands.c |  6 ++--
 drivers/net/wireless/quantenna/qtnfmac/core.h     |  2 +-
 drivers/net/wireless/quantenna/qtnfmac/event.c    |  7 ++--
 4 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 77afc0fc8c03..3f90f57ed595 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -408,11 +408,19 @@ static int qtnf_start_ap(struct wiphy *wiphy, struct net_device *dev,
 			 struct cfg80211_ap_settings *settings)
 {
 	struct qtnf_vif *vif = qtnf_netdev_get_priv(dev);
+	struct qtnf_wmac *mac = wiphy_priv(wiphy);
 	struct qtnf_bss_config *bss_cfg;
 	int ret;
 
-	bss_cfg = &vif->bss_cfg;
+	if (!cfg80211_chandef_identical(&mac->chandef, &settings->chandef)) {
+		memcpy(&mac->chandef, &settings->chandef, sizeof(mac->chandef));
+		if (vif->vifid != 0)
+			pr_warn("%s: unexpected chan %u (%u MHz)\n", dev->name,
+				settings->chandef.chan->hw_value,
+				settings->chandef.chan->center_freq);
+	}
 
+	bss_cfg = &vif->bss_cfg;
 	memset(bss_cfg, 0, sizeof(*bss_cfg));
 
 	bss_cfg->bcn_period = settings->beacon_interval;
@@ -423,8 +431,6 @@ static int qtnf_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	bss_cfg->ssid_len = settings->ssid_len;
 	memcpy(&bss_cfg->ssid, settings->ssid, bss_cfg->ssid_len);
 
-	memcpy(&bss_cfg->chandef, &settings->chandef,
-	       sizeof(struct cfg80211_chan_def));
 	memcpy(&bss_cfg->crypto, &settings->crypto,
 	       sizeof(struct cfg80211_crypto_settings));
 
@@ -755,6 +761,7 @@ qtnf_connect(struct wiphy *wiphy, struct net_device *dev,
 	     struct cfg80211_connect_params *sme)
 {
 	struct qtnf_vif *vif = qtnf_netdev_get_priv(dev);
+	struct qtnf_wmac *mac = wiphy_priv(wiphy);
 	struct cfg80211_chan_def chandef;
 	struct qtnf_bss_config *bss_cfg;
 	int ret;
@@ -777,7 +784,7 @@ qtnf_connect(struct wiphy *wiphy, struct net_device *dev,
 			cfg80211_chandef_create(&chandef, sme->channel,
 						NL80211_CHAN_HT20);
 
-		memcpy(&bss_cfg->chandef, &chandef, sizeof(bss_cfg->chandef));
+		memcpy(&mac->chandef, &chandef, sizeof(mac->chandef));
 	}
 
 	bss_cfg->ssid_len = sme->ssid_len;
@@ -857,14 +864,14 @@ qtnf_dump_survey(struct wiphy *wiphy, struct net_device *dev,
 {
 	struct qtnf_wmac *mac = wiphy_priv(wiphy);
 	struct ieee80211_supported_band *sband;
-	struct cfg80211_chan_def *bss_chandef;
+	struct cfg80211_chan_def *chandef;
 	struct ieee80211_channel *chan;
 	struct qtnf_chan_stats stats;
 	struct qtnf_vif *vif;
 	int ret;
 
 	vif = qtnf_netdev_get_priv(dev);
-	bss_chandef = &vif->bss_cfg.chandef;
+	chandef = &mac->chandef;
 
 	sband = wiphy->bands[NL80211_BAND_2GHZ];
 	if (sband && idx >= sband->n_channels) {
@@ -884,9 +891,10 @@ qtnf_dump_survey(struct wiphy *wiphy, struct net_device *dev,
 	survey->channel = chan;
 	survey->filled = 0x0;
 
-	if (bss_chandef->chan)
-		if (chan->hw_value == bss_chandef->chan->hw_value)
-			survey->filled |= SURVEY_INFO_IN_USE;
+	if (chandef->chan) {
+		if (chan->hw_value == chandef->chan->hw_value)
+			survey->filled = SURVEY_INFO_IN_USE;
+	}
 
 	ret = qtnf_cmd_get_chan_stats(mac, chan->hw_value, &stats);
 	switch (ret) {
@@ -898,7 +906,7 @@ qtnf_dump_survey(struct wiphy *wiphy, struct net_device *dev,
 			break;
 		}
 
-		survey->filled = SURVEY_INFO_TIME |
+		survey->filled |= SURVEY_INFO_TIME |
 				 SURVEY_INFO_TIME_SCAN |
 				 SURVEY_INFO_TIME_BUSY |
 				 SURVEY_INFO_TIME_RX |
@@ -930,15 +938,14 @@ static int
 qtnf_get_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
 		 struct cfg80211_chan_def *chandef)
 {
+	struct qtnf_wmac *mac = wiphy_priv(wiphy);
 	struct net_device *ndev = wdev->netdev;
-	struct qtnf_bss_config *bss_cfg;
 	struct qtnf_vif *vif;
 
 	if (!ndev)
 		return -ENODEV;
 
 	vif = qtnf_netdev_get_priv(wdev->netdev);
-	bss_cfg = &vif->bss_cfg;
 
 	switch (vif->wdev.iftype) {
 	case NL80211_IFTYPE_STATION:
@@ -950,7 +957,6 @@ qtnf_get_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
 	case NL80211_IFTYPE_AP_VLAN:
 		/* AP_VLAN: get primary vif and pass through to AP */
 		vif = vif->u.vlan.parent;
-		bss_cfg = &vif->bss_cfg;
 	case NL80211_IFTYPE_AP:
 		if (!(vif->bss_status & QTNF_STATE_AP_START)) {
 			pr_warn("%s: AP not started\n", ndev->name);
@@ -962,7 +968,12 @@ qtnf_get_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
 		return -ENODATA;
 	}
 
-	memcpy(chandef, &bss_cfg->chandef, sizeof(*chandef));
+	if (!cfg80211_chandef_valid(&mac->chandef)) {
+		pr_err("invalid channel settings on %s\n", ndev->name);
+		return -ENODATA;
+	}
+
+	memcpy(chandef, &mac->chandef, sizeof(*chandef));
 	return 0;
 }
 
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 2e7034a560aa..f44ae71047c3 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -185,7 +185,7 @@ int qtnf_cmd_send_config_ap(struct qtnf_vif *vif)
 {
 	struct sk_buff *cmd_skb;
 	struct qtnf_bss_config *bss_cfg = &vif->bss_cfg;
-	struct cfg80211_chan_def *chandef = &bss_cfg->chandef;
+	struct cfg80211_chan_def *chandef = &vif->mac->chandef;
 	struct qlink_tlv_channel *qchan;
 	struct qlink_auth_encr aen;
 	u16 res_code = QLINK_CMD_RESULT_OK;
@@ -2086,8 +2086,8 @@ int qtnf_cmd_send_connect(struct qtnf_vif *vif,
 
 	ether_addr_copy(cmd->bssid, bss_cfg->bssid);
 
-	if (bss_cfg->chandef.chan)
-		cmd->channel = cpu_to_le16(bss_cfg->chandef.chan->hw_value);
+	if (vif->mac->chandef.chan)
+		cmd->channel = cpu_to_le16(vif->mac->chandef.chan->hw_value);
 
 	cmd->bg_scan_period = cpu_to_le16(bss_cfg->bg_scan_period);
 
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index 8824e3d0b5bc..3c62f5bd28e4 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -68,7 +68,6 @@ struct qtnf_bss_config {
 	u16 auth_type;
 	bool privacy;
 	enum nl80211_mfp mfp;
-	struct cfg80211_chan_def chandef;
 	struct cfg80211_crypto_settings crypto;
 	u16 bg_scan_period;
 	u32 connect_flags;
@@ -155,6 +154,7 @@ struct qtnf_wmac {
 	struct qtnf_mac_info macinfo;
 	struct qtnf_vif iflist[QTNF_MAX_INTF];
 	struct cfg80211_scan_request *scan_req;
+	struct cfg80211_chan_def chandef;
 };
 
 struct qtnf_hw_info {
diff --git a/drivers/net/wireless/quantenna/qtnfmac/event.c b/drivers/net/wireless/quantenna/qtnfmac/event.c
index 6e1e93ccc40e..d1edeb3892df 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/event.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/event.c
@@ -358,7 +358,6 @@ qtnf_event_handle_freq_change(struct qtnf_wmac *mac,
 	struct wiphy *wiphy = priv_to_wiphy(mac);
 	struct cfg80211_chan_def chandef;
 	struct ieee80211_channel *chan;
-	struct qtnf_bss_config *bss_cfg;
 	struct qtnf_vif *vif;
 	int freq;
 	int i;
@@ -383,16 +382,14 @@ qtnf_event_handle_freq_change(struct qtnf_wmac *mac,
 	if (!cfg80211_chandef_valid(&chandef))
 		cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_HT20);
 
+	memcpy(&mac->chandef, &chandef, sizeof(mac->chandef));
+
 	for (i = 0; i < QTNF_MAX_INTF; i++) {
 		vif = &mac->iflist[i];
 		if (vif->wdev.iftype == NL80211_IFTYPE_UNSPECIFIED)
 			continue;
 
 		if (vif->netdev) {
-			bss_cfg = &vif->bss_cfg;
-			memcpy(&bss_cfg->chandef, &chandef,
-			       sizeof(bss_cfg->chandef));
-
 			mutex_lock(&vif->wdev.mtx);
 			cfg80211_ch_switch_notify(vif->netdev, &chandef);
 			mutex_unlock(&vif->wdev.mtx);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 7/8] qtnfmac: implement cfg80211 channel_switch handler
  2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
                   ` (5 preceding siblings ...)
  2017-06-20 19:55 ` [PATCH 6/8] qtnfmac: move current channel info from vif to mac Sergey Matyukevich
@ 2017-06-20 19:55 ` Sergey Matyukevich
  2017-06-27 17:29   ` Kalle Valo
  2017-06-20 19:55 ` [PATCH 8/8] qtnfmac: implement scan timeout Sergey Matyukevich
  7 siblings, 1 reply; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich

This patch implements cfg80211 channel_switch handler enabling CSA
channel-switch procedure.

At the moment qtnfmac host driver performs only basic validation of
the requested new channel and then sends command to firmware.
Beacon IEs are not sent since beacon update is handled by firmware.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 60 ++++++++++++++++++++++-
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 55 +++++++++++++++++++++
 drivers/net/wireless/quantenna/qtnfmac/commands.h |  2 +
 drivers/net/wireless/quantenna/qtnfmac/core.h     |  6 +++
 drivers/net/wireless/quantenna/qtnfmac/event.c    |  8 +++
 drivers/net/wireless/quantenna/qtnfmac/qlink.h    | 17 +++++++
 6 files changed, 146 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 3f90f57ed595..097f29189339 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -977,6 +977,59 @@ qtnf_get_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
 	return 0;
 }
 
+static int qtnf_channel_switch(struct wiphy *wiphy, struct net_device *dev,
+			       struct cfg80211_csa_settings *params)
+{
+	struct qtnf_wmac *mac = wiphy_priv(wiphy);
+	struct qtnf_vif *vif = qtnf_netdev_get_priv(dev);
+	int ret;
+
+	pr_info("%s: chan(%u) count(%u) radar(%u) block_tx(%u)\n", dev->name,
+		params->chandef.chan->hw_value, params->count,
+		params->radar_required, params->block_tx);
+
+	switch (vif->wdev.iftype) {
+	case NL80211_IFTYPE_AP:
+		if (!(vif->bss_status & QTNF_STATE_AP_START)) {
+			pr_warn("AP not started on %s\n", dev->name);
+			return -ENOTCONN;
+		}
+		break;
+	default:
+		pr_err("unsupported vif type (%d) on %s\n",
+		       vif->wdev.iftype, dev->name);
+		return -EOPNOTSUPP;
+	}
+
+	if (vif->vifid != 0) {
+		if (!(mac->status & QTNF_MAC_CSA_ACTIVE))
+			return -EOPNOTSUPP;
+
+		if (!cfg80211_chandef_identical(&params->chandef,
+						&mac->csa_chandef))
+			return -EINVAL;
+
+		return 0;
+	}
+
+	if (!cfg80211_chandef_valid(&params->chandef)) {
+		pr_err("%s: invalid channel\n", dev->name);
+		return -EINVAL;
+	}
+
+	if (cfg80211_chandef_identical(&params->chandef, &mac->chandef)) {
+		pr_err("%s: switch request to the same channel\n", dev->name);
+		return -EALREADY;
+	}
+
+	ret = qtnf_cmd_send_chan_switch(mac, params);
+	if (ret)
+		pr_warn("%s: failed to switch to channel (%u)\n",
+			dev->name, params->chandef.chan->hw_value);
+
+	return ret;
+}
+
 static struct cfg80211_ops qtn_cfg80211_ops = {
 	.add_virtual_intf	= qtnf_add_virtual_intf,
 	.change_virtual_intf	= qtnf_change_virtual_intf,
@@ -999,7 +1052,8 @@ static struct cfg80211_ops qtn_cfg80211_ops = {
 	.connect		= qtnf_connect,
 	.disconnect		= qtnf_disconnect,
 	.dump_survey		= qtnf_dump_survey,
-	.get_channel		= qtnf_get_channel
+	.get_channel		= qtnf_get_channel,
+	.channel_switch		= qtnf_channel_switch
 };
 
 static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy_in,
@@ -1149,6 +1203,7 @@ int qtnf_wiphy_register(struct qtnf_hw_info *hw_info, struct qtnf_wmac *mac)
 
 	wiphy->iface_combinations = iface_comb;
 	wiphy->n_iface_combinations = 1;
+	wiphy->max_num_csa_counters = 2;
 
 	/* Initialize cipher suits */
 	wiphy->cipher_suites = qtnf_cipher_suites;
@@ -1156,7 +1211,8 @@ int qtnf_wiphy_register(struct qtnf_hw_info *hw_info, struct qtnf_wmac *mac)
 	wiphy->signal_type = CFG80211_SIGNAL_TYPE_MBM;
 	wiphy->flags |= WIPHY_FLAG_HAVE_AP_SME |
 			WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD |
-			WIPHY_FLAG_AP_UAPSD;
+			WIPHY_FLAG_AP_UAPSD |
+			WIPHY_FLAG_HAS_CHANNEL_SWITCH;
 
 	wiphy->probe_resp_offload = NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WPS |
 				    NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WPS2;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index f44ae71047c3..900159e82bc8 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -2334,3 +2334,58 @@ int qtnf_cmd_get_chan_stats(struct qtnf_wmac *mac, u16 channel,
 	consume_skb(resp_skb);
 	return ret;
 }
+
+int qtnf_cmd_send_chan_switch(struct qtnf_wmac *mac,
+			      struct cfg80211_csa_settings *params)
+{
+	struct qlink_cmd_chan_switch *cmd;
+	struct sk_buff *cmd_skb;
+	u16 res_code = QLINK_CMD_RESULT_OK;
+	int ret;
+
+	cmd_skb = qtnf_cmd_alloc_new_cmdskb(mac->macid, 0x0,
+					    QLINK_CMD_CHAN_SWITCH,
+					    sizeof(*cmd));
+
+	if (unlikely(!cmd_skb))
+		return -ENOMEM;
+
+	qtnf_bus_lock(mac->bus);
+
+	cmd = (struct qlink_cmd_chan_switch *)cmd_skb->data;
+	cmd->channel = cpu_to_le16(params->chandef.chan->hw_value);
+	cmd->radar_required = params->radar_required;
+	cmd->block_tx = params->block_tx;
+	cmd->beacon_count = params->count;
+
+	ret = qtnf_cmd_send(mac->bus, cmd_skb, &res_code);
+
+	if (unlikely(ret))
+		goto out;
+
+	switch (res_code) {
+	case QLINK_CMD_RESULT_OK:
+		memcpy(&mac->csa_chandef, &params->chandef,
+		       sizeof(mac->csa_chandef));
+		mac->status |= QTNF_MAC_CSA_ACTIVE;
+		ret = 0;
+		break;
+	case QLINK_CMD_RESULT_ENOTFOUND:
+		ret = -ENOENT;
+		break;
+	case QLINK_CMD_RESULT_ENOTSUPP:
+		ret = -EOPNOTSUPP;
+		break;
+	case QLINK_CMD_RESULT_EALREADY:
+		ret = -EALREADY;
+		break;
+	case QLINK_CMD_RESULT_INVALID:
+	default:
+		ret = -EFAULT;
+		break;
+	}
+
+out:
+	qtnf_bus_unlock(mac->bus);
+	return ret;
+}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.h b/drivers/net/wireless/quantenna/qtnfmac/commands.h
index 41e2d50988b7..783b20364296 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.h
@@ -73,5 +73,7 @@ int qtnf_cmd_send_updown_intf(struct qtnf_vif *vif,
 int qtnf_cmd_reg_notify(struct qtnf_bus *bus, struct regulatory_request *req);
 int qtnf_cmd_get_chan_stats(struct qtnf_wmac *mac, u16 channel,
 			    struct qtnf_chan_stats *stats);
+int qtnf_cmd_send_chan_switch(struct qtnf_wmac *mac,
+			      struct cfg80211_csa_settings *params);
 
 #endif /* QLINK_COMMANDS_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index 3c62f5bd28e4..efe078a6d1d3 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -90,6 +90,10 @@ enum qtnf_sta_state {
 	QTNF_STA_CONNECTED
 };
 
+enum qtnf_mac_status {
+	QTNF_MAC_CSA_ACTIVE	= BIT(0)
+};
+
 struct qtnf_vif_vlan {
 	struct qtnf_vif *parent;
 	struct list_head list;
@@ -150,11 +154,13 @@ struct qtnf_wmac {
 	u8 macid;
 	u8 wiphy_registered;
 	u8 macaddr[ETH_ALEN];
+	u32 status;
 	struct qtnf_bus *bus;
 	struct qtnf_mac_info macinfo;
 	struct qtnf_vif iflist[QTNF_MAX_INTF];
 	struct cfg80211_scan_request *scan_req;
 	struct cfg80211_chan_def chandef;
+	struct cfg80211_chan_def csa_chandef;
 };
 
 struct qtnf_hw_info {
diff --git a/drivers/net/wireless/quantenna/qtnfmac/event.c b/drivers/net/wireless/quantenna/qtnfmac/event.c
index d1edeb3892df..b48d9e8b6935 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/event.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/event.c
@@ -376,6 +376,14 @@ qtnf_event_handle_freq_change(struct qtnf_wmac *mac,
 
 	pr_info("MAC%d switch to new channel %u MHz\n", mac->macid, freq);
 
+	if (mac->status & QTNF_MAC_CSA_ACTIVE) {
+		mac->status &= ~QTNF_MAC_CSA_ACTIVE;
+		if (chan->hw_value != mac->csa_chandef.chan->hw_value)
+			pr_warn("unexpected switch to %u during CSA to %u\n",
+				chan->hw_value,
+				mac->csa_chandef.chan->hw_value);
+	}
+
 	/* FIXME: need to figure out proper nl80211_channel_type value */
 	cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_HT20);
 	/* fall-back to minimal safe chandef description */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 5859ef624058..5acd3da5e6d0 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -154,6 +154,7 @@ enum qlink_cmd_type {
 	QLINK_CMD_UPDOWN_INTF		= 0x0018,
 	QLINK_CMD_REG_NOTIFY		= 0x0019,
 	QLINK_CMD_CHANS_INFO_GET	= 0x001A,
+	QLINK_CMD_CHAN_SWITCH		= 0x001B,
 	QLINK_CMD_CONFIG_AP		= 0x0020,
 	QLINK_CMD_START_AP		= 0x0021,
 	QLINK_CMD_STOP_AP		= 0x0022,
@@ -488,6 +489,22 @@ struct qlink_cmd_reg_notify {
 	u8 user_reg_hint_type;
 } __packed;
 
+/**
+ * struct qlink_cmd_chan_switch - data for QLINK_CMD_CHAN_SWITCH command
+ *
+ * @channel: channel number according to 802.11 17.3.8.3.2 and Annex J
+ * @radar_required: whether radar detection is required on the new channel
+ * @block_tx: whether transmissions should be blocked while changing
+ * @beacon_count: number of beacons until switch
+ */
+struct qlink_cmd_chan_switch {
+	struct qlink_cmd chdr;
+	__le16 channel;
+	u8 radar_required;
+	u8 block_tx;
+	u8 beacon_count;
+} __packed;
+
 /* QLINK Command Responses messages related definitions
  */
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 7/8] qtnfmac: implement cfg80211 channel_switch handler
  2017-06-20 19:55 ` [PATCH 7/8] qtnfmac: implement cfg80211 channel_switch handler Sergey Matyukevich
@ 2017-06-27 17:29   ` Kalle Valo
  0 siblings, 0 replies; 31+ messages in thread
From: Kalle Valo @ 2017-06-27 17:29 UTC (permalink / raw)
  To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> This patch implements cfg80211 channel_switch handler enabling CSA
> channel-switch procedure.
>
> At the moment qtnfmac host driver performs only basic validation of
> the requested new channel and then sends command to firmware.
> Beacon IEs are not sent since beacon update is handled by firmware.
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

[...]

> +static int qtnf_channel_switch(struct wiphy *wiphy, struct net_device *dev,
> +			       struct cfg80211_csa_settings *params)
> +{
> +	struct qtnf_wmac *mac = wiphy_priv(wiphy);
> +	struct qtnf_vif *vif = qtnf_netdev_get_priv(dev);
> +	int ret;
> +
> +	pr_info("%s: chan(%u) count(%u) radar(%u) block_tx(%u)\n", dev->name,
> +		params->chandef.chan->hw_value, params->count,
> +		params->radar_required, params->block_tx);

Should this be a debug message?

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 8/8] qtnfmac: implement scan timeout
  2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
                   ` (6 preceding siblings ...)
  2017-06-20 19:55 ` [PATCH 7/8] qtnfmac: implement cfg80211 channel_switch handler Sergey Matyukevich
@ 2017-06-20 19:55 ` Sergey Matyukevich
  2017-06-27 17:27   ` Kalle Valo
  7 siblings, 1 reply; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-20 19:55 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich

Userspace tools may hang on scan in the case when scan completion event
is not returned by firmware. This patch implements the scan timeout
to avoid such situation.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 22 ++++++++++++++++++----
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.h |  4 ++++
 drivers/net/wireless/quantenna/qtnfmac/core.c     |  2 ++
 drivers/net/wireless/quantenna/qtnfmac/core.h     | 13 +++++++++++++
 drivers/net/wireless/quantenna/qtnfmac/event.c    |  2 ++
 5 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 097f29189339..a0ab7d289684 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -741,19 +741,33 @@ qtnf_del_station(struct wiphy *wiphy, struct net_device *dev,
 	return ret;
 }
 
+static void qtnf_scan_timeout(unsigned long data)
+{
+	struct qtnf_wmac *mac = (struct qtnf_wmac *)data;
+
+	pr_warn("mac%d scan timed out\n", mac->macid);
+	qtnf_scan_done(mac, true);
+}
+
 static int
 qtnf_scan(struct wiphy *wiphy, struct cfg80211_scan_request *request)
 {
 	struct qtnf_wmac *mac = wiphy_priv(wiphy);
-	int ret;
 
 	mac->scan_req = request;
 
-	ret = qtnf_cmd_send_scan(mac);
-	if (ret)
+	if (qtnf_cmd_send_scan(mac)) {
 		pr_err("MAC%u: failed to start scan\n", mac->macid);
+		mac->scan_req = NULL;
+		return -EFAULT;
+	}
 
-	return ret;
+	mac->scan_timeout.data = (unsigned long)mac;
+	mac->scan_timeout.function = qtnf_scan_timeout;
+	mod_timer(&mac->scan_timeout,
+		  jiffies + QTNF_SCAN_TIMEOUT_SEC * HZ);
+
+	return 0;
 }
 
 static int
diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h
index 5bd33124a7c8..eddd040b8869 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h
@@ -34,10 +34,14 @@ static inline void qtnf_scan_done(struct qtnf_wmac *mac, bool aborted)
 		.aborted = aborted,
 	};
 
+	qtnf_wmac_lock(mac);
+
 	if (mac->scan_req) {
 		cfg80211_scan_done(mac->scan_req, &info);
 		mac->scan_req = NULL;
 	}
+
+	qtnf_wmac_unlock(mac);
 }
 
 #endif /* _QTN_FMAC_CFG80211_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c
index 3d9b217790ed..8ce9c370dc94 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -336,6 +336,8 @@ static struct qtnf_wmac *qtnf_core_mac_alloc(struct qtnf_bus *bus,
 		mac->iflist[i].vifid = i;
 		qtnf_list_init(&mac->iflist[i].sta_list);
 		qtnf_list_init(&mac->iflist[i].vlan_list);
+		mutex_init(&mac->mac_lock);
+		init_timer(&mac->scan_timeout);
 	}
 
 	qtnf_mac_init_primary_intf(mac);
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index efe078a6d1d3..883b052ccb01 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -46,6 +46,7 @@
 #define QTNF_MAX_EVENT_QUEUE_LEN	255
 #define QTNF_DEFAULT_BG_SCAN_PERIOD	300
 #define QTNF_MAX_BG_SCAN_PERIOD		0xffff
+#define QTNF_SCAN_TIMEOUT_SEC		15
 
 #define QTNF_DEF_BSS_PRIORITY		0
 #define QTNF_DEF_WDOG_TIMEOUT		5
@@ -161,6 +162,8 @@ struct qtnf_wmac {
 	struct cfg80211_scan_request *scan_req;
 	struct cfg80211_chan_def chandef;
 	struct cfg80211_chan_def csa_chandef;
+	struct mutex mac_lock;	/* lock during wmac speicific ops */
+	struct timer_list scan_timeout;
 };
 
 struct qtnf_hw_info {
@@ -197,4 +200,14 @@ static inline struct qtnf_vif *qtnf_netdev_get_priv(struct net_device *dev)
 	return *((void **)netdev_priv(dev));
 }
 
+static __always_inline void qtnf_wmac_lock(struct qtnf_wmac *mac)
+{
+	mutex_lock(&mac->mac_lock);
+}
+
+static __always_inline void qtnf_wmac_unlock(struct qtnf_wmac *mac)
+{
+	mutex_unlock(&mac->mac_lock);
+}
+
 #endif /* _QTN_FMAC_CORE_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/event.c b/drivers/net/wireless/quantenna/qtnfmac/event.c
index b48d9e8b6935..2c0d6095544f 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/event.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/event.c
@@ -345,6 +345,8 @@ qtnf_event_handle_scan_complete(struct qtnf_wmac *mac,
 		return -EINVAL;
 	}
 
+	if (timer_pending(&mac->scan_timeout))
+		del_timer_sync(&mac->scan_timeout);
 	qtnf_scan_done(mac, le32_to_cpu(status->flags) & QLINK_SCAN_ABORTED);
 
 	return 0;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] qtnfmac: implement scan timeout
  2017-06-20 19:55 ` [PATCH 8/8] qtnfmac: implement scan timeout Sergey Matyukevich
@ 2017-06-27 17:27   ` Kalle Valo
  2017-06-29 16:49     ` Sergey Matyukevich
  0 siblings, 1 reply; 31+ messages in thread
From: Kalle Valo @ 2017-06-27 17:27 UTC (permalink / raw)
  To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> Userspace tools may hang on scan in the case when scan completion event
> is not returned by firmware. This patch implements the scan timeout
> to avoid such situation.
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

[...]

> +static __always_inline void qtnf_wmac_lock(struct qtnf_wmac *mac)
> +{
> +	mutex_lock(&mac->mac_lock);
> +}
> +
> +static __always_inline void qtnf_wmac_unlock(struct qtnf_wmac *mac)
> +{
> +	mutex_unlock(&mac->mac_lock);
> +}

Why? These look pointless to me.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] qtnfmac: implement scan timeout
  2017-06-27 17:27   ` Kalle Valo
@ 2017-06-29 16:49     ` Sergey Matyukevich
  2017-06-30  5:36       ` Kalle Valo
  2017-07-25 11:56       ` Kalle Valo
  0 siblings, 2 replies; 31+ messages in thread
From: Sergey Matyukevich @ 2017-06-29 16:49 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Igor Mitsyanko

On Tue, Jun 27, 2017 at 08:27:37PM +0300, Kalle Valo wrote:
> 
> External Email
> 
> 
> Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:
> 
> > Userspace tools may hang on scan in the case when scan completion event
> > is not returned by firmware. This patch implements the scan timeout
> > to avoid such situation.
> >
> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> 
> [...]
> 
> > +static __always_inline void qtnf_wmac_lock(struct qtnf_wmac *mac)
> > +{
> > +     mutex_lock(&mac->mac_lock);
> > +}
> > +
> > +static __always_inline void qtnf_wmac_unlock(struct qtnf_wmac *mac)
> > +{
> > +     mutex_unlock(&mac->mac_lock);
> > +}
> 
> Why? These look pointless to me.

Could you please clarify a bit. You mean, locking/unlocking directly instead of
using inline wrappers ?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] qtnfmac: implement scan timeout
  2017-06-29 16:49     ` Sergey Matyukevich
@ 2017-06-30  5:36       ` Kalle Valo
  2017-07-25 11:56       ` Kalle Valo
  1 sibling, 0 replies; 31+ messages in thread
From: Kalle Valo @ 2017-06-30  5:36 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> On Tue, Jun 27, 2017 at 08:27:37PM +0300, Kalle Valo wrote:
>> 
>> External Email
>> 
>> 
>> Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:
>> 
>> > Userspace tools may hang on scan in the case when scan completion event
>> > is not returned by firmware. This patch implements the scan timeout
>> > to avoid such situation.
>> >
>> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
>> 
>> [...]
>> 
>> > +static __always_inline void qtnf_wmac_lock(struct qtnf_wmac *mac)
>> > +{
>> > +     mutex_lock(&mac->mac_lock);
>> > +}
>> > +
>> > +static __always_inline void qtnf_wmac_unlock(struct qtnf_wmac *mac)
>> > +{
>> > +     mutex_unlock(&mac->mac_lock);
>> > +}
>> 
>> Why? These look pointless to me.
>
> Could you please clarify a bit. You mean, locking/unlocking directly instead of
> using inline wrappers ?

Yeah, adding unnecessary abstractions is very much frowned upon.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] qtnfmac: implement scan timeout
  2017-06-29 16:49     ` Sergey Matyukevich
  2017-06-30  5:36       ` Kalle Valo
@ 2017-07-25 11:56       ` Kalle Valo
  1 sibling, 0 replies; 31+ messages in thread
From: Kalle Valo @ 2017-07-25 11:56 UTC (permalink / raw)
  To: linux-wireless; +Cc: Igor Mitsyanko

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> On Tue, Jun 27, 2017 at 08:27:37PM +0300, Kalle Valo wrote:
>> 
>> External Email
>> 
>> 
>> Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:
>> 
>> > Userspace tools may hang on scan in the case when scan completion event
>> > is not returned by firmware. This patch implements the scan timeout
>> > to avoid such situation.
>> >
>> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
>> 
>> [...]
>> 
>> > +static __always_inline void qtnf_wmac_lock(struct qtnf_wmac *mac)
>> > +{
>> > +     mutex_lock(&mac->mac_lock);
>> > +}
>> > +
>> > +static __always_inline void qtnf_wmac_unlock(struct qtnf_wmac *mac)
>> > +{
>> > +     mutex_unlock(&mac->mac_lock);
>> > +}
>> 
>> Why? These look pointless to me.
>
> Could you please clarify a bit. You mean, locking/unlocking directly instead of
> using inline wrappers ?

Yeah, these kind of simple locking wrappers should not be used. They
only add confusion.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2017-09-07  6:45 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-20 19:55 [PATCH 0/8] qtnfmac: add more features to driver Sergey Matyukevich
2017-06-20 19:55 ` [PATCH 1/8] qtnfmac: updates and fixes for regulatory support Sergey Matyukevich
2017-06-27 17:20   ` Kalle Valo
2017-06-29 16:43     ` Sergey Matyukevich
2017-06-20 19:55 ` [PATCH 2/8] qtnfmac: cleanup and fixes preparing AP_VLAN support Sergey Matyukevich
2017-06-27 17:21   ` Kalle Valo
2017-06-29 16:46     ` Sergey Matyukevich
2017-06-20 19:55 ` [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support Sergey Matyukevich
2017-06-27 17:35   ` Kalle Valo
2017-07-01  8:45     ` Sergey Matyukevich
2017-09-05 13:45   ` Johannes Berg
2017-09-05 14:20     ` VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support) Johannes Berg
2017-09-05 14:20       ` Johannes Berg
2017-09-06 15:45       ` Sergey Matyukevich
2017-09-06 15:45         ` Sergey Matyukevich
2017-09-07  6:45         ` Johannes Berg
2017-09-06 16:22     ` [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support Sergey Matyukevich
2017-06-20 19:55 ` [PATCH 4/8] qtnfmac: implement cfg80211 dump_survey handler Sergey Matyukevich
2017-06-20 19:55 ` [PATCH 5/8] qtnfmac: enable reporting the current operating channel Sergey Matyukevich
2017-06-20 19:59   ` Johannes Berg
2017-06-20 20:09     ` Sergey Matyukevich
2017-06-20 20:17       ` Johannes Berg
2017-06-27 17:30   ` Kalle Valo
2017-06-20 19:55 ` [PATCH 6/8] qtnfmac: move current channel info from vif to mac Sergey Matyukevich
2017-06-20 19:55 ` [PATCH 7/8] qtnfmac: implement cfg80211 channel_switch handler Sergey Matyukevich
2017-06-27 17:29   ` Kalle Valo
2017-06-20 19:55 ` [PATCH 8/8] qtnfmac: implement scan timeout Sergey Matyukevich
2017-06-27 17:27   ` Kalle Valo
2017-06-29 16:49     ` Sergey Matyukevich
2017-06-30  5:36       ` Kalle Valo
2017-07-25 11:56       ` Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.