* [PATCH 1/3] mac80211: remove unnecessary num_mcast_sta user @ 2016-10-05 4:55 Michael Braun 2016-10-05 4:55 ` [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN Michael Braun 2016-10-05 4:55 ` [PATCH 3/3] mac80211: multicast to unicast conversion Michael Braun 0 siblings, 2 replies; 11+ messages in thread 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 [flat|nested] 11+ messages in thread
* [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN 2016-10-05 4:55 [PATCH 1/3] mac80211: remove unnecessary num_mcast_sta user Michael Braun @ 2016-10-05 4:55 ` Michael Braun 2016-10-05 9:56 ` Johannes Berg 2016-10-05 10:01 ` Johannes Berg 2016-10-05 4:55 ` [PATCH 3/3] mac80211: multicast to unicast conversion Michael Braun 1 sibling, 2 replies; 11+ messages in thread From: Michael Braun @ 2016-10-05 4:55 UTC (permalink / raw) To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan 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 [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN 2016-10-05 4:55 ` [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN Michael Braun @ 2016-10-05 9:56 ` Johannes Berg 2016-10-05 10:01 ` Johannes Berg 1 sibling, 0 replies; 11+ messages in thread From: Johannes Berg @ 2016-10-05 9:56 UTC (permalink / raw) To: Michael Braun; +Cc: linux-wireless, projekt-wlan > 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 [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN 2016-10-05 4:55 ` [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN Michael Braun 2016-10-05 9:56 ` Johannes Berg @ 2016-10-05 10:01 ` Johannes Berg 1 sibling, 0 replies; 11+ messages in thread From: Johannes Berg @ 2016-10-05 10:01 UTC (permalink / raw) To: Michael Braun; +Cc: linux-wireless, projekt-wlan 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 [flat|nested] 11+ messages in thread
* [PATCH 3/3] mac80211: multicast to unicast conversion 2016-10-05 4:55 [PATCH 1/3] mac80211: remove unnecessary num_mcast_sta user Michael Braun 2016-10-05 4:55 ` [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN Michael Braun @ 2016-10-05 4:55 ` Michael Braun 2016-10-05 10:19 ` Johannes Berg 1 sibling, 1 reply; 11+ messages in thread From: Michael Braun @ 2016-10-05 4:55 UTC (permalink / raw) To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan 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 [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mac80211: multicast to unicast conversion 2016-10-05 4:55 ` [PATCH 3/3] mac80211: multicast to unicast conversion Michael Braun @ 2016-10-05 10:19 ` Johannes Berg 2016-10-05 11:40 ` michael-dev 0 siblings, 1 reply; 11+ messages in thread From: Johannes Berg @ 2016-10-05 10:19 UTC (permalink / raw) To: Michael Braun; +Cc: linux-wireless, projekt-wlan, netdev +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 [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mac80211: multicast to unicast conversion 2016-10-05 10:19 ` Johannes Berg @ 2016-10-05 11:40 ` michael-dev 2016-10-05 11:58 ` Johannes Berg 0 siblings, 1 reply; 11+ messages in thread From: michael-dev @ 2016-10-05 11:40 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, projekt-wlan, netdev 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 [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mac80211: multicast to unicast conversion 2016-10-05 11:40 ` michael-dev @ 2016-10-05 11:58 ` Johannes Berg 2016-10-06 11:53 ` michael-dev 0 siblings, 1 reply; 11+ messages in thread From: Johannes Berg @ 2016-10-05 11:58 UTC (permalink / raw) To: michael-dev; +Cc: linux-wireless, projekt-wlan, netdev 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 [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mac80211: multicast to unicast conversion 2016-10-05 11:58 ` Johannes Berg @ 2016-10-06 11:53 ` michael-dev 2016-10-06 11:55 ` Johannes Berg 0 siblings, 1 reply; 11+ messages in thread From: michael-dev @ 2016-10-06 11:53 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, projekt-wlan, netdev Am 05.10.2016 13:58, schrieb Johannes Berg: > > 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... as I see it, that is exactly how DMS is standarized. IEEE 802.11-2012 section 10.23.15 DMS procedures: "If the requested DMS is accepted by the AP, the AP shall send subsequent group addressed MSDUs that match the frame classifier specified in the DMS Descriptors to the requesting STA as A-MSDU subframes within an individually addressed A-MSDU frame (see 8.3.2.2 and 9.11)." -> so the multicast packets shall go out as unicast A-MSDU frames to stations that requested this "The AP shall continue to transmit the matching frames as group addressed frames (see 9.3.6, and 10.2.1.16) if at least one associated STA has not requested DMS for these frames." -> so it will continue to send it as multicast frames as well. As with DMS the station requested DMS for a specific multicast address, it could then drop multicast frames addressed to the multicast address it registered for DMS. Regards, M. Braun ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mac80211: multicast to unicast conversion 2016-10-06 11:53 ` michael-dev @ 2016-10-06 11:55 ` Johannes Berg 0 siblings, 0 replies; 11+ messages in thread From: Johannes Berg @ 2016-10-06 11:55 UTC (permalink / raw) To: michael-dev; +Cc: linux-wireless, projekt-wlan, netdev On Thu, 2016-10-06 at 13:53 +0200, michael-dev wrote: > Am 05.10.2016 13:58, schrieb Johannes Berg: > > > > > > 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... > > as I see it, that is exactly how DMS is standarized. > > IEEE 802.11-2012 section 10.23.15 DMS procedures: > > "If the requested DMS is accepted by the AP, the AP shall send > subsequent group addressed MSDUs that > match the frame classifier specified in the DMS Descriptors to the > requesting STA as A-MSDU subframes > within an individually addressed A-MSDU frame (see 8.3.2.2 and > 9.11)." > > -> so the multicast packets shall go out as unicast A-MSDU frames > to stations that requested this Correct. > "The AP shall continue to transmit the matching frames as group > addressed frames (see 9.3.6, and 10.2.1.16) if at least one > associated > STA has not requested DMS for these frames." > > -> so it will continue to send it as multicast frames as well. > > As with DMS the station requested DMS for a specific multicast > address, it could then drop multicast frames addressed to the > multicast address it registered for DMS. Yes, the DMS spec tells it to do this. However, we can't implement non- DMS similarly, because then the station won't request it and won't drop the duplicates. So for this non-standard multicast-to-unicast, it's all or nothing, it can't be done for some stations only. johannes ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] mac80211: remove unnecessary num_mcast_sta user @ 2016-10-04 20:31 Michael Braun 2016-10-04 20:31 ` [PATCH 3/3] mac80211: multicast to unicast conversion Michael Braun 0 siblings, 1 reply; 11+ messages in thread From: Michael Braun @ 2016-10-04 20:31 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 [flat|nested] 11+ messages in thread
* [PATCH 3/3] mac80211: multicast to unicast conversion 2016-10-04 20:31 [PATCH 1/3] mac80211: remove unnecessary num_mcast_sta user Michael Braun @ 2016-10-04 20:31 ` Michael Braun 0 siblings, 0 replies; 11+ messages in thread From: Michael Braun @ 2016-10-04 20:31 UTC (permalink / raw) To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan 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> -- 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 | 6 +++ 9 files changed, 209 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..5bce212 100644 --- a/net/wireless/trace.h +++ b/net/wireless/trace.h @@ -773,6 +773,12 @@ DEFINE_EVENT(wiphy_netdev_mac_evt, rdev_set_wds_peer, TP_ARGS(wiphy, netdev, mac) ); +DEFINE_EVENT(wiphy_netdev_mac_evt, rdev_set_ap_unicast, + TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, + const bool unicast), + TP_ARGS(wiphy, netdev, mac) +); + TRACE_EVENT(rdev_dump_station, TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, int idx, u8 *mac), -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-10-06 11:55 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-05 4:55 [PATCH 1/3] mac80211: remove unnecessary num_mcast_sta user Michael Braun 2016-10-05 4:55 ` [PATCH 2/3] mac80211: filter multicast data packets on AP / AP_VLAN Michael Braun 2016-10-05 9:56 ` Johannes Berg 2016-10-05 10:01 ` Johannes Berg 2016-10-05 4:55 ` [PATCH 3/3] mac80211: multicast to unicast conversion Michael Braun 2016-10-05 10:19 ` Johannes Berg 2016-10-05 11:40 ` michael-dev 2016-10-05 11:58 ` Johannes Berg 2016-10-06 11:53 ` michael-dev 2016-10-06 11:55 ` Johannes Berg -- strict thread matches above, loose matches on Subject: below -- 2016-10-04 20:31 [PATCH 1/3] mac80211: remove unnecessary num_mcast_sta user Michael Braun 2016-10-04 20:31 ` [PATCH 3/3] mac80211: multicast to unicast conversion Michael Braun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).