EAPoL over NL80211

RFC Superseded Denis Kenzior 2 participants 21 messages Mar 13, 2018 – Mar 23, 2018

Subsystems (2)

mac80211 802.11 (including cfg80211/NL80211)

lore

Share & tools

lore

Denis Kenzior

author

2018-03-13 21:59 UTC #0 lore raw

v5 cover EAPoL over NL80211

This patchset adds support for running 802.11 authentication mechanisms (e.g. 802.1X, 4-Way Handshake, etc) over NL80211 instead of putting them onto the network device. This has the advantage of fixing several long-standing race conditions that result from userspace operating on multiple transports in order to manage a 802.11 connection (e.g. NL80211 and wireless netdev, wlan0, etc).

For example, userspace would sometimes see 4-Way handshake packets before NL80211 signaled that the connection has been established. Leading to ugly hacks or having the STA wait for retransmissions from the AP.

This also provides a way to mitigate a particularly nasty race condition where the encryption key could be set prior to the 4-way handshake packet 4/4 being sent. This would result in the packet being sent encrypted and discarded by the peer. The mitigation strategy for this race is for userspace to explicitly tell the kernel that a particular EAPoL packet should not be encrypted.

To make this possible this patchset introduces a new NL80211 command and several new attributes. A userspace that is capable of processing EAPoL packets over NL80211 includes a new NL80211_ATTR_CONTROL_PORT_OVER_NL80211 attribute in its NL80211_CMD_ASSOCIATE or NL80211_CMD_CONNECT requests being sent to the kernel. The previously added NL80211_ATTR_SOCKET_OWNER attribute must also be included. The latter is used by the kernel to send NL80211_CMD_CONTROL_PORT_FRAME notifications back to userspace via a netlink unicast. If the NL80211_ATTR_CONTROL_PORT_OVER_NL80211 attribute is not specified, then legacy behavior is kept and control port packets continue to flow over the network interface.

If control port over nl80211 transport is requested, then control port packets are intercepted just prior to being handed to the network device and sent over netlink via the NL80211_CMD_CONTROL_PORT_FRAME notification. NL80211_ATTR_CONTROL_PORT_ETHERTYPE and NL80211_ATTR_MAC are included to specify the control port frame protocol and source address respectively. If the control port frame was received unencrypted then NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT flag is also included. NL80211_ATTR_FRAME attribute contains the raw control port frame with all transport layer headers stripped (e.g. this would be the raw EAPoL frame).

Userspace can reply to control port frames either via legacy methods (by sending frames to the network device) or via NL80211_CMD_CONTROL_PORT_FRAME request. Userspace would included NL80211_ATTR_FRAME with the raw control port frame as well as NL80211_Attr_MAC and NL80211_ATTR_CONTROL_PORT_ETHERTYPE attributes to specify the destination address and protocol respectively. This allows Pre-Authentication (protocol 0x88c7) frames to be sent via this mechanism as well. Finally, NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT flag can be included to tell the driver to send the frame unencrypted, e.g. for 4-Way handshake 4/4 frames.

The proposed patchset has been tested in a mac80211_hwsim based environment with hostapd and iwd.

ChangeLog

- Johannes' main comment was that we're not handling interface types other than STATION inside tx_control_port (patch 2). This patch was modified to support all interface types that seemed relevant.
- Since tx_control_port relies on wdev->conn_owner_nlportid being set, SOCKET_OWNER support was added to JOIN_IBSS, JOIN_MESH and START_AP
- SOCKET_OWNER auto-destruction logic was updated to support interface types other than STATION/P2P_CLIENT
- Last patch was modified to support control_port_over_nl80211 for mac80211 based AP mode. It also copies necessary bits for AP_VLAN interfaces.

This version has been tested on both STATION and AP mode interfaces with SOCKET_OWNER & CONTROL_PORT_OVER_NL80211 attributes provided to CMD_CONNECT and CMD_START_AP.

TODO:

- It is unclear to me how AP_VLAN and AP interfaces should synchronize on conn_owner_nlportid. This is required for tx_control_port to work.
- JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup?

- Reordered the patches to make sure that: when CONTROL_PORT_OVER_NL80211 is provided by userspace, nl80211 checks that both EXT_FEATURE bit is set and the tx_control_port is present in rdev ops.
- Fixed up various issues Johannes found in his review

- Added ETH_P_PREAUTH to if_ether.h
- Moved NL80211 feature bit from wiphy features to ext features
- Addressed various comments from Johannes

- Added WIPHY_FLAG_CONTROL_PORT_OVER_NL80211 flag. This is a capability flag used by the drivers, e.g. that the driver supports control port over nl80211 capability. This capability is now checked when CONTROL_PORT_OVER_NL80211 is requested.

- mac80211 rx path now forwards Pre-Authentication frames over NL80211 as well, if requested. Tweaked the signature of cfg80211_rx_control_port.

- TX path reworked completely. tx_control_port method has been introduced to cfg80211_ops. An implementation of tx_control_port for mac80211 was added.

Denis Kenzior (9):
nl80211: Add CMD_CONTROL_PORT_FRAME API
nl80211: Implement TX of control port frames
nl80211: Add CONTROL_PORT_OVER_NL80211 attribute
cfg80211: Support all iftypes in autodisconnect_wk
nl80211: Add SOCKET_OWNER support to JOIN_IBSS
nl80211: Add SOCKET_OWNER support to JOIN_MESH
nl80211: Add SOCKET_OWNER support to START_AP
mac80211: Add support for tx_control_port
mac80211: Send control port frames over nl80211

 include/net/cfg80211.h       |  34 +++++++++
 include/uapi/linux/nl80211.h |  36 +++++++++-
 net/mac80211/cfg.c           |   5 ++
 net/mac80211/ieee80211_i.h   |   4 ++
 net/mac80211/iface.c         |   2 +
 net/mac80211/main.c          |   2 +
 net/mac80211/mlme.c          |   2 +
 net/mac80211/rx.c            |  33 +++++++--
 net/mac80211/tx.c            |  46 +++++++++++++
 net/wireless/ap.c            |   1 +
 net/wireless/ibss.c          |   1 +
 net/wireless/mesh.c          |   1 +
 net/wireless/nl80211.c       | 161 ++++++++++++++++++++++++++++++++++++++++++-
 net/wireless/rdev-ops.h      |  15 ++++
 net/wireless/sme.c           |  43 +++++++++---
 net/wireless/trace.h         |  47 +++++++++++++
 16 files changed, 414 insertions(+), 19 deletions(-)

Signature · 1 line

2.13.5

Denis Kenzior

author

2018-03-13 21:59 UTC #1 lore raw

RFC v5 1/9 nl80211: Add CMD_CONTROL_PORT_FRAME API

This commit also adds cfg80211_rx_control_port function. This is used to generate a CMD_CONTROL_PORT_FRAME event out to userspace. The conn_owner_nlportid is used as the unicast destination. This means that userspace must specify NL80211_ATTR_SOCKET_OWNER flag if control port over nl80211 routing is requested in NL80211_CMD_CONNECT, NL80211_CMD_ASSOCIATE or NL80211_CMD_START_AP

Signed-off-by Denis Kenzior <[email protected]>

---
 include/net/cfg80211.h       | 22 +++++++++++++++++
 include/uapi/linux/nl80211.h | 13 ++++++++++
 net/wireless/nl80211.c       | 58 ++++++++++++++++++++++++++++++++++++++++++++
 net/wireless/trace.h         | 21 ++++++++++++++++
 4 files changed, 114 insertions(+)

include/net/cfg80211.h +22 -0

index fc40843baed3..6dee630ee66d 100644
@@ -5694,6 +5694,28 @@ void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie,
 
 
 /**
+ * cfg80211_rx_control_port - notification about a received control port frame
+ * @dev: The device the frame matched to
+ * @buf: control port frame
+ * @len: length of the frame data
+ * @addr: The peer from which the frame was received
+ * @proto: frame protocol, typically PAE or Pre-authentication
+ * @unencrypted: Whether the frame was received unencrypted
+ *
+ * This function is used to inform userspace about a received control port
+ * frame.  It should only be used if userspace indicated it wants to receive
+ * control port frames over NL80211.
+ *
+ * The frame is the data portion of the 802.3 or 802.11 data frame with all
+ * network layer headers removed (e.g. the raw EAPoL frame).
+ *
+ * Return: %true if the frame was passed to userspace
+ */
+bool cfg80211_rx_control_port(struct net_device *dev,
+			      const u8 *buf, size_t len,
+			      const u8 *addr, u16 proto, bool unencrypted);
+
+/**
  * cfg80211_cqm_rssi_notify - connection quality monitoring rssi event
  * @dev: network device
  * @rssi_event: the triggered RSSI event

include/uapi/linux/nl80211.h +13 -0

index c13c84304be3..1334f810f7b4 100644
@@ -990,6 +990,17 @@
  *	&NL80211_CMD_CONNECT or &NL80211_CMD_ROAM. If the 4 way handshake failed
  *	&NL80211_CMD_DISCONNECT should be indicated instead.
  *
+ * @NL80211_CMD_CONTROL_PORT_FRAME: Control Port (e.g. PAE) frame TX request
+ *	and RX notification.  This command is used both as a request to transmit
+ *	a control port frame and as a notification that a control port frame
+ *	has been received. %NL80211_ATTR_FRAME is used to specify the
+ *	frame contents.  The frame is the raw EAPoL data, without ethernet or
+ *	802.11 headers.
+ *	When used as an event indication %NL80211_ATTR_CONTROL_PORT_ETHERTYPE,
+ *	%NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT and %NL80211_ATTR_MAC are added
+ *	indicating the protocol type of the received frame; whether the frame
+ *	was received unencrypted and the MAC address of the peer respectively.
+ *
  * @NL80211_CMD_RELOAD_REGDB: Request that the regdb firmware file is reloaded.
  *
  * @NL80211_CMD_EXTERNAL_AUTH: This interface is exclusively defined for host

@@ -1228,6 +1239,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_STA_OPMODE_CHANGED,
 
+	NL80211_CMD_CONTROL_PORT_FRAME,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */

net/wireless/nl80211.c +58 -0

64 lines not on this page · raw message

net/wireless/trace.h +21 -0

index 5152938b358d..24e84dfe54fd 100644
@@ -2600,6 +2600,27 @@ TRACE_EVENT(cfg80211_mgmt_tx_status,
 		  WDEV_PR_ARG, __entry->cookie, BOOL_TO_STR(__entry->ack))
 );
 
+TRACE_EVENT(cfg80211_rx_control_port,
+	TP_PROTO(struct net_device *netdev, const u8 *buf, size_t len,
+		 const u8 *addr, u16 proto, bool unencrypted),
+	TP_ARGS(netdev, buf, len, addr, proto, unencrypted),
+	TP_STRUCT__entry(
+		NETDEV_ENTRY
+		MAC_ENTRY(addr)
+		__field(u16, proto)
+		__field(bool, unencrypted)
+	),
+	TP_fast_assign(
+		NETDEV_ASSIGN;
+		MAC_ASSIGN(addr, addr);
+		__entry->proto = proto;
+		__entry->unencrypted = unencrypted;
+	),
+	TP_printk(NETDEV_PR_FMT ", " MAC_PR_FMT " proto: %x, unencrypted: %s",
+		  NETDEV_PR_ARG, MAC_PR_ARG(addr),
+		  __entry->proto, BOOL_TO_STR(__entry->unencrypted))
+);
+
 TRACE_EVENT(cfg80211_cqm_rssi_notify,
 	TP_PROTO(struct net_device *netdev,
 		 enum nl80211_cqm_rssi_threshold_event rssi_event,

Signature · 1 line

2.13.5

Denis Kenzior

author

2018-03-13 21:59 UTC #2 lore raw

RFC v5 2/9 nl80211: Implement TX of control port frames

In reply to Denis Kenzior

This commit implements the TX side of NL80211_CMD_CONTROL_PORT_FRAME. Userspace provides the raw EAPoL frame using NL80211_ATTR_FRAME. Userspace should also provide the destination address and the protocol type to use when sending the frame. This is used to implement TX of Pre-authentication frames. If CONTROL_PORT_ETHERTYPE_NO_ENCRYPT is specified, then the driver will be asked not to encrypt the outgoing frame.

A new EXT_FEATURE flag is introduced so that nl80211 code can check whether a given wiphy has capability to pass EAPoL frames over NL80211.

Signed-off-by Denis Kenzior <[email protected]>

---
 include/net/cfg80211.h       |  9 ++++++
 include/uapi/linux/nl80211.h |  3 ++
 net/wireless/nl80211.c       | 70 +++++++++++++++++++++++++++++++++++++++++++-
 net/wireless/rdev-ops.h      | 15 ++++++++++
 net/wireless/trace.h         | 26 ++++++++++++++++
 5 files changed, 122 insertions(+), 1 deletion(-)

include/net/cfg80211.h +9 -0

index 6dee630ee66d..76b6783f35f6 100644
@@ -2960,6 +2960,9 @@ struct cfg80211_external_auth_params {
  *
  * @external_auth: indicates result of offloaded authentication processing from
  *     user space
+ *
+ * @tx_control_port: TX a control port frame (EAPoL).  The noencrypt parameter
+ *	tells the driver that the frame should not be encrypted.
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);

@@ -3255,6 +3258,12 @@ struct cfg80211_ops {
 			   const u8 *aa);
 	int     (*external_auth)(struct wiphy *wiphy, struct net_device *dev,
 				 struct cfg80211_external_auth_params *params);
+
+	int	(*tx_control_port)(struct wiphy *wiphy,
+				   struct net_device *dev,
+				   const u8 *buf, size_t len,
+				   const u8 *dest, const __be16 proto,
+				   const bool noencrypt);
 };
 
 /*

include/uapi/linux/nl80211.h +3 -0

index 1334f810f7b4..77675ae3e475 100644
@@ -5012,6 +5012,8 @@ enum nl80211_feature_flags {
  * @NL80211_EXT_FEATURE_LOW_SPAN_SCAN: Driver supports low span scan.
  * @NL80211_EXT_FEATURE_LOW_POWER_SCAN: Driver supports low power scan.
  * @NL80211_EXT_FEATURE_HIGH_ACCURACY_SCAN: Driver supports high accuracy scan.
+ * @NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211: Driver supports sending and
+ *     receiving control port frames over NL80211 instead of the netdevice.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.

@@ -5042,6 +5044,7 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_LOW_SPAN_SCAN,
 	NL80211_EXT_FEATURE_LOW_POWER_SCAN,
 	NL80211_EXT_FEATURE_HIGH_ACCURACY_SCAN,
+	NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,

net/wireless/nl80211.c +69 -1

82 lines not on this page · raw message

net/wireless/rdev-ops.h +15 -0

index 84f23ae015fc..87479a53411b 100644
@@ -714,6 +714,21 @@ static inline int rdev_mgmt_tx(struct cfg80211_registered_device *rdev,
 	return ret;
 }
 
+static inline int rdev_tx_control_port(struct cfg80211_registered_device *rdev,
+				       struct net_device *dev,
+				       const void *buf, size_t len,
+				       const u8 *dest, __be16 proto,
+				       const bool noencrypt)
+{
+	int ret;
+	trace_rdev_tx_control_port(&rdev->wiphy, dev, buf, len,
+				   dest, proto, noencrypt);
+	ret = rdev->ops->tx_control_port(&rdev->wiphy, dev, buf, len,
+					 dest, proto, noencrypt);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
+
 static inline int
 rdev_mgmt_tx_cancel_wait(struct cfg80211_registered_device *rdev,
 			 struct wireless_dev *wdev, u64 cookie)

net/wireless/trace.h +26 -0

index 24e84dfe54fd..5794cfa371c7 100644
@@ -1882,6 +1882,32 @@ TRACE_EVENT(rdev_mgmt_tx,
 		  BOOL_TO_STR(__entry->dont_wait_for_ack))
 );
 
+TRACE_EVENT(rdev_tx_control_port,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
+		 const u8 *buf, size_t len, const u8 *dest, __be16 proto,
+		 bool unencrypted),
+	TP_ARGS(wiphy, netdev, buf, len, dest, proto, unencrypted),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		NETDEV_ENTRY
+		MAC_ENTRY(dest)
+		__field(__be16, proto)
+		__field(bool, unencrypted)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		NETDEV_ASSIGN;
+		MAC_ASSIGN(dest, dest);
+		__entry->proto = proto;
+		__entry->unencrypted = unencrypted;
+	),
+	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", " MAC_PR_FMT ","
+		  " proto: %x, unencrypted: %s",
+		  WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(dest),
+		  be16_to_cpu(__entry->proto),
+		  BOOL_TO_STR(__entry->unencrypted))
+);
+
 TRACE_EVENT(rdev_set_noack_map,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
 		 u16 noack_map),

Signature · 1 line

2.13.5

Denis Kenzior

author

2018-03-13 21:59 UTC #3 lore raw

RFC v5 3/9 nl80211: Add CONTROL_PORT_OVER_NL80211 attribute

In reply to Denis Kenzior

Signed-off-by Denis Kenzior <[email protected]>

---
 include/net/cfg80211.h       |  3 +++
 include/uapi/linux/nl80211.h | 14 +++++++++++++-
 net/wireless/nl80211.c       | 13 +++++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

include/net/cfg80211.h +3 -0

index 76b6783f35f6..2e7f30c66913 100644
@@ -646,6 +646,8 @@ struct survey_info {
  *	allowed through even on unauthorized ports
  * @control_port_no_encrypt: TRUE to prevent encryption of control port
  *	protocol frames.
+ * @control_port_over_nl80211: TRUE if userspace expects to exchange control
+ *	port frames over NL80211 instead of the network interface.
  * @wep_keys: static WEP keys, if not NULL points to an array of
  *	CFG80211_MAX_WEP_KEYS WEP keys
  * @wep_tx_key: key index (0..3) of the default TX static WEP key

@@ -661,6 +663,7 @@ struct cfg80211_crypto_settings {
 	bool control_port;
 	__be16 control_port_ethertype;
 	bool control_port_no_encrypt;
+	bool control_port_over_nl80211;
 	struct key_params *wep_keys;
 	int wep_tx_key;
 	const u8 *psk;

include/uapi/linux/nl80211.h +13 -1

index 77675ae3e475..1cdac3d732c1 100644
@@ -542,7 +542,8 @@
  *	IEs in %NL80211_ATTR_IE, %NL80211_ATTR_AUTH_TYPE, %NL80211_ATTR_USE_MFP,
  *	%NL80211_ATTR_MAC, %NL80211_ATTR_WIPHY_FREQ, %NL80211_ATTR_CONTROL_PORT,
  *	%NL80211_ATTR_CONTROL_PORT_ETHERTYPE,
- *	%NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT, %NL80211_ATTR_MAC_HINT, and
+ *	%NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT,
+ *	%NL80211_ATTR_CONTROL_PORT_OVER_NL80211, %NL80211_ATTR_MAC_HINT, and
  *	%NL80211_ATTR_WIPHY_FREQ_HINT.
  *	If included, %NL80211_ATTR_MAC and %NL80211_ATTR_WIPHY_FREQ are
  *	restrictions on BSS selection, i.e., they effectively prevent roaming

@@ -1488,6 +1489,15 @@ enum nl80211_commands {
  * @NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT: When included along with
  *	%NL80211_ATTR_CONTROL_PORT_ETHERTYPE, indicates that the custom
  *	ethertype frames used for key negotiation must not be encrypted.
+ * @NL80211_ATTR_CONTROL_PORT_OVER_NL80211: A flag indicating whether control
+ *	port frames (e.g. of type given in %NL80211_ATTR_CONTROL_PORT_ETHERTYPE)
+ *	will be sent directly to the network interface or sent via the NL80211
+ *	socket.  If this attribute is missing, then legacy behavior of sending
+ *	control port frames directly to the network interface is used.  If the
+ *	flag is included, then control port frames are sent over NL80211 instead
+ *	using %CMD_CONTROL_PORT_FRAME.  If control port routing over NL80211 is
+ *	to be used then userspace must also use the %NL80211_ATTR_SOCKET_OWNER
+ *	flag.
  *
  * @NL80211_ATTR_TESTDATA: Testmode data blob, passed through to the driver.
  *	We recommend using nested, driver-specific attributes within this.

@@ -2641,6 +2651,8 @@ enum nl80211_attrs {
 	NL80211_ATTR_NSS,
 	NL80211_ATTR_ACK_SIGNAL,
 
+	NL80211_ATTR_CONTROL_PORT_OVER_NL80211,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,

net/wireless/nl80211.c +13 -0

index 3c4dbfa0ca71..24b1bd940fca 100644
@@ -287,6 +287,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 	[NL80211_ATTR_CONTROL_PORT] = { .type = NLA_FLAG },
 	[NL80211_ATTR_CONTROL_PORT_ETHERTYPE] = { .type = NLA_U16 },
 	[NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT] = { .type = NLA_FLAG },
+	[NL80211_ATTR_CONTROL_PORT_OVER_NL80211] = { .type = NLA_FLAG },
 	[NL80211_ATTR_PRIVACY] = { .type = NLA_FLAG },
 	[NL80211_ATTR_CIPHER_SUITE_GROUP] = { .type = NLA_U32 },
 	[NL80211_ATTR_WPA_VERSIONS] = { .type = NLA_U32 },

@@ -8227,6 +8228,18 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev,
 	} else
 		settings->control_port_ethertype = cpu_to_be16(ETH_P_PAE);
 
+	if (info->attrs[NL80211_ATTR_CONTROL_PORT_OVER_NL80211]) {
+		if (!info->attrs[NL80211_ATTR_SOCKET_OWNER])
+			return -EINVAL;
+
+		if (!rdev->ops->tx_control_port ||
+		    !wiphy_ext_feature_isset(&rdev->wiphy,
+					     NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211))
+			return -EOPNOTSUPP;
+
+		settings->control_port_over_nl80211 = true;
+	}
+
 	if (info->attrs[NL80211_ATTR_CIPHER_SUITES_PAIRWISE]) {
 		void *data;
 		int len, i;

Signature · 1 line

2.13.5

Johannes Berg

maintainer

2018-03-21 07:48 UTC #4 lore raw

Re: [RFC v5 3/9] nl80211: Add CONTROL_PORT_OVER_NL80211 attribute

net/wireless/nl80211.c static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev, · line 8233

Quoted 5 lines · Denis Kenzior

net/wireless/nl80211.c +3 -0

+	if (info->attrs[NL80211_ATTR_CONTROL_PORT_OVER_NL80211]) {
+		if (!info->attrs[NL80211_ATTR_SOCKET_OWNER])
+			return -EINVAL;

There might be value in adding GENL_SET_ERR_MSG() calls to new instances of -EINVAL, but if you don't want to do that now I won't insist (and perhaps add some when I apply the patches).

johannes

Denis Kenzior

author

2018-03-21 15:01 UTC #5 lore raw

Re: [RFC v5 3/9] nl80211: Add CONTROL_PORT_OVER_NL80211 attribute

Hi Johannes,

Quoted 10 lines · Johannes Berg

net/wireless/nl80211.c static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev, · line 8233
Quoted 5 lines · Denis Kenzior
net/wireless/nl80211.c +3 -0
+	if (info->attrs[NL80211_ATTR_CONTROL_PORT_OVER_NL80211]) {
+		if (!info->attrs[NL80211_ATTR_SOCKET_OWNER])
+			return -EINVAL;
There might be value in adding GENL_SET_ERR_MSG() calls to new instances of -EINVAL, but if you don't want to do that now I won't insist (and perhaps add some when I apply the patches).

Sure, that sounds easy enough. Did you see the TODO comments I added in RFC v5 0/9 message? I need your help figuring out how you want to handle those. Those are pretty esoteric though and would require more surgery.

Any chance that we can merge the non-controversial bits of this RFC so that we can get some wider testing and start encouraging non-mac80211 based drivers to support these mechanisms?

Regards,
-Denis

Johannes Berg

maintainer

2018-03-21 15:09 UTC #6 lore raw

Re: [RFC v5 3/9] nl80211: Add CONTROL_PORT_OVER_NL80211 attribute

Quoted 15 lines · Denis Kenzior

Hi Johannes,
Quoted 11 lines · Johannes Berg
net/wireless/nl80211.c static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev, · line 8233
Quoted 5 lines · Denis Kenzior
net/wireless/nl80211.c +3 -0
+	if (info->attrs[NL80211_ATTR_CONTROL_PORT_OVER_NL80211]) {
+		if (!info->attrs[NL80211_ATTR_SOCKET_OWNER])
+			return -EINVAL;
There might be value in adding GENL_SET_ERR_MSG() calls to new instances of -EINVAL, but if you don't want to do that now I won't insist (and perhaps add some when I apply the patches).

Quoted 4 lines

Sure, that sounds easy enough. Did you see the TODO comments I added in RFC v5 0/9 message? I need your help figuring out how you want to handle those. Those are pretty esoteric though and would require more surgery.

Sorry, I hadn't. I'll take a look now.

Quoted 3 lines

Any chance that we can merge the non-controversial bits of this RFC so that we can get some wider testing and start encouraging non-mac80211 based drivers to support these mechanisms?

Yeah, sure, we can do that.

johannes

Denis Kenzior

author

2018-03-13 21:59 UTC #7 lore raw

RFC v5 4/9 cfg80211: Support all iftypes in autodisconnect_wk

In reply to Denis Kenzior

Currently autodisconnect_wk assumes that only interface types of P2P_CLIENT and STATION use conn_owner_nlportid. Change this so all interface types are supported.

Signed-off-by Denis Kenzior <[email protected]>

---
 net/wireless/sme.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

net/wireless/sme.c +32 ~~-11~~

index 701cfd7acc1b..5df6b33db786 100644
@@ -1239,17 +1239,38 @@ void cfg80211_autodisconnect_wk(struct work_struct *work)
 	wdev_lock(wdev);
 
 	if (wdev->conn_owner_nlportid) {
-		/*
-		 * Use disconnect_bssid if still connecting and ops->disconnect
-		 * not implemented.  Otherwise we can use cfg80211_disconnect.
-		 */
-		if (rdev->ops->disconnect || wdev->current_bss)
-			cfg80211_disconnect(rdev, wdev->netdev,
-					    WLAN_REASON_DEAUTH_LEAVING, true);
-		else
-			cfg80211_mlme_deauth(rdev, wdev->netdev,
-					     wdev->disconnect_bssid, NULL, 0,
-					     WLAN_REASON_DEAUTH_LEAVING, false);
+		switch (wdev->iftype) {
+		case NL80211_IFTYPE_ADHOC:
+			cfg80211_leave_ibss(rdev, wdev->netdev, false);
+			break;
+		case NL80211_IFTYPE_AP:
+		case NL80211_IFTYPE_P2P_GO:
+			cfg80211_stop_ap(rdev, wdev->netdev, false);
+			break;
+		case NL80211_IFTYPE_MESH_POINT:
+			cfg80211_leave_mesh(rdev, wdev->netdev);
+			break;
+		case NL80211_IFTYPE_STATION:
+		case NL80211_IFTYPE_P2P_CLIENT:
+			/*
+			 * Use disconnect_bssid if still connecting and
+			 * ops->disconnect not implemented.  Otherwise we can
+			 * use cfg80211_disconnect.
+			 */
+			if (rdev->ops->disconnect || wdev->current_bss)
+				cfg80211_disconnect(rdev, wdev->netdev,
+						    WLAN_REASON_DEAUTH_LEAVING,
+						    true);
+			else
+				cfg80211_mlme_deauth(rdev, wdev->netdev,
+						     wdev->disconnect_bssid,
+						     NULL, 0,
+						     WLAN_REASON_DEAUTH_LEAVING,
+						     false);
+			break;
+		default:
+			break;
+		}
 	}
 
 	wdev_unlock(wdev);

Signature · 1 line

2.13.5

Johannes Berg

maintainer

2018-03-21 07:46 UTC #8 lore raw

Re: [RFC v5 4/9] cfg80211: Support all iftypes in autodisconnect_wk

Quoted 3 lines · Denis Kenzior

Currently autodisconnect_wk assumes that only interface types of P2P_CLIENT and STATION use conn_owner_nlportid. Change this so all interface types are supported.

Hmm. This sort of duplicates cfg80211_leave() for the most part, apart from the special case with cfg80211_mlme_deauth(). I wonder why we need that, and if we could use cfg80211_leave()?

johannes

Denis Kenzior

author

2018-03-21 15:27 UTC #9 lore raw

Re: [RFC v5 4/9] cfg80211: Support all iftypes in autodisconnect_wk

Hi Johannes,

Quoted 8 lines · Johannes Berg

Quoted 3 lines · Denis Kenzior
Currently autodisconnect_wk assumes that only interface types of P2P_CLIENT and STATION use conn_owner_nlportid. Change this so all interface types are supported.
Hmm. This sort of duplicates cfg80211_leave() for the most part, apart from the special case with cfg80211_mlme_deauth(). I wonder why we need that, and if we could use cfg80211_leave()?

cfg80211_leave also messes with scans and autodisconnect_wk didn't. So I played it safe as I didn't want to introduce any silent semantic changes.

Also, cfg80211_leave uses stop_ap/leave_ibss with notify argument being true, while I thought it made more sense to use false (as there's arguably nobody left to pay attention to it) and to be consistent with nl80211_stop_ap and nl80211_leave_ibss which also use false.

I don't see an issue with using cfg80211_leave though. Just tell me which way you prefer.

Regards,
-Denis

Johannes Berg

maintainer

2018-03-21 15:40 UTC #10 lore raw

Re: [RFC v5 4/9] cfg80211: Support all iftypes in autodisconnect_wk

Quoted 3 lines · Denis Kenzior

cfg80211_leave also messes with scans and autodisconnect_wk didn't. So I played it safe as I didn't want to introduce any silent semantic changes.

Makes sense, but we could pass an argument and avoid duplicating the code?

Quoted 4 lines

Also, cfg80211_leave uses stop_ap/leave_ibss with notify argument being true, while I thought it made more sense to use false (as there's arguably nobody left to pay attention to it) and to be consistent with nl80211_stop_ap and nl80211_leave_ibss which also use false.

Fair enough.

Quoted 2 lines

I don't see an issue with using cfg80211_leave though. Just tell me which way you prefer.

It just seemed duplicated, ultimately I don't think I care all that much.

johannes

Denis Kenzior

author

2018-03-13 22:00 UTC #11 lore raw

RFC v5 5/9 nl80211: Add SOCKET_OWNER support to JOIN_IBSS

In reply to Denis Kenzior

Signed-off-by Denis Kenzior <[email protected]>

---
 include/uapi/linux/nl80211.h | 2 ++
 net/wireless/ibss.c          | 1 +
 net/wireless/nl80211.c       | 6 ++++++
 3 files changed, 9 insertions(+)

include/uapi/linux/nl80211.h +2 -0

index 1cdac3d732c1..877fab2836ec 100644
@@ -1985,6 +1985,8 @@ enum nl80211_commands {
  *	multicast group.
  *	If set during %NL80211_CMD_ASSOCIATE or %NL80211_CMD_CONNECT the
  *	station will deauthenticate when the socket is closed.
+ *	If set during %NL80211_CMD_JOIN_IBSS the IBSS will be automatically
+ *	torn down when the socket is closed.
  *
  * @NL80211_ATTR_TDLS_INITIATOR: flag attribute indicating the current end is
  *	the TDLS link initiator.

net/wireless/ibss.c +1 -0

index a1d10993d08a..d5d26fc5b853 100644
@@ -224,6 +224,7 @@ int __cfg80211_leave_ibss(struct cfg80211_registered_device *rdev,
 	if (err)
 		return err;
 
+	wdev->conn_owner_nlportid = 0;
 	__cfg80211_clear_ibss(dev, nowext);
 
 	return 0;

net/wireless/nl80211.c +6 -0

index 24b1bd940fca..e678dc510f3a 100644
@@ -8691,6 +8691,12 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
 	err = cfg80211_join_ibss(rdev, dev, &ibss, connkeys);
 	if (err)
 		kzfree(connkeys);
+	else if (info->attrs[NL80211_ATTR_SOCKET_OWNER]) {
+		wdev_lock(dev->ieee80211_ptr);
+		dev->ieee80211_ptr->conn_owner_nlportid = info->snd_portid;
+		wdev_unlock(dev->ieee80211_ptr);
+	}
+
 	return err;
 }

Signature · 1 line

2.13.5

Denis Kenzior

author

2018-03-13 22:00 UTC #12 lore raw

RFC v5 6/9 nl80211: Add SOCKET_OWNER support to JOIN_MESH

In reply to Denis Kenzior

Signed-off-by Denis Kenzior <[email protected]>

---
 include/uapi/linux/nl80211.h |  2 ++
 net/wireless/mesh.c          |  1 +
 net/wireless/nl80211.c       | 10 +++++++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

include/uapi/linux/nl80211.h +2 -0

index 877fab2836ec..e3329bc4644b 100644
@@ -1987,6 +1987,8 @@ enum nl80211_commands {
  *	station will deauthenticate when the socket is closed.
  *	If set during %NL80211_CMD_JOIN_IBSS the IBSS will be automatically
  *	torn down when the socket is closed.
+ *	If set during %NL80211_CMD_JOIN_MESH the mesh setup will be
+ *	automatically torn down when the socket is closed.
  *
  * @NL80211_ATTR_TDLS_INITIATOR: flag attribute indicating the current end is
  *	the TDLS link initiator.

net/wireless/mesh.c +1 -0

index b12da6ef3c12..e91a5078615b 100644
@@ -286,6 +286,7 @@ int __cfg80211_leave_mesh(struct cfg80211_registered_device *rdev,
 
 	err = rdev_leave_mesh(rdev, dev);
 	if (!err) {
+		wdev->conn_owner_nlportid = 0;
 		wdev->mesh_id_len = 0;
 		wdev->beacon_interval = 0;
 		memset(&wdev->chandef, 0, sizeof(wdev->chandef));

net/wireless/nl80211.c +9 -1

index e678dc510f3a..e38d55cf34f7 100644
@@ -10140,7 +10140,15 @@ static int nl80211_join_mesh(struct sk_buff *skb, struct genl_info *info)
 	setup.userspace_handles_dfs =
 		nla_get_flag(info->attrs[NL80211_ATTR_HANDLE_DFS]);
 
-	return cfg80211_join_mesh(rdev, dev, &setup, &cfg);
+	err = cfg80211_join_mesh(rdev, dev, &setup, &cfg);
+
+	if (!err && info->attrs[NL80211_ATTR_SOCKET_OWNER]) {
+		wdev_lock(dev->ieee80211_ptr);
+		dev->ieee80211_ptr->conn_owner_nlportid = info->snd_portid;
+		wdev_unlock(dev->ieee80211_ptr);
+	}
+
+	return err;
 }
 
 static int nl80211_leave_mesh(struct sk_buff *skb, struct genl_info *info)

Signature · 1 line

2.13.5

Denis Kenzior

author

2018-03-13 22:00 UTC #13 lore raw

RFC v5 7/9 nl80211: Add SOCKET_OWNER support to START_AP

In reply to Denis Kenzior

Signed-off-by Denis Kenzior <[email protected]>

---
 include/uapi/linux/nl80211.h | 2 ++
 net/wireless/ap.c            | 1 +
 net/wireless/nl80211.c       | 4 ++++
 3 files changed, 7 insertions(+)

include/uapi/linux/nl80211.h +2 -0

index e3329bc4644b..9b4fd4bca141 100644
@@ -1989,6 +1989,8 @@ enum nl80211_commands {
  *	torn down when the socket is closed.
  *	If set during %NL80211_CMD_JOIN_MESH the mesh setup will be
  *	automatically torn down when the socket is closed.
+ *	If set during %NL80211_CMD_START_AP the AP will be automatically
+ *	disabled when the socket is closed.
  *
  * @NL80211_ATTR_TDLS_INITIATOR: flag attribute indicating the current end is
  *	the TDLS link initiator.

net/wireless/ap.c +1 -0

index 63682176c96c..882d97bdc6bf 100644
@@ -27,6 +27,7 @@ int __cfg80211_stop_ap(struct cfg80211_registered_device *rdev,
 
 	err = rdev_stop_ap(rdev, dev);
 	if (!err) {
+		wdev->conn_owner_nlportid = 0;
 		wdev->beacon_interval = 0;
 		memset(&wdev->chandef, 0, sizeof(wdev->chandef));
 		wdev->ssid_len = 0;

net/wireless/nl80211.c +4 -0

index e38d55cf34f7..bbdcc61a738c 100644
@@ -4135,6 +4135,10 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 		wdev->chandef = params.chandef;
 		wdev->ssid_len = params.ssid_len;
 		memcpy(wdev->ssid, params.ssid, wdev->ssid_len);
+
+		if (info->attrs[NL80211_ATTR_SOCKET_OWNER])
+			wdev->conn_owner_nlportid = info->snd_portid;
+
 	}
 	wdev_unlock(wdev);

Signature · 1 line

2.13.5

Denis Kenzior

author

2018-03-13 22:00 UTC #14 lore raw

RFC v5 8/9 mac80211: Add support for tx_control_port

In reply to Denis Kenzior

Signed-off-by Denis Kenzior <[email protected]>

---
 net/mac80211/cfg.c         |  1 +
 net/mac80211/ieee80211_i.h |  3 +++
 net/mac80211/tx.c          | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)

net/mac80211/cfg.c +1 -0

index fd68f6fb02d7..9294acb495ee 100644
@@ -3786,4 +3786,5 @@ const struct cfg80211_ops mac80211_config_ops = {
 	.add_nan_func = ieee80211_add_nan_func,
 	.del_nan_func = ieee80211_del_nan_func,
 	.set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
+	.tx_control_port = ieee80211_tx_control_port,
 };

net/mac80211/ieee80211_i.h +3 -0

index ae9c33cd8ada..a52bd2a61a27 100644
@@ -1734,6 +1734,9 @@ void ieee80211_check_fast_xmit(struct sta_info *sta);
 void ieee80211_check_fast_xmit_all(struct ieee80211_local *local);
 void ieee80211_check_fast_xmit_iface(struct ieee80211_sub_if_data *sdata);
 void ieee80211_clear_fast_xmit(struct sta_info *sta);
+int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
+			      const u8 *buf, size_t len,
+			      const u8 *dest, __be16 proto, bool unencrypted);
 
 /* HT */
 void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,

net/mac80211/tx.c +46 -0

49 lines not on this page · raw message

Signature · 1 line

2.13.5

Denis Kenzior

author

2018-03-13 22:00 UTC #15 lore raw

RFC v5 9/9 mac80211: Send control port frames over nl80211

In reply to Denis Kenzior

If userspace requested control port frames to go over 80211, then do so. The control packets are intercepted just prior to delivery of the packet to the underlying network device.

Pre-authentication type frames (protocol: 0x88c7) are also forwarded over nl80211.

Signed-off-by Denis Kenzior <[email protected]>

---
 net/mac80211/cfg.c         |  4 ++++
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/iface.c       |  2 ++
 net/mac80211/main.c        |  2 ++
 net/mac80211/mlme.c        |  2 ++
 net/mac80211/rx.c          | 33 ++++++++++++++++++++++++++++-----
 6 files changed, 39 insertions(+), 5 deletions(-)

net/mac80211/cfg.c +4 -0

index 9294acb495ee..180653fcdb94 100644
@@ -925,6 +925,8 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	 */
 	sdata->control_port_protocol = params->crypto.control_port_ethertype;
 	sdata->control_port_no_encrypt = params->crypto.control_port_no_encrypt;
+	sdata->control_port_over_nl80211 =
+				params->crypto.control_port_over_nl80211;
 	sdata->encrypt_headroom = ieee80211_cs_headroom(sdata->local,
 							&params->crypto,
 							sdata->vif.type);

@@ -934,6 +936,8 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 			params->crypto.control_port_ethertype;
 		vlan->control_port_no_encrypt =
 			params->crypto.control_port_no_encrypt;
+		vlan->control_port_over_nl80211 =
+			params->crypto.control_port_over_nl80211;
 		vlan->encrypt_headroom =
 			ieee80211_cs_headroom(sdata->local,
 					      &params->crypto,

net/mac80211/ieee80211_i.h +1 -0

index a52bd2a61a27..00dbc6a1b79d 100644
@@ -899,6 +899,7 @@ struct ieee80211_sub_if_data {
 	u16 sequence_number;
 	__be16 control_port_protocol;
 	bool control_port_no_encrypt;
+	bool control_port_over_nl80211;
 	int encrypt_headroom;
 
 	atomic_t num_tx_queued;

net/mac80211/iface.c +2 -0

index d13ba064951f..555e389b7dfa 100644
@@ -519,6 +519,8 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 			master->control_port_protocol;
 		sdata->control_port_no_encrypt =
 			master->control_port_no_encrypt;
+		sdata->control_port_over_nl80211 =
+			master->control_port_over_nl80211;
 		sdata->vif.cab_queue = master->vif.cab_queue;
 		memcpy(sdata->vif.hw_queue, master->vif.hw_queue,
 		       sizeof(sdata->vif.hw_queue));

net/mac80211/main.c +2 -0

index 0785d04a80bc..e5a51267c75d 100644
@@ -554,6 +554,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
 			   NL80211_FEATURE_USERSPACE_MPM |
 			   NL80211_FEATURE_FULL_AP_CLIENT_STATE;
 	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_FILS_STA);
+	wiphy_ext_feature_set(wiphy,
+			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211);
 
 	if (!ops->hw_scan)
 		wiphy->features |= NL80211_FEATURE_LOW_PRIORITY_SCAN |

net/mac80211/mlme.c +2 -0

index 0024eff9bb84..b3665b857883 100644
@@ -4844,6 +4844,8 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
 
 	sdata->control_port_protocol = req->crypto.control_port_ethertype;
 	sdata->control_port_no_encrypt = req->crypto.control_port_no_encrypt;
+	sdata->control_port_over_nl80211 =
+					req->crypto.control_port_over_nl80211;
 	sdata->encrypt_headroom = ieee80211_cs_headroom(local, &req->crypto,
 							sdata->vif.type);

net/mac80211/rx.c +28 -5

index de7d10732fd5..bbb8bc6cac2a 100644
@@ -2245,6 +2245,32 @@ static bool ieee80211_frame_allowed(struct ieee80211_rx_data *rx, __le16 fc)
 	return true;
 }
 
+static void ieee80211_deliver_skb_to_local_stack(struct sk_buff *skb,
+						 struct ieee80211_rx_data *rx)
+{
+	struct ieee80211_sub_if_data *sdata = rx->sdata;
+	struct net_device *dev = sdata->dev;
+
+	if (unlikely((skb->protocol == sdata->control_port_protocol ||
+		      skb->protocol == cpu_to_be16(ETH_P_PREAUTH)) &&
+		     sdata->control_port_over_nl80211)) {
+		struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
+		bool noencrypt = status->flag & RX_FLAG_DECRYPTED;
+		struct ethhdr *ehdr = eth_hdr(skb);
+
+		cfg80211_rx_control_port(dev, skb->data, skb->len,
+					 ehdr->h_source,
+					 be16_to_cpu(skb->protocol), noencrypt);
+		dev_kfree_skb(skb);
+	} else {
+		/* deliver to local stack */
+		if (rx->napi)
+			napi_gro_receive(rx->napi, skb);
+		else
+			netif_receive_skb(skb);
+	}
+}
+
 /*
  * requires that rx->skb is a frame with ethernet header
  */

@@ -2329,13 +2355,10 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 #endif
 
 	if (skb) {
-		/* deliver to local stack */
 		skb->protocol = eth_type_trans(skb, dev);
 		memset(skb->cb, 0, sizeof(skb->cb));
-		if (rx->napi)
-			napi_gro_receive(rx->napi, skb);
-		else
-			netif_receive_skb(skb);
+
+		ieee80211_deliver_skb_to_local_stack(skb, rx);
 	}
 
 	if (xmit_skb) {

Signature · 1 line

2.13.5

Johannes Berg

maintainer

2018-03-21 15:13 UTC #16 lore raw

In reply to Denis Kenzior

Quoted 2 lines

TODO:

That was well-hidden :)

Quoted 2 lines

- It is unclear to me how AP_VLAN and AP interfaces should synchronize on conn_owner_nlportid. This is required for tx_control_port to work.

I'm not really sure what you mean? Technically I guess an AP_VLAN could have a different owner from an AP, but if the AP goes down all the AP_VLANs go down with it already anyway.

Quoted 3 lines

- JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup?

I don't think it matters - they just don't support this now and don't really need to.

johannes

Denis Kenzior

author

2018-03-21 15:18 UTC #17 lore raw

Hi Johannes,

Quoted 4 lines · Johannes Berg

Quoted 2 lines
TODO:
That was well-hidden :)

Sorry. I assumed people read the change log :)

Quoted 7 lines

Quoted 3 lines
- It is unclear to me how AP_VLAN and AP interfaces should synchronize on conn_owner_nlportid. This is required for tx_control_port to work.
I'm not really sure what you mean? Technically I guess an AP_VLAN could have a different owner from an AP, but if the AP goes down all the AP_VLANs go down with it already anyway.

So the issue is that when mac80211 calls cfg80211_rx_control_port and subsequently __nl80211_rx_control_port, we grab the nlportid from the wdev. So if that isn't synchronized, then AP_VLAN devices won't be sending the EAPoL frames to the right place.

Quoted 8 lines

Quoted 4 lines
- JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup?
I don't think it matters - they just don't support this now and don't really need to.

Except that the eapol over nl80211 flag is being sent in security settings. This covers STA/AP/P2P_GO/P2P_CLIENT. We need some way of passing this information for mesh & ibss.

Regards,
-Denis

Johannes Berg

maintainer

2018-03-22 10:28 UTC #18 lore raw

Quoted 2 lines · Denis Kenzior

Sorry. I assumed people read the change log :)

And I should :-)

Quoted 11 lines

Quoted 6 lines
Quoted 2 lines
- It is unclear to me how AP_VLAN and AP interfaces should synchronize on conn_owner_nlportid. This is required for tx_control_port to work.
I'm not really sure what you mean? Technically I guess an AP_VLAN could have a different owner from an AP, but if the AP goes down all the AP_VLANs go down with it already anyway.
So the issue is that when mac80211 calls cfg80211_rx_control_port and subsequently __nl80211_rx_control_port, we grab the nlportid from the wdev. So if that isn't synchronized, then AP_VLAN devices won't be sending the EAPoL frames to the right place.

Oh, ok, gotcha. I guess mac80211 would have to sync over the data, just like it does for other things? You also copied over the control_port_over_nl80211 setting, so could do the same here?

But no, maybe that doesn't make sense, since ...

Hmm. Ok, I think I see what you're getting at.

The key point seems to be that we don't have any sort of "add VLAN to AP operation" - it's auto-detected based on the MAC address.

However, it doesn't actually matter at all - we shouldn't get there with VLAN interface. EAPOL frames are always sent out to the corresponding AP interface, see ieee80211_rx_h_data:

        if (rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
            unlikely(port_control) && sdata->bss) {
                sdata = container_of(sdata->bss, struct ieee80211_sub_if_data,
                                     u.ap);
                dev = sdata->dev;
                rx->sdata = sdata;
        }

Quoted 11 lines

Quoted 7 lines
Quoted 3 lines
- JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup?
I don't think it matters - they just don't support this now and don't really need to.
Except that the eapol over nl80211 flag is being sent in security settings. This covers STA/AP/P2P_GO/P2P_CLIENT. We need some way of passing this information for mesh & ibss.

Not sure I understand what you're saying. Can't we just say the flag isn't permitted in those modes?

johannes

Denis Kenzior

author

2018-03-22 13:54 UTC #19 lore raw

Hi Johannes,

Quoted 4 lines

However, it doesn't actually matter at all - we shouldn't get there with VLAN interface. EAPOL frames are always sent out to the corresponding AP interface, see ieee80211_rx_h_data:

Quoted 8 lines

         if (rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
             unlikely(port_control) && sdata->bss) {
                 sdata = container_of(sdata->bss, struct ieee80211_sub_if_data,
                                      u.ap);
                 dev = sdata->dev;
                 rx->sdata = sdata;
         }

Okay, that makes things easier. However, it does bring up a question. Should we be symmetric and remove AP_VLAN as a valid target of control port frames? E.g. drop NL80211_IFTYPE_AP_VLAN case in patch 2 of the series. In effect we'd require all control port frame traffic to go over the master interface.

Quoted 9 lines

Quoted 9 lines
Quoted 8 lines
Quoted 4 lines
- JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup?
I don't think it matters - they just don't support this now and don't really need to.

Quoted 6 lines

Quoted 3 lines
Except that the eapol over nl80211 flag is being sent in security settings. This covers STA/AP/P2P_GO/P2P_CLIENT. We need some way of passing this information for mesh & ibss.
Not sure I understand what you're saying. Can't we just say the flag isn't permitted in those modes?

I'm the one confused now. You wanted me to add IFTYPE_IBSS in nl80211_tx_control_port in your earlier feedback :)

Let me try to restate what I said earlier in a different way and see if it makes things a bit clearer:

So in patch 9, we set sdata->control_port_over_nl80211 based on parameters passed into start_ap or mgd_assoc. The control_port_over_nl80211 flag is passed in cfg80211_crypto_settings structure that is part of the relevant parameters structure. If sdata->control_port_over_nl80211 is true, then we actually redirect the control port frames to nl80211.

So my question is, if we want to support IBSS/MESH, should we:
1. add the whole cfg80211_crypto_settings to the IBSS/MESH parameters,
2. add the control_port_over_nl80211 flag directly to IBSS/MESH parameters
3. Pass the flag some other way?
4. Or drop IBSS/MESH from patch 2 (nl80211_tx_control_port) completely?

Regards,
-Denis

Johannes Berg

maintainer

2018-03-23 08:04 UTC #20 lore raw

Hi,

Quoted 5 lines

Okay, that makes things easier. However, it does bring up a question. Should we be symmetric and remove AP_VLAN as a valid target of control port frames? E.g. drop NL80211_IFTYPE_AP_VLAN case in patch 2 of the series. In effect we'd require all control port frame traffic to go over the master interface.

Yeah, I think that'd make sense.

Quoted 2 lines

I'm the one confused now. You wanted me to add IFTYPE_IBSS in nl80211_tx_control_port in your earlier feedback :)

Hah. My memory is like a sieve...

Quoted 9 lines

Let me try to restate what I said earlier in a different way and see if it makes things a bit clearer:
So in patch 9, we set sdata->control_port_over_nl80211 based on parameters passed into start_ap or mgd_assoc. The control_port_over_nl80211 flag is passed in cfg80211_crypto_settings structure that is part of the relevant parameters structure. If sdata->control_port_over_nl80211 is true, then we actually redirect the control port frames to nl80211.

Right.

Quoted 8 lines

So my question is, if we want to support IBSS/MESH, should we:
1. add the whole cfg80211_crypto_settings to the IBSS/MESH parameters,
2. add the control_port_over_nl80211 flag directly to IBSS/MESH parameters
3. Pass the flag some other way?
4. Or drop IBSS/MESH from patch 2 (nl80211_tx_control_port) completely?

I'd say 2 or 4. Having all the crypto settings would be confusing, since they aren't (and possibly cannot) be used.

4 is a bit annoying - not that I remember - because it eventually means that when we do want to support it in IBSS/MESH, we'd have to add another flag indicating that now it's supported, etc. But I think we can live with that too, if it's too complex to add this for real.

johannes