This series adds basic kernel-mode TDLS support for nl80211 based drivers.
It is based in part on patches by Kalyan C. Gaddam, cc-ed here.
Support is added for peer discovery and data path setup/teardown. More
advanced features are not implemented. These include QoS/HT, peer PSM,
peer U-APSD and channel switching.
Notably, this patch-set does not include locking in the data path,
to switch between AP-based and direct Tx during link setup/tear-down.
This will be added by a later patch-set, if/when this series is
accepted. In practice it seems to work quite well without additional
locking.
User-mode support is added in a companion series.
Tested with wl12xx hardware, with a nl80211/mac80211 based driver.
Comments/reviews are welcome.
Arik
Cc: Kalyan C Gaddam <[email protected]>
Arik Nemtsov (5):
nl80211: support sending TDLS commands/frames
mac80211: handle TDLS high-level commands and frames
nl80211/mac80211: allow adding TDLS peers as stations
mac80211: add a HW flag for TDLS support
mac80211: send data directly to TDLS peers
include/linux/ieee80211.h | 75 +++++++++
include/linux/if_ether.h | 1 +
include/linux/nl80211.h | 40 +++++
include/net/cfg80211.h | 9 +
include/net/mac80211.h | 4 +
net/mac80211/Kconfig | 12 ++
net/mac80211/cfg.c | 383 +++++++++++++++++++++++++++++++++++++++++++++
net/mac80211/sta_info.h | 4 +
net/mac80211/tx.c | 24 +++-
net/wireless/nl80211.c | 73 +++++++++-
net/wireless/util.c | 5 +-
11 files changed, 623 insertions(+), 7 deletions(-)
--
1.7.4.1
Register and implement the TDLS cfg80211 callback functions.
Internally prepare and send TDLS management frames. We incorporate
local STA capabilities and supported rates with extra IEs given by
usermode. The resulting packet is either encapsulated in a data frame,
or assembled as an action frame. It is transmitted directly or using
the AP as a relay, as mandated by the TDLS specification.
Handle TDLS link enable/disable high-level commands, and refuse handling
link setup/teardown and discovery requests. This forces usermode to
handle these processes on its own and only use mac80211 for sending
TDLS mgmt packets.
Signed-off-by: Arik Nemtsov <[email protected]>
Cc: Kalyan C Gaddam <[email protected]>
---
include/linux/ieee80211.h | 75 +++++++++
include/linux/if_ether.h | 1 +
net/mac80211/Kconfig | 12 ++
net/mac80211/cfg.c | 366 +++++++++++++++++++++++++++++++++++++++++++++
net/mac80211/sta_info.h | 2 +
5 files changed, 456 insertions(+), 0 deletions(-)
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 37f95f2..8973c85 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -757,6 +757,12 @@ struct ieee80211_mgmt {
u8 action;
u8 smps_control;
} __attribute__ ((packed)) ht_smps;
+ struct {
+ u8 action_code;
+ u8 dialog_token;
+ __le16 capability;
+ u8 variable[0];
+ } __packed tdls_discover_resp;
} u;
} __attribute__ ((packed)) action;
} u;
@@ -796,6 +802,52 @@ struct ieee80211_pspoll {
u8 ta[6];
} __attribute__ ((packed));
+/* TDLS */
+
+/* Link-id information element */
+struct ieee80211_tdls_lnkie {
+ u8 ie_type; /* Link Identifier IE */
+ u8 ie_len;
+ u8 bssid[6];
+ u8 init_sta[6];
+ u8 resp_sta[6];
+} __packed;
+
+struct ieee80211_tdls_data {
+ u8 da[6];
+ u8 sa[6];
+ __be16 ether_type;
+ u8 payload_type;
+ u8 category;
+ u8 action_code;
+ union {
+ struct {
+ u8 dialog_token;
+ __le16 capability;
+ u8 variable[0];
+ } __packed setup_req;
+ struct {
+ __le16 status_code;
+ u8 dialog_token;
+ __le16 capability;
+ u8 variable[0];
+ } __packed setup_resp;
+ struct {
+ __le16 status_code;
+ u8 dialog_token;
+ u8 variable[0];
+ } __packed setup_cfm;
+ struct {
+ __le16 reason_code;
+ u8 variable[0];
+ } __packed teardown;
+ struct {
+ u8 dialog_token;
+ u8 variable[0];
+ } __packed discover_req;
+ } u;
+} __packed;
+
/**
* struct ieee80211_bar - HT Block Ack Request
*
@@ -1187,6 +1239,8 @@ enum ieee80211_eid {
WLAN_EID_TS_DELAY = 43,
WLAN_EID_TCLAS_PROCESSING = 44,
WLAN_EID_QOS_CAPA = 46,
+ /* 802.11z */
+ WLAN_EID_LINK_ID = 101,
/* 802.11s */
WLAN_EID_MESH_CONFIG = 113,
WLAN_EID_MESH_ID = 114,
@@ -1270,6 +1324,7 @@ enum ieee80211_category {
WLAN_CATEGORY_HT = 7,
WLAN_CATEGORY_SA_QUERY = 8,
WLAN_CATEGORY_PROTECTED_DUAL_OF_ACTION = 9,
+ WLAN_CATEGORY_TDLS = 12,
WLAN_CATEGORY_MESH_ACTION = 13,
WLAN_CATEGORY_MULTIHOP_ACTION = 14,
WLAN_CATEGORY_SELF_PROTECTED = 15,
@@ -1333,6 +1388,26 @@ enum ieee80211_key_len {
WLAN_KEY_LEN_AES_CMAC = 16,
};
+/* Public action codes */
+enum ieee80211_pub_actioncode {
+ WLAN_PUB_ACTION_TDLS_DISCOVER_RES = 14,
+};
+
+/* TDLS action codes */
+enum ieee80211_tdls_actioncode {
+ WLAN_TDLS_SETUP_REQUEST = 0,
+ WLAN_TDLS_SETUP_RESPONSE = 1,
+ WLAN_TDLS_SETUP_CONFIRM = 2,
+ WLAN_TDLS_TEARDOWN = 3,
+ WLAN_TDLS_PEER_TRAFFIC_INDICATION = 4,
+ WLAN_TDLS_CHANNEL_SWITCH_REQUEST = 5,
+ WLAN_TDLS_CHANNEL_SWITCH_RESPONSE = 6,
+ WLAN_TDLS_PEER_PSM_REQUEST = 7,
+ WLAN_TDLS_PEER_PSM_RESPONSE = 8,
+ WLAN_TDLS_PEER_TRAFFIC_RESPONSE = 9,
+ WLAN_TDLS_DISCOVERY_REQUEST = 10,
+};
+
/**
* enum - mesh path selection protocol identifier
*
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index a3d99ff..49c38fc 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -83,6 +83,7 @@
#define ETH_P_8021AH 0x88E7 /* 802.1ah Backbone Service Tag */
#define ETH_P_1588 0x88F7 /* IEEE 1588 Timesync */
#define ETH_P_FCOE 0x8906 /* Fibre Channel over Ethernet */
+#define ETH_P_TDLS 0x890D /* TDLS */
#define ETH_P_FIP 0x8914 /* FCoE Initialization Protocol */
#define ETH_P_QINQ1 0x9100 /* deprecated QinQ VLAN [ NOT AN OFFICIALLY REGISTERED ID ] */
#define ETH_P_QINQ2 0x9200 /* deprecated QinQ VLAN [ NOT AN OFFICIALLY REGISTERED ID ] */
diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index d1886b5..7d3b438 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -225,6 +225,18 @@ config MAC80211_VERBOSE_MHWMP_DEBUG
Do not select this option.
+config MAC80211_VERBOSE_TDLS_DEBUG
+ bool "Verbose TDLS debugging"
+ depends on MAC80211_DEBUG_MENU
+ ---help---
+ Selecting this option causes mac80211 to print out very
+ verbose TDLS selection debugging messages (when mac80211
+ is a TDLS STA).
+ It should not be selected on production systems as those
+ messages are remotely triggerable.
+
+ Do not select this option.
+
config MAC80211_DEBUG_COUNTERS
bool "Extra statistics for TX/RX debugging"
depends on MAC80211_DEBUG_MENU
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 7d17a91..27dda09 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include <net/net_namespace.h>
#include <linux/rcupdate.h>
+#include <linux/if_ether.h>
#include <net/cfg80211.h>
#include "ieee80211_i.h"
#include "driver-ops.h"
@@ -2092,6 +2093,369 @@ static int ieee80211_set_rekey_data(struct wiphy *wiphy,
return 0;
}
+static void ieee80211_tdls_add_supp_rates(struct ieee80211_sub_if_data *sdata,
+ struct sk_buff *skb)
+{
+ struct ieee80211_local *local = sdata->local;
+ struct wiphy *wiphy = local->hw.wiphy;
+ enum ieee80211_band band = local->oper_channel->band;
+ struct ieee80211_supported_band *sband = wiphy->bands[band];
+ int rates, i;
+ u8 *pos;
+
+ rates = sband->n_bitrates;
+ if (rates > 8)
+ rates = 8;
+
+ pos = skb_put(skb, rates + 2);
+ *pos++ = WLAN_EID_SUPP_RATES;
+ *pos++ = rates;
+
+ /* No basic rates */
+ for (i = 0; i < rates; i++) {
+ int rate = sband->bitrates[i].bitrate;
+ *pos++ = (u8) (rate / 5);
+ }
+
+ if (wiphy->bands[band]->n_bitrates <= 8)
+ return;
+
+ rates = wiphy->bands[band]->n_bitrates - 8;
+ pos = skb_put(skb, rates + 2);
+ *pos++ = WLAN_EID_EXT_SUPP_RATES;
+ *pos++ = rates;
+
+ for (i = 0; i < rates; i++) {
+ int rate = wiphy->bands[band]->bitrates[i + 8].bitrate;
+ *pos++ = (u8) (rate / 5);
+ }
+}
+
+static void ieee80211_tdls_add_ext_capab(struct sk_buff *skb)
+{
+ u8 *pos = (void *)skb_put(skb, 7);
+
+ *pos++ = WLAN_EID_EXT_CAPABILITY;
+ *pos++ = 5; /* len */
+ *pos++ = 0x0;
+ *pos++ = 0x0;
+ *pos++ = 0x0;
+ *pos++ = 0x0;
+ *pos++ = 0x20; /* TDLS enabled */
+}
+
+static u16 ieee80211_get_tdls_sta_capab(struct ieee80211_sub_if_data *sdata)
+{
+ u16 capab;
+ struct ieee80211_local *local = sdata->local;
+
+ /* We do not support any exotic capabilities yet */
+
+ capab = WLAN_CAPABILITY_ESS;
+ if (local->oper_channel->band != IEEE80211_BAND_2GHZ)
+ return capab;
+
+ if (!(local->hw.flags & IEEE80211_HW_2GHZ_SHORT_SLOT_INCAPABLE))
+ capab |= WLAN_CAPABILITY_SHORT_SLOT_TIME;
+ if (!(local->hw.flags & IEEE80211_HW_2GHZ_SHORT_PREAMBLE_INCAPABLE))
+ capab |= WLAN_CAPABILITY_SHORT_PREAMBLE;
+
+ return capab;
+}
+
+static void ieee80211_tdls_add_link_ie(struct sk_buff *skb, u8 *src_addr,
+ u8 *peer, u8 *bssid)
+{
+ struct ieee80211_tdls_lnkie *lnkid;
+
+ lnkid = (void *)skb_put(skb, sizeof(struct ieee80211_tdls_lnkie));
+
+ lnkid->ie_type = WLAN_EID_LINK_ID;
+ lnkid->ie_len = 18;
+
+ memcpy(lnkid->bssid, bssid, ETH_ALEN);
+ memcpy(lnkid->init_sta, src_addr, ETH_ALEN);
+ memcpy(lnkid->resp_sta, peer, ETH_ALEN);
+}
+
+static int
+ieee80211_prep_tdls_encap_data(struct wiphy *wiphy, struct net_device *dev,
+ u8 *peer, u8 action_code, u8 dialog_token,
+ u16 status_code, struct sk_buff *skb)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_tdls_data *tf;
+
+ tf = (void *)skb_put(skb, offsetof(struct ieee80211_tdls_data, u));
+
+ memcpy(tf->da, peer, ETH_ALEN);
+ memcpy(tf->sa, sdata->vif.addr, ETH_ALEN);
+ tf->ether_type = cpu_to_be16(ETH_P_TDLS);
+ tf->payload_type = 2; /* RFTYPE_TDLS */
+
+ switch (action_code) {
+ case WLAN_TDLS_SETUP_REQUEST:
+ tf->category = WLAN_CATEGORY_TDLS;
+ tf->action_code = WLAN_TDLS_SETUP_REQUEST;
+
+ skb_put(skb, sizeof(tf->u.setup_req));
+ tf->u.setup_req.dialog_token = dialog_token;
+ tf->u.setup_req.capability =
+ cpu_to_le16(ieee80211_get_tdls_sta_capab(sdata));
+
+ ieee80211_tdls_add_supp_rates(sdata, skb);
+ ieee80211_tdls_add_ext_capab(skb);
+ break;
+ case WLAN_TDLS_SETUP_RESPONSE:
+ tf->category = WLAN_CATEGORY_TDLS;
+ tf->action_code = WLAN_TDLS_SETUP_RESPONSE;
+
+ skb_put(skb, sizeof(tf->u.setup_resp));
+ tf->u.setup_resp.status_code = cpu_to_le16(status_code);
+ tf->u.setup_resp.dialog_token = dialog_token;
+ tf->u.setup_resp.capability =
+ cpu_to_le16(ieee80211_get_tdls_sta_capab(sdata));
+
+ ieee80211_tdls_add_supp_rates(sdata, skb);
+ ieee80211_tdls_add_ext_capab(skb);
+ break;
+ case WLAN_TDLS_SETUP_CONFIRM:
+ tf->category = WLAN_CATEGORY_TDLS;
+ tf->action_code = WLAN_TDLS_SETUP_CONFIRM;
+
+ skb_put(skb, sizeof(tf->u.setup_cfm));
+ tf->u.setup_cfm.status_code = cpu_to_le16(status_code);
+ tf->u.setup_cfm.dialog_token = dialog_token;
+ break;
+ case WLAN_TDLS_TEARDOWN:
+ tf->category = WLAN_CATEGORY_TDLS;
+ tf->action_code = WLAN_TDLS_TEARDOWN;
+
+ skb_put(skb, sizeof(tf->u.teardown));
+ tf->u.teardown.reason_code = cpu_to_le16(status_code);
+ break;
+ case WLAN_TDLS_DISCOVERY_REQUEST:
+ tf->category = WLAN_CATEGORY_TDLS;
+ tf->action_code = WLAN_TDLS_DISCOVERY_REQUEST;
+
+ skb_put(skb, sizeof(tf->u.discover_req));
+ tf->u.discover_req.dialog_token = dialog_token;
+ break;
+ default:
+ /* we don't support anything else for now */
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int
+ieee80211_prep_tdls_direct(struct wiphy *wiphy, struct net_device *dev,
+ u8 *peer, u8 action_code, u8 dialog_token,
+ u16 status_code, struct sk_buff *skb)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_mgmt *mgmt;
+
+ mgmt = (void *)skb_put(skb, 24);
+ memset(mgmt, 0, 24);
+ memcpy(mgmt->da, peer, ETH_ALEN);
+ memcpy(mgmt->sa, sdata->vif.addr, ETH_ALEN);
+ memcpy(mgmt->bssid, sdata->u.mgd.bssid, ETH_ALEN);
+
+ mgmt->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
+ IEEE80211_STYPE_ACTION);
+
+ switch (action_code) {
+ case WLAN_PUB_ACTION_TDLS_DISCOVER_RES:
+ skb_put(skb, 1 + sizeof(mgmt->u.action.u.tdls_discover_resp));
+ mgmt->u.action.category = WLAN_CATEGORY_PUBLIC;
+ mgmt->u.action.u.tdls_discover_resp.action_code =
+ WLAN_PUB_ACTION_TDLS_DISCOVER_RES;
+ mgmt->u.action.u.tdls_discover_resp.dialog_token =
+ dialog_token;
+ mgmt->u.action.u.tdls_discover_resp.capability =
+ cpu_to_le16(ieee80211_get_tdls_sta_capab(sdata));
+
+ ieee80211_tdls_add_supp_rates(sdata, skb);
+ ieee80211_tdls_add_ext_capab(skb);
+ break;
+ default:
+ /* we don't support other actions for now */
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ieee80211_tdls_mgmt(struct wiphy *wiphy, struct net_device *dev,
+ u8 *peer, u8 action_code, u8 dialog_token,
+ u16 status_code, const u8 *extra_ies,
+ size_t extra_ies_len)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
+ struct ieee80211_tx_info *info;
+ struct sk_buff *skb = NULL;
+ bool send_direct;
+ int ret;
+
+#ifdef CONFIG_MAC80211_VERBOSE_TDLS_DEBUG
+ printk(KERN_DEBUG "TDLS mgmt action %d peer %pM\n", action_code, peer);
+#endif
+
+ /* make sure we are in managed mode, and associated */
+ if (sdata->vif.type != NL80211_IFTYPE_STATION ||
+ !sdata->u.mgd.associated) {
+ return -EINVAL;
+ }
+
+ skb = dev_alloc_skb(local->hw.extra_tx_headroom +
+ max(sizeof(struct ieee80211_mgmt),
+ sizeof(struct ieee80211_tdls_data)) +
+ 50 + /* supported rates */
+ 7 + /* ext capab */
+ sizeof(struct ieee80211_tdls_lnkie));
+ if (!skb)
+ return -ENOMEM;
+
+ info = IEEE80211_SKB_CB(skb);
+ skb_reserve(skb, local->hw.extra_tx_headroom);
+
+ switch (action_code) {
+ case WLAN_TDLS_SETUP_REQUEST:
+ case WLAN_TDLS_SETUP_RESPONSE:
+ case WLAN_TDLS_SETUP_CONFIRM:
+ case WLAN_TDLS_TEARDOWN:
+ case WLAN_TDLS_DISCOVERY_REQUEST:
+ ret = ieee80211_prep_tdls_encap_data(wiphy, dev, peer,
+ action_code, dialog_token,
+ status_code, skb);
+ send_direct = false;
+ break;
+ case WLAN_PUB_ACTION_TDLS_DISCOVER_RES:
+ ret = ieee80211_prep_tdls_direct(wiphy, dev, peer, action_code,
+ dialog_token, status_code,
+ skb);
+ send_direct = true;
+ break;
+ default:
+ /* we don't support other actions for now */
+ ret = -EINVAL;
+ break;
+ }
+
+ if (ret < 0)
+ goto fail;
+
+ if (extra_ies_len)
+ memcpy(skb_put(skb, extra_ies_len), extra_ies, extra_ies_len);
+
+ /* the TDLS link IE is always added last */
+ switch (action_code) {
+ case WLAN_TDLS_SETUP_REQUEST:
+ case WLAN_TDLS_SETUP_CONFIRM:
+ case WLAN_TDLS_TEARDOWN:
+ case WLAN_TDLS_DISCOVERY_REQUEST:
+ /* we are the initiator */
+ ieee80211_tdls_add_link_ie(skb, sdata->vif.addr, peer,
+ sdata->u.mgd.bssid);
+ break;
+ case WLAN_TDLS_SETUP_RESPONSE:
+ case WLAN_PUB_ACTION_TDLS_DISCOVER_RES:
+ /* we are the responder */
+ ieee80211_tdls_add_link_ie(skb, peer, sdata->vif.addr,
+ sdata->u.mgd.bssid);
+ break;
+ default:
+ WARN_ON(1);
+ ret = -EINVAL;
+ goto fail;
+ break;
+ }
+
+ if (send_direct) {
+ ieee80211_tx_skb(sdata, skb);
+ return 0;
+ }
+
+ /*
+ * According to 802.11z: Setup req/resp are sent in AC_BK, otherwise
+ * we should default to AC_VI.
+ */
+ switch (action_code) {
+ case WLAN_TDLS_SETUP_REQUEST:
+ case WLAN_TDLS_SETUP_RESPONSE:
+ skb_set_queue_mapping(skb, IEEE80211_AC_BK);
+ skb->priority = 2;
+ break;
+ default:
+ skb_set_queue_mapping(skb, IEEE80211_AC_VI);
+ skb->priority = 5;
+ break;
+ }
+
+ /* disable bottom halves when entering the Tx path */
+ local_bh_disable();
+ ret = ieee80211_subif_start_xmit(skb, dev);
+ local_bh_enable();
+
+ return ret;
+
+fail:
+ if (skb)
+ dev_kfree_skb(skb);
+ return ret;
+}
+
+static int ieee80211_tdls_oper(struct wiphy *wiphy, struct net_device *dev,
+ u8 *peer, enum nl80211_tdls_operation oper)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct sta_info *sta = NULL;
+
+#ifdef CONFIG_MAC80211_VERBOSE_TDLS_DEBUG
+ printk(KERN_DEBUG "TDLS oper %d peer %pM\n", oper, peer);
+#endif
+
+ switch (oper) {
+ case NL80211_TDLS_ENABLE_LINK:
+ rcu_read_lock();
+ sta = sta_info_get(sdata, peer);
+ if (sta) {
+ set_sta_flags(sta, WLAN_STA_AUTHORIZED);
+ sta->tdls_link_enabled = true;
+ }
+ rcu_read_unlock();
+ break;
+ case NL80211_TDLS_DISABLE_LINK:
+ rcu_read_lock();
+ sta = sta_info_get(sdata, peer);
+ if (sta) {
+ sta->tdls_link_enabled = false;
+ sta_info_destroy_addr(sdata, peer);
+ }
+ rcu_read_unlock();
+ break;
+ case NL80211_TDLS_TEARDOWN:
+ case NL80211_TDLS_SETUP:
+ case NL80211_TDLS_DISCOVERY_REQ:
+ /* We don't support in-driver setup/teardown/discovery */
+ return -ENOTSUPP;
+ case NL80211_TDLS_DISABLE:
+ case NL80211_TDLS_ENABLE:
+ /* TODO */
+ return -ENOTSUPP;
+ default:
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -2155,4 +2519,6 @@ struct cfg80211_ops mac80211_config_ops = {
.set_ringparam = ieee80211_set_ringparam,
.get_ringparam = ieee80211_get_ringparam,
.set_rekey_data = ieee80211_set_rekey_data,
+ .tdls_oper = ieee80211_tdls_oper,
+ .tdls_mgmt = ieee80211_tdls_mgmt,
};
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 28beb78..25fb49e 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -336,6 +336,8 @@ struct sta_info {
unsigned int lost_packets;
+ bool tdls_link_enabled;
+
/* keep last! */
struct ieee80211_sta sta;
};
--
1.7.4.1
On Wed, 2011-09-21 at 20:36 -0400, Kalyan Chakravarthy wrote:
> I thought ATTR_FRAME would be more appropriate since it carries multiple IE's, such as RSN, FTIE etc.
ATTR_FRAME is typically used to carry a frame including header/fixed
portion, so ATTR_IE would be more appropriate if it doesn't include that
-- and the docs say multiple IEs can be there.
johannes
On Thu, Sep 15, 2011 at 13:25, Arik Nemtsov <[email protected]> wrote:
> Allow TDLS operations and peers only in supporting hardware.
>
> Signed-off-by: Arik Nemtsov <[email protected]>
> Cc: Kalyan C Gaddam <[email protected]>
> ---
> ?include/net/mac80211.h | ? ?4 ++++
> ?net/mac80211/cfg.c ? ? | ? 11 +++++++++++
> ?2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 2f01d84..25bd038 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1094,6 +1094,9 @@ enum sta_notify_cmd {
> ?* ? ? stations based on the PM bit of incoming frames.
> ?* ? ? Use ieee80211_start_ps()/ieee8021_end_ps() to manually configure
> ?* ? ? the PS mode of connected stations.
> + *
> + * @IEEE80211_HW_SUPPORTS_TDLS:
> + * ? ? This device can operate as a TDLS (802.11z) peer.
> ?*/
I think that for v2 I'll replace this with a wiphy flag that will also
be propagated to usermode. An additional wiphy flag (something like
TDLS_EXTERNAL_LINK_SETUP) will toggle between the full-mac mode and
the mode used in mac80211.
Arik
On Fri, Sep 16, 2011 at 15:59, Johannes Berg <[email protected]> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>
>> + ? ? case NL80211_TDLS_ENABLE_LINK:
>> + ? ? ? ? ? ? rcu_read_lock();
>> + ? ? ? ? ? ? sta = sta_info_get(sdata, peer);
>> + ? ? ? ? ? ? if (sta) {
>> + ? ? ? ? ? ? ? ? ? ? set_sta_flags(sta, WLAN_STA_AUTHORIZED);
>> + ? ? ? ? ? ? ? ? ? ? sta->tdls_link_enabled = true;
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? rcu_read_unlock();
>> + ? ? ? ? ? ? break;
>
> This seems to require the station already having been added, but
> couldn't this create the data race you were worried about? Could you not
> simply just create the station once it is authorized?
The station is added only when the link is already authorized (see
wpa_tdls_enable_link() in wpa_s). This whole bit is kind of redundant.
A cleaner solution would be to just automatically authorize tdls
stations in ieee80211_add_station() and use WLAN_STA_TDLS_PEER instead
of tdls_link_enabled. I'll add this to my v2 list. Thanks.
While this creates a race, it's not the one i'm worried about. It will
also be solved automatically when the TDLS locking requirements are
met (as mentioned in a previous email).
But I agree your approach is cleaner.
>
>> + ? ? case NL80211_TDLS_DISABLE_LINK:
>> + ? ? ? ? ? ? rcu_read_lock();
>> + ? ? ? ? ? ? sta = sta_info_get(sdata, peer);
>> + ? ? ? ? ? ? if (sta) {
>> + ? ? ? ? ? ? ? ? ? ? sta->tdls_link_enabled = false;
>> + ? ? ? ? ? ? ? ? ? ? sta_info_destroy_addr(sdata, peer);
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? rcu_read_unlock();
>> + ? ? ? ? ? ? break;
>
> Isn't that equivalent to just deleting the station?
Yep. The whole sta->tdls_link_enabled thing is kind of redundant. v2.
Arik
Allow TDLS operations and peers only in supporting hardware.
Signed-off-by: Arik Nemtsov <[email protected]>
Cc: Kalyan C Gaddam <[email protected]>
---
include/net/mac80211.h | 4 ++++
net/mac80211/cfg.c | 11 +++++++++++
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2f01d84..25bd038 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1094,6 +1094,9 @@ enum sta_notify_cmd {
* stations based on the PM bit of incoming frames.
* Use ieee80211_start_ps()/ieee8021_end_ps() to manually configure
* the PS mode of connected stations.
+ *
+ * @IEEE80211_HW_SUPPORTS_TDLS:
+ * This device can operate as a TDLS (802.11z) peer.
*/
enum ieee80211_hw_flags {
IEEE80211_HW_HAS_RATE_CONTROL = 1<<0,
@@ -1119,6 +1122,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SUPPORTS_CQM_RSSI = 1<<20,
IEEE80211_HW_SUPPORTS_PER_STA_GTK = 1<<21,
IEEE80211_HW_AP_LINK_PS = 1<<22,
+ IEEE80211_HW_SUPPORTS_TDLS = 1<<23,
};
/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 3ef0bc6..9c6ac93 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -797,6 +797,11 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev,
sta_apply_parameters(local, sta, params);
+ if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_TDLS) &&
+ (sta->flags & WLAN_STA_TDLS_PEER)) {
+ return -ENOTSUPP;
+ }
+
rate_control_rate_init(sta);
layer2_update = sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
@@ -2308,6 +2313,9 @@ static int ieee80211_tdls_mgmt(struct wiphy *wiphy, struct net_device *dev,
bool send_direct;
int ret;
+ if (!(sdata->local->hw.flags & IEEE80211_HW_SUPPORTS_TDLS))
+ return -ENOTSUPP;
+
#ifdef CONFIG_MAC80211_VERBOSE_TDLS_DEBUG
printk(KERN_DEBUG "TDLS mgmt action %d peer %pM\n", action_code, peer);
#endif
@@ -2422,6 +2430,9 @@ static int ieee80211_tdls_oper(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct sta_info *sta = NULL;
+ if (!(sdata->local->hw.flags & IEEE80211_HW_SUPPORTS_TDLS))
+ return -ENOTSUPP;
+
#ifdef CONFIG_MAC80211_VERBOSE_TDLS_DEBUG
printk(KERN_DEBUG "TDLS oper %d peer %pM\n", oper, peer);
#endif
--
1.7.4.1
On Wed, Sep 21, 2011 at 15:06, Johannes Berg <[email protected]> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>> Allow TDLS operations and peers only in supporting hardware.
>
> In terms of patch ordering I think this should be part of patch 1 (and
> maybe patch 3 can be too), otherwise patch 2 adds some incomplete
> mac80211 stuff and even enables it ...
Sure.
Arik
I thought ATTR_FRAME would be more appropriate since it carries multiple IE's, such as RSN, FTIE etc.
----- Original Message -----
From: Johannes Berg <[email protected]>
Date: Wednesday, September 21, 2011 7:54 am
Subject: Re: [RFC 1/5] nl80211: support sending TDLS commands/frames
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>
> > +static int nl80211_tdls_mgmt(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];
> > + u8 action_code, dialog_token;
> > + u16 status_code;
> > + u8 *peer;
> > +
> > + if (!rdev->ops->tdls_mgmt)
> > + return -EOPNOTSUPP;
> > +
> > + if (!info->attrs[NL80211_ATTR_TDLS_ACTION] ||
> > + !info->attrs[NL80211_ATTR_STATUS_CODE] ||
> > + !info->attrs[NL80211_ATTR_TDLS_DIALOG_TOKEN] ||
> > + !info->attrs[NL80211_ATTR_FRAME] ||
> > + !info->attrs[NL80211_ATTR_MAC])
> > + return -EINVAL;
>
> Should that really use ATTR_FRAME as opposed to ATTR_IE?
>
> johannes
>
>
On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
> +static void ieee80211_tdls_add_supp_rates(struct ieee80211_sub_if_data *sdata,
> + struct sk_buff *skb)
maybe it's time to refactor adding supported rates? This must be like
the 5th implementation of it ... mesh has exactly the same at least.
> +static void ieee80211_tdls_add_ext_capab(struct sk_buff *skb)
> +{
> + u8 *pos = (void *)skb_put(skb, 7);
> +
> + *pos++ = WLAN_EID_EXT_CAPABILITY;
> + *pos++ = 5; /* len */
> + *pos++ = 0x0;
> + *pos++ = 0x0;
> + *pos++ = 0x0;
> + *pos++ = 0x0;
> + *pos++ = 0x20; /* TDLS enabled */
That might benefit from a constant in ieee80211.h
> +static u16 ieee80211_get_tdls_sta_capab(struct ieee80211_sub_if_data *sdata)
> +{
> + u16 capab;
> + struct ieee80211_local *local = sdata->local;
> +
> + /* We do not support any exotic capabilities yet */
> +
> + capab = WLAN_CAPABILITY_ESS;
Should we really set the ESS bit for TDLS?
> +static void ieee80211_tdls_add_link_ie(struct sk_buff *skb, u8 *src_addr,
> + u8 *peer, u8 *bssid)
> +{
> + struct ieee80211_tdls_lnkie *lnkid;
> +
> + lnkid = (void *)skb_put(skb, sizeof(struct ieee80211_tdls_lnkie));
> +
> + lnkid->ie_type = WLAN_EID_LINK_ID;
> + lnkid->ie_len = 18;
sizeof too?
> + memcpy(tf->da, peer, ETH_ALEN);
> + memcpy(tf->sa, sdata->vif.addr, ETH_ALEN);
> + tf->ether_type = cpu_to_be16(ETH_P_TDLS);
> + tf->payload_type = 2; /* RFTYPE_TDLS */
another constant?
> + default:
> + /* we don't support other actions for now */
> + WARN_ON(1);
> + return -EINVAL;
Wouldn't that warning be triggerable from userspace?
> +static int ieee80211_tdls_mgmt(struct wiphy *wiphy, struct net_device *dev,
> + u8 *peer, u8 action_code, u8 dialog_token,
> + u16 status_code, const u8 *extra_ies,
> + size_t extra_ies_len)
> +{
> + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> + struct ieee80211_local *local = sdata->local;
> + struct ieee80211_tx_info *info;
> + struct sk_buff *skb = NULL;
> + bool send_direct;
> + int ret;
> +
> +#ifdef CONFIG_MAC80211_VERBOSE_TDLS_DEBUG
> + printk(KERN_DEBUG "TDLS mgmt action %d peer %pM\n", action_code, peer);
> +#endif
> +
> + /* make sure we are in managed mode, and associated */
> + if (sdata->vif.type != NL80211_IFTYPE_STATION ||
> + !sdata->u.mgd.associated) {
> + return -EINVAL;
> + }
braces here seem useless :)
Also accessing u.mgd.associated as a bool like this is fine, but
obviously racy. How do we deal with that? Do we even tear down links
when disassociating, and do we even need to beyond just killing the
station entries?
> + skb = dev_alloc_skb(local->hw.extra_tx_headroom +
> + max(sizeof(struct ieee80211_mgmt),
> + sizeof(struct ieee80211_tdls_data)) +
> + 50 + /* supported rates */
> + 7 + /* ext capab */
> + sizeof(struct ieee80211_tdls_lnkie));
Where are the extra IEs used? They should be accounted for too.
> +fail:
> + if (skb)
> + dev_kfree_skb(skb);
I believe the if() is pointless.
> + case NL80211_TDLS_DISABLE_LINK:
> + rcu_read_lock();
> + sta = sta_info_get(sdata, peer);
> + if (sta) {
> + sta->tdls_link_enabled = false;
> + sta_info_destroy_addr(sdata, peer);
> + }
> + rcu_read_unlock();
You can't call destroy_addr within rcu section.
> + case NL80211_TDLS_TEARDOWN:
> + case NL80211_TDLS_SETUP:
> + case NL80211_TDLS_DISCOVERY_REQ:
> + /* We don't support in-driver setup/teardown/discovery */
> + return -ENOTSUPP;
> + case NL80211_TDLS_DISABLE:
> + case NL80211_TDLS_ENABLE:
> + /* TODO */
> + return -ENOTSUPP;
> + default:
> + WARN_ON(1);
> + return -EINVAL;
might want to use EOPNOTSUPP and not have a warning -- if nl80211 ever
gets extended with more actions you might run into the default case.
johannes
On Wed, Sep 21, 2011 at 15:06, Johannes Berg <[email protected]> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>> When a TDLS peer STA is added it is marked with a new flag in both
>> nl80211 and in mac80211.
>
> I think it'd make some sense to allow only TDLS stations on a managed
> mode interface?
Yea i'll add the check.
Arik
On Fri, 2011-09-16 at 19:48 +0300, Arik Nemtsov wrote:
> We had some qualms about the design ourselves. This is a question
> Kalyan sent to the hostap list a while back -
> http://lists.shmoo.com/pipermail/hostap/2011-June/023311.html
>
> The locking requirement (elaborated at the end of the email) was also
> part of the decision to make mac80211 more aware of the connection
> state. Also, eventually mac80211 will have to add some IEs of it's own
> to the setup packets.
Hmm, ok, adding the IEs would be a deal-breaker, it didn't seem
necessary though.
> wpa_supplicant needs to add some IEs to the packet (RSN, FTIE, ..).
> mac80211 will also have to add some IEs (U-APSD, HT, ...)
> Does it really matter if the packet's headers are created by usermode
> or by kernel? Even if we change the "owner" of the packet, the API
> would still remain about the same. tdls_mgmt would simply get the
> frame instead of extra IEs.
Not really. But adding the extra datapath seemed a little pointless.
> Jouni's original wpa_s TDLS code was written for a full-mac driver
> where the kernel/firmware side created the packet. wpa_supplicant only
> provided auth IEs. Keeping it this way also avoids refactoring the
> code and introducing bugs.
Yeah that's another good argument :-)
> >> Notably, this patch-set does not include locking in the data path,
> >> to switch between AP-based and direct Tx during link setup/tear-down.
> >> This will be added by a later patch-set, if/when this series is
> >> accepted. In practice it seems to work quite well without additional
> >> locking.
> >
> > That ought to work without locks if you create/destroy the station
> > entries appropriately?
>
> I should elaborate on the locking requirement. The TDLS link setup is
> made up 3 frames - a setup request, followed by a response from the
> peer, and finally a confirm.
> This is an excerpt from the spec:
>
> "To avoid possible reordering of MSDUs, a TDLS initiator STA shall
> cease transmitting MSDUs to the
> TDLS responder STA through the AP after sending a TDLS Setup Request
> frame, and a TDLS responder
> STA shall cease transmitting MSDUs to the TDLS initiator STA through
> the AP after sending a TDLS
> Setup Response frame indicating status code 0 (Success)."
>
> This requires a locking mechanism similar to AP-mode buffering for
> stations in PS (since we probably don't want to stop all queues).
Ok, so the special thing is really that you need to stop after the setup
request frame. Makes sense.
johannes
On Thu, 2011-09-22 at 10:34 +0300, Arik Nemtsov wrote:
> > Also accessing u.mgd.associated as a bool like this is fine, but
> > obviously racy. How do we deal with that? Do we even tear down links
> > when disassociating, and do we even need to beyond just killing the
> > station entries?
>
> We don't tear down links just before disassociating (there are too
> many corner cases here). We just disable the links post-factum.
> Killing the station entries won't help for packets meant to be sent
> over the AP. I guess we can take the mutex for a little extra safely
> (but it won't do much).
>
> The race is even worse - from queuing until the actual Tx, we could
> have disconnected from this AP and connected do a totally different
> one. But this shouldn't happen in reality (and we can add some guards
> to wpa_supplicant to make sure).
> Do you see this as a security threat?
I don't really know :-) I just saw the possible race here. If you remove
the TDLS station entries when disassociating then it shouldn't be an
issue, right? Since you require a station entry pretty early on anyway.
johannes
On Fri, Sep 16, 2011 at 16:03, Johannes Berg <[email protected]> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>> ? ? ? case NL80211_IFTYPE_STATION:
>> - ? ? ? ? ? ? memcpy(hdr.addr1, sdata->u.mgd.bssid, ETH_ALEN);
>> - ? ? ? ? ? ? if (sdata->u.mgd.use_4addr &&
>> - ? ? ? ? ? ? ? ? cpu_to_be16(ethertype) != sdata->control_port_protocol) {
>> - ? ? ? ? ? ? ? ? ? ? fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS);
>> + ? ? ? ? ? ? if (local->hw.flags & IEEE80211_HW_SUPPORTS_TDLS) {
>> + ? ? ? ? ? ? ? ? ? ? rcu_read_lock();
>> + ? ? ? ? ? ? ? ? ? ? sta = sta_info_get(sdata, skb->data);
>> + ? ? ? ? ? ? ? ? ? ? tdls_link = (sta && sta->tdls_link_enabled);
>
> Why don't you test WLAN_STA_TDLS_PEER and add the station only when the
> session is set up?
>
> That would avoid the problem here when setting up the session.
Will do (like I wrote in a previous email). Thanks.
>
>> --- a/net/wireless/util.c
>> +++ b/net/wireless/util.c
>> @@ -392,8 +392,9 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
>> ? ? ? ? ? ? ? }
>> ? ? ? ? ? ? ? break;
>> ? ? ? case cpu_to_le16(0):
>> - ? ? ? ? ? ? if (iftype != NL80211_IFTYPE_ADHOC)
>> - ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? if (iftype != NL80211_IFTYPE_ADHOC &&
>> + ? ? ? ? ? ? ? ? iftype != NL80211_IFTYPE_STATION)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? return -1;
>> ? ? ? ? ? ? ? break;
>> ? ? ? }
>>
>
> It seems there needs to be a check somewhere that this packet was
> received from a TDLS peer? OTOH, if somebody is spoofing it crypto will
> reject it or you're vulnerable anyway...
My thoughts exactly. It's not like a rouge STA can't fake the
frame_control and bssid fields..
I didn't want to encumber the Rx path with redundant checks.
Arik
On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
> Allow TDLS operations and peers only in supporting hardware.
In terms of patch ordering I think this should be part of patch 1 (and
maybe patch 3 can be too), otherwise patch 2 adds some incomplete
mac80211 stuff and even enables it ...
johannes
On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
> + case NL80211_TDLS_ENABLE_LINK:
> + rcu_read_lock();
> + sta = sta_info_get(sdata, peer);
> + if (sta) {
> + set_sta_flags(sta, WLAN_STA_AUTHORIZED);
> + sta->tdls_link_enabled = true;
> + }
> + rcu_read_unlock();
> + break;
This seems to require the station already having been added, but
couldn't this create the data race you were worried about? Could you not
simply just create the station once it is authorized?
> + case NL80211_TDLS_DISABLE_LINK:
> + rcu_read_lock();
> + sta = sta_info_get(sdata, peer);
> + if (sta) {
> + sta->tdls_link_enabled = false;
> + sta_info_destroy_addr(sdata, peer);
> + }
> + rcu_read_unlock();
> + break;
Isn't that equivalent to just deleting the station?
johannes
On Wed, Sep 21, 2011 at 15:05, Johannes Berg <[email protected]> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>
>> +static void ieee80211_tdls_add_supp_rates(struct ieee80211_sub_if_data *sdata,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct sk_buff *skb)
>
> maybe it's time to refactor adding supported rates? This must be like
> the 5th implementation of it ... mesh has exactly the same at least.
They're not all exactly the same, but yeah, mesh has exactly the same
one (with no basic rates).
I'll move it to util.c and have both places use it.
>> + ? ? *pos++ = 0x20; /* TDLS enabled */
>
> That might benefit from a constant in ieee80211.h
Will do.
>
>> +static u16 ieee80211_get_tdls_sta_capab(struct ieee80211_sub_if_data *sdata)
>> +{
>> + ? ? u16 capab;
>> + ? ? struct ieee80211_local *local = sdata->local;
>> +
>> + ? ? /* We do not support any exotic capabilities yet */
>> +
>> + ? ? capab = WLAN_CAPABILITY_ESS;
>
> Should we really set the ESS bit for TDLS?
Well I saw this bit set in an example tdls pcap that Jouni provided
the wireshark list a while ago. I haven't seen anything about it in
the spec, but I assumed there was some inter-op cause for this.
This was on in Kalyan's original patches as well.
Kalyan, Jouni - care to comment?
>> + ? ? lnkid->ie_len = 18;
>
> sizeof too?
Sure.
>
>> + ? ? memcpy(tf->da, peer, ETH_ALEN);
>> + ? ? memcpy(tf->sa, sdata->vif.addr, ETH_ALEN);
>> + ? ? tf->ether_type = cpu_to_be16(ETH_P_TDLS);
>> + ? ? tf->payload_type = 2; /* RFTYPE_TDLS */
>
> another constant?
Right.
>
>> + ? ? default:
>> + ? ? ? ? ? ? /* we don't support other actions for now */
>> + ? ? ? ? ? ? WARN_ON(1);
>> + ? ? ? ? ? ? return -EINVAL;
>
> Wouldn't that warning be triggerable from userspace?
That was more of a debugging thing. I guess I should remove these.
>
>> +static int ieee80211_tdls_mgmt(struct wiphy *wiphy, struct net_device *dev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?u8 *peer, u8 action_code, u8 dialog_token,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?u16 status_code, const u8 *extra_ies,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t extra_ies_len)
>> +{
>> + ? ? struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>> + ? ? struct ieee80211_local *local = sdata->local;
>> + ? ? struct ieee80211_tx_info *info;
>> + ? ? struct sk_buff *skb = NULL;
>> + ? ? bool send_direct;
>> + ? ? int ret;
>> +
>> +#ifdef CONFIG_MAC80211_VERBOSE_TDLS_DEBUG
>> + ? ? printk(KERN_DEBUG "TDLS mgmt action %d peer %pM\n", action_code, peer);
>> +#endif
>> +
>> + ? ? /* make sure we are in managed mode, and associated */
>> + ? ? if (sdata->vif.type != NL80211_IFTYPE_STATION ||
>> + ? ? ? ? !sdata->u.mgd.associated) {
>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? }
>
> braces here seem useless :)
Will fix.
>
> Also accessing u.mgd.associated as a bool like this is fine, but
> obviously racy. How do we deal with that? Do we even tear down links
> when disassociating, and do we even need to beyond just killing the
> station entries?
We don't tear down links just before disassociating (there are too
many corner cases here). We just disable the links post-factum.
Killing the station entries won't help for packets meant to be sent
over the AP. I guess we can take the mutex for a little extra safely
(but it won't do much).
The race is even worse - from queuing until the actual Tx, we could
have disconnected from this AP and connected do a totally different
one. But this shouldn't happen in reality (and we can add some guards
to wpa_supplicant to make sure).
Do you see this as a security threat?
>
>> + ? ? skb = dev_alloc_skb(local->hw.extra_tx_headroom +
>> + ? ? ? ? ? ? ? ? ? ? ? ? max(sizeof(struct ieee80211_mgmt),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(struct ieee80211_tdls_data)) +
>> + ? ? ? ? ? ? ? ? ? ? ? ? 50 + /* supported rates */
>> + ? ? ? ? ? ? ? ? ? ? ? ? 7 + /* ext capab */
>> + ? ? ? ? ? ? ? ? ? ? ? ? sizeof(struct ieee80211_tdls_lnkie));
>
> Where are the extra IEs used? They should be accounted for too.
Good catch.
>
>> +fail:
>> + ? ? if (skb)
>> + ? ? ? ? ? ? dev_kfree_skb(skb);
>
> I believe the if() is pointless.
ok.
>
>> + ? ? case NL80211_TDLS_DISABLE_LINK:
>> + ? ? ? ? ? ? rcu_read_lock();
>> + ? ? ? ? ? ? sta = sta_info_get(sdata, peer);
>> + ? ? ? ? ? ? if (sta) {
>> + ? ? ? ? ? ? ? ? ? ? sta->tdls_link_enabled = false;
>> + ? ? ? ? ? ? ? ? ? ? sta_info_destroy_addr(sdata, peer);
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? rcu_read_unlock();
>
> You can't call destroy_addr within rcu section.
Another good catch.
>
>> + ? ? case NL80211_TDLS_TEARDOWN:
>> + ? ? case NL80211_TDLS_SETUP:
>> + ? ? case NL80211_TDLS_DISCOVERY_REQ:
>> + ? ? ? ? ? ? /* We don't support in-driver setup/teardown/discovery */
>> + ? ? ? ? ? ? return -ENOTSUPP;
>> + ? ? case NL80211_TDLS_DISABLE:
>> + ? ? case NL80211_TDLS_ENABLE:
>> + ? ? ? ? ? ? /* TODO */
>> + ? ? ? ? ? ? return -ENOTSUPP;
>> + ? ? default:
>> + ? ? ? ? ? ? WARN_ON(1);
>> + ? ? ? ? ? ? return -EINVAL;
>
> might want to use EOPNOTSUPP and not have a warning -- if nl80211 ever
> gets extended with more actions you might run into the default case.
Yea I'll remove this one as well as a couple more.
Arik
On Thu, Sep 22, 2011 at 14:40, Johannes Berg <[email protected]> wrote:
>> The race is even worse - from queuing until the actual Tx, we could
>> have disconnected from this AP and connected do a totally different
>> one. But this shouldn't happen in reality (and we can add some guards
>> to wpa_supplicant to make sure).
>> Do you see this as a security threat?
>
> I don't really know :-) I just saw the possible race here. If you remove
> the TDLS station entries when disassociating then it shouldn't be an
> issue, right? Since you require a station entry pretty early on anyway.
As discussed on IRC - this race is not really an issue. In the next
version of the patch stations will be purged by kernel-mode on
disassociation.
Arik
Add support for sending high-level TDLS commands and TDLS frames via
NL80211_CMD_TDLS_OPER and NL80211_CMD_TDLS_MGMT, respectively.
Add appropriate cfg80211 callbacks for lower level drivers.
Signed-off-by: Arik Nemtsov <[email protected]>
Cc: Kalyan C Gaddam <[email protected]>
---
include/linux/nl80211.h | 38 +++++++++++++++++++++++++
include/net/cfg80211.h | 9 ++++++
net/wireless/nl80211.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 117 insertions(+), 0 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 3769303..e21e023 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -499,6 +499,9 @@
* this command may also be sent by the driver as an MLME event to
* inform userspace of the new replay counter.
*
+ * @NL80211_CMD_TDLS_OPER: Perform a high-level TDLS command (e.g. link setup).
+ * @NL80211_CMD_TDLS_MGMT: Send a TDLS management frame.
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -623,6 +626,9 @@ enum nl80211_commands {
NL80211_CMD_SET_REKEY_OFFLOAD,
+ NL80211_CMD_TDLS_OPER,
+ NL80211_CMD_TDLS_MGMT,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -1042,6 +1048,14 @@ enum nl80211_commands {
* (Re)Association Response frames when the driver (or firmware) replies to
* (Re)Association Request frames.
*
+ * @NL80211_ATTR_TDLS_ACTION: Low level TDLS action code (e.g. link setup
+ * request, link setup confirm, link teardown, etc.). Values are
+ * described in the TDLS (802.11z) specification.
+ * @NL80211_ATTR_TDLS_DIALOG_TOKEN: Non-zero token for uniquely identifying a
+ * TDLS conversation between two devices.
+ * @NL80211_ATTR_TDLS_OPERATION: High level TDLS operation; see
+ * &enum nl80211_tdls_operation, represented as a u8.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1252,6 +1266,10 @@ enum nl80211_attrs {
NL80211_ATTR_IE_PROBE_RESP,
NL80211_ATTR_IE_ASSOC_RESP,
+ NL80211_ATTR_TDLS_ACTION,
+ NL80211_ATTR_TDLS_DIALOG_TOKEN,
+ NL80211_ATTR_TDLS_OPERATION,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -2482,4 +2500,24 @@ enum nl80211_hidden_ssid {
NL80211_HIDDEN_SSID_ZERO_CONTENTS
};
+/**
+ * enum nl80211_tdls_operation - values for %NL80211_ATTR_TDLS_OPERATION
+ * @NL80211_TDLS_DISCOVERY_REQ: Send a TDLS discovery request
+ * @NL80211_TDLS_SETUP: Setup TDLS link
+ * @NL80211_TDLS_TEARDOWN: Teardown a TDLS link which is already established
+ * @NL80211_TDLS_ENABLE_LINK: Enable TDLS link
+ * @NL80211_TDLS_DISABLE_LINK: Disable TDLS link
+ * @NL80211_TDLS_ENABLE: (Re-)Enable TDLS operation
+ * @NL80211_TDLS_DISABLE: Disable TDLS operation
+ */
+enum nl80211_tdls_operation {
+ NL80211_TDLS_DISCOVERY_REQ,
+ NL80211_TDLS_SETUP,
+ NL80211_TDLS_TEARDOWN,
+ NL80211_TDLS_ENABLE_LINK,
+ NL80211_TDLS_DISABLE_LINK,
+ NL80211_TDLS_ENABLE,
+ NL80211_TDLS_DISABLE
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a37f264..a899e34 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1386,6 +1386,9 @@ struct cfg80211_gtk_rekey_data {
* @set_ringparam: Set tx and rx ring sizes.
*
* @get_ringparam: Get tx and rx ring current and maximum sizes.
+ *
+ * @tdls_mgmt: Transmit a TDLS management frame.
+ * @tdls_oper: Perform a high-level TDLS operation (e.g. TDLS link setup).
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -1568,6 +1571,12 @@ struct cfg80211_ops {
int (*set_rekey_data)(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_gtk_rekey_data *data);
+
+ int (*tdls_mgmt)(struct wiphy *wiphy, struct net_device *dev,
+ u8 *peer, u8 action_code, u8 dialog_token,
+ u16 status_code, const u8 *buf, size_t len);
+ int (*tdls_oper)(struct wiphy *wiphy, struct net_device *dev,
+ u8 *peer, enum nl80211_tdls_operation oper);
};
/*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 57ecfa4..74a4ad4 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -189,6 +189,9 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
.len = IEEE80211_MAX_DATA_LEN },
[NL80211_ATTR_IE_ASSOC_RESP] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
+ [NL80211_ATTR_TDLS_ACTION] = { .type = NLA_U8 },
+ [NL80211_ATTR_TDLS_DIALOG_TOKEN] = { .type = NLA_U8 },
+ [NL80211_ATTR_TDLS_OPERATION] = { .type = NLA_U8 },
};
/* policy for the key attributes */
@@ -861,6 +864,8 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
}
CMD(set_channel, SET_CHANNEL);
CMD(set_wds_peer, SET_WDS_PEER);
+ CMD(tdls_mgmt, TDLS_MGMT);
+ CMD(tdls_oper, TDLS_OPER);
if (dev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN)
CMD(sched_scan_start, START_SCHED_SCAN);
@@ -4869,6 +4874,55 @@ static int nl80211_flush_pmksa(struct sk_buff *skb, struct genl_info *info)
return rdev->ops->flush_pmksa(&rdev->wiphy, dev);
}
+static int nl80211_tdls_mgmt(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];
+ u8 action_code, dialog_token;
+ u16 status_code;
+ u8 *peer;
+
+ if (!rdev->ops->tdls_mgmt)
+ return -EOPNOTSUPP;
+
+ if (!info->attrs[NL80211_ATTR_TDLS_ACTION] ||
+ !info->attrs[NL80211_ATTR_STATUS_CODE] ||
+ !info->attrs[NL80211_ATTR_TDLS_DIALOG_TOKEN] ||
+ !info->attrs[NL80211_ATTR_FRAME] ||
+ !info->attrs[NL80211_ATTR_MAC])
+ return -EINVAL;
+
+ peer = nla_data(info->attrs[NL80211_ATTR_MAC]);
+ action_code = nla_get_u8(info->attrs[NL80211_ATTR_TDLS_ACTION]);
+ status_code = nla_get_u16(info->attrs[NL80211_ATTR_STATUS_CODE]);
+ dialog_token = nla_get_u8(info->attrs[NL80211_ATTR_TDLS_DIALOG_TOKEN]);
+
+ return rdev->ops->tdls_mgmt(&rdev->wiphy, dev, peer, action_code,
+ dialog_token, status_code,
+ nla_data(info->attrs[NL80211_ATTR_FRAME]),
+ nla_len(info->attrs[NL80211_ATTR_FRAME]));
+}
+
+static int nl80211_tdls_oper(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];
+ enum nl80211_tdls_operation operation;
+ u8 *peer;
+
+ if (!rdev->ops->tdls_oper)
+ return -EOPNOTSUPP;
+
+ if (!info->attrs[NL80211_ATTR_TDLS_OPERATION] ||
+ !info->attrs[NL80211_ATTR_MAC])
+ return -EINVAL;
+
+ operation = nla_get_u8(info->attrs[NL80211_ATTR_TDLS_OPERATION]);
+ peer = nla_data(info->attrs[NL80211_ATTR_MAC]);
+
+ return rdev->ops->tdls_oper(&rdev->wiphy, dev, peer, operation);
+}
+
static int nl80211_remain_on_channel(struct sk_buff *skb,
struct genl_info *info)
{
@@ -6181,6 +6235,22 @@ static struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
+ {
+ .cmd = NL80211_CMD_TDLS_MGMT,
+ .doit = nl80211_tdls_mgmt,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
+ .cmd = NL80211_CMD_TDLS_OPER,
+ .doit = nl80211_tdls_oper,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
};
static struct genl_multicast_group nl80211_mlme_mcgrp = {
--
1.7.4.1
On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
> This series adds basic kernel-mode TDLS support for nl80211 based drivers.
> It is based in part on patches by Kalyan C. Gaddam, cc-ed here.
I'm not convinced of the design. I don't see anything in the setup
frames that will require creating them in the kernel. If they are data
frames, the supplicant can create them and send them as such, if they
are public action frames we have nl80211 API for that.
> Support is added for peer discovery and data path setup/teardown. More
> advanced features are not implemented. These include QoS/HT, peer PSM,
> peer U-APSD and channel switching.
That will certainly require kernel help, but I don't think it requires
that the setup frames be created in mac80211.
> Notably, this patch-set does not include locking in the data path,
> to switch between AP-based and direct Tx during link setup/tear-down.
> This will be added by a later patch-set, if/when this series is
> accepted. In practice it seems to work quite well without additional
> locking.
That ought to work without locks if you create/destroy the station
entries appropriately?
johannes
When a TDLS peer STA is added it is marked with a new flag in both
nl80211 and in mac80211.
Signed-off-by: Arik Nemtsov <[email protected]>
Cc: Kalyan C Gaddam <[email protected]>
---
include/linux/nl80211.h | 2 ++
net/mac80211/cfg.c | 6 ++++++
net/mac80211/sta_info.h | 2 ++
net/wireless/nl80211.c | 3 ++-
4 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index e21e023..34415e81 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1367,6 +1367,7 @@ enum nl80211_iftype {
* @NL80211_STA_FLAG_WME: station is WME/QoS capable
* @NL80211_STA_FLAG_MFP: station uses management frame protection
* @NL80211_STA_FLAG_AUTHENTICATED: station is authenticated
+ * @NL80211_STA_FLAG_TDLS_PEER: station is a TDLS peer
* @NL80211_STA_FLAG_MAX: highest station flag number currently defined
* @__NL80211_STA_FLAG_AFTER_LAST: internal use
*/
@@ -1377,6 +1378,7 @@ enum nl80211_sta_flags {
NL80211_STA_FLAG_WME,
NL80211_STA_FLAG_MFP,
NL80211_STA_FLAG_AUTHENTICATED,
+ NL80211_STA_FLAG_TDLS_PEER,
/* keep last */
__NL80211_STA_FLAG_AFTER_LAST,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 27dda09..3ef0bc6 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -696,6 +696,12 @@ static void sta_apply_parameters(struct ieee80211_local *local,
if (set & BIT(NL80211_STA_FLAG_AUTHENTICATED))
sta->flags |= WLAN_STA_AUTH;
}
+
+ if (mask & BIT(NL80211_STA_FLAG_TDLS_PEER)) {
+ sta->flags &= ~WLAN_STA_TDLS_PEER;
+ if (set & BIT(NL80211_STA_FLAG_TDLS_PEER))
+ sta->flags |= WLAN_STA_TDLS_PEER;
+ }
spin_unlock_irqrestore(&sta->flaglock, flags);
/*
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 25fb49e..ae497e3 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -45,6 +45,7 @@
* station in power-save mode, reply when the driver unblocks.
* @WLAN_STA_PS_DRIVER_BUF: Station has frames pending in driver internal
* buffers. Automatically cleared on station wake-up.
+ * @WLAN_STA_TDLS_PEER: station is a TDLS peer.
*/
enum ieee80211_sta_info_flags {
WLAN_STA_AUTH = 1<<0,
@@ -61,6 +62,7 @@ enum ieee80211_sta_info_flags {
WLAN_STA_PS_DRIVER = 1<<12,
WLAN_STA_PSPOLL = 1<<13,
WLAN_STA_PS_DRIVER_BUF = 1<<14,
+ WLAN_STA_TDLS_PEER = 1<<15,
};
#define STA_TID_NUM 16
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 74a4ad4..81b67a8 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2598,7 +2598,8 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN &&
dev->ieee80211_ptr->iftype != NL80211_IFTYPE_MESH_POINT &&
- dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
+ dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO &&
+ dev->ieee80211_ptr->iftype != NL80211_IFTYPE_STATION)
return -EINVAL;
err = get_vlan(info, rdev, ¶ms.vlan);
--
1.7.4.1
On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
> +static int nl80211_tdls_mgmt(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];
> + u8 action_code, dialog_token;
> + u16 status_code;
> + u8 *peer;
> +
> + if (!rdev->ops->tdls_mgmt)
> + return -EOPNOTSUPP;
> +
> + if (!info->attrs[NL80211_ATTR_TDLS_ACTION] ||
> + !info->attrs[NL80211_ATTR_STATUS_CODE] ||
> + !info->attrs[NL80211_ATTR_TDLS_DIALOG_TOKEN] ||
> + !info->attrs[NL80211_ATTR_FRAME] ||
> + !info->attrs[NL80211_ATTR_MAC])
> + return -EINVAL;
Should that really use ATTR_FRAME as opposed to ATTR_IE?
johannes
When a station is determined to be a TDLS peer, send the data directly,
bypassing the AP.
In addition, allow data to be received directly from TDLS peers.
Signed-off-by: Arik Nemtsov <[email protected]>
Cc: Kalyan C Gaddam <[email protected]>
---
net/mac80211/tx.c | 24 ++++++++++++++++++++----
net/wireless/util.c | 5 +++--
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 0107263..43c08ce 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1725,6 +1725,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
struct sta_info *sta = NULL;
u32 sta_flags = 0;
struct sk_buff *tmp_skb;
+ bool tdls_link = false;
if (unlikely(skb->len < ETH_HLEN)) {
ret = NETDEV_TX_OK;
@@ -1836,11 +1837,25 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
break;
#endif
case NL80211_IFTYPE_STATION:
- memcpy(hdr.addr1, sdata->u.mgd.bssid, ETH_ALEN);
- if (sdata->u.mgd.use_4addr &&
- cpu_to_be16(ethertype) != sdata->control_port_protocol) {
- fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS);
+ if (local->hw.flags & IEEE80211_HW_SUPPORTS_TDLS) {
+ rcu_read_lock();
+ sta = sta_info_get(sdata, skb->data);
+ tdls_link = (sta && sta->tdls_link_enabled);
+ rcu_read_unlock();
+ }
+
+ if (tdls_link) {
+ /* DA SA BSSID */
+ memcpy(hdr.addr1, skb->data, ETH_ALEN);
+ memcpy(hdr.addr2, skb->data + ETH_ALEN, ETH_ALEN);
+ memcpy(hdr.addr3, sdata->u.mgd.bssid, ETH_ALEN);
+ hdrlen = 24;
+ } else if (sdata->u.mgd.use_4addr &&
+ cpu_to_be16(ethertype) != sdata->control_port_protocol) {
+ fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS |
+ IEEE80211_FCTL_TODS);
/* RA TA DA SA */
+ memcpy(hdr.addr1, sdata->u.mgd.bssid, ETH_ALEN);
memcpy(hdr.addr2, sdata->vif.addr, ETH_ALEN);
memcpy(hdr.addr3, skb->data, ETH_ALEN);
memcpy(hdr.addr4, skb->data + ETH_ALEN, ETH_ALEN);
@@ -1848,6 +1863,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
} else {
fc |= cpu_to_le16(IEEE80211_FCTL_TODS);
/* BSSID SA DA */
+ memcpy(hdr.addr1, sdata->u.mgd.bssid, ETH_ALEN);
memcpy(hdr.addr2, skb->data + ETH_ALEN, ETH_ALEN);
memcpy(hdr.addr3, skb->data, ETH_ALEN);
hdrlen = 24;
diff --git a/net/wireless/util.c b/net/wireless/util.c
index eef82f7..c28afee 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -392,8 +392,9 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
}
break;
case cpu_to_le16(0):
- if (iftype != NL80211_IFTYPE_ADHOC)
- return -1;
+ if (iftype != NL80211_IFTYPE_ADHOC &&
+ iftype != NL80211_IFTYPE_STATION)
+ return -1;
break;
}
--
1.7.4.1
On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
> When a TDLS peer STA is added it is marked with a new flag in both
> nl80211 and in mac80211.
I think it'd make some sense to allow only TDLS stations on a managed
mode interface?
johannes
On Wed, Sep 21, 2011 at 14:54, Johannes Berg <[email protected]> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>
>> +static int nl80211_tdls_mgmt(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];
>> + ? ? u8 action_code, dialog_token;
>> + ? ? u16 status_code;
>> + ? ? u8 *peer;
>> +
>> + ? ? if (!rdev->ops->tdls_mgmt)
>> + ? ? ? ? ? ? return -EOPNOTSUPP;
>> +
>> + ? ? if (!info->attrs[NL80211_ATTR_TDLS_ACTION] ||
>> + ? ? ? ? !info->attrs[NL80211_ATTR_STATUS_CODE] ||
>> + ? ? ? ? !info->attrs[NL80211_ATTR_TDLS_DIALOG_TOKEN] ||
>> + ? ? ? ? !info->attrs[NL80211_ATTR_FRAME] ||
>> + ? ? ? ? !info->attrs[NL80211_ATTR_MAC])
>> + ? ? ? ? ? ? return -EINVAL;
>
> Should that really use ATTR_FRAME as opposed to ATTR_IE?
>
I guess ATTR_IE is more appropriate (looking at the docs).
Arik
On Wed, Sep 21, 2011 at 15:07, Johannes Berg <[email protected]> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>> When a station is determined to be a TDLS peer, send the data directly,
>> bypassing the AP.
>>
>> In addition, allow data to be received directly from TDLS peers.
>
> We talked a bit about this but I forgot -- did you want to add the
> buffering here?
Not as part of this patch series but yes. If the TDLS connection is in
setup, don't do anything with the packet, just put it on a queue.
Later when the connection succeeds or fails we can these packets
direct or via the AP, accordingly.
Arik
On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
> When a station is determined to be a TDLS peer, send the data directly,
> bypassing the AP.
>
> In addition, allow data to be received directly from TDLS peers.
We talked a bit about this but I forgot -- did you want to add the
buffering here?
johannes
On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1725,6 +1725,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> struct sta_info *sta = NULL;
> u32 sta_flags = 0;
> struct sk_buff *tmp_skb;
> + bool tdls_link = false;
>
> if (unlikely(skb->len < ETH_HLEN)) {
> ret = NETDEV_TX_OK;
> @@ -1836,11 +1837,25 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> break;
> #endif
> case NL80211_IFTYPE_STATION:
> - memcpy(hdr.addr1, sdata->u.mgd.bssid, ETH_ALEN);
> - if (sdata->u.mgd.use_4addr &&
> - cpu_to_be16(ethertype) != sdata->control_port_protocol) {
> - fc |= cpu_to_le16(IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS);
> + if (local->hw.flags & IEEE80211_HW_SUPPORTS_TDLS) {
> + rcu_read_lock();
> + sta = sta_info_get(sdata, skb->data);
> + tdls_link = (sta && sta->tdls_link_enabled);
Why don't you test WLAN_STA_TDLS_PEER and add the station only when the
session is set up?
That would avoid the problem here when setting up the session.
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -392,8 +392,9 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
> }
> break;
> case cpu_to_le16(0):
> - if (iftype != NL80211_IFTYPE_ADHOC)
> - return -1;
> + if (iftype != NL80211_IFTYPE_ADHOC &&
> + iftype != NL80211_IFTYPE_STATION)
> + return -1;
> break;
> }
>
It seems there needs to be a check somewhere that this packet was
received from a TDLS peer? OTOH, if somebody is spoofing it crypto will
reject it or you're vulnerable anyway...
johannes
On Fri, Sep 16, 2011 at 15:58, Johannes Berg <[email protected]> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>> This series adds basic kernel-mode TDLS support for nl80211 based drivers.
>> It is based in part on patches by Kalyan C. Gaddam, cc-ed here.
>
> I'm not convinced of the design. I don't see anything in the setup
> frames that will require creating them in the kernel. If they are data
> frames, the supplicant can create them and send them as such, if they
> are public action frames we have nl80211 API for that.
We had some qualms about the design ourselves. This is a question
Kalyan sent to the hostap list a while back -
http://lists.shmoo.com/pipermail/hostap/2011-June/023311.html
The locking requirement (elaborated at the end of the email) was also
part of the decision to make mac80211 more aware of the connection
state. Also, eventually mac80211 will have to add some IEs of it's own
to the setup packets.
>
>> Support is added for peer discovery and data path setup/teardown. More
>> advanced features are not implemented. These include QoS/HT, peer PSM,
>> peer U-APSD and channel switching.
>
> That will certainly require kernel help, but I don't think it requires
> that the setup frames be created in mac80211.
wpa_supplicant needs to add some IEs to the packet (RSN, FTIE, ..).
mac80211 will also have to add some IEs (U-APSD, HT, ...)
Does it really matter if the packet's headers are created by usermode
or by kernel? Even if we change the "owner" of the packet, the API
would still remain about the same. tdls_mgmt would simply get the
frame instead of extra IEs.
Jouni's original wpa_s TDLS code was written for a full-mac driver
where the kernel/firmware side created the packet. wpa_supplicant only
provided auth IEs. Keeping it this way also avoids refactoring the
code and introducing bugs.
>
>> Notably, this patch-set does not include locking in the data path,
>> to switch between AP-based and direct Tx during link setup/tear-down.
>> This will be added by a later patch-set, if/when this series is
>> accepted. In practice it seems to work quite well without additional
>> locking.
>
> That ought to work without locks if you create/destroy the station
> entries appropriately?
I should elaborate on the locking requirement. The TDLS link setup is
made up 3 frames - a setup request, followed by a response from the
peer, and finally a confirm.
This is an excerpt from the spec:
"To avoid possible reordering of MSDUs, a TDLS initiator STA shall
cease transmitting MSDUs to the
TDLS responder STA through the AP after sending a TDLS Setup Request
frame, and a TDLS responder
STA shall cease transmitting MSDUs to the TDLS initiator STA through
the AP after sending a TDLS
Setup Response frame indicating status code 0 (Success)."
This requires a locking mechanism similar to AP-mode buffering for
stations in PS (since we probably don't want to stop all queues).
Arik