[PATCH 0/3] mac80211: multicast with AP_VLAN optimizations

[PATCH 0/3] mac80211: multicast with AP_VLAN optimizations

[Michael Braun]

Hi,

this series tries to optimize multicast delivery on access points with
AP_VLAN interfaces.

My setup is as follows: hostapd creates one AP_VLAN interface per station
(per_sta_vif=1), which enables bridge igmp snooping to decide which
stations need to receive a multicast packet.

This series then
 - avoids multicast packets by tracking the number of authenticated
   stations per interface more accurately and drops packets if there is no
   receiver connected,
 - converts multicast packets to unicast packets for the most common
   protocols, so they get delivered faster and more reliable. Additionally,
   waking up non-receivers in other VLANs is avoided.
 - speeds up station lookup if there is only one authenticated station
   assigned to the AP_VLAN interface.

Sincerely,
M. Braun

Michael Braun (3):
  mac80211: filter multicast data packets on AP / AP_VLAN
  mac80211: multicast to unicast conversion
  mac80211: cache the only AP_VLAN station

 net/mac80211/cfg.c            |  26 +++++-----
 net/mac80211/debugfs_netdev.c |  38 ++++++++++++++
 net/mac80211/ieee80211_i.h    |  61 ++++++++++++++++++++++
 net/mac80211/rx.c             |   5 +-
 net/mac80211/sta_info.c       |  41 +++++++++++----
 net/mac80211/tx.c             | 115 ++++++++++++++++++++++++++++++++++++++++--
 6 files changed, 258 insertions(+), 28 deletions(-)

-- 
2.1.4

[PATCH 1/3] mac80211: filter multicast data packets on AP / AP_VLAN

[Michael Braun]

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>

--

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    | 54 +++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/rx.c             |  5 ++--
 net/mac80211/sta_info.c       | 10 ++------
 net/mac80211/tx.c             |  5 ++--
 6 files changed, 78 insertions(+), 27 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 543b1d4..078e837 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..d97756d 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -477,6 +477,8 @@ 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_if, u.vlan.num_mcast_sta_if, ATOMIC);
+IEEE80211_IF_FILE(num_mcast_sta_novlan_if, u.ap.num_mcast_sta_if, ATOMIC);
 
 static ssize_t ieee80211_if_fmt_num_buffered_multicast(
 	const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -636,6 +638,7 @@ static void add_sta_files(struct ieee80211_sub_if_data *sdata)
 static void add_ap_files(struct ieee80211_sub_if_data *sdata)
 {
 	DEBUGFS_ADD(num_mcast_sta);
+	DEBUGFS_ADD(num_mcast_sta_novlan_if);
 	DEBUGFS_ADD_MODE(smps, 0600);
 	DEBUGFS_ADD(num_sta_ps);
 	DEBUGFS_ADD(dtim_count);
@@ -643,6 +646,11 @@ 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)
+{
+	DEBUGFS_ADD(num_mcast_sta_vlan_if);
+}
+
 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..b10e8be 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -289,6 +289,7 @@ struct ieee80211_if_ap {
 
 	struct ps_data ps;
 	atomic_t num_mcast_sta; /* number of stations receiving multicast */
+	atomic_t num_mcast_sta_if; /* num_mcast_sta w/o sta in AP_VLAN */
 	enum ieee80211_smps_mode req_smps, /* requested smps mode */
 			 driver_smps_mode; /* smps mode request */
 
@@ -305,6 +306,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_if; /* number of stations receiving multicast */
 };
 
 struct mesh_stats {
@@ -1496,6 +1498,58 @@ 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_if);
+	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+		atomic_inc(&sdata->u.vlan.num_mcast_sta_if);
+
+	if (sdata->vif.type != NL80211_IFTYPE_AP_VLAN ||
+	    !sdata->u.vlan.sta) /* except 4addr mode */
+		atomic_inc(&sdata->bss->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_if);
+	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+		atomic_dec(&sdata->u.vlan.num_mcast_sta_if);
+
+	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+	    sdata->u.vlan.sta)
+		return; /* 4addr mode */
+
+	atomic_dec(&sdata->bss->num_mcast_sta);
+}
+
+/**
+ * @returns number of multicast stations connected
+ *  -1 if unsupported (no-AP, 4addr mode)
+ */
+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_if);
+	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+		 !sdata->u.vlan.sta)
+		return atomic_read(&sdata->u.vlan.num_mcast_sta_if);
+	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 9dce3b1..4c6adc9 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2160,7 +2160,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
@@ -2169,7 +2170,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..7130862 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) != -1 &&
+			    ieee80211_is_data(hdr->frame_control))) {
 		/*
 		 * No associated STAs - no need to send multicast
 		 * frames.
-- 
2.1.4

Re: [PATCH 1/3] mac80211: filter multicast data packets on AP / AP_VLAN

[Johannes Berg]

[snip]

I think this makes sense, but it's not clear to me why you add two
counters and keep the old one? It seems to me that it would be
sufficient to have a single counter per AP/VLAN interface?

The usage in __ieee80211_request_smps_ap() can just be removed since it
goes to iterate the stations next. That should be a separate, first,
patch in the series, but after that I don't see a need to keep
num_mcast_sta, or rather, I see no reason not to remove the VLAN
stations from the AP's num_mcast_sta, and add a new per-VLAN
num_mcast_sta.

> +/**
> + * @returns number of multicast stations connected
> + *  -1 if unsupported (no-AP, 4addr mode)
> + */
> +static inline int
> +ieee80211_vif_get_num_mcast_if(struct ieee80211_sub_if_data *sdata)

That's not a valid kernel-doc comment, but you've tagged it as one with
the /** - please fix by removing the /** and the @, and writing a real
sentence out of that, or by making it a real kernel-doc comment.

johannes

[PATCH 2/3] mac80211: multicast to unicast conversion

[Michael Braun]

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>
---
 net/mac80211/debugfs_netdev.c |  27 +++++++++++
 net/mac80211/ieee80211_i.h    |   1 +
 net/mac80211/tx.c             | 105 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)

diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index d97756d..cfffbc0 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -488,6 +488,32 @@ 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)
@@ -644,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 b10e8be..7b3de28 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -294,6 +294,7 @@ struct ieee80211_if_ap {
 			 driver_smps_mode; /* smps mode request */
 
 	struct work_struct request_smps_work;
+	int unicast;
 };
 
 struct ieee80211_if_wds {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7130862..9c82fd8 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,106 @@ bool ieee80211_tx_prepare_skb(struct ieee80211_hw *hw,
 }
 EXPORT_SYMBOL(ieee80211_tx_prepare_skb);
 
+/* rewrite destination mac address */
+static int ieee80211_tx_dnat(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_tx_dnat(cloned_skb, prev)))
+				ieee80211_subif_start_xmit(cloned_skb,
+							   cloned_skb->dev);
+			else
+				dev_kfree_skb(cloned_skb);
+		}
+		prev = sta;
+	}
+
+	if (likely(prev)) {
+		ieee80211_tx_dnat(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 +3454,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;
 
-- 
2.1.4

Re: [PATCH 2/3] mac80211: multicast to unicast conversion

[Johannes Berg]

> +static ssize_t ieee80211_if_fmt_unicast(
> +	const struct ieee80211_sub_if_data *sdata, char *buf, int
> buflen)

That's a very ... unusual way to break the lines here - please either
break after the first argument, or before the function name. You should
see lots of examples for both.

However, regardless of that, I don't think that debugfs is an
appropriate way to configure a production option like this.

> +	int unicast;

bool, quite obviously.

> +/* rewrite destination mac address */
> +static int ieee80211_tx_dnat(struct sk_buff *skb, struct sta_info
> *sta)

How is this DNAT?

johannes

Re: [PATCH 2/3] mac80211: multicast to unicast conversion

[M. Braun]

Am 30.09.2016 um 09:29 schrieb Johannes Berg:
>> +static ssize_t ieee80211_if_fmt_unicast(
>> +	const struct ieee80211_sub_if_data *sdata, char *buf, int
>> buflen)
> 
> That's a very ... unusual way to break the lines here

it is the same style as used in debugfs_netdev.c for other attributes,
see e.g. ieee80211_if_fmt_tsf, ieee80211_if_parse_tsf.

Michael

Re: [PATCH 2/3] mac80211: multicast to unicast conversion

[Johannes Berg]

On Tue, 2016-10-04 at 06:36 +0200, M. Braun wrote:
> Am 30.09.2016 um 09:29 schrieb Johannes Berg:
> > 
> > > 
> > > +static ssize_t ieee80211_if_fmt_unicast(
> > > +	const struct ieee80211_sub_if_data *sdata, char *buf,
> > > int
> > > buflen)
> > 
> > That's a very ... unusual way to break the lines here
> 
> it is the same style as used in debugfs_netdev.c for other
> attributes,
> see e.g. ieee80211_if_fmt_tsf, ieee80211_if_parse_tsf.
> 

Fair enough, I didn't check. Maybe I'll change it all wholesale at some
other point then.

johannes

[PATCH 3/3] mac80211: cache the only AP_VLAN station

[Michael Braun]

If an AP_VLAN is only used with one station, cache a pointer to this station to
avoid lookup. This should speed up station lookup when there is only one
station assigned to the AP_VLAN interface, e.g. when using hostapd with
per_sta_vif.

Assigning only one station per AP_VLAN interfaces enables bridge IGMP snooping
to track multicast subscriptions by station to selectively forward to only
those stations that subscribed.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
 net/mac80211/cfg.c         | 10 ++++++++--
 net/mac80211/ieee80211_i.h | 14 ++++++++++----
 net/mac80211/sta_info.c    | 33 +++++++++++++++++++++++++++++++--
 net/mac80211/tx.c          |  5 +++++
 4 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 078e837..a69e6f2 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -67,6 +67,7 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
 	if (type == NL80211_IFTYPE_AP_VLAN &&
 	    params && params->use_4addr == 0) {
 		RCU_INIT_POINTER(sdata->u.vlan.sta, NULL);
+		RCU_INIT_POINTER(sdata->u.vlan.sta0, NULL);
 		ieee80211_check_fast_rx_iface(sdata);
 	} else if (type == NL80211_IFTYPE_STATION &&
 		   params && params->use_4addr >= 0) {
@@ -1379,8 +1380,13 @@ static int ieee80211_change_station(struct wiphy *wiphy,
 		sta->sdata = vlansdata;
 		ieee80211_check_fast_xmit(sta);
 
-		if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
-			ieee80211_vif_inc_num_mcast(sta->sdata);
+		if (test_sta_flag(sta, WLAN_STA_AUTHORIZED) &&
+		    ieee80211_vif_inc_num_mcast(sta->sdata) == 1 &&
+		    sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+			rcu_assign_pointer(vlansdata->u.vlan.sta0, sta);
+		else if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+			 test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+			RCU_INIT_POINTER(vlansdata->u.vlan.sta0, NULL);
 
 		ieee80211_send_layer2_update(sta);
 	}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 7b3de28..48a141f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -307,6 +307,8 @@ struct ieee80211_if_vlan {
 
 	/* used for all tx if the VLAN is configured to 4-addr mode */
 	struct sta_info __rcu *sta;
+	/* the one and only authenticated station in non 4-addr mode if set */
+	struct sta_info __rcu *sta0;
 	atomic_t num_mcast_sta_if; /* number of stations receiving multicast */
 };
 
@@ -1499,21 +1501,25 @@ ieee80211_have_rx_timestamp(struct ieee80211_rx_status *status)
 	return false;
 }
 
-static inline void
+static inline int
 ieee80211_vif_inc_num_mcast(struct ieee80211_sub_if_data *sdata)
 {
+	int ret;
+
 	if (sdata->vif.type != NL80211_IFTYPE_AP &&
 	    sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
-		return;
+		return -1;
 
 	if (sdata->vif.type == NL80211_IFTYPE_AP)
-		atomic_inc(&sdata->u.ap.num_mcast_sta_if);
+		ret = atomic_inc_return(&sdata->u.ap.num_mcast_sta_if);
 	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
-		atomic_inc(&sdata->u.vlan.num_mcast_sta_if);
+		ret = atomic_inc_return(&sdata->u.vlan.num_mcast_sta_if);
 
 	if (sdata->vif.type != NL80211_IFTYPE_AP_VLAN ||
 	    !sdata->u.vlan.sta) /* except 4addr mode */
 		atomic_inc(&sdata->bss->num_mcast_sta);
+
+	return ret;
 }
 
 static inline void
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 216ef65..d1c5d96 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -162,11 +162,28 @@ struct sta_info *sta_info_get(struct ieee80211_sub_if_data *sdata,
 			      const u8 *addr)
 {
 	struct ieee80211_local *local = sdata->local;
-	struct sta_info *sta;
+	struct sta_info *sta = NULL;
 	struct rhash_head *tmp;
 	const struct bucket_table *tbl;
 
 	rcu_read_lock();
+
+	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+	    !sdata->u.vlan.sta)
+		sta = rcu_dereference(sdata->u.vlan.sta0);
+
+	WARN_ONCE((sta && sta->sdata != sdata),
+		  "sdata->u.vlan.sta0->sdata != sdata");
+
+	if (sta && sta->sdata == sdata &&
+	    ether_addr_equal(sta->sta.addr, addr)) {
+		rcu_read_unlock();
+		/* this is safe as the caller must already hold
+		 * another rcu read section or the mutex
+		 */
+		return sta;
+	}
+
 	tbl = rht_dereference_rcu(local->sta_hash.tbl, &local->sta_hash);
 
 	for_each_sta_info(local, tbl, addr, sta, tmp) {
@@ -920,6 +937,10 @@ static int __must_check __sta_info_destroy_part1(struct sta_info *sta)
 	    rcu_access_pointer(sdata->u.vlan.sta) == sta)
 		RCU_INIT_POINTER(sdata->u.vlan.sta, NULL);
 
+	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+	    rcu_access_pointer(sdata->u.vlan.sta0) == sta)
+		RCU_INIT_POINTER(sdata->u.vlan.sta0, NULL);
+
 	return 0;
 }
 
@@ -1883,6 +1904,9 @@ int sta_info_move_state(struct sta_info *sta,
 				ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
 		} else if (sta->sta_state == IEEE80211_STA_AUTHORIZED) {
 			ieee80211_vif_dec_num_mcast(sta->sdata);
+			if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+			    rcu_access_pointer(sta->sdata->u.vlan.sta0) == sta)
+				RCU_INIT_POINTER(sta->sdata->u.vlan.sta0, NULL);
 			clear_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
 			ieee80211_clear_fast_xmit(sta);
 			ieee80211_clear_fast_rx(sta);
@@ -1890,7 +1914,12 @@ int sta_info_move_state(struct sta_info *sta,
 		break;
 	case IEEE80211_STA_AUTHORIZED:
 		if (sta->sta_state == IEEE80211_STA_ASSOC) {
-			ieee80211_vif_inc_num_mcast(sta->sdata);
+			if (ieee80211_vif_inc_num_mcast(sta->sdata) == 1 &&
+			    sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+				rcu_assign_pointer(sta->sdata->u.vlan.sta0,
+						   sta);
+			else if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+				RCU_INIT_POINTER(sta->sdata->u.vlan.sta0, NULL);
 			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 9c82fd8..c06d5f9 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1808,6 +1808,7 @@ ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
 		sta = rcu_dereference(sdata->u.vlan.sta);
 		if (sta) /* 4addr */
 			return 0;
+		prev = rcu_dereference(sdata->u.vlan.sta0);
 	case NL80211_IFTYPE_AP:
 		break;
 	default:
@@ -1839,6 +1840,9 @@ ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
 		return 0;
 	}
 
+	if (prev)
+		goto skip_lookup;
+
 	/* clone packets and update destination mac */
 	list_for_each_entry_rcu(sta, &local->sta_list, list) {
 		if (sdata != sta->sdata)
@@ -1862,6 +1866,7 @@ ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
 		prev = sta;
 	}
 
+skip_lookup:
 	if (likely(prev)) {
 		ieee80211_tx_dnat(skb, prev);
 		return 0;
-- 
2.1.4

Re: [PATCH 3/3] mac80211: cache the only AP_VLAN station

[Johannes Berg]

On Sun, 2016-09-25 at 18:39 +0200, Michael Braun wrote:
> If an AP_VLAN is only used with one station, cache a pointer to this
> station to avoid lookup. This should speed up station lookup when 

"should speed up"? Do you have numbers to back that up?

Read this as - you need to do more to convince me that the double
accounting and software complexity is worth it.

And then, if we really want/need to have the double accounting and all
the associated complexity and potential problems, perhaps making per-
interface lists, perhaps even replacing the global list (since we still
have a global hash table), or something similar would make more sense?

johannes