Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH 1/3] mac80211: remove unnecessary num_mcast_sta user
From: Michael Braun @ 2016-10-05  4:55 UTC (permalink / raw)
  To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan

Checking for num_mcast_sta in __ieee80211_request_smps_ap() is unnecessary,
as sta list will be empty in this case anyway, so list_for_each_entry(sta,
...) will exit immediately.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
 net/mac80211/cfg.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 543b1d4..24133f5 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2313,13 +2313,6 @@ int __ieee80211_request_smps_ap(struct ieee80211_sub_if_data *sdata,
 	    smps_mode == IEEE80211_SMPS_AUTOMATIC)
 		return 0;
 
-	 /* If no associated stations, there's no need to do anything */
-	if (!atomic_read(&sdata->u.ap.num_mcast_sta)) {
-		sdata->smps_mode = smps_mode;
-		ieee80211_queue_work(&sdata->local->hw, &sdata->recalc_smps);
-		return 0;
-	}
-
 	ht_dbg(sdata,
 	       "SMPS %d requested in AP mode, sending Action frame to %d stations\n",
 	       smps_mode, atomic_read(&sdata->u.ap.num_mcast_sta));
-- 
2.1.4

^ permalink raw reply related

* [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN
From: Michael Braun @ 2016-10-05  4:55 UTC (permalink / raw)
  To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan
In-Reply-To: <1475643324-2845-1-git-send-email-michael-dev@fami-braun.de>

This patch adds filtering for multicast data packets on AP_VLAN interfaces that
have no authorized station connected and changes filtering on AP interfaces to
not count stations assigned to AP_VLAN interfaces.

This saves airtime and avoids waking up other stations currently authorized in
this BSS. When using WPA, the packets dropped could not be decrypted by any
station.

The behaviour when there are no AP_VLAN interfaces is left unchanged.
When there are AP_VLAN interfaces, this patch
1. adds filtering multicast data packets sent on AP_VLAN interfaces that
   have no authorized station connected.
   No filtering happens on 4addr AP_VLAN interfaces.
2. makes filtering of multicast data packets sent on AP interfaces depend on
   the number of authorized stations in this bss not assigned to an AP_VLAN
   interface.

Therefore, two new num_mcast_sta like counters are added: one for the number of
authorized stations connected to an AP_VLAN interface and one for the number of
authorized stations connected to the bss and not assigned to any AP_VLAN. The
already existing num_mcast_sta counter is left unchanged as it is used by SMPS.

The new counters are exposed in debugfs.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>

--
v3:
 - reuse existing num_mcast_sta
v2:
 - use separate function to inc/dec mcast_sta counters
 - do not filter in 4addr mode
 - change description
 - change filtering on AP interface (do not count AP_VLAN sta)
 - use new counters regardless of 4addr or not
 - simplify cfg.c:change_station
 - remove no-op change in __cleanup_single_sta
---
 net/mac80211/cfg.c            | 20 ++++++--------------
 net/mac80211/debugfs_netdev.c | 11 +++++++++++
 net/mac80211/ieee80211_i.h    | 42 ++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/rx.c             |  5 +++--
 net/mac80211/sta_info.c       | 10 ++--------
 net/mac80211/tx.c             |  5 ++---
 6 files changed, 66 insertions(+), 27 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 24133f5..1edb017 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1357,9 +1357,6 @@ static int ieee80211_change_station(struct wiphy *wiphy,
 		goto out_err;
 
 	if (params->vlan && params->vlan != sta->sdata->dev) {
-		bool prev_4addr = false;
-		bool new_4addr = false;
-
 		vlansdata = IEEE80211_DEV_TO_SUB_IF(params->vlan);
 
 		if (params->vlan->ieee80211_ptr->use_4addr) {
@@ -1369,26 +1366,21 @@ static int ieee80211_change_station(struct wiphy *wiphy,
 			}
 
 			rcu_assign_pointer(vlansdata->u.vlan.sta, sta);
-			new_4addr = true;
 			__ieee80211_check_fast_rx_iface(vlansdata);
 		}
 
 		if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-		    sta->sdata->u.vlan.sta) {
+		    sta->sdata->u.vlan.sta)
 			RCU_INIT_POINTER(sta->sdata->u.vlan.sta, NULL);
-			prev_4addr = true;
-		}
+
+		if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+			ieee80211_vif_dec_num_mcast(sta->sdata);
 
 		sta->sdata = vlansdata;
 		ieee80211_check_fast_xmit(sta);
 
-		if (sta->sta_state == IEEE80211_STA_AUTHORIZED &&
-		    prev_4addr != new_4addr) {
-			if (new_4addr)
-				atomic_dec(&sta->sdata->bss->num_mcast_sta);
-			else
-				atomic_inc(&sta->sdata->bss->num_mcast_sta);
-		}
+		if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+			ieee80211_vif_inc_num_mcast(sta->sdata);
 
 		ieee80211_send_layer2_update(sta);
 	}
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index a5ba739..7fe468e 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -477,6 +477,7 @@ IEEE80211_IF_FILE_RW(tdls_wider_bw);
 IEEE80211_IF_FILE(num_mcast_sta, u.ap.num_mcast_sta, ATOMIC);
 IEEE80211_IF_FILE(num_sta_ps, u.ap.ps.num_sta_ps, ATOMIC);
 IEEE80211_IF_FILE(dtim_count, u.ap.ps.dtim_count, DEC);
+IEEE80211_IF_FILE(num_mcast_sta_vlan, u.vlan.num_mcast_sta, ATOMIC);
 
 static ssize_t ieee80211_if_fmt_num_buffered_multicast(
 	const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -643,6 +644,13 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
 	DEBUGFS_ADD_MODE(tkip_mic_test, 0200);
 }
 
+static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
+{
+	// add num_mcast_sta_vlan using name num_mcast_sta
+	debugfs_create_file("num_mcast_sta", 0400, sdata->vif.debugfs_dir, \
+			    sdata, &num_mcast_sta_vlan_ops);
+}
+
 static void add_ibss_files(struct ieee80211_sub_if_data *sdata)
 {
 	DEBUGFS_ADD_MODE(tsf, 0600);
@@ -746,6 +754,9 @@ static void add_files(struct ieee80211_sub_if_data *sdata)
 	case NL80211_IFTYPE_AP:
 		add_ap_files(sdata);
 		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		add_vlan_files(sdata);
+		break;
 	case NL80211_IFTYPE_WDS:
 		add_wds_files(sdata);
 		break;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f56d342..933688e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -305,6 +305,7 @@ struct ieee80211_if_vlan {
 
 	/* used for all tx if the VLAN is configured to 4-addr mode */
 	struct sta_info __rcu *sta;
+	atomic_t num_mcast_sta; /* number of stations receiving multicast */
 };
 
 struct mesh_stats {
@@ -1496,6 +1497,47 @@ ieee80211_have_rx_timestamp(struct ieee80211_rx_status *status)
 	return false;
 }
 
+static inline void
+ieee80211_vif_inc_num_mcast(struct ieee80211_sub_if_data *sdata)
+{
+	if (sdata->vif.type != NL80211_IFTYPE_AP &&
+	    sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
+		return;
+
+	if (sdata->vif.type == NL80211_IFTYPE_AP)
+		atomic_inc(&sdata->u.ap.num_mcast_sta);
+	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+		atomic_inc(&sdata->u.vlan.num_mcast_sta);
+}
+
+static inline void
+ieee80211_vif_dec_num_mcast(struct ieee80211_sub_if_data *sdata)
+{
+	if (sdata->vif.type != NL80211_IFTYPE_AP &&
+	    sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
+		return;
+
+	if (sdata->vif.type == NL80211_IFTYPE_AP)
+		atomic_dec(&sdata->u.ap.num_mcast_sta);
+	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+		atomic_dec(&sdata->u.vlan.num_mcast_sta);
+}
+
+/* This function returns the number of multicast stations connected to this
+ * interface. It returns -1 if that number is not tracked, that is for netdevs
+ * not in AP or AP_VLAN mode or when using 4addr. */
+static inline int
+ieee80211_vif_get_num_mcast_if(struct ieee80211_sub_if_data *sdata)
+{
+	if (sdata->vif.type == NL80211_IFTYPE_AP)
+		return atomic_read(&sdata->u.ap.num_mcast_sta);
+	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+		 !sdata->u.vlan.sta)
+		return atomic_read(&sdata->u.vlan.num_mcast_sta);
+	else
+		return -1;
+}
+
 u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
 				     struct ieee80211_rx_status *status,
 				     unsigned int mpdu_len,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index fbf99b8..9c5d222 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2161,7 +2161,8 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 	     sdata->vif.type == NL80211_IFTYPE_AP_VLAN) &&
 	    !(sdata->flags & IEEE80211_SDATA_DONT_BRIDGE_PACKETS) &&
 	    (sdata->vif.type != NL80211_IFTYPE_AP_VLAN || !sdata->u.vlan.sta)) {
-		if (is_multicast_ether_addr(ehdr->h_dest)) {
+		if (is_multicast_ether_addr(ehdr->h_dest) &&
+		    ieee80211_vif_get_num_mcast_if(sdata) != 0) {
 			/*
 			 * send multicast frames both to higher layers in
 			 * local net stack and back to the wireless medium
@@ -2170,7 +2171,7 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 			if (!xmit_skb)
 				net_info_ratelimited("%s: failed to clone multicast frame\n",
 						    dev->name);
-		} else {
+		} else if (!is_multicast_ether_addr(ehdr->h_dest)) {
 			dsta = sta_info_get(sdata, skb->data);
 			if (dsta) {
 				/*
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 76b737d..216ef65 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1882,10 +1882,7 @@ int sta_info_move_state(struct sta_info *sta,
 			if (!sta->sta.support_p2p_ps)
 				ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
 		} else if (sta->sta_state == IEEE80211_STA_AUTHORIZED) {
-			if (sta->sdata->vif.type == NL80211_IFTYPE_AP ||
-			    (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-			     !sta->sdata->u.vlan.sta))
-				atomic_dec(&sta->sdata->bss->num_mcast_sta);
+			ieee80211_vif_dec_num_mcast(sta->sdata);
 			clear_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
 			ieee80211_clear_fast_xmit(sta);
 			ieee80211_clear_fast_rx(sta);
@@ -1893,10 +1890,7 @@ int sta_info_move_state(struct sta_info *sta,
 		break;
 	case IEEE80211_STA_AUTHORIZED:
 		if (sta->sta_state == IEEE80211_STA_ASSOC) {
-			if (sta->sdata->vif.type == NL80211_IFTYPE_AP ||
-			    (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-			     !sta->sdata->u.vlan.sta))
-				atomic_inc(&sta->sdata->bss->num_mcast_sta);
+			ieee80211_vif_inc_num_mcast(sta->sdata);
 			set_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
 			ieee80211_check_fast_xmit(sta);
 			ieee80211_check_fast_rx(sta);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 5023966..aed375f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -331,9 +331,8 @@ ieee80211_tx_h_check_assoc(struct ieee80211_tx_data *tx)
 			I802_DEBUG_INC(tx->local->tx_handlers_drop_not_assoc);
 			return TX_DROP;
 		}
-	} else if (unlikely(tx->sdata->vif.type == NL80211_IFTYPE_AP &&
-			    ieee80211_is_data(hdr->frame_control) &&
-			    !atomic_read(&tx->sdata->u.ap.num_mcast_sta))) {
+	} else if (unlikely(ieee80211_vif_get_num_mcast_if(tx->sdata) == 0 &&
+			    ieee80211_is_data(hdr->frame_control))) {
 		/*
 		 * No associated STAs - no need to send multicast
 		 * frames.
-- 
2.1.4

^ permalink raw reply related

* [PATCH 3/3] mac80211: multicast to unicast conversion
From: Michael Braun @ 2016-10-05  4:55 UTC (permalink / raw)
  To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan
In-Reply-To: <1475643324-2845-1-git-send-email-michael-dev@fami-braun.de>

This patch adds support for sending multicast data packets with ARP, IPv4 and
IPv6 payload (possible 802.1q tagged) as 802.11 unicast frames to all stations.

IEEE 802.11 multicast has well known issues, among them:
 1. packets are not acked and hence not retransmitted, resulting in decreased
    reliablity
 2. packets are send at low rate, increasing time required on air

When used with AP_VLAN, there is another disadvantage:
 3. all stations in the BSS are woken up, regardsless of their AP_VLAN
    assignment.

By doing multicast to unicast conversion, all three issus are solved.

IEEE802.11-2012 proposes directed multicast service (DMS) using A-MSDU frames
and a station initiated control protocol. It has the advantage that the station
can recover the destination multicast mac address, but it is not backward
compatible with non QOS stations and does not enable the administrator of a BSS
to force this mode of operation within a BSS. Additionally, it would require
both the ap and the station to implement the control protocol, which is
optional on both ends. Furthermore, I've seen a few mobile phone stations
locally that indicate qos support but won't complete DHCP if their broadcasts
are encapsulated as A-MSDU. Though they work fine with this series approach.

This patch therefore does not opt to implement DMS but instead just replicates
the packet and changes the destination address. As this works fine with ARP,
IPv4 and IPv6, it is limited to these protocols and normal 802.11 multicast
frames are send out for all other payload protocols.

There is a runtime toggle to enable multicast conversion in a per-bss fashion.

When there is only a single station assigned to the AP_VLAN interface, no
packet replication will occur. 4addr mode of operation is unchanged.

This change opts for iterating all BSS stations for finding the stations
assigned to this AP/AP_VLAN interface, as there currently is no per AP_VLAN
list to iterate and multicast packets are expected to be few. If needed, such
a list could be added later.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>

--
v3: fix compile error for trace.h
v2: add nl80211 toggle
    rename tx_dnat to change_da
    change int to bool unicast
---
 include/net/cfg80211.h        |   5 ++
 include/uapi/linux/nl80211.h  |   7 +++
 net/mac80211/cfg.c            |  14 ++++++
 net/mac80211/debugfs_netdev.c |  29 ++++++++++++
 net/mac80211/ieee80211_i.h    |   1 +
 net/mac80211/tx.c             | 103 ++++++++++++++++++++++++++++++++++++++++++
 net/wireless/nl80211.c        |  33 ++++++++++++++
 net/wireless/rdev-ops.h       |  11 +++++
 net/wireless/trace.h          |  19 ++++++++
 9 files changed, 222 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d768fcd..89049d9 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2460,6 +2460,8 @@ struct cfg80211_qos_map {
  *
  * @set_wds_peer: set the WDS peer for a WDS interface
  *
+ * @set_ap_unicast: set the multicast to unicast flag for a AP interface
+ *
  * @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting
  *	functions to adjust rfkill hw state
  *
@@ -2722,6 +2724,9 @@ struct cfg80211_ops {
 	int	(*set_wds_peer)(struct wiphy *wiphy, struct net_device *dev,
 				const u8 *addr);
 
+	int	(*set_ap_unicast)(struct wiphy *wiphy, struct net_device *dev,
+				  const bool unicast);
+
 	void	(*rfkill_poll)(struct wiphy *wiphy);
 
 #ifdef CONFIG_NL80211_TESTMODE
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 2206941..cfbeebc 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -599,6 +599,9 @@
  *
  * @NL80211_CMD_SET_WDS_PEER: Set the MAC address of the peer on a WDS interface.
  *
+ * @NL80211_CMD_SET_AP_UNICAST: Set the multicast to unicast toggle on a AP
+ *                              interface.
+ *
  * @NL80211_CMD_JOIN_MESH: Join a mesh. The mesh ID must be given, and initial
  *	mesh config parameters may be given.
  * @NL80211_CMD_LEAVE_MESH: Leave the mesh network -- no special arguments, the
@@ -1026,6 +1029,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_ABORT_SCAN,
 
+	NL80211_CMD_SET_AP_UNICAST,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -2261,6 +2266,8 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_MESH_PEER_AID,
 
+	NL80211_ATTR_UNICAST,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1edb017..a543e46 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2242,6 +2242,19 @@ static int ieee80211_set_wds_peer(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
+static int ieee80211_set_ap_unicast(struct wiphy *wiphy, struct net_device *dev,
+				    const bool unicast)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+	if (sdata->vif.type != NL80211_IFTYPE_AP)
+		return -1;
+
+	sdata->u.ap.unicast = unicast;
+
+	return 0;
+}
+
 static void ieee80211_rfkill_poll(struct wiphy *wiphy)
 {
 	struct ieee80211_local *local = wiphy_priv(wiphy);
@@ -3400,6 +3413,7 @@ const struct cfg80211_ops mac80211_config_ops = {
 	.set_tx_power = ieee80211_set_tx_power,
 	.get_tx_power = ieee80211_get_tx_power,
 	.set_wds_peer = ieee80211_set_wds_peer,
+	.set_ap_unicast = ieee80211_set_ap_unicast,
 	.rfkill_poll = ieee80211_rfkill_poll,
 	CFG80211_TESTMODE_CMD(ieee80211_testmode_cmd)
 	CFG80211_TESTMODE_DUMP(ieee80211_testmode_dump)
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index 7fe468e..03ff0ab 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -487,6 +487,34 @@ static ssize_t ieee80211_if_fmt_num_buffered_multicast(
 }
 IEEE80211_IF_FILE_R(num_buffered_multicast);
 
+static ssize_t
+ieee80211_if_fmt_unicast(const struct ieee80211_sub_if_data *sdata,
+			 char *buf, int buflen)
+{
+	const struct ieee80211_if_ap *ifap = &sdata->u.ap;
+
+	return snprintf(buf, buflen, "0x%x\n", ifap->unicast);
+}
+
+static ssize_t
+ieee80211_if_parse_unicast(struct ieee80211_sub_if_data *sdata,
+			   const char *buf, int buflen)
+{
+	struct ieee80211_if_ap *ifap = &sdata->u.ap;
+	u8 val;
+	int ret;
+
+	ret = kstrtou8(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ifap->unicast = val ? 1 : 0;
+
+	return buflen;
+}
+
+IEEE80211_IF_FILE_RW(unicast);
+
 /* IBSS attributes */
 static ssize_t ieee80211_if_fmt_tsf(
 	const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -642,6 +670,7 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
 	DEBUGFS_ADD(dtim_count);
 	DEBUGFS_ADD(num_buffered_multicast);
 	DEBUGFS_ADD_MODE(tkip_mic_test, 0200);
+	DEBUGFS_ADD_MODE(unicast, 0600);
 }
 
 static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 933688e..99f990a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -293,6 +293,7 @@ struct ieee80211_if_ap {
 			 driver_smps_mode; /* smps mode request */
 
 	struct work_struct request_smps_work;
+	bool unicast;
 };
 
 struct ieee80211_if_wds {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index aed375f..b230aa2 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/skbuff.h>
+#include <linux/if_vlan.h>
 #include <linux/etherdevice.h>
 #include <linux/bitmap.h>
 #include <linux/rcupdate.h>
@@ -1770,6 +1771,104 @@ bool ieee80211_tx_prepare_skb(struct ieee80211_hw *hw,
 }
 EXPORT_SYMBOL(ieee80211_tx_prepare_skb);
 
+/* rewrite destination mac address */
+static int ieee80211_change_da(struct sk_buff *skb, struct sta_info *sta)
+{
+	struct ethhdr *eth;
+	int err;
+
+	err = skb_ensure_writable(skb, ETH_HLEN);
+	if (unlikely(err))
+		return err;
+
+	eth = (void *)skb->data;
+	ether_addr_copy(eth->h_dest, sta->sta.addr);
+
+	return 0;
+}
+
+/* Check if multicast to unicast conversion is needed and do it.
+ * Returns 1 if skb was freed and should not be send out. */
+static int
+ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
+				  struct sk_buff *skb, u32  info_flags)
+{
+	struct ieee80211_local *local = sdata->local;
+	const struct ethhdr *eth = (void *)skb->data;
+	const struct vlan_ethhdr *ethvlan = (void *)skb->data;
+	struct sta_info *sta, *prev = NULL;
+	struct sk_buff *cloned_skb;
+	u16 ethertype;
+
+	/* multicast to unicast conversion only for AP interfaces */
+	switch (sdata->vif.type) {
+	case NL80211_IFTYPE_AP_VLAN:
+		sta = rcu_dereference(sdata->u.vlan.sta);
+		if (sta) /* 4addr */
+			return 0;
+	case NL80211_IFTYPE_AP:
+		break;
+	default:
+		return 0;
+	}
+
+	/* check runtime toggle for this bss */
+	if (!sdata->bss->unicast)
+		return 0;
+
+	/* check if this is a multicast frame */
+	if (!is_multicast_ether_addr(eth->h_dest))
+		return 0;
+
+	/* info_flags would not get preserved, used only by TLDS */
+	if (info_flags)
+		return 0;
+
+	/* multicast to unicast conversion only for some payload */
+	ethertype = ntohs(eth->h_proto);
+	if (ethertype == ETH_P_8021Q && skb->len >= VLAN_ETH_HLEN)
+		ethertype = ntohs(ethvlan->h_vlan_encapsulated_proto);
+	switch (ethertype) {
+	case ETH_P_ARP:
+	case ETH_P_IP:
+	case ETH_P_IPV6:
+		break;
+	default:
+		return 0;
+	}
+
+	/* clone packets and update destination mac */
+	list_for_each_entry_rcu(sta, &local->sta_list, list) {
+		if (sdata != sta->sdata)
+			continue;
+		if (unlikely(!memcmp(eth->h_source, sta->sta.addr, ETH_ALEN)))
+			/* do not send back to source */
+			continue;
+		if (unlikely(is_multicast_ether_addr(sta->sta.addr))) {
+			WARN_ONCE(1, "sta with multicast address %pM",
+				  sta->sta.addr);
+			continue;
+		}
+		if (prev) {
+			cloned_skb = skb_clone(skb, GFP_ATOMIC);
+			if (likely(!ieee80211_change_da(cloned_skb, prev)))
+				ieee80211_subif_start_xmit(cloned_skb,
+							   cloned_skb->dev);
+			else
+				dev_kfree_skb(cloned_skb);
+		}
+		prev = sta;
+	}
+
+	if (likely(prev)) {
+		ieee80211_change_da(skb, prev);
+		return 0;
+	}
+
+	/* no STA connected, drop */
+	return 1;
+}
+
 /*
  * Returns false if the frame couldn't be transmitted but was queued instead.
  */
@@ -3353,6 +3452,10 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
 
 	rcu_read_lock();
 
+	/* AP multicast to unicast conversion */
+	if (ieee80211_tx_multicast_to_unicast(sdata, skb, info_flags))
+		goto out_free;
+
 	if (ieee80211_lookup_ra_sta(sdata, skb, &sta))
 		goto out_free;
 
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f02653a..2eefee7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -409,6 +409,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 		.len = VHT_MUMIMO_GROUPS_DATA_LEN
 	},
 	[NL80211_ATTR_MU_MIMO_FOLLOW_MAC_ADDR] = { .len = ETH_ALEN },
+	[NL80211_ATTR_UNICAST] = { .type = NLA_U8, },
 };
 
 /* policy for the key attributes */
@@ -1538,6 +1539,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 				goto nla_put_failure;
 		}
 		CMD(set_wds_peer, SET_WDS_PEER);
+		CMD(set_ap_unicast, SET_AP_UNICAST);
 		if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS) {
 			CMD(tdls_mgmt, TDLS_MGMT);
 			CMD(tdls_oper, TDLS_OPER);
@@ -2164,6 +2166,29 @@ static int nl80211_set_wds_peer(struct sk_buff *skb, struct genl_info *info)
 	return rdev_set_wds_peer(rdev, dev, bssid);
 }
 
+static int nl80211_set_ap_unicast(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	bool unicast;
+
+	if (!info->attrs[NL80211_ATTR_UNICAST])
+		return -EINVAL;
+
+	if (netif_running(dev))
+		return -EBUSY;
+
+	if (!rdev->ops->set_ap_unicast)
+		return -EOPNOTSUPP;
+
+	if (wdev->iftype != NL80211_IFTYPE_AP)
+		return -EOPNOTSUPP;
+
+	unicast = nla_data(info->attrs[NL80211_ATTR_UNICAST]);
+	return rdev_set_ap_unicast(rdev, dev, unicast);
+}
+
 static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev;
@@ -11574,6 +11599,14 @@ static const struct genl_ops nl80211_ops[] = {
 				  NL80211_FLAG_NEED_RTNL,
 	},
 	{
+		.cmd = NL80211_CMD_SET_AP_UNICAST,
+		.doit = nl80211_set_ap_unicast,
+		.policy = nl80211_policy,
+		.flags = GENL_UNS_ADMIN_PERM,
+		.internal_flags = NL80211_FLAG_NEED_NETDEV |
+				  NL80211_FLAG_NEED_RTNL,
+	},
+	{
 		.cmd = NL80211_CMD_JOIN_MESH,
 		.doit = nl80211_join_mesh,
 		.policy = nl80211_policy,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 85ff30b..af3ca14 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -562,6 +562,17 @@ static inline int rdev_set_wds_peer(struct cfg80211_registered_device *rdev,
 	return ret;
 }
 
+static inline int rdev_set_ap_unicast(struct cfg80211_registered_device *rdev,
+				      struct net_device *dev,
+				      const bool unicast)
+{
+	int ret;
+	trace_rdev_set_ap_unicast(&rdev->wiphy, dev, unicast);
+	ret = rdev->ops->set_ap_unicast(&rdev->wiphy, dev, unicast);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
+
 static inline void rdev_rfkill_poll(struct cfg80211_registered_device *rdev)
 {
 	trace_rdev_rfkill_poll(&rdev->wiphy);
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 72b5255..6016821 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2940,6 +2940,25 @@ DEFINE_EVENT(wiphy_wdev_evt, rdev_abort_scan,
 	TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
 	TP_ARGS(wiphy, wdev)
 );
+
+TRACE_EVENT(rdev_set_ap_unicast,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
+		 const bool unicast),
+	TP_ARGS(wiphy, netdev, unicast),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		NETDEV_ENTRY
+		__field(bool, unicast)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		NETDEV_ASSIGN;
+		__entry->unicast = unicast;
+	),
+	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", unicast: %s",
+		  WIPHY_PR_ARG, NETDEV_PR_ARG,
+		  BOOL_TO_STR(__entry->unicast))
+);
 #endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCHv3] wireless: check A-MSDU inner frame source address on AP interfaces
From: Johannes Berg @ 2016-10-05  8:14 UTC (permalink / raw)
  To: M. Braun
  Cc: kvalo, akarwar, nishants, Larry.Finger, Jes.Sorensen,
	linux-wireless, projekt-wlan
In-Reply-To: <3948ce82-05d8-5c77-0ef1-04045555f48e@fami-braun.de>

On Tue, 2016-10-04 at 23:12 +0200, M. Braun wrote:
> > 
> > Obviously, now that I think about it, your patch also would break
> > client mode since it would refuse to accept any A-MSDU with SA !=
> > TA, which is highly unlikely in most cases, since traffic doesn't
> > usually originate from the AP.
> 
> I still don't think my patch would break anything here, as it does
> only filter on AP/AP_VLAN interfaces so stations are not affected at
> all.

Yes, that's true.

> I agree that asking for more cases to be filtered seems natural. But
> is fixing all possible A-MSDU address mismatch problems required to
> fix the one I currently care about most?

Maybe not. But we're changing the API here, and doing that just a
single time would be easier, I think.

> > We verify today that only multicast frames can be encrypted with
> > the GTK, but that applies to the outer header, so we're susceptible
> > to a variant of the hole-196 attack, afaict?
> 
> That exploited that an unicast arp reply can be injected using GTK,
> thus bypassing AP filtering, right?

No, more generally, that exploits that you often can send unicast L3
(e.g. IP) frames in multicast L2 frames, so that even checking that GTK
is used only for multicast L2 frames.

This case is different, not really quite a variant thereof maybe.

But you could possibly send a unicast inner-L2 frame in a multicast
outer-L2 frame, so we should discard multicast A-MSDUs I guess? But
that actually seems like a separate problem.

> To counter, it would suffice to required multicast A-MSDU frames to
> only carry multicast (inner) MSDUs?

Or that, but I don't even think that multicast A-MSDU is allowed, see
9.11.

> I don't see how this could be inversed, that is an attack exploiting
> multicast encrypted with PTK.

Yeah, you're right, anyone who is in possession of a PTK better also be
able to send multicast frames *anyway* somehow.

> > Overall, it seems to me we should do the following:
> 
> I'm wondering if all this filtering is actually required or if it
> filters out more than required?

It seems it should be required?

> >  * pass DA == RA for client mode (not when 4-addr)
> 
> If this implies that encapsulating multicast as unicast A-MSDU is not
> permitted, then why? This would break multicast to unicast using A-
> MSDU frames, which currently works with most clients for me.

But arguably it shouldn't work unless DMS was negotiated? However, I
also don't see an attack vector right now, since, as I wrote above,
peers in possession of a PTK should have a way to send multicast frames
anyway.


> >  * pass both on TDLS links
> 
> I don't know why TDLS links should carry multicast at all, so this
> seems reasonable to me.

Yeah, they can't really carry multicast traffic.

> >  * pass both for IBSS mode (I think)
> 
> Why is multicast to unicast not permitted within IBSS?

Well, you're thinking of this only from a multicast angle and I was
thinking only from a "fake DA/SA" angle.

Combining the two, I suppose we could accept a multicast DA even when a
DA match was requested.

Obviously, when no DA match is requested, like in the AP (or VLAN)
case, this question is irrelevant. When a DA match *is* requested, i.e.
cases where we have real multicast over the air (client/IBSS/TDLS),
arguably inner multicast should *not* be accepted by spec, since the
multicast DA cannot map to a unicast RA.

I'll send out a patch or two to fix the most glaring issues here, and
we can continue discussing the inner-L2 filtering.

johannes

^ permalink raw reply

* [PATCH] mac80211: discard multicast and 4-addr A-MSDUs
From: Johannes Berg @ 2016-10-05  8:19 UTC (permalink / raw)
  To: linux-wireless; +Cc: M . Braun, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

In mac80211, multicast A-MSDUs are accepted in many cases that
they shouldn't be accepted in:
 * drop A-MSDUs with a multicast A1 (RA), as required by the
   spec in 9.11 (802.11-2012 version)
 * drop A-MSDUs with a 4-addr header, since the fourth address
   can't actually be useful for them; this was already done in
   the case where 4-addr behaviour was requested, but bizarrely
   not in the common cases

Accepting the first case, in particular, is very problematic
since it allows anyone else with possession of a GTK to send
unicast frames encapsulated in a multicast A-MSDU, even when
the AP has client isolation enabled.

Cc: stable@vger.kernel.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/rx.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 6175db385ba7..31aadc769021 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2308,16 +2308,10 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
 	if (!(status->rx_flags & IEEE80211_RX_AMSDU))
 		return RX_CONTINUE;
 
-	if (ieee80211_has_a4(hdr->frame_control) &&
-	    rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-	    !rx->sdata->u.vlan.sta)
+	if (ieee80211_has_a4(hdr->frame_control))
 		return RX_DROP_UNUSABLE;
 
-	if (is_multicast_ether_addr(hdr->addr1) &&
-	    ((rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-	      rx->sdata->u.vlan.sta) ||
-	     (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
-	      rx->sdata->u.mgd.use_4addr)))
+	if (is_multicast_ether_addr(hdr->addr1))
 		return RX_DROP_UNUSABLE;
 
 	skb->dev = dev;
-- 
2.8.1

^ permalink raw reply related

* Re: BCM43602 firmware reports multiple BRCMF_E_DEAUTH
From: Arend Van Spriel @ 2016-10-05  9:08 UTC (permalink / raw)
  To: Rafał Miłecki, linux-wireless@vger.kernel.org,
	brcm80211 development
In-Reply-To: <f97678dc-2869-1b6a-33a3-f829cc30918a@gmail.com>

On 4-10-2016 20:15, Rafał Miłecki wrote:
> On 09/28/2015 11:00 AM, Rafał Miłecki wrote:
>> I'm using recent brcmfmac and brcmfmac43602-pcie.ap.bin that currently
>> sits in linux-firmware.git.
>>
>> In OpenWrt we have hostapd with a feature of banning STAs. It works in
>> a quite simple way. Whenever hostapd gets NL80211_CMD_NEW_STATION for
>> STA that is banned it sends NL80211_CMD_DEL_STATION.
>>
>> The problem is that in such case BCM43602 firmware happens to randomly
>> send more than 1 BRCMF_E_DEAUTH event. It seems it can send random
>> amount between 1 and 3. Looks a bit like some kind of race. It's
>> nothing really critical, just makes hostapd log a bit confusing.
>>
>> Could someone at Broadcom look at firmware source to see if you can
>> fix this, please?
> 
> Hey, I didn't get any reply on this for a year. I just saw similar
> problem with
> BCM4366. Below you will find a nice log with my extra comments.
> 
> Could take a look at this issue this time, please?

Can try.

> I think it may be another problem related to the A-MPDU thing (bug?) I
> reported
> in "AMPDU stalls with brcmfmac4366b-pcie.bin triggering WARNINGs" e-mail
> thread.

So what firmware version do you have? A colleague pointed me to firmware
fix that may be related so I want to know the target string to build.
Firmware version is in the bin file:

$ hexdump -C fw.bin | tail -40

Regards,
Arend

> # My smartphone remains in the same place (1 m from the AP) but there is
> some
> # connection/A-MPDU problem.
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509120] brcmfmac:
> CONSOLE: 026970.308 ampdu_dbg: wl0.0 scb:0035ee78 tid:0
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509250] brcmfmac:
> CONSOLE: 026970.308 ampdu_dbg: wl0.0 dead_cnt 2 tx_in_transit 1 psm_mux
> 0xfff0 aqmqmap 0x0x101 aqmfifo_status 0x0x4000 fifordy 0x0 cpbusy 0x0
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509304] brcmfmac:
> CONSOLE: 026970.308 ampdu_dbg: ifsstat 0xaf nav_stat 0x0 txop 110486
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509346] brcmfmac:
> CONSOLE: 026970.308 ampdu_dbg: pktpend: 0 0 0 0 0 ap 1
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509411] brcmfmac:
> CONSOLE: 026970.308 ampdu_dbg: txall 4 txbcn 0 txrts 0 rxcts 0 rsptmout
> 0 rxstrt 0
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509477] brcmfmac:
> CONSOLE: 026970.308 ampdu_dbg: cwcur0-3 f f 7 3 bslots cur/0-3 4 0 0 0 0
> ifs_boff 0
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509527] brcmfmac:
> CONSOLE: 026970.308 ampdu_dbg: again1 ifsstat 0xaf nav_stat 0x0
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509576] brcmfmac:
> CONSOLE: 026970.308 ampdu_dbg: again2 ifsstat 0xaf nav_stat 0x0
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509665] brcmfmac:
> CONSOLE: 026970.308 wl0: wlc_ampdu_watchdog: cleaning up ini tid 0 due
> to no progress for 2 secs tx_in_transit 1
> Tue Oct  4 17:22:22 2016 kern.debug kernel: [  247.509726] brcmfmac:
> CONSOLE: 026970.308 wl0: wlc_ampdu_tx_send_delba: tid 0 initiator 1
> reason 39
> Tue Oct  4 17:22:41 2016 kern.debug kernel: [  266.456860] brcmfmac:
> CONSOLE: 026990.068 wl0.0: wlc_send_bar: seq 0x7c tid 0
> Tue Oct  4 17:22:43 2016 kern.debug kernel: [  268.178234] brcmfmac:
> CONSOLE: 026991.783 pktid is NULL
> 
> # After recovering from A-MPDU thing firmware sends BRCMF_E_DEAUTH and
> # BRCMF_E_DISASSOC_IND events.
> # My smartphone never receives deauth/disassoc and it believes it's still
> # connected to the AP.
> Tue Oct  4 17:23:24 2016 kern.debug kernel: [  309.275305] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 4
> Tue Oct  4 17:23:24 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:24 2016 kern.debug kernel: [  309.275354] brcmfmac:
> brcmf_notify_connect_status_ap event 12, reason 8
> Tue Oct  4 17:23:24 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:24 2016 kern.debug kernel: [  309.275865] brcmfmac:
> brcmf_cfg80211_del_key key index (0)
> Tue Oct  4 17:23:24 2016 kern.debug kernel: [  309.276177] brcmfmac:
> brcmf_cfg80211_del_key key index (0)
> Tue Oct  4 17:23:24 2016 kern.debug kernel: [  309.276188] brcmfmac:
> brcmf_cfg80211_del_key Ignore clearing of (never configured) key
> 
> # My smartphone starts sending packets. It seems brcmfmac refuses them
> due to
> # STA not being connected and for each packet it reports BRCMF_E_DEAUTH
> to the
> # driver.
> Tue Oct  4 17:23:58 2016 kern.debug kernel: [  343.000406] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 7
> Tue Oct  4 17:23:58 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:58 2016 kern.debug kernel: [  343.001227] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 7
> Tue Oct  4 17:23:58 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:58 2016 kern.debug kernel: [  343.001894] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 7
> Tue Oct  4 17:23:58 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:58 2016 kern.debug kernel: [  343.002594] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 7
> Tue Oct  4 17:23:58 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:58 2016 kern.debug kernel: [  343.003741] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 7
> Tue Oct  4 17:23:58 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:58 2016 kern.debug kernel: [  343.004096] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 7
> Tue Oct  4 17:23:58 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:58 2016 kern.debug kernel: [  343.004490] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 7
> Tue Oct  4 17:23:58 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> Tue Oct  4 17:23:58 2016 kern.debug kernel: [  343.004936] brcmfmac:
> brcmf_notify_connect_status_ap event 5, reason 7
> Tue Oct  4 17:23:58 2016 daemon.info hostapd: wlan1: STA
> 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
> 
> As you can see in above example I got more than just 3 BRCMF_E_DEAUTH
> reported
> originally for BCM43602 firmware. But this isn't the worst case. In one
> extreme
> situation I got 9714 of these events!
> 
> 
> Mon Oct  3 09:10:04 2016 kern.err kernel: [227426.890053] brcmfmac:
> brcmf_netdev_wait_pend8021x: Timed out waiting for no pending 802.1x
> packets
> Mon Oct  3 09:10:04 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:06 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:06 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:06 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:06 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:06 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 472 identical lines were here)
> Mon Oct  3 09:10:07 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:07 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:07 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:07 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:07 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 623 identical lines were here)
> Mon Oct  3 09:10:08 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:08 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:08 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:08 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:08 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 443 identical lines were here)
> Mon Oct  3 09:10:09 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:09 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:09 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:09 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:09 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 434 identical lines were here)
> Mon Oct  3 09:10:10 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:10 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:10 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:10 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:10 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 577 identical lines were here)
> Mon Oct  3 09:10:11 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:11 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:11 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:11 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:11 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 562 identical lines were here)
> Mon Oct  3 09:10:12 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:12 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:12 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:12 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:12 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 451 identical lines were here)
> Mon Oct  3 09:10:13 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:13 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:13 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:13 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:13 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 557 identical lines were here)
> Mon Oct  3 09:10:14 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:14 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:14 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:14 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:14 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 426 identical lines were here)
> Mon Oct  3 09:10:15 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:15 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:15 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:15 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:15 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 340 identical lines were here)
> Mon Oct  3 09:10:16 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:16 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:16 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:16 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:16 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 115 identical lines were here)
> Mon Oct  3 09:10:17 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:17 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:17 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:17 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:17 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 115 identical lines were here)
> Mon Oct  3 09:10:18 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:18 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:18 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:18 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:18 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 150 identical lines were here)
> Mon Oct  3 09:10:19 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:19 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:19 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:19 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:19 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 668 identical lines were here)
> Mon Oct  3 09:10:20 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:20 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:20 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:20 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:20 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 602 identical lines were here)
> Mon Oct  3 09:10:21 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:21 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:21 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:21 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:21 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 482 identical lines were here)
> Mon Oct  3 09:10:22 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:22 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:22 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:22 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:22 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 419 identical lines were here)
> Mon Oct  3 09:10:23 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:23 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:23 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:23 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:23 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 462 identical lines were here)
> Mon Oct  3 09:10:24 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:24 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:24 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:24 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:24 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 584 identical lines were here)
> Mon Oct  3 09:10:25 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:25 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:25 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:25 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:25 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 417 identical lines were here)
> Mon Oct  3 09:10:26 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:26 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:26 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:26 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:26 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 485 identical lines were here)
> Mon Oct  3 09:10:27 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:27 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:27 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:27 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:27 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 200 identical lines were here)
> Mon Oct  3 09:10:28 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:28 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:28 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:28 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> Mon Oct  3 09:10:28 2016 daemon.info hostapd: wlan1: STA
> 3c:15:c2:dd:ce:de IEEE 802.11: disassociated
> (another 14 identical lines were here)

^ permalink raw reply

* Re: bcmdhd: Strange Power Save messages
From: Arend Van Spriel @ 2016-10-05  9:12 UTC (permalink / raw)
  To: Gucea Doru, Krishna Chaitanya
  Cc: Arend van Spriel, Andra Paraschiv, linux-wireless
In-Reply-To: <CANfLQrbRvQb5a2AsqY0jGYeuDqC1c_7x8tUbQSm2=QPt1gT5eA@mail.gmail.com>

On 4-10-2016 13:39, Gucea Doru wrote:
> On Sat, Oct 1, 2016 at 2:52 PM, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>>
>>>
>>> On 29-09-16 13:32, Gucea Doru wrote:
>>>> On Tue, Sep 27, 2016 at 12:03 PM, Gucea Doru <gucea.doru@gmail.com> wrote:
>>>>> What is the decision triggering the exit from the PS mode immediately
>>>>> after the ping request? I am asking this because 802.11 PS legacy
>>>>> specifies that the client should wait for a beacon with TIM set in
>>>>> order to wake up: in my case, there is no beacon between the ping
>>>>> request message and the Null frame that announces the exit from the PS
>>>>> mode.
>>>>
>>>>
>>>> Any help would be highly appreciated :)
>>>
>>> Actually though I already sent you are reply, but alas here it is.
>>>
>>> bcmdhd is our aosp driver. I am maintaining the upstream brcm80211
>>> drivers. Regardless your question is more for firmware running on the
>>> device. So like the same behavior would be observed when using brcmfmac
>>> with same firmware.
>>>
>>>> IEEE Std 802.11-2012, section 10.2.1.8 specifies that "when the STA
>>>> detects that the bit corresponding to its AID is 1 i the TIM, the STA
>>>> shall issue a PS Poll". In my capture there are cases when the STA
>>>> exits the PS mode without waiting for a beacon.
>>>
>>> It is a bit tricky, but the standard does not explicitly say the STA
>>> should be in power-save at any other time. So it is difficult to say
>>> what event occurred on the STA side to exit PS mode. Also STA means
>>> P2P-Client as you say. That means that you have multiple interfaces:
>>> regular STA and P2P-Client. So is the STA connected to some other AP or
>>> just not connected. wpa_supplicant will do intermittent scan or initiate
>>> scheduled scan by which firmware will scan at a certain interval. That
>>> is just some things I can come up with and I am sure there are more.
> 
> I agree that there may be some events belonging to the regular STA
> interface that could trigger the Null Frame (which includes the exit
> from PS Mode). However, I would expect to see some management frames
> in the air before/after the Null Packet (e.g.: a Probe request in case
> of a scheduled scan). But in my case the trigger for the Null frame
> seems to be the ping request packet, the scenario is the same every
> time: ping request -> Block ACK -> Null Frame (Wireshark trace
> confirms this behavior).
> 
> I thought that you had a power save optimization algorithm that keeps
> the card on a few milliseconds just to see if we can have a fast reply
> from the peer. Does this ring a bell? :)

It does not. That would be implemented in firmware. As said I am working
on brcmfmac/brcmsmac. So bcmdhd and firmware are not my expertise.

Regards,
Arend

> On Sat, Oct 1, 2016 at 3:02 PM, Krishna Chaitanya
> <chaitanya.mgit@gmail.com> wrote:
>> Keeping the STA aside, as far as AP is concerned the STA is still in PS,
>> so it should set the TIM/DTIM bit to 1 before sending out data to the STA.
> 
> Not necessarily, see section 10.2.1.6/l from IEEE Std 802.11-2012:
> "When an AP is informed that a STA has changed to the Active mode,
> then the AP shall send buffered BUs (if any exist) to the STA without
> waiting for a PS Poll...."
> 

^ permalink raw reply

* Re: [PATCH] mac80211: discard multicast and 4-addr A-MSDUs
From: Felix Fietkau @ 2016-10-05  9:31 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless; +Cc: M . Braun, Johannes Berg
In-Reply-To: <1475655551-13504-1-git-send-email-johannes@sipsolutions.net>

On 2016-10-05 10:19, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> In mac80211, multicast A-MSDUs are accepted in many cases that
> they shouldn't be accepted in:
>  * drop A-MSDUs with a multicast A1 (RA), as required by the
>    spec in 9.11 (802.11-2012 version)
>  * drop A-MSDUs with a 4-addr header, since the fourth address
>    can't actually be useful for them; this was already done in
>    the case where 4-addr behaviour was requested, but bizarrely
>    not in the common cases
Won't this break the use of A-MSDU in existing 4-addr AP/STA setups?

- Felix

^ permalink raw reply

* Re: [PATCH] mac80211: discard multicast and 4-addr A-MSDUs
From: Johannes Berg @ 2016-10-05  9:38 UTC (permalink / raw)
  To: Felix Fietkau, linux-wireless; +Cc: M . Braun
In-Reply-To: <90c1bacf-3ea3-0486-96f4-be878a4a759e@nbd.name>


> Won't this break the use of A-MSDU in existing 4-addr AP/STA setups?

I didn't think it did, but looking closer, that does seem indeed to be
the case.

Do you remember why you explicitly added code to *not* accept 4-addr
frames in non-4addr AP_VLAN, but no other cases? This seems oddly
specific.

I can change it to accept 4-addr frames in 4-addr cases, but I'll note
that it's completely pointless to carry A4 since it will not be used
for decapsulation.

johannes

^ permalink raw reply

* [PATCH] rsi: update in boot parameters
From: Prameela Rani Garnepudi @ 2016-10-05  9:35 UTC (permalink / raw)
  To: linux-wireless
  Cc: kvalo, johannes.berg, hofrat, xypron.glpk, prameela.garnepudi,
	Prameela Rani Garnepudi

Added more clock switch fields in boot parameters configured to device

Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
---
 drivers/net/wireless/rsi/rsi_91x_mgmt.c    | 110 ++++++++++++++++++++---------
 drivers/net/wireless/rsi/rsi_boot_params.h | 101 ++++++++++++++++++--------
 2 files changed, 149 insertions(+), 62 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_mgmt.c b/drivers/net/wireless/rsi/rsi_91x_mgmt.c
index 35c14cc..046b354 100644
--- a/drivers/net/wireless/rsi/rsi_91x_mgmt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_mgmt.c
@@ -18,6 +18,7 @@
 #include "rsi_mgmt.h"
 #include "rsi_common.h"
 
+/* Bootup Parameters for 20MHz */
 static struct bootup_params boot_params_20 = {
 	.magic_number = cpu_to_le16(0x5aa5),
 	.crystal_good_time = 0x0,
@@ -28,14 +29,15 @@ static struct bootup_params boot_params_20 = {
 	.rtls_timestamp_en = 0x0,
 	.host_spi_intr_cfg = 0x0,
 	.device_clk_info = {{
+		/* WLAN params */
 		.pll_config_g = {
 			.tapll_info_g = {
-				.pll_reg_1 = cpu_to_le16((TA_PLL_N_VAL_20 << 8)|
-					      (TA_PLL_M_VAL_20)),
-				.pll_reg_2 = cpu_to_le16(TA_PLL_P_VAL_20),
+				.pll_reg_1 = cpu_to_le16((TAPLL_N_VAL_20 << 8) |
+							 (TAPLL_M_VAL_20)),
+				.pll_reg_2 = cpu_to_le16(TAPLL_P_VAL_20),
 			},
 			.pll960_info_g = {
-				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_20 << 8)|
+				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_20 << 8) |
 							 (PLL960_N_VAL_20)),
 				.pll_reg_2 = cpu_to_le16(PLL960_M_VAL_20),
 				.pll_reg_3 = 0x0,
@@ -45,21 +47,27 @@ static struct bootup_params boot_params_20 = {
 			}
 		},
 		.switch_clk_g = {
-			.switch_clk_info = cpu_to_le16(BIT(3)),
+			.switch_umac_clk = 0x0,
+			.switch_qspi_clk = 0x0,
+			.switch_slp_clk_2_32 = 0x0,
+			.switch_bbp_lmac_clk_reg = 0x1,
+			.switch_mem_ctrl_cfg = 0x0,
+			.reserved = 0x0,
 			.bbp_lmac_clk_reg_val = cpu_to_le16(0x121),
 			.umac_clock_reg_config = 0x0,
 			.qspi_uart_clock_reg_config = 0x0
 		}
 	},
+	/* Bluetooth params */
 	{
 		.pll_config_g = {
 			.tapll_info_g = {
-				.pll_reg_1 = cpu_to_le16((TA_PLL_N_VAL_20 << 8)|
-							 (TA_PLL_M_VAL_20)),
-				.pll_reg_2 = cpu_to_le16(TA_PLL_P_VAL_20),
+				.pll_reg_1 = cpu_to_le16((TAPLL_N_VAL_20 << 8) |
+							 (TAPLL_M_VAL_20)),
+				.pll_reg_2 = cpu_to_le16(TAPLL_P_VAL_20),
 			},
 			.pll960_info_g = {
-				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_20 << 8)|
+				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_20 << 8) |
 							 (PLL960_N_VAL_20)),
 				.pll_reg_2 = cpu_to_le16(PLL960_M_VAL_20),
 				.pll_reg_3 = 0x0,
@@ -69,21 +77,27 @@ static struct bootup_params boot_params_20 = {
 			}
 		},
 		.switch_clk_g = {
-			.switch_clk_info = 0x0,
+			.switch_umac_clk = 0x0,
+			.switch_qspi_clk = 0x0,
+			.switch_slp_clk_2_32 = 0x0,
+			.switch_bbp_lmac_clk_reg = 0x0,
+			.switch_mem_ctrl_cfg = 0x0,
+			.reserved = 0x0,
 			.bbp_lmac_clk_reg_val = 0x0,
 			.umac_clock_reg_config = 0x0,
 			.qspi_uart_clock_reg_config = 0x0
 		}
 	},
+	/* Zigbee params */
 	{
 		.pll_config_g = {
 			.tapll_info_g = {
-				.pll_reg_1 = cpu_to_le16((TA_PLL_N_VAL_20 << 8)|
-							 (TA_PLL_M_VAL_20)),
-				.pll_reg_2 = cpu_to_le16(TA_PLL_P_VAL_20),
+				.pll_reg_1 = cpu_to_le16((TAPLL_N_VAL_20 << 8) |
+							 (TAPLL_M_VAL_20)),
+				.pll_reg_2 = cpu_to_le16(TAPLL_P_VAL_20),
 			},
 			.pll960_info_g = {
-				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_20 << 8)|
+				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_20 << 8) |
 							 (PLL960_N_VAL_20)),
 				.pll_reg_2 = cpu_to_le16(PLL960_M_VAL_20),
 				.pll_reg_3 = 0x0,
@@ -93,12 +107,18 @@ static struct bootup_params boot_params_20 = {
 			}
 		},
 		.switch_clk_g = {
-			.switch_clk_info = 0x0,
+			.switch_umac_clk = 0x0,
+			.switch_qspi_clk = 0x0,
+			.switch_slp_clk_2_32 = 0x0,
+			.switch_bbp_lmac_clk_reg = 0x0,
+			.switch_mem_ctrl_cfg = 0x0,
+			.reserved = 0x0,
 			.bbp_lmac_clk_reg_val = 0x0,
 			.umac_clock_reg_config = 0x0,
 			.qspi_uart_clock_reg_config = 0x0
 		}
 	} },
+	/* ULP Params */
 	.buckboost_wakeup_cnt = 0x0,
 	.pmu_wakeup_wait = 0x0,
 	.shutdown_wait_time = 0x0,
@@ -106,9 +126,13 @@ static struct bootup_params boot_params_20 = {
 	.wdt_prog_value = 0x0,
 	.wdt_soc_rst_delay = 0x0,
 	.dcdc_operation_mode = 0x0,
-	.soc_reset_wait_cnt = 0x0
+	.soc_reset_wait_cnt = 0x0,
+	.waiting_time_at_fresh_sleep = 0x0,
+	.max_threshold_to_avoid_sleep = 0x0,
+	.beacon_resedue_alg_en = 0,
 };
 
+/* Bootup parameters for 40MHz */
 static struct bootup_params boot_params_40 = {
 	.magic_number = cpu_to_le16(0x5aa5),
 	.crystal_good_time = 0x0,
@@ -119,14 +143,15 @@ static struct bootup_params boot_params_40 = {
 	.rtls_timestamp_en = 0x0,
 	.host_spi_intr_cfg = 0x0,
 	.device_clk_info = {{
+		/* WLAN params */
 		.pll_config_g = {
 			.tapll_info_g = {
-				.pll_reg_1 = cpu_to_le16((TA_PLL_N_VAL_40 << 8)|
-							 (TA_PLL_M_VAL_40)),
-				.pll_reg_2 = cpu_to_le16(TA_PLL_P_VAL_40),
+				.pll_reg_1 = cpu_to_le16((TAPLL_N_VAL_40 << 8) |
+							 (TAPLL_M_VAL_40)),
+				.pll_reg_2 = cpu_to_le16(TAPLL_P_VAL_40),
 			},
 			.pll960_info_g = {
-				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_40 << 8)|
+				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_40 << 8) |
 							 (PLL960_N_VAL_40)),
 				.pll_reg_2 = cpu_to_le16(PLL960_M_VAL_40),
 				.pll_reg_3 = 0x0,
@@ -136,21 +161,27 @@ static struct bootup_params boot_params_40 = {
 			}
 		},
 		.switch_clk_g = {
-			.switch_clk_info = cpu_to_le16(0x09),
+			.switch_umac_clk = 0x1,
+			.switch_qspi_clk = 0x0,
+			.switch_slp_clk_2_32 = 0x0,
+			.switch_bbp_lmac_clk_reg = 0x1,
+			.switch_mem_ctrl_cfg = 0x0,
+			.reserved = 0x0,
 			.bbp_lmac_clk_reg_val = cpu_to_le16(0x1121),
 			.umac_clock_reg_config = cpu_to_le16(0x48),
 			.qspi_uart_clock_reg_config = 0x0
 		}
 	},
+	/* Bluetooth Params */
 	{
 		.pll_config_g = {
 			.tapll_info_g = {
-				.pll_reg_1 = cpu_to_le16((TA_PLL_N_VAL_40 << 8)|
-							 (TA_PLL_M_VAL_40)),
-				.pll_reg_2 = cpu_to_le16(TA_PLL_P_VAL_40),
+				.pll_reg_1 = cpu_to_le16((TAPLL_N_VAL_40 << 8) |
+							 (TAPLL_M_VAL_40)),
+				.pll_reg_2 = cpu_to_le16(TAPLL_P_VAL_40),
 			},
 			.pll960_info_g = {
-				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_40 << 8)|
+				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_40 << 8) |
 							 (PLL960_N_VAL_40)),
 				.pll_reg_2 = cpu_to_le16(PLL960_M_VAL_40),
 				.pll_reg_3 = 0x0,
@@ -160,21 +191,27 @@ static struct bootup_params boot_params_40 = {
 			}
 		},
 		.switch_clk_g = {
-			.switch_clk_info = 0x0,
+			.switch_umac_clk = 0x0,
+			.switch_qspi_clk = 0x0,
+			.switch_slp_clk_2_32 = 0x0,
+			.switch_bbp_lmac_clk_reg = 0x0,
+			.switch_mem_ctrl_cfg = 0x0,
+			.reserved = 0x0,
 			.bbp_lmac_clk_reg_val = 0x0,
 			.umac_clock_reg_config = 0x0,
 			.qspi_uart_clock_reg_config = 0x0
 		}
 	},
+	/* Zigbee Params */
 	{
 		.pll_config_g = {
 			.tapll_info_g = {
-				.pll_reg_1 = cpu_to_le16((TA_PLL_N_VAL_40 << 8)|
-							 (TA_PLL_M_VAL_40)),
-				.pll_reg_2 = cpu_to_le16(TA_PLL_P_VAL_40),
+				.pll_reg_1 = cpu_to_le16((TAPLL_N_VAL_40 << 8) |
+							 (TAPLL_M_VAL_40)),
+				.pll_reg_2 = cpu_to_le16(TAPLL_P_VAL_40),
 			},
 			.pll960_info_g = {
-				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_40 << 8)|
+				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_40 << 8) |
 							 (PLL960_N_VAL_40)),
 				.pll_reg_2 = cpu_to_le16(PLL960_M_VAL_40),
 				.pll_reg_3 = 0x0,
@@ -184,12 +221,18 @@ static struct bootup_params boot_params_40 = {
 			}
 		},
 		.switch_clk_g = {
-			.switch_clk_info = 0x0,
+			.switch_umac_clk = 0x0,
+			.switch_qspi_clk = 0x0,
+			.switch_slp_clk_2_32 = 0x0,
+			.switch_bbp_lmac_clk_reg = 0x0,
+			.switch_mem_ctrl_cfg = 0x0,
+			.reserved = 0x0,
 			.bbp_lmac_clk_reg_val = 0x0,
 			.umac_clock_reg_config = 0x0,
 			.qspi_uart_clock_reg_config = 0x0
 		}
 	} },
+	/* ULP Params */
 	.buckboost_wakeup_cnt = 0x0,
 	.pmu_wakeup_wait = 0x0,
 	.shutdown_wait_time = 0x0,
@@ -197,7 +240,10 @@ static struct bootup_params boot_params_40 = {
 	.wdt_prog_value = 0x0,
 	.wdt_soc_rst_delay = 0x0,
 	.dcdc_operation_mode = 0x0,
-	.soc_reset_wait_cnt = 0x0
+	.soc_reset_wait_cnt = 0x0,
+	.waiting_time_at_fresh_sleep = 0x0,
+	.max_threshold_to_avoid_sleep = 0x0,
+	.beacon_resedue_alg_en = 0,
 };
 
 static u16 mcs[] = {13, 26, 39, 52, 78, 104, 117, 130};
diff --git a/drivers/net/wireless/rsi/rsi_boot_params.h b/drivers/net/wireless/rsi/rsi_boot_params.h
index 5e2721f..9f5d61d 100644
--- a/drivers/net/wireless/rsi/rsi_boot_params.h
+++ b/drivers/net/wireless/rsi/rsi_boot_params.h
@@ -17,71 +17,77 @@
 #ifndef __RSI_BOOTPARAMS_HEADER_H__
 #define __RSI_BOOTPARAMS_HEADER_H__
 
-#define CRYSTAL_GOOD_TIME                BIT(0)
-#define BOOTUP_MODE_INFO                 BIT(1)
-#define WIFI_TAPLL_CONFIGS               BIT(5)
-#define WIFI_PLL960_CONFIGS              BIT(6)
-#define WIFI_AFEPLL_CONFIGS              BIT(7)
-#define WIFI_SWITCH_CLK_CONFIGS          BIT(8)
+#define CRYSTAL_GOOD_TIME		BIT(0)
+#define BOOTUP_MODE_INFO                BIT(1)
+#define WIFI_TAPLL_CONFIGS              BIT(5)
+#define WIFI_PLL960_CONFIGS             BIT(6)
+#define WIFI_AFEPLL_CONFIGS             BIT(7)
+#define WIFI_SWITCH_CLK_CONFIGS         BIT(8)
 
-#define TA_PLL_M_VAL_20                  8
-#define TA_PLL_N_VAL_20                  1
-#define TA_PLL_P_VAL_20                  4
+#define TAPLL_M_VAL_20			8
+#define TAPLL_N_VAL_20			1
+#define TAPLL_P_VAL_20			4
 
-#define PLL960_M_VAL_20                  0x14
-#define PLL960_N_VAL_20                  0
-#define PLL960_P_VAL_20                  5
+#define PLL960_M_VAL_20                 0x14
+#define PLL960_N_VAL_20                 0
+#define PLL960_P_VAL_20                 5
 
-#define UMAC_CLK_40MHZ                   40
+#define UMAC_CLK_40MHZ                  40
 
-#define TA_PLL_M_VAL_40                  46
-#define TA_PLL_N_VAL_40                  3
-#define TA_PLL_P_VAL_40                  3
+#define TAPLL_M_VAL_40                  46
+#define TAPLL_N_VAL_40                  3
+#define TAPLL_P_VAL_40                  3
 
-#define PLL960_M_VAL_40                  0x14
-#define PLL960_N_VAL_40                  0
-#define PLL960_P_VAL_40                  5
+#define PLL960_M_VAL_40                 0x14
+#define PLL960_N_VAL_40                 0
+#define PLL960_P_VAL_40                 5
 
 #define UMAC_CLK_20BW \
-	(((TA_PLL_M_VAL_20 + 1) * 40) / \
-	 ((TA_PLL_N_VAL_20 + 1) * (TA_PLL_P_VAL_20 + 1)))
+	(((TAPLL_M_VAL_20 + 1) * 40) / \
+	 ((TAPLL_N_VAL_20 + 1) * (TAPLL_P_VAL_20 + 1)))
 #define VALID_20 \
 	(WIFI_PLL960_CONFIGS | WIFI_AFEPLL_CONFIGS | WIFI_SWITCH_CLK_CONFIGS)
 #define UMAC_CLK_40BW   \
-	(((TA_PLL_M_VAL_40 + 1) * 40) / \
-	 ((TA_PLL_N_VAL_40 + 1) * (TA_PLL_P_VAL_40 + 1)))
+	(((TAPLL_M_VAL_40 + 1) * 40) / \
+	 ((TAPLL_N_VAL_40 + 1) * (TAPLL_P_VAL_40 + 1)))
 #define VALID_40 \
 	(WIFI_PLL960_CONFIGS | WIFI_AFEPLL_CONFIGS | WIFI_SWITCH_CLK_CONFIGS | \
 	 WIFI_TAPLL_CONFIGS | CRYSTAL_GOOD_TIME | BOOTUP_MODE_INFO)
 
-/* structure to store configs related to TAPLL programming */
+/* TAPLL programming configurations */
 struct tapll_info {
 	__le16 pll_reg_1;
 	__le16 pll_reg_2;
 } __packed;
 
-/* structure to store configs related to PLL960 programming */
+/* PLL960 programming configurations */
 struct pll960_info {
 	__le16 pll_reg_1;
 	__le16 pll_reg_2;
 	__le16 pll_reg_3;
 } __packed;
 
-/* structure to store configs related to AFEPLL programming */
+/* AFEPLL programming configurations */
 struct afepll_info {
 	__le16 pll_reg;
 } __packed;
 
-/* structure to store configs related to pll configs */
+/* PLL configurations */
 struct pll_config {
 	struct tapll_info tapll_info_g;
 	struct pll960_info pll960_info_g;
 	struct afepll_info afepll_info_g;
 } __packed;
 
-/* structure to store configs related to UMAC clk programming */
+/* UMAC clk programming configurations */
 struct switch_clk {
-	__le16 switch_clk_info;
+	__le16 switch_umac_clk : 1; /* If set rest is valid */
+	__le16 switch_qspi_clk : 1; /* If set qspi clk will be changed */
+	__le16 switch_slp_clk_2_32 : 1;
+	__le16 switch_bbp_lmac_clk_reg : 1;
+	__le16 switch_mem_ctrl_cfg : 1;
+	__le16 reserved : 11;
+
 	/* If switch_bbp_lmac_clk_reg is set then this value will be programmed
 	 * into reg
 	 */
@@ -99,11 +105,43 @@ struct device_clk_info {
 
 struct bootup_params {
 	__le16 magic_number;
+#define LOADED_TOKEN  0x5AA5   /* Bootup params are installed by host
+				* or OTP/FLASH (Bootloader)
+				*/
+#define ROM_TOKEN     0x55AA   /* Bootup params are taken from ROM
+				* itself in MCU mode.
+				*/
 	__le16 crystal_good_time;
 	__le32 valid;
+#define CRYSTAL_GOOD_TIME                BIT(0)
+#define BOOTUP_MODE_INFO                 BIT(1)
+#define DIGITAL_LOOP_BACK_PARAMS         BIT(2)
+#define RTLS_TIMESTAMP_EN                BIT(3)
+#define HOST_SPI_INTR_CFG                BIT(4)
+#define WIFI_TAPLL_CONFIGS               BIT(5)
+#define WIFI_PLL960_CONFIGS              BIT(6)
+#define WIFI_AFEPLL_CONFIGS              BIT(7)
+#define WIFI_SWITCH_CLK_CONFIGS          BIT(8)
+#define BT_TAPLL_CONFIGS                 BIT(9)
+#define BT_PLL960_CONFIGS                BIT(10)
+#define BT_AFEPLL_CONFIGS                BIT(11)
+#define BT_SWITCH_CLK_CONFIGS            BIT(12)
+#define ZB_TAPLL_CONFIGS                 BIT(13)
+#define ZB_PLL960_CONFIGS                BIT(14)
+#define ZB_AFEPLL_CONFIGS                BIT(15)
+#define ZB_SWITCH_CLK_CONFIGS            BIT(16)
+#define BUCKBOOST_WAIT_INFO              BIT(17)
+#define PMU_WAKEUP_SHUTDOWN_W            BIT(18)
+#define WDT_PROG_VALUES                  BIT(19)
+#define WDT_RESET_DELAY_VALUE            BIT(20)
+#define DCDC_OPERATION_MODE_VALID        BIT(21)
+#define PMU_SLP_CLKOUT_SEL               BIT(22)
+#define SOC_RESET_WAIT_CNT               BIT(23)
 	__le32 reserved_for_valids;
 	__le16 bootup_mode_info;
-	/* configuration used for digital loop back */
+#define BT_COEXIST                       BIT(0)
+#define BOOTUP_MODE                     (BIT(2) | BIT(1))
+#define CUR_DEV_MODE                    (bootup_params.bootup_mode_info >> 1)
 	__le16 digital_loop_back_params;
 	__le16 rtls_timestamp_en;
 	__le16 host_spi_intr_cfg;
@@ -122,5 +160,8 @@ struct bootup_params {
 	/* dcdc modes configs */
 	__le32 dcdc_operation_mode;
 	__le32 soc_reset_wait_cnt;
+	__le32 waiting_time_at_fresh_sleep;
+	__le32 max_threshold_to_avoid_sleep;
+	u8 beacon_resedue_alg_en;
 } __packed;
 #endif
-- 
2.4.11

^ permalink raw reply related

* Re: [PATCH] mac80211: discard multicast and 4-addr A-MSDUs
From: Johannes Berg @ 2016-10-05  9:39 UTC (permalink / raw)
  To: Felix Fietkau, linux-wireless; +Cc: M . Braun
In-Reply-To: <1475660290.4994.21.camel@sipsolutions.net>


> Do you remember why you explicitly added code to *not* accept 4-addr
> frames in non-4addr AP_VLAN, but no other cases? This seems oddly
> specific.
> 

Oh, maybe that's because more checks are done
in __ieee80211_data_to_8023().

johannes

^ permalink raw reply

* [PATCH v2] mac80211: discard multicast and 4-addr A-MSDUs
From: Johannes Berg @ 2016-10-05  9:51 UTC (permalink / raw)
  To: linux-wireless; +Cc: M . Braun, Felix Fietkau, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

In mac80211, multicast A-MSDUs are accepted in many cases that
they shouldn't be accepted in:
 * drop A-MSDUs with a multicast A1 (RA), as required by the
   spec in 9.11 (802.11-2012 version)
 * drop A-MSDUs with a 4-addr header, since the fourth address
   can't actually be useful for them; unless 4-address frame
   format is actually requested, even though the fourth address
   is still not useful in this case, but ignored

Accepting the first case, in particular, is very problematic
since it allows anyone else with possession of a GTK to send
unicast frames encapsulated in a multicast A-MSDU, even when
the AP has client isolation enabled.

Cc: stable@vger.kernel.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/rx.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 6175db385ba7..3ac2f1cba317 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2308,16 +2308,22 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
 	if (!(status->rx_flags & IEEE80211_RX_AMSDU))
 		return RX_CONTINUE;
 
-	if (ieee80211_has_a4(hdr->frame_control) &&
-	    rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-	    !rx->sdata->u.vlan.sta)
-		return RX_DROP_UNUSABLE;
+	if (unlikely(ieee80211_has_a4(hdr->frame_control))) {
+		switch (rx->sdata->vif.type) {
+		case NL80211_IFTYPE_AP_VLAN:
+			if (!rx->sdata->u.vlan.sta)
+				return RX_DROP_UNUSABLE;
+			break;
+		case NL80211_IFTYPE_STATION:
+			if (!rx->sdata->u.mgd.use_4addr)
+				return RX_DROP_UNUSABLE;
+			break;
+		default:
+			return RX_DROP_UNUSABLE;
+		}
+	}
 
-	if (is_multicast_ether_addr(hdr->addr1) &&
-	    ((rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-	      rx->sdata->u.vlan.sta) ||
-	     (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
-	      rx->sdata->u.mgd.use_4addr)))
+	if (is_multicast_ether_addr(hdr->addr1))
 		return RX_DROP_UNUSABLE;
 
 	skb->dev = dev;
-- 
2.8.1

^ permalink raw reply related

* Re: [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN
From: Johannes Berg @ 2016-10-05  9:56 UTC (permalink / raw)
  To: Michael Braun; +Cc: linux-wireless, projekt-wlan
In-Reply-To: <1475643324-2845-2-git-send-email-michael-dev@fami-braun.de>


> Therefore, two new num_mcast_sta like counters are added: one for the
> number of authorized stations connected to an AP_VLAN interface and
> one for the number of authorized stations connected to the bss and
> not assigned to any AP_VLAN. The already existing num_mcast_sta
> counter is left unchanged as it is used by SMPS.

I think this is no longer accurate?

johannes

^ permalink raw reply

* Re: [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN
From: Johannes Berg @ 2016-10-05 10:01 UTC (permalink / raw)
  To: Michael Braun; +Cc: linux-wireless, projekt-wlan
In-Reply-To: <1475643324-2845-2-git-send-email-michael-dev@fami-braun.de>

Please also add "v3" to the subject line, e.g. using --subject-prefix
'PATCH v3' when using git send-email.

> +static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
> +{
> +	// add num_mcast_sta_vlan using name num_mcast_sta

Please don't use // style comments.

> +static inline void
> +ieee80211_vif_inc_num_mcast(struct ieee80211_sub_if_data *sdata)
> +{
> +	if (sdata->vif.type != NL80211_IFTYPE_AP &&
> +	    sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
> +		return;

That's pointless, given this:

> +	if (sdata->vif.type == NL80211_IFTYPE_AP)
> +		atomic_inc(&sdata->u.ap.num_mcast_sta);
> +	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
> +		atomic_inc(&sdata->u.vlan.num_mcast_sta);
> +}
> +
> +static inline void
> +ieee80211_vif_dec_num_mcast(struct ieee80211_sub_if_data *sdata)
> +{
> +	if (sdata->vif.type != NL80211_IFTYPE_AP &&
> +	    sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
> +		return;
> +
> +	if (sdata->vif.type == NL80211_IFTYPE_AP)
> +		atomic_dec(&sdata->u.ap.num_mcast_sta);
> +	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
> +		atomic_dec(&sdata->u.vlan.num_mcast_sta);

Same here.

> +}
> +
> +/* This function returns the number of multicast stations connected
> to this
> + * interface. It returns -1 if that number is not tracked, that is
> for netdevs
> + * not in AP or AP_VLAN mode or when using 4addr. */
> +static inline int
> +ieee80211_vif_get_num_mcast_if(struct ieee80211_sub_if_data *sdata)
> +{
> +	if (sdata->vif.type == NL80211_IFTYPE_AP)
> +		return atomic_read(&sdata->u.ap.num_mcast_sta);
> +	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
> +		 !sdata->u.vlan.sta)
> +		return atomic_read(&sdata->u.vlan.num_mcast_sta);
> +	else
> +		return -1;

All the "else" branches are useless since you return immediately inside
each if.

> -       } else if (unlikely(tx->sdata->vif.type == NL80211_IFTYPE_AP &&
> -                           ieee80211_is_data(hdr->frame_control) &&
> -                           !atomic_read(&tx->sdata->u.ap.num_mcast_sta))) {
> +       } else if (unlikely(ieee80211_vif_get_num_mcast_if(tx->sdata) == 0 &&
> +                           ieee80211_is_data(hdr->frame_control))) {

any particular reason to invert the order of the checks? seems checking
for data first should be faster/cheaper from the cache, than accessing
the counters?

johannes

^ permalink raw reply

* [PATCHv4] mac80211: check A-MSDU inner frame source address on AP interfaces
From: Michael Braun @ 2016-10-05 10:02 UTC (permalink / raw)
  To: johannes
  Cc: Michael Braun, linux-wireless, projekt-wlan, kvalo, akarwar,
	nishants, Larry.Finger, Jes.Sorensen

When using WPA security, the station and thus the required key is
identified by its mac address when packets are received. So a
station usually cannot spoof its source mac address.

But when a station sends an A-MSDU frame, port control and crypto
is done using the outer mac address, while the packets delivered
and forwarded use the inner mac address.
This might affect ARP/IP filtering on the AccessPoint.

IEEE 802.11-2012 mandates that the outer source mac address should
match the inner source address (section 8.3.2.2). For the destination
mac address, matching is not required, as a wifi client may send all
its traffic to the AP in order to have it forwarded.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>

To: johannes@sipsolutions.net
Cc: linux-wireless@vger.kernel.org
Cc: projekt-wlan@fem.tu-ilmenau.de
Cc: kvalo@codeaurora.org
Cc: akarwar@marvell.com
Cc: nishants@marvell.com
Cc: Larry.Finger@lwfinger.net
Cc: Jes.Sorensen@redhat.com

v4: check for IBSS, STATION, TDLS data frame
---
 drivers/net/wireless/intel/iwlwifi/mvm/d3.c        |  3 +-
 .../net/wireless/marvell/mwifiex/11n_rxreorder.c   |  3 +-
 drivers/staging/rtl8723au/core/rtw_recv.c          |  2 +-
 include/net/cfg80211.h                             | 16 ++++---
 net/mac80211/rx.c                                  | 25 ++++++++---
 net/wireless/util.c                                | 52 +++++++++++++++-------
 6 files changed, 70 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
index 4fdc3da..05dcaef 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
@@ -1436,7 +1436,8 @@ static void iwl_mvm_report_wakeup_reasons(struct iwl_mvm *mvm,
 
 			memcpy(skb_put(pkt, pktsize), pktdata, pktsize);
 
-			if (ieee80211_data_to_8023(pkt, vif->addr, vif->type))
+			if (ieee80211_data_to_8023(pkt, NULL, vif->addr,
+						   vif->type))
 				goto report;
 			wakeup.packet = pkt->data;
 			wakeup.packet_present_len = pkt->len;
diff --git a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
index a74cc43..298e447 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
@@ -45,7 +45,8 @@ static int mwifiex_11n_dispatch_amsdu_pkt(struct mwifiex_private *priv,
 		skb_trim(skb, le16_to_cpu(local_rx_pd->rx_pkt_length));
 
 		ieee80211_amsdu_to_8023s(skb, &list, priv->curr_addr,
-					 priv->wdev.iftype, 0, false);
+					 priv->wdev.iftype, 0, NULL, NULL, 0,
+					 0);
 
 		while (!skb_queue_empty(&list)) {
 			struct rx_packet_hdr *rx_hdr;
diff --git a/drivers/staging/rtl8723au/core/rtw_recv.c b/drivers/staging/rtl8723au/core/rtw_recv.c
index 150dabc..8f39a8b 100644
--- a/drivers/staging/rtl8723au/core/rtw_recv.c
+++ b/drivers/staging/rtl8723au/core/rtw_recv.c
@@ -1687,7 +1687,7 @@ int amsdu_to_msdu(struct rtw_adapter *padapter, struct recv_frame *prframe)
 	skb_pull(skb, prframe->attrib.hdrlen);
 	__skb_queue_head_init(&skb_list);
 
-	ieee80211_amsdu_to_8023s(skb, &skb_list, NULL, 0, 0, false);
+	ieee80211_amsdu_to_8023s(skb, &skb_list, NULL, 0, 0, NULL, NULL, 0, 0);
 
 	while (!skb_queue_empty(&skb_list)) {
 		sub_skb = __skb_dequeue(&skb_list);
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index beb7610..b550314 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3905,12 +3905,13 @@ unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr);
 /**
  * ieee80211_data_to_8023 - convert an 802.11 data frame to 802.3
  * @skb: the 802.11 data frame
+ * @ehdr: (out) buffer for source/destination address (optional)
  * @addr: the device MAC address
  * @iftype: the virtual interface type
  * Return: 0 on success. Non-zero on error.
  */
-int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
-			   enum nl80211_iftype iftype);
+int ieee80211_data_to_8023(struct sk_buff *skb, struct ethhdr *ehdr,
+			   const u8 *addr, enum nl80211_iftype iftype);
 
 /**
  * ieee80211_data_from_8023 - convert an 802.3 frame to 802.11
@@ -3938,12 +3939,17 @@ int ieee80211_data_from_8023(struct sk_buff *skb, const u8 *addr,
  * @addr: The device MAC address.
  * @iftype: The device interface type.
  * @extra_headroom: The hardware extra headroom for SKBs in the @list.
- * @has_80211_header: Set it true if SKB is with IEEE 802.11 header.
+ * @ta: transmitter address (or NULL)
+ * @ra: receiver address (or NULL)
+ * @is_4addr: indicates that interface is in 4addr mode
+ * @is_tlds_data: indicates that frame as been received from TLDS peer
  */
 void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
-			      const u8 *addr, enum nl80211_iftype iftype,
+			      const u8 *addr,
+			      enum nl80211_iftype iftype,
 			      const unsigned int extra_headroom,
-			      bool has_80211_header);
+			      const u8 *ta, const u8 *ra, bool is_4addr,
+			      bool is_tdls_data);
 
 /**
  * cfg80211_classify8021d - determine the 802.1p/1d tag for a data frame
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 9dce3b1..3d8d889 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2090,7 +2090,8 @@ __ieee80211_data_to_8023(struct ieee80211_rx_data *rx, bool *port_control)
 	    sdata->vif.type == NL80211_IFTYPE_AP_VLAN && sdata->u.vlan.sta)
 		return -1;
 
-	ret = ieee80211_data_to_8023(rx->skb, sdata->vif.addr, sdata->vif.type);
+	ret = ieee80211_data_to_8023(rx->skb, NULL, sdata->vif.addr,
+				     sdata->vif.type);
 	if (ret < 0)
 		return ret;
 
@@ -2243,6 +2244,9 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
 	__le16 fc = hdr->frame_control;
 	struct sk_buff_head frame_list;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(rx->skb);
+	struct ethhdr eth_80211;
+	bool is_4addr;
+	bool is_tdls_data;
 
 	if (unlikely(!ieee80211_is_data(fc)))
 		return RX_CONTINUE;
@@ -2258,19 +2262,26 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
 	    !rx->sdata->u.vlan.sta)
 		return RX_DROP_UNUSABLE;
 
-	if (is_multicast_ether_addr(hdr->addr1) &&
-	    ((rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-	      rx->sdata->u.vlan.sta) ||
-	     (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
-	      rx->sdata->u.mgd.use_4addr)))
+	is_4addr = ((rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+		     rx->sdata->u.vlan.sta) ||
+		    (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
+		     rx->sdata->u.mgd.use_4addr));
+	if (is_multicast_ether_addr(hdr->addr1) && is_4addr)
 		return RX_DROP_UNUSABLE;
 
 	skb->dev = dev;
 	__skb_queue_head_init(&frame_list);
 
+	if (ieee80211_data_to_8023(skb, &eth_80211, dev->dev_addr,
+				   rx->sdata->vif.type) < 0)
+		return RX_DROP_UNUSABLE;
+
+	is_tdls_data = !ieee80211_has_tods(fc) && !ieee80211_has_fromds(fc);
 	ieee80211_amsdu_to_8023s(skb, &frame_list, dev->dev_addr,
 				 rx->sdata->vif.type,
-				 rx->local->hw.extra_tx_headroom, true);
+				 rx->local->hw.extra_tx_headroom,
+				 eth_80211.h_source,
+				 eth_80211.h_dest, is_4addr, is_tdls_data);
 
 	while (!skb_queue_empty(&frame_list)) {
 		rx->skb = __skb_dequeue(&frame_list);
diff --git a/net/wireless/util.c b/net/wireless/util.c
index b7d1592..84da5c2 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -414,8 +414,8 @@ unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr)
 }
 EXPORT_SYMBOL(ieee80211_get_mesh_hdrlen);
 
-static int __ieee80211_data_to_8023(struct sk_buff *skb, struct ethhdr *ehdr,
-				    const u8 *addr, enum nl80211_iftype iftype)
+int ieee80211_data_to_8023(struct sk_buff *skb, struct ethhdr *ehdr,
+			   const u8 *addr, enum nl80211_iftype iftype)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	struct {
@@ -519,12 +519,6 @@ static int __ieee80211_data_to_8023(struct sk_buff *skb, struct ethhdr *ehdr,
 
 	return 0;
 }
-
-int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
-			   enum nl80211_iftype iftype)
-{
-	return __ieee80211_data_to_8023(skb, NULL, addr, iftype);
-}
 EXPORT_SYMBOL(ieee80211_data_to_8023);
 
 int ieee80211_data_from_8023(struct sk_buff *skb, const u8 *addr,
@@ -738,24 +732,41 @@ __ieee80211_amsdu_copy(struct sk_buff *skb, unsigned int hlen,
 }
 
 void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
-			      const u8 *addr, enum nl80211_iftype iftype,
+			      const u8 *addr,
+			      enum nl80211_iftype iftype,
 			      const unsigned int extra_headroom,
-			      bool has_80211_header)
+			      const u8 *ta, const u8 *ra, bool is_4addr,
+			      bool is_tdls_data)
 {
 	unsigned int hlen = ALIGN(extra_headroom, 4);
 	struct sk_buff *frame = NULL;
 	u16 ethertype;
 	u8 *payload;
-	int offset = 0, remaining, err;
+	int offset = 0, remaining;
 	struct ethhdr eth;
 	bool reuse_frag = skb->head_frag && !skb_has_frag_list(skb);
 	bool reuse_skb = false;
 	bool last = false;
 
-	if (has_80211_header) {
-		err = __ieee80211_data_to_8023(skb, &eth, addr, iftype);
-		if (err)
-			goto out;
+	/* limit inner src/dst checks depending on iftype */
+	switch (iftype) {
+	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_AP_VLAN:
+		if (is_4addr)
+			ta = NULL;
+		ra = NULL;
+		break;
+	case NL80211_IFTYPE_ADHOC:
+		break;
+	case NL80211_IFTYPE_STATION:
+		if (is_4addr || !is_tdls_data)
+			ta = NULL;
+		if (is_4addr)
+			ra = NULL;
+		break;
+	default:
+		ta = NULL;
+		ra = NULL;
 	}
 
 	while (!last) {
@@ -768,6 +779,16 @@ void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
 		subframe_len = sizeof(struct ethhdr) + len;
 		padding = (4 - subframe_len) & 0x3;
 
+		if (unlikely(ta && !ether_addr_equal(ta, eth.h_source)))
+			goto purge;
+		/* for unicast frames, we accept multicast inner MSDUs
+		 * to enable multicast to unicast conversion
+		 */
+		if (unlikely(ra && !ether_addr_equal(ra, eth.h_dest) &&
+			     (is_multicast_ether_addr(ra) ||
+			      !is_multicast_ether_addr(eth.h_dest))))
+			goto purge;
+
 		/* the last MSDU has no padding */
 		remaining = skb->len - offset;
 		if (subframe_len > remaining)
@@ -813,7 +834,6 @@ void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
 
  purge:
 	__skb_queue_purge(list);
- out:
 	dev_kfree_skb(skb);
 }
 EXPORT_SYMBOL(ieee80211_amsdu_to_8023s);
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH 3/3] mac80211: multicast to unicast conversion
From: Johannes Berg @ 2016-10-05 10:19 UTC (permalink / raw)
  To: Michael Braun; +Cc: linux-wireless, projekt-wlan, netdev
In-Reply-To: <1475643324-2845-3-git-send-email-michael-dev@fami-braun.de>

+netdev

> IEEE802.11-2012 proposes directed multicast service (DMS) using A-
> MSDU frames and a station initiated control protocol. It has the
> advantage that the station can recover the destination multicast mac
> address, but it is not backward compatible with non QOS stations and
> does not enable the administrator of a BSS to force this mode of
> operation within a BSS. Additionally, it would require both the ap
> and the station to implement the control protocol, which is optional
> on both ends. Furthermore, I've seen a few mobile phone stations
> locally that indicate qos support but won't complete DHCP if their
> broadcasts are encapsulated as A-MSDU. Though they work fine with
> this series approach.

Presumably those phones also don't even try to use DMS, right?

> This patch therefore does not opt to implement DMS but instead just
> replicates the packet and changes the destination address. As this
> works fine with ARP, IPv4 and IPv6, it is limited to these protocols
> and normal 802.11 multicast frames are send out for all other payload
> protocols.

How did you determine that it "works fine"?

I see at least one undesirable impact of this, which DMS doesn't have;
it breaks a client's MUST NOT requirement from RFC 1122:

         An ICMP error message MUST NOT be sent as the result of
         receiving:
[...]
         *    a datagram sent as a link-layer broadcast, or
[...]

since the client can no longer realize that the datagram was in fact
sent as a link-layer broadcast (or multicast).

>  include/net/cfg80211.h        |   5 ++
>  include/uapi/linux/nl80211.h  |   7 +++
>  net/mac80211/cfg.c            |  14 ++++++
>  net/mac80211/debugfs_netdev.c |  29 ++++++++++++
>  net/mac80211/ieee80211_i.h    |   1 +
>  net/mac80211/tx.c             | 103
> ++++++++++++++++++++++++++++++++++++++++++
>  net/wireless/nl80211.c        |  33 ++++++++++++++
>  net/wireless/rdev-ops.h       |  11 +++++
>  net/wireless/trace.h          |  19 ++++++++
>  9 files changed, 222 insertions(+)

You should split the patch into cfg80211 and mac80211, IMHO it's big
enough to do that.

> + * @set_ap_unicast: set the multicast to unicast flag for a AP
> interface

That API name isn't very descriptive, I'm sure we can do something
better there.

Also, perhaps we should structure this already like we would DMS, with
a per-station toggle or even list of multicast addresses?

> @@ -2261,6 +2266,8 @@ enum nl80211_attrs {
>  
>  	NL80211_ATTR_MESH_PEER_AID,
>  
> +	NL80211_ATTR_UNICAST,

missing docs, but likely doesn't matter after the comment above

> +static int ieee80211_set_ap_unicast(struct wiphy *wiphy, struct
> net_device *dev,
> +				    const bool unicast)
> +{
> +	struct ieee80211_sub_if_data *sdata =
> IEEE80211_DEV_TO_SUB_IF(dev);
> +
> +	if (sdata->vif.type != NL80211_IFTYPE_AP)
> +		return -1;

Was this not documented but also intended to apply to its dependent
VLANs?

> +static ssize_t
> +ieee80211_if_fmt_unicast(const struct ieee80211_sub_if_data *sdata,
> +			 char *buf, int buflen)
> +{
> +	const struct ieee80211_if_ap *ifap = &sdata->u.ap;
> +
> +	return snprintf(buf, buflen, "0x%x\n", ifap->unicast);
> +}
> +
> +static ssize_t
> +ieee80211_if_parse_unicast(struct ieee80211_sub_if_data *sdata,
> +			   const char *buf, int buflen)
> +{
> +	struct ieee80211_if_ap *ifap = &sdata->u.ap;
> +	u8 val;
> +	int ret;
> +
> +	ret = kstrtou8(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	ifap->unicast = val ? 1 : 0;
> +
> +	return buflen;
> +}
> +
> +IEEE80211_IF_FILE_RW(unicast);

No need for this, at least the setter, any more.

> +/* Check if multicast to unicast conversion is needed and do it.
> + * Returns 1 if skb was freed and should not be send out. */

wrong comment style :)

> +static int
> +ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data
> *sdata,
> +				  struct sk_buff *skb,
> u32  info_flags)
> +{
> +	struct ieee80211_local *local = sdata->local;
> +	const struct ethhdr *eth = (void *)skb->data;
> +	const struct vlan_ethhdr *ethvlan = (void *)skb->data;
> +	struct sta_info *sta, *prev = NULL;
> +	struct sk_buff *cloned_skb;
> +	u16 ethertype;
> +
> +	/* multicast to unicast conversion only for AP interfaces */
> +	switch (sdata->vif.type) {
> +	case NL80211_IFTYPE_AP_VLAN:
> +		sta = rcu_dereference(sdata->u.vlan.sta);
> +		if (sta) /* 4addr */
> +			return 0;
> +	case NL80211_IFTYPE_AP:
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	/* check runtime toggle for this bss */
> +	if (!sdata->bss->unicast)
> +		return 0;
> +
> +	/* check if this is a multicast frame */
> +	if (!is_multicast_ether_addr(eth->h_dest))
> +		return 0;

That should probably come first, would make this far easier to read.

> +		if (unlikely(!memcmp(eth->h_source, sta->sta.addr,
> ETH_ALEN)))
> +			/* do not send back to source */
> +			continue;

ether_addr_something, instead of memcmp?

> +		if (unlikely(is_multicast_ether_addr(sta-
> >sta.addr))) {
> +			WARN_ONCE(1, "sta with multicast address
> %pM",
> +				  sta->sta.addr);
> +			continue;
> +		}

Err, no, remove this... it cannot happen. We could move the check into
cfg80211 from mac80211, but we surely shouldn't add it into the TX
hotpath!

> +		if (prev) {
> +			cloned_skb = skb_clone(skb, GFP_ATOMIC);
> +			if (likely(!ieee80211_change_da(cloned_skb,
> prev)))
> +				ieee80211_subif_start_xmit(cloned_sk
> b,
> +							   cloned_sk
> b->dev);

I'm not very happy with this recursion, but I guess it can't be avoided
easily. However, you can easily call the more
sensible __ieee80211_subif_start_xmit() instead of this one.

> +	unicast = nla_data(info->attrs[NL80211_ATTR_UNICAST]);

What's this supposed to mean?

johannes

^ permalink raw reply

* Re: [PATCH 3/3] mac80211: multicast to unicast conversion
From: michael-dev @ 2016-10-05 11:40 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, projekt-wlan, netdev
In-Reply-To: <1475662791.4994.39.camel@sipsolutions.net>

Am 05.10.2016 12:19, schrieb Johannes Berg:
>> on both ends. Furthermore, I've seen a few mobile phone stations
>> locally that indicate qos support but won't complete DHCP if their
>> broadcasts are encapsulated as A-MSDU. Though they work fine with
>> this series approach.
>=20
> Presumably those phones also don't even try to use DMS, right?

When I traced this two years ago, almost no device indicated DMS=20
support, even though almost all seem to accepted multicast in unicast=20
a-msdu frames.

>=20
>> This patch therefore does not opt to implement DMS but instead just
>> replicates the packet and changes the destination address. As this
>> works fine with ARP, IPv4 and IPv6, it is limited to these protocols
>> and normal 802.11 multicast frames are send out for all other payload
>> protocols.
>=20
> How did you determine that it "works fine"?

First, I tested this manually using my own devices or asked friends. I=20
think this covered at least a recent debian x64 with an intel wireless=20
card, a windows 7 x64 with an intel wireless card, an android phone, an=20
ios phone and some recent macbook. Manually testing included visiting an=20
IPv6 only website (this network uses IPv6 router advertismentens (RA)=20
but no DHCPv6), so RA is accepted and ND working. Additionally,=20
arping'ing these station using broadcast arp request worked fine, so=20
broadcast arp requests are working. Finally, DHCP worked fine and UPNP=20
multicast discovery for some closed source media streaming wireless=20
device was reported working.

Next, that change was rolled out. It is now in use for at least three=20
years with about 300 simulatenously online stations and >2000 currently=20
registered devices and there hasn't been a single problem report that=20
could be related to that change. Though, e.g. our samsung galaxy users=20
report consistently that their devices refuse to connect using WPA-PSK=20
as our network advertises FT-PSK next to WPA-PSK and I learned that=20
there was at least one device there that did not like the=20
multicast-as-unicast-amsdu packets due to a user problem report.

> I see at least one undesirable impact of this, which DMS doesn't have;
> it breaks a client's MUST NOT requirement from RFC 1122:

Okay, so this cannot go into linux, right?

The thing I dislike most about DMS is that it is client driven, that is=20
an AP will only apply unicast conversion if a station actively requests=20
so.

> You should split the patch into cfg80211 and mac80211, IMHO it's big
> enough to do that.

ok

>> + * @set_ap_unicast: set the multicast to unicast flag for a AP
>> interface
>=20
> That API name isn't very descriptive, I'm sure we can do something
> better there.

proposal: "request multicast packets to be trasnmitted as unicast" ?

> Also, perhaps we should structure this already like we would DMS, with
> a per-station toggle or even list of multicast addresses?

should be possible, yes


>> +static int ieee80211_set_ap_unicast(struct wiphy *wiphy, struct
>> net_device *dev,
>> +				=C2=A0=C2=A0=C2=A0=C2=A0const bool unicast)
>> +{
>> +	struct ieee80211_sub_if_data *sdata =3D
>> IEEE80211_DEV_TO_SUB_IF(dev);
>> +
>> +	if (sdata->vif.type !=3D NL80211_IFTYPE_AP)
>> +		return -1;
>=20
> Was this not documented but also intended to apply to its dependent
> VLANs?

it was intended as a per per-BSS toggle, so it applies to all dependent=20
VLANs automatically.

>> +/* Check if multicast to unicast conversion is needed and do it.
>> + * Returns 1 if skb was freed and should not be send out. */
>=20
> wrong comment style :)

you mean the */ at end of line instead of on a new line?

>> +	unicast =3D nla_data(info->attrs[NL80211_ATTR_UNICAST]);
>=20
> What's this supposed to mean?

this was supposed to be nla_get_u8.

michael

^ permalink raw reply

* [PATCH] [wl18xx] Fix memory leakage if kzalloc fails
From: Souptick Joarder @ 2016-10-05 11:50 UTC (permalink / raw)
  To: linux-wireless; +Cc: sahu.rameshwar73

This patch is added to properly handle memory leak if kzalloc fails
in wl18xx_scan_send() and wl18xx_scan_sched_scan_config()

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Signed-off-by: Rameshwar Sahu <sahu.rameshwar73@gmail.com>
---
 drivers/net/wireless/ti/wl18xx/scan.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ti/wl18xx/scan.c b/drivers/net/wireless/ti/wl18xx/scan.c
index 4e522154..aed22e1 100644
--- a/drivers/net/wireless/ti/wl18xx/scan.c
+++ b/drivers/net/wireless/ti/wl18xx/scan.c
@@ -41,14 +41,13 @@ static void wl18xx_adjust_channels(struct wl18xx_cmd_scan_params *cmd,
 static int wl18xx_scan_send(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			    struct cfg80211_scan_request *req)
 {
-	struct wl18xx_cmd_scan_params *cmd;
+	struct wl18xx_cmd_scan_params *cmd = NULL;
 	struct wlcore_scan_channels *cmd_channels = NULL;
 	int ret;
 
 	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
 	if (!cmd) {
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	/* scan on the dev role if the regular one is not started */
@@ -59,7 +58,7 @@ static int wl18xx_scan_send(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 
 	if (WARN_ON(cmd->role_id == WL12XX_INVALID_ROLE_ID)) {
 		ret = -EINVAL;
-		goto out;
+		goto err_cmd_free;
 	}
 
 	cmd->scan_type = SCAN_TYPE_SEARCH;
@@ -84,7 +83,7 @@ static int wl18xx_scan_send(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 	cmd_channels = kzalloc(sizeof(*cmd_channels), GFP_KERNEL);
 	if (!cmd_channels) {
 		ret = -ENOMEM;
-		goto out;
+		goto err_cmd_free;
 	}
 
 	wlcore_set_scan_chan_params(wl, cmd_channels, req->channels,
@@ -153,6 +152,7 @@ static int wl18xx_scan_send(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 
 out:
 	kfree(cmd_channels);
+err_cmd_free:
 	kfree(cmd);
 	return ret;
 }
@@ -171,7 +171,7 @@ int wl18xx_scan_sched_scan_config(struct wl1271 *wl,
 				  struct cfg80211_sched_scan_request *req,
 				  struct ieee80211_scan_ies *ies)
 {
-	struct wl18xx_cmd_scan_params *cmd;
+	struct wl18xx_cmd_scan_params *cmd = NULL;
 	struct wlcore_scan_channels *cmd_channels = NULL;
 	struct conf_sched_scan_settings *c = &wl->conf.sched_scan;
 	int ret;
@@ -185,15 +185,14 @@ int wl18xx_scan_sched_scan_config(struct wl1271 *wl,
 
 	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
 	if (!cmd) {
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	cmd->role_id = wlvif->role_id;
 
 	if (WARN_ON(cmd->role_id == WL12XX_INVALID_ROLE_ID)) {
 		ret = -EINVAL;
-		goto out;
+		goto err_cmd_free;
 	}
 
 	cmd->scan_type = SCAN_TYPE_PERIODIC;
@@ -218,7 +217,7 @@ int wl18xx_scan_sched_scan_config(struct wl1271 *wl,
 	cmd_channels = kzalloc(sizeof(*cmd_channels), GFP_KERNEL);
 	if (!cmd_channels) {
 		ret = -ENOMEM;
-		goto out;
+		goto err_cmd_free;
 	}
 
 	/* configure channels */
@@ -296,6 +295,7 @@ int wl18xx_scan_sched_scan_config(struct wl1271 *wl,
 
 out:
 	kfree(cmd_channels);
+err_cmd_free:
 	kfree(cmd);
 	return ret;
 }
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 3/3] mac80211: multicast to unicast conversion
From: Johannes Berg @ 2016-10-05 11:58 UTC (permalink / raw)
  To: michael-dev; +Cc: linux-wireless, projekt-wlan, netdev
In-Reply-To: <1690671fb30f19c53c7883a5207c5721@fami-braun.de>

On Wed, 2016-10-05 at 13:40 +0200, michael-dev wrote:
> Am 05.10.2016 12:19, schrieb Johannes Berg:
> > 
> > > 
> > > on both ends. Furthermore, I've seen a few mobile phone stations
> > > locally that indicate qos support but won't complete DHCP if
> > > their broadcasts are encapsulated as A-MSDU. Though they work
> > > fine with this series approach.
> > 
> > Presumably those phones also don't even try to use DMS, right?
> 
> When I traced this two years ago, almost no device indicated DMS 
> support, even though almost all seem to accepted multicast in unicast
> a-msdu frames.

Right, that's what I suspected. I'm a bit surprised they accepted
multicast in unicast A-MSDU too, though I don't actually see any big
problem with it.

> > How did you determine that it "works fine"?
> 
> First, I tested this manually using my own devices or asked friends. 
[snip

Thanks!

> > I see at least one undesirable impact of this, which DMS doesn't
> > have; it breaks a client's MUST NOT requirement from RFC 1122:
> 
> Okay, so this cannot go into linux, right?

I'm not necessarily saying that, I just think we need to be careful
documenting possibly unexpected/undesired side-effects.

> > > + * @set_ap_unicast: set the multicast to unicast flag for a AP
> > > interface
> > 
> > That API name isn't very descriptive, I'm sure we can do something
> > better there.
> 
> proposal: "request multicast packets to be trasnmitted as unicast" ?

I was thinking more of the function name ("set_ap_unicast") which by
itself makes no sense - set_multicast_to_unicast or something like that
would be better, no?

> > Also, perhaps we should structure this already like we would DMS,
> > with a per-station toggle or even list of multicast addresses?
> 
> should be possible, yes

I'm mostly handwaving though, haven't really looked at what DMS really
would require from the API, even assuming that hostapd would implement
all the action frame handling etc.

It's quite possible that on the *client* side, mac80211 should
implement the DMS client, if supported, and perhaps only if enabled by
some kind of configuration knob.

> > Was this not documented but also intended to apply to its dependent
> > VLANs?
> 
> it was intended as a per per-BSS toggle, so it applies to all
> dependent VLANs automatically.

makes sense, but you should document it in the API documentation, which
today says "for a AP interface" or so (see above)

(btw - writing that out I see that it should be "an AP interface" too)

> > 
> > > 
> > > +/* Check if multicast to unicast conversion is needed and do it.
> > > + * Returns 1 if skb was freed and should not be send out. */
> > 
> > wrong comment style :)
> 
> you mean the */ at end of line instead of on a new line?

yeah, no big deal though.

I've also mostly gone back to non-davem style with /* also on its own
line, but it's not so important. :)

> > > +	unicast = nla_data(info->attrs[NL80211_ATTR_UNICAST]);
> > 
> > What's this supposed to mean?
> 
> this was supposed to be nla_get_u8.

Shouldn't it just be nla_get_flag()? I mean, why do you have a u8 with
values 0/1 rather than just flag attribute absent/present?

Anyway, perhaps this needs to change to take DMS/per-station into
account?

Then again, this kind of setting - global multicast-to-unicast -
fundamentally *cannot* be done on a per-station basis, since if you
enable it for one station and not for another, the first station that
has it enabled would get the packets twice...

johannes

^ permalink raw reply

* RE: [PATCH v2 2/2] mwifiex: check hw_status in suspend and resume handlers
From: Amitkumar Karwar @ 2016-10-05 12:26 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-wireless@vger.kernel.org, Cathy Luo, Nishant Sarmukadam,
	rajatja@google.com, briannorris@google.com, Xinming Hu
In-Reply-To: <20161004210435.GA31652@localhost>

Hi Brian,

Thanks for your thorough review.

> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Wednesday, October 05, 2016 2:35 AM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> rajatja@google.com; briannorris@google.com; Xinming Hu
> Subject: Re: [PATCH v2 2/2] mwifiex: check hw_status in suspend and
> resume handlers
> 
> Hi,
> 
> On Tue, Oct 04, 2016 at 10:38:25PM +0530, Amitkumar Karwar wrote:
> > From: Xinming Hu <huxm@marvell.com>
> >
> > We have observed a kernel crash when system immediately suspends after
> > booting. There is a race between suspend and driver initialization
> > paths.
> > This patch adds hw_status checks to fix the problem
> >
> > Signed-off-by: Xinming Hu <huxm@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> > v2: Return failure in suspend/resume handler in this scenario.
> > ---
> >  drivers/net/wireless/marvell/mwifiex/pcie.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c
> > b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > index ba9e068..fa6bf85 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > @@ -122,9 +122,10 @@ static int mwifiex_pcie_suspend(struct device
> > *dev)
> >
> >  	if (pdev) {
> >  		card = pci_get_drvdata(pdev);
> > -		if (!card || !card->adapter) {
> > +		if (!card || !card->adapter ||
> > +		    card->adapter->hw_status != MWIFIEX_HW_STATUS_READY) {
> 
> Wait, is there no locking on the 'hw_status' field? That is inherently
> an unsafe race all on its own; you're not guaranteed that this will be
> read/written atomically. And you also aren't guaranteed that writes to
> this happen in the order they appear in the code -- in other words,
> reading this flag doesn't necessarily guarantee that initialization is
> actually complete (even if that's very likely to be true, given that
> it's probably just a single-instruction word-access, and any prior HW
> polling or interrupts likely have done some synchronization and can't be
> reordered).
> actually complete

Here is the brief info on how "hw_status" flag is updated.
1) It gets changed incrementally during initialization.
    MWIFIEX_HW_STATUS_INITIALIZING -> MWIFIEX_HW_STATUS_INIT_DONE -> MWIFIEX_HW_STATUS_READY

2) Status will remain READY once driver+firmware is up and running.

3) Below is status during teardown
MWIFIEX_HW_STATUS_READY -> MWIFIEX_HW_STATUS_RESET -> MWIFIEX_HW_STATUS_CLOSING -> MWIFIEX_HW_STATUS_NOT_READY

As the events occur one after another, we don't expect a race and don't need locking here for the flag. Flag status MWIFIEX_HW_STATUS_READY guarantees that initialization is completed.

In worst case scenario, only first system suspend attempt issued immediately after system boot will be aborted with BUSY error. I think, that should be fine.

Let me know if you have any concerns.

> 
> This is probably better than nothing, but it's not very good.
> 
> >  			pr_err("Card or adapter structure is not valid\n");
> > -			return 0;
> > +			return -EBUSY;
> 
> So the above cases all mean that the driver hasn't finished loading,
> right?
> 
> !card => can't happen (PCIe probe() would have failed
> mwifiex_add_card()), but fine to check

NULL "card" or "card->adapter" pointers are expected in below cases
1) Driver initialization failed after downloading the firmware. Driver is performing cleanup in init failure path. Now system suspends.
2) Race of teardown + suspend
 
> 
> !card->adapter => only happens after patch 1; i.e., when tearing down
> the device and detaching it from the driver
> 
> card->adapter->hw_status != MWIFIEX_HW_STATUS_READY => FW is not loaded
> (i.e., in the process of starting or stopping FW?)
> 
> I guess all of those cases make sense to be -EBUSY.

Yes. we can keep -EBUSY for all of the cases.

> 
> >  		}
> >  	} else {
> >  		pr_err("PCIE device is not specified\n");
> 
> I was going to complain about this branch (!pdev) returning 0, since
> that looked asymmetric. But this case will never happen, actually, since
> to_pci_dev() is just doing struct offset arithmetic on struct device,
> and struct device is never going to be NULL. It probably makes more
> sense to just remove this branch entirely, but that's for another patch.

Thanks for pointing this out. I will create separate patch for this.

> 
> > @@ -166,9 +167,10 @@ static int mwifiex_pcie_resume(struct device
> > *dev)
> >
> >  	if (pdev) {
> >  		card = pci_get_drvdata(pdev);
> > -		if (!card || !card->adapter) {
> > +		if (!card || !card->adapter ||
> > +		    card->adapter->hw_status != MWIFIEX_HW_STATUS_READY) {
> 
> Same complaint, except if you've screwed up here, you probably already
> screwed up in suspend().
> 
> Brian
> 
> >  			pr_err("Card or adapter structure is not valid\n");
> > -			return 0;
> > +			return -EBUSY;
> >  		}
> >  	} else {
> >  		pr_err("PCIE device is not specified\n");
> > --
> > 1.9.1
> >

Regards,
Amitkumar

^ permalink raw reply

* Test mail please ignore
From: Souptick Joarder @ 2016-10-05 13:03 UTC (permalink / raw)
  To: linux-wireless



^ permalink raw reply

* [PATCH] [wl18xx] Fix memory leakage if kzalloc fails
From: Souptick Joarder @ 2016-10-05 13:09 UTC (permalink / raw)
  To: linux-wireless

This patch is added to properly handle memory leak if kzalloc fails
in wl18xx_scan_send() and wl18xx_scan_sched_scan_config()

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Signed-off-by: Rameshwar Sahu <sahu.rameshwar73@gmail.com>
---
 drivers/net/wireless/ti/wl18xx/scan.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ti/wl18xx/scan.c b/drivers/net/wireless/ti/wl18xx/scan.c
index 4e522154..aed22e1 100644
--- a/drivers/net/wireless/ti/wl18xx/scan.c
+++ b/drivers/net/wireless/ti/wl18xx/scan.c
@@ -41,14 +41,13 @@ static void wl18xx_adjust_channels(struct wl18xx_cmd_scan_params *cmd,
 static int wl18xx_scan_send(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			    struct cfg80211_scan_request *req)
 {
-	struct wl18xx_cmd_scan_params *cmd;
+	struct wl18xx_cmd_scan_params *cmd = NULL;
 	struct wlcore_scan_channels *cmd_channels = NULL;
 	int ret;
 
 	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
 	if (!cmd) {
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	/* scan on the dev role if the regular one is not started */
@@ -59,7 +58,7 @@ static int wl18xx_scan_send(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 
 	if (WARN_ON(cmd->role_id == WL12XX_INVALID_ROLE_ID)) {
 		ret = -EINVAL;
-		goto out;
+		goto err_cmd_free;
 	}
 
 	cmd->scan_type = SCAN_TYPE_SEARCH;
@@ -84,7 +83,7 @@ static int wl18xx_scan_send(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 	cmd_channels = kzalloc(sizeof(*cmd_channels), GFP_KERNEL);
 	if (!cmd_channels) {
 		ret = -ENOMEM;
-		goto out;
+		goto err_cmd_free;
 	}
 
 	wlcore_set_scan_chan_params(wl, cmd_channels, req->channels,
@@ -153,6 +152,7 @@ static int wl18xx_scan_send(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 
 out:
 	kfree(cmd_channels);
+err_cmd_free:
 	kfree(cmd);
 	return ret;
 }
@@ -171,7 +171,7 @@ int wl18xx_scan_sched_scan_config(struct wl1271 *wl,
 				  struct cfg80211_sched_scan_request *req,
 				  struct ieee80211_scan_ies *ies)
 {
-	struct wl18xx_cmd_scan_params *cmd;
+	struct wl18xx_cmd_scan_params *cmd = NULL;
 	struct wlcore_scan_channels *cmd_channels = NULL;
 	struct conf_sched_scan_settings *c = &wl->conf.sched_scan;
 	int ret;
@@ -185,15 +185,14 @@ int wl18xx_scan_sched_scan_config(struct wl1271 *wl,
 
 	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
 	if (!cmd) {
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	cmd->role_id = wlvif->role_id;
 
 	if (WARN_ON(cmd->role_id == WL12XX_INVALID_ROLE_ID)) {
 		ret = -EINVAL;
-		goto out;
+		goto err_cmd_free;
 	}
 
 	cmd->scan_type = SCAN_TYPE_PERIODIC;
@@ -218,7 +217,7 @@ int wl18xx_scan_sched_scan_config(struct wl1271 *wl,
 	cmd_channels = kzalloc(sizeof(*cmd_channels), GFP_KERNEL);
 	if (!cmd_channels) {
 		ret = -ENOMEM;
-		goto out;
+		goto err_cmd_free;
 	}
 
 	/* configure channels */
@@ -296,6 +295,7 @@ int wl18xx_scan_sched_scan_config(struct wl1271 *wl,
 
 out:
 	kfree(cmd_channels);
+err_cmd_free:
 	kfree(cmd);
 	return ret;
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH] cfg80211: let ieee80211_amsdu_to_8023s() take only 802.3 header
From: Johannes Berg @ 2016-10-05 13:33 UTC (permalink / raw)
  To: linux-wireless; +Cc: Michael Braun, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There's only a single case where has_80211_header is passed as true,
which is in mac80211. Given that it's only a very simple check that
needs to be done before calling it (calling __ieee80211_data_to_8023()
and checking its return value), simple move that into the caller in
order to simplify this function. The only additional cost this adds
is the extra skb_push(), so reduce that by using __skb_push() as we
know there's enough space, having pulled just before.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 .../net/wireless/marvell/mwifiex/11n_rxreorder.c   |  2 +-
 drivers/staging/rtl8723au/core/rtw_recv.c          |  2 +-
 include/net/cfg80211.h                             |  7 +++---
 net/mac80211/rx.c                                  |  6 ++++-
 net/wireless/util.c                                | 26 +++++-----------------
 5 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
index 94480123efa3..e9f462e3730b 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
@@ -45,7 +45,7 @@ static int mwifiex_11n_dispatch_amsdu_pkt(struct mwifiex_private *priv,
 		skb_trim(skb, le16_to_cpu(local_rx_pd->rx_pkt_length));
 
 		ieee80211_amsdu_to_8023s(skb, &list, priv->curr_addr,
-					 priv->wdev.iftype, 0, false);
+					 priv->wdev.iftype, 0);
 
 		while (!skb_queue_empty(&list)) {
 			struct rx_packet_hdr *rx_hdr;
diff --git a/drivers/staging/rtl8723au/core/rtw_recv.c b/drivers/staging/rtl8723au/core/rtw_recv.c
index 150dabc2a58d..e1873c906c42 100644
--- a/drivers/staging/rtl8723au/core/rtw_recv.c
+++ b/drivers/staging/rtl8723au/core/rtw_recv.c
@@ -1687,7 +1687,7 @@ int amsdu_to_msdu(struct rtw_adapter *padapter, struct recv_frame *prframe)
 	skb_pull(skb, prframe->attrib.hdrlen);
 	__skb_queue_head_init(&skb_list);
 
-	ieee80211_amsdu_to_8023s(skb, &skb_list, NULL, 0, 0, false);
+	ieee80211_amsdu_to_8023s(skb, &skb_list, NULL, 0, 0);
 
 	while (!skb_queue_empty(&skb_list)) {
 		sub_skb = __skb_dequeue(&skb_list);
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index fe78f02a242e..8d1a67fc2983 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4067,7 +4067,8 @@ int ieee80211_data_from_8023(struct sk_buff *skb, const u8 *addr,
  *
  * Decode an IEEE 802.11n A-MSDU frame and convert it to a list of
  * 802.3 frames. The @list will be empty if the decode fails. The
- * @skb is consumed after the function returns.
+ * @skb must already have been converted to 802.3 format. It is
+ * consumed after the function returns.
  *
  * @skb: The input IEEE 802.11n A-MSDU frame.
  * @list: The output list of 802.3 frames. It must be allocated and
@@ -4075,12 +4076,10 @@ int ieee80211_data_from_8023(struct sk_buff *skb, const u8 *addr,
  * @addr: The device MAC address.
  * @iftype: The device interface type.
  * @extra_headroom: The hardware extra headroom for SKBs in the @list.
- * @has_80211_header: Set it true if SKB is with IEEE 802.11 header.
  */
 void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
 			      const u8 *addr, enum nl80211_iftype iftype,
-			      const unsigned int extra_headroom,
-			      bool has_80211_header);
+			      const unsigned int extra_headroom);
 
 /**
  * cfg80211_classify8021d - determine the 802.1p/1d tag for a data frame
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 6175db385ba7..3a912e6c585f 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2323,9 +2323,13 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
 	skb->dev = dev;
 	__skb_queue_head_init(&frame_list);
 
+	if (ieee80211_data_to_8023(skb, rx->sdata->vif.addr,
+				   rx->sdata->vif.type))
+		return RX_DROP_UNUSABLE;
+
 	ieee80211_amsdu_to_8023s(skb, &frame_list, dev->dev_addr,
 				 rx->sdata->vif.type,
-				 rx->local->hw.extra_tx_headroom, true);
+				 rx->local->hw.extra_tx_headroom);
 
 	while (!skb_queue_empty(&frame_list)) {
 		rx->skb = __skb_dequeue(&frame_list);
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 8edce22d1b93..ed22d0336f1b 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -420,8 +420,8 @@ unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr)
 }
 EXPORT_SYMBOL(ieee80211_get_mesh_hdrlen);
 
-static int __ieee80211_data_to_8023(struct sk_buff *skb, struct ethhdr *ehdr,
-				    const u8 *addr, enum nl80211_iftype iftype)
+int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
+			   enum nl80211_iftype iftype)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	struct {
@@ -519,18 +519,10 @@ static int __ieee80211_data_to_8023(struct sk_buff *skb, struct ethhdr *ehdr,
 
 	pskb_pull(skb, hdrlen);
 
-	if (!ehdr)
-		ehdr = (struct ethhdr *) skb_push(skb, sizeof(struct ethhdr));
-	memcpy(ehdr, &tmp, sizeof(tmp));
+	memcpy(__skb_push(skb, sizeof(tmp)), &tmp, sizeof(tmp));
 
 	return 0;
 }
-
-int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
-			   enum nl80211_iftype iftype)
-{
-	return __ieee80211_data_to_8023(skb, NULL, addr, iftype);
-}
 EXPORT_SYMBOL(ieee80211_data_to_8023);
 
 int ieee80211_data_from_8023(struct sk_buff *skb, const u8 *addr,
@@ -745,25 +737,18 @@ __ieee80211_amsdu_copy(struct sk_buff *skb, unsigned int hlen,
 
 void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
 			      const u8 *addr, enum nl80211_iftype iftype,
-			      const unsigned int extra_headroom,
-			      bool has_80211_header)
+			      const unsigned int extra_headroom)
 {
 	unsigned int hlen = ALIGN(extra_headroom, 4);
 	struct sk_buff *frame = NULL;
 	u16 ethertype;
 	u8 *payload;
-	int offset = 0, remaining, err;
+	int offset = 0, remaining;
 	struct ethhdr eth;
 	bool reuse_frag = skb->head_frag && !skb_has_frag_list(skb);
 	bool reuse_skb = false;
 	bool last = false;
 
-	if (has_80211_header) {
-		err = __ieee80211_data_to_8023(skb, &eth, addr, iftype);
-		if (err)
-			goto out;
-	}
-
 	while (!last) {
 		unsigned int subframe_len;
 		int len;
@@ -819,7 +804,6 @@ void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
 
  purge:
 	__skb_queue_purge(list);
- out:
 	dev_kfree_skb(skb);
 }
 EXPORT_SYMBOL(ieee80211_amsdu_to_8023s);
-- 
2.8.1

^ permalink raw reply related

* Re: [PATCH] cfg80211: let ieee80211_amsdu_to_8023s() take only 802.3 header
From: Johannes Berg @ 2016-10-05 13:57 UTC (permalink / raw)
  To: linux-wireless; +Cc: Michael Braun
In-Reply-To: <1475674414-23086-1-git-send-email-johannes@sipsolutions.net>

On Wed, 2016-10-05 at 15:33 +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> There's only a single case where has_80211_header is passed as true,
> which is in mac80211. Given that it's only a very simple check that
> needs to be done before calling it (calling
> __ieee80211_data_to_8023()
> and checking its return value), simple move that into the caller in
> order to simplify this function. The only additional cost this adds
> is the extra skb_push(), so reduce that by using __skb_push() as we
> know there's enough space, having pulled just before.

Obviously this is broken ... we need to have a frame without the outer
802.3 header, not with it, passed into the function.

johannes

^ permalink raw reply

* Re: [PATCH v4] ath9k: Switch to using mac80211 intermediate software queues.
From: Toke Høiland-Jørgensen @ 2016-10-05 14:02 UTC (permalink / raw)
  To: Kalle Valo
  Cc: linux-wireless, make-wifi-fast, ath9k-devel, Tim Shepard,
	Felix Fietkau
In-Reply-To: <877fb8ug0x.fsf@kamboji.qca.qualcomm.com>

Kalle Valo <kvalo@codeaurora.org> writes:

> Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk> writes:
>
>>>>> This is great work but due to the regressions I'm not sure if this
>>>>> will be ready for 4.9. To get more testing time I wonder if we should
>>>>> wait for 4.10? IMHO applying this in the end of the cycle is too risky
>>>>> and we should try to maximise the time linux-next by applying this
>>>>> just after -rc1 is released.
>>>>>
>>>>> Thoughts?
>>>>
>>>> Well, now that we understand what is causing the throughput regression=
s,
>>>> fixing them should be fairly straight forward (yeah, famous last words,
>>>> but still...). I already have a patch for the fast path and will go po=
ke
>>>> at the slow path next. It'll probably require another workaround or tw=
o,
>>>> so I guess it won't be the architecturally clean ideal solution; but it
>>>> would make it possible to have something that works for 4.9 and then
>>>> iterate for a cleaner design for 4.10.
>>>
>>> But if we try to rush this to 4.9 it won't be in linux-next for long. We
>>> are now in -rc3 and let's say that the patches are ready to apply in two
>>> weeks. That would leave us only two weeks of -next time before the merge
>>> window, which I think is not enough for a controversial patch like this
>>> one. There might be other bugs lurking which haven't been found yet.
>>
>> What, other hidden bugs? Unpossible! :)
>
> Yeah, right ;)
>
>> Would it be possible to merge the partial solution (which is ready now,
>> basically) and fix the slow path in a separate patch later?
>
> What do you mean with partial solution? You mean ath9k users would
> suffer from regressions until they are fixed? We can't do that.
>
>> (Just spit-balling here; I'm still fairly new to this process. But I am
>> concerned that we'll hit a catch-22 where we can't get wider testing
>> before it's "ready" and we can't prove that it's "ready" until we've had
>> wider testing...)
>
> I understand your point, but I don't want to rush this to 4.9 and then
> start getting lots of bug reports and eventually forced to revert it. If
> we just found a new serious regression the chances are that there are
> more lurking somewhere and this patch is just not ready yet.

So, the changes to mac80211 that fixes the known regressions of this
patch have gone in. Any chance of seeing this merged during the current
merge window? :)

-Toke

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox