[PATCH 0/3] nl80211/mac80211: add tx status for ctrl port

[PATCH 0/3] nl80211/mac80211: add tx status for ctrl port

[Markus Theil]

When sending EAPOL frames from an AP to a STA, SO_WIFI_STATUS can be used
on raw/packet sockets to get notified of (un-)successful frame
transmissions in order to trigger retransmits earlier.

The nl80211 control port currently lacks such a feature, which therefore
gets added in this series, in order to function as a drop in replacement
for using dedicated AF_PACKET sockets for handling EAPOL messages.
With this series it is finally possible to run hostapd in AP mode with
mgmt and EAPOL control port frames solely over nl80211 and data frames on
the kernel data path.

The intended behaviour stems from tx notifications for mgmt frames over
nl80211. A cookie is provided to match request and reply, furthermore the
frame content is also included in status messages.

This series has been tested with a modified hostapd version. Patches for
hostapd will follow soon on its mailing list.

Markus Theil (3):
  nl80211: cookie arg for tx control port
  nl80211: add control port tx status method
  nl80211: add feature flag for control port tx status capability

 include/net/cfg80211.h       | 18 +++++++-
 include/uapi/linux/nl80211.h | 12 ++++++
 net/mac80211/ieee80211_i.h   |  8 +++-
 net/mac80211/main.c          |  2 +
 net/mac80211/status.c        |  7 ++-
 net/mac80211/tdls.c          |  2 +-
 net/mac80211/tx.c            | 83 +++++++++++++++++++++++++++---------
 net/wireless/nl80211.c       | 73 ++++++++++++++++++++++++++-----
 net/wireless/rdev-ops.h      |  9 ++--
 net/wireless/trace.h         | 17 ++++++++
 10 files changed, 193 insertions(+), 38 deletions(-)

-- 
2.26.2

[PATCH 1/3] nl80211: cookie arg for tx control port

[Markus Theil]

This patch is a preparation for getting tx status reports for nl80211
control port frames. It adds a cookie parameter and replies the cookie
to the sending process. This cookie can later be used to identify
succesfull or lost ACKs (analog to already existing status reports for
mgmt frames).

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 include/net/cfg80211.h     |  3 ++-
 net/mac80211/ieee80211_i.h |  3 ++-
 net/mac80211/tx.c          |  3 ++-
 net/wireless/nl80211.c     | 45 +++++++++++++++++++++++++++++++++-----
 net/wireless/rdev-ops.h    |  4 ++--
 5 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a82fc59a1d82..d3d18481f6da 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4067,7 +4067,8 @@ struct cfg80211_ops {
 				   struct net_device *dev,
 				   const u8 *buf, size_t len,
 				   const u8 *dest, const __be16 proto,
-				   const bool noencrypt);
+				   const bool noencrypt,
+				   u64 *cookie);
 
 	int	(*get_ftm_responder_stats)(struct wiphy *wiphy,
 				struct net_device *dev,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 8cbae66b5cdb..4f6432c7e150 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1800,7 +1800,8 @@ 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);
+			      const u8 *dest, __be16 proto, bool unencrypted,
+			      u64 *cookie);
 int ieee80211_probe_mesh_link(struct wiphy *wiphy, struct net_device *dev,
 			      const u8 *buf, size_t len);
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 6dad67eb60b2..ba34cc392ea8 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -5339,7 +5339,8 @@ void __ieee80211_tx_skb_tid_band(struct ieee80211_sub_if_data *sdata,
 
 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)
+			      const u8 *dest, __be16 proto, bool unencrypted,
+			      u64 *cookie)
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = sdata->local;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3d27b24c68b2..4fe232a51078 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -13862,14 +13862,18 @@ static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info)
 
 static int nl80211_tx_control_port(struct sk_buff *skb, struct genl_info *info)
 {
+	bool dont_wait_for_ack = info->attrs[NL80211_ATTR_DONT_WAIT_FOR_ACK];
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct sk_buff *msg = NULL;
+	void *hdr = NULL;
 	const u8 *buf;
 	size_t len;
 	u8 *dest;
 	u16 proto;
 	bool noencrypt;
+	u64 cookie;
 	int err;
 
 	if (!wiphy_ext_feature_isset(&rdev->wiphy,
@@ -13899,10 +13903,10 @@ static int nl80211_tx_control_port(struct sk_buff *skb, struct genl_info *info)
 		if (wdev->current_bss)
 			break;
 		err = -ENOTCONN;
-		goto out;
+		goto out_unlock;
 	default:
 		err = -EOPNOTSUPP;
-		goto out;
+		goto out_unlock;
 	}
 
 	wdev_unlock(wdev);
@@ -13914,11 +13918,42 @@ static int nl80211_tx_control_port(struct sk_buff *skb, struct genl_info *info)
 	noencrypt =
 		nla_get_flag(info->attrs[NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT]);
 
-	return rdev_tx_control_port(rdev, dev, buf, len,
-				    dest, cpu_to_be16(proto), noencrypt);
+	if (!dont_wait_for_ack) {
+		msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+		if (!msg)
+			return -ENOMEM;
 
- out:
+		hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
+				     NL80211_CMD_CONTROL_PORT_FRAME);
+		if (!hdr) {
+			err = -ENOBUFS;
+			goto free_msg;
+		}
+	}
+
+	err = rdev_tx_control_port(rdev, dev, buf, len,
+				   dest, cpu_to_be16(proto), noencrypt,
+				   dont_wait_for_ack ? NULL : &cookie);
+	if (err)
+		goto free_msg;
+
+	if (msg) {
+		if (nla_put_u64_64bit(msg, NL80211_ATTR_COOKIE, cookie,
+				      NL80211_ATTR_PAD))
+			goto nla_put_failure;
+
+		genlmsg_end(msg, hdr);
+		return genlmsg_reply(msg, info);
+	}
+
+	return 0;
+
+out_unlock:
 	wdev_unlock(wdev);
+nla_put_failure:
+	err = -ENOBUFS;
+free_msg:
+	nlmsg_free(msg);
 	return err;
 }
 
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index df5142e86c4f..63b5786572d0 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -748,13 +748,13 @@ 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)
+				       const bool noencrypt, u64 *cookie)
 {
 	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);
+					 dest, proto, noencrypt, cookie);
 	trace_rdev_return_int(&rdev->wiphy, ret);
 	return ret;
 }
-- 
2.26.2

[PATCH 2/3] nl80211: add control port tx status method

[Markus Theil]

This patch adds the actual code for returning the tx status of
control port frames sent over nl80211.

NL80211_CMD_CONTROL_PORT_FRAME_TX_STATUS is a new command which is used
when returning the tx status. Its availability can be queried by checking
against NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211_TX_STATUS, which is
added in the next patch of this series.

The ctrl port tx status over nl80211 path re-uses some code from the path
for advertising the tx status over socket control messages, when
SKBTX_WIFI_STATUS is set.

The tx status can be used in a similar fashion as the mgmt tx status
already allows for. A cookie is included in a optional reply to
NL80211_CMD_CONTROL_PORT_FRAME, which can be matched against the cookie
in NL80211_CMD_CONTROL_PORT_FRAME_TX_STATUS. The frame content is also
included, as for example hostapd currently uses it to match request and
reply.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 include/net/cfg80211.h       | 17 ++++++++
 include/uapi/linux/nl80211.h |  8 ++++
 net/mac80211/ieee80211_i.h   |  5 ++-
 net/mac80211/status.c        |  9 +++-
 net/mac80211/tdls.c          |  2 +-
 net/mac80211/tx.c            | 80 +++++++++++++++++++++++++++---------
 net/wireless/nl80211.c       | 30 +++++++++++---
 net/wireless/rdev-ops.h      |  5 ++-
 net/wireless/trace.h         | 17 ++++++++
 9 files changed, 145 insertions(+), 28 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d3d18481f6da..0f8bbd6579c1 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -7023,6 +7023,23 @@ bool cfg80211_rx_mgmt(struct wireless_dev *wdev, int freq, int sig_dbm,
 void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie,
 			     const u8 *buf, size_t len, bool ack, gfp_t gfp);
 
+/**
+ * cfg80211_control_port_tx_status - notification of TX status for control
+ *                                   port frames
+ * @wdev: wireless device receiving the frame
+ * @cookie: Cookie returned by cfg80211_ops::tx_control_port()
+ * @buf: Data frame (header + body)
+ * @len: length of the frame data
+ * @ack: Whether frame was acknowledged
+ * @gfp: context flags
+ *
+ * This function is called whenever a control port frame was requested to be
+ * transmitted with cfg80211_ops::tx_control_port() to report the TX status of
+ * the transmission attempt.
+ */
+void cfg80211_control_port_tx_status(struct wireless_dev *wdev, u64 cookie,
+				     const u8 *buf, size_t len, bool ack,
+				     gfp_t gfp);
 
 /**
  * cfg80211_rx_control_port - notification about a received control port frame
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9679d561f7d0..e3b7a911b35c 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1160,6 +1160,12 @@
  *	dropped because it did not include a valid MME MIC while beacon
  *	protection was enabled (BIGTK configured in station mode).
  *
+ * @NL80211_CMD_CONTROL_PORT_FRAME_TX_STATUS: Report TX status of a control
+ *	port frame transmitted with %NL80211_CMD_CONTROL_PORT_FRAME.
+ *	%NL80211_ATTR_COOKIE identifies the TX command and %NL80211_ATTR_FRAME
+ *	includes the contents of the frame. %NL80211_ATTR_ACK flag is included
+ *	if the recipient acknowledged the frame.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1388,6 +1394,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_UNPROT_BEACON,
 
+	NL80211_CMD_CONTROL_PORT_FRAME_TX_STATUS,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4f6432c7e150..32ecb1b34371 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1407,6 +1407,8 @@ struct ieee80211_local {
 	struct idr ack_status_frames;
 	spinlock_t ack_status_lock;
 
+	u64 ctrl_port_cookie_counter;
+
 	struct ieee80211_sub_if_data __rcu *p2p_sdata;
 
 	/* virtual monitor interface */
@@ -1783,7 +1785,8 @@ netdev_tx_t ieee80211_subif_start_xmit_8023(struct sk_buff *skb,
 void __ieee80211_subif_start_xmit(struct sk_buff *skb,
 				  struct net_device *dev,
 				  u32 info_flags,
-				  u32 ctrl_flags);
+				  u32 ctrl_flags,
+				  u64 *cookie);
 void ieee80211_purge_tx_queue(struct ieee80211_hw *hw,
 			      struct sk_buff_head *skbs);
 struct sk_buff *
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 22512805eafb..7b1bacac39c6 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -649,10 +649,17 @@ static void ieee80211_report_ack_skb(struct ieee80211_local *local,
 						      info->status.ack_signal,
 						      info->status.is_valid_ack_signal,
 						      GFP_ATOMIC);
-			else
+			else if (ieee80211_is_mgmt(hdr->frame_control))
 				cfg80211_mgmt_tx_status(&sdata->wdev, cookie,
 							skb->data, skb->len,
 							acked, GFP_ATOMIC);
+			else
+				cfg80211_control_port_tx_status(&sdata->wdev,
+								cookie,
+								skb->data,
+								skb->len,
+								acked,
+								GFP_ATOMIC);
 		}
 		rcu_read_unlock();
 
diff --git a/net/mac80211/tdls.c b/net/mac80211/tdls.c
index 8ad420db3766..4b0cff4a07bd 100644
--- a/net/mac80211/tdls.c
+++ b/net/mac80211/tdls.c
@@ -1054,7 +1054,7 @@ ieee80211_tdls_prep_mgmt_packet(struct wiphy *wiphy, struct net_device *dev,
 
 	/* disable bottom halves when entering the Tx path */
 	local_bh_disable();
-	__ieee80211_subif_start_xmit(skb, dev, flags, 0);
+	__ieee80211_subif_start_xmit(skb, dev, flags, 0, NULL);
 	local_bh_enable();
 
 	return ret;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index ba34cc392ea8..092f282a6a5c 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2436,13 +2436,33 @@ int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
 	return 0;
 }
 
-static int ieee80211_store_ack_skb(struct ieee80211_local *local,
+static u64 ieee80211_ctrl_port_tx_cookie(struct ieee80211_local *local)
+{
+	lockdep_assert_held(&local->mtx);
+
+	local->ctrl_port_cookie_counter++;
+
+	/* wow, you wrapped 64 bits ... more likely a bug */
+	if (WARN_ON(local->ctrl_port_cookie_counter == 0))
+		local->ctrl_port_cookie_counter++;
+
+	return local->ctrl_port_cookie_counter;
+}
+
+static u16 ieee80211_store_ack_skb(struct ieee80211_local *local,
 				   struct sk_buff *skb,
-				   u32 *info_flags)
+				   u32 *info_flags,
+				   bool use_socket,
+				   u64 *cookie)
 {
-	struct sk_buff *ack_skb = skb_clone_sk(skb);
+	struct sk_buff *ack_skb;
 	u16 info_id = 0;
 
+	if (use_socket)
+		ack_skb = skb_clone_sk(skb);
+	else
+		ack_skb = skb_clone(skb, GFP_ATOMIC);
+
 	if (ack_skb) {
 		unsigned long flags;
 		int id;
@@ -2455,6 +2475,10 @@ static int ieee80211_store_ack_skb(struct ieee80211_local *local,
 		if (id >= 0) {
 			info_id = id;
 			*info_flags |= IEEE80211_TX_CTL_REQ_TX_STATUS;
+			if (cookie) {
+				*cookie = ieee80211_ctrl_port_tx_cookie(local);
+				IEEE80211_SKB_CB(ack_skb)->ack.cookie = *cookie;
+			}
 		} else {
 			kfree_skb(ack_skb);
 		}
@@ -2484,7 +2508,8 @@ static int ieee80211_store_ack_skb(struct ieee80211_local *local,
  */
 static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
 					   struct sk_buff *skb, u32 info_flags,
-					   struct sta_info *sta, u32 ctrl_flags)
+					   struct sta_info *sta, u32 ctrl_flags,
+					   u64 *cookie)
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_tx_info *info;
@@ -2757,7 +2782,12 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
 
 	if (unlikely(!multicast && skb->sk &&
 		     skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS))
-		info_id = ieee80211_store_ack_skb(local, skb, &info_flags);
+		info_id = ieee80211_store_ack_skb(local, skb, &info_flags,
+						  true, NULL);
+
+	if (unlikely(!multicast && ctrl_flags & IEEE80211_TX_CTL_REQ_TX_STATUS))
+		info_id = ieee80211_store_ack_skb(local, skb, &info_flags,
+						  false, cookie);
 
 	/*
 	 * If the skb is shared we need to obtain our own copy.
@@ -2765,8 +2795,9 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
 	if (skb_shared(skb)) {
 		struct sk_buff *tmp_skb = skb;
 
-		/* can't happen -- skb is a clone if info_id != 0 */
-		WARN_ON(info_id);
+		if (!(ctrl_flags & IEEE80211_TX_CTL_REQ_TX_STATUS))
+			/* can't happen -- skb is a clone if info_id != 0 */
+			WARN_ON(info_id);
 
 		skb = skb_clone(skb, GFP_ATOMIC);
 		kfree_skb(tmp_skb);
@@ -3913,7 +3944,8 @@ EXPORT_SYMBOL(ieee80211_txq_schedule_start);
 void __ieee80211_subif_start_xmit(struct sk_buff *skb,
 				  struct net_device *dev,
 				  u32 info_flags,
-				  u32 ctrl_flags)
+				  u32 ctrl_flags,
+				  u64 *cookie)
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = sdata->local;
@@ -3983,7 +4015,7 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
 		skb_mark_not_on_list(skb);
 
 		skb = ieee80211_build_hdr(sdata, skb, info_flags,
-					  sta, ctrl_flags);
+					  sta, ctrl_flags, cookie);
 		if (IS_ERR(skb)) {
 			kfree_skb_list(next);
 			goto out;
@@ -4125,9 +4157,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		__skb_queue_head_init(&queue);
 		ieee80211_convert_to_unicast(skb, dev, &queue);
 		while ((skb = __skb_dequeue(&queue)))
-			__ieee80211_subif_start_xmit(skb, dev, 0, 0);
+			__ieee80211_subif_start_xmit(skb, dev, 0, 0, NULL);
 	} else {
-		__ieee80211_subif_start_xmit(skb, dev, 0, 0);
+		__ieee80211_subif_start_xmit(skb, dev, 0, 0, NULL);
 	}
 
 	return NETDEV_TX_OK;
@@ -4215,7 +4247,7 @@ static void ieee80211_8023_xmit(struct ieee80211_sub_if_data *sdata,
 
 	if (unlikely(!multicast && skb->sk &&
 		     skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS))
-		ieee80211_store_ack_skb(local, skb, &info->flags);
+		ieee80211_store_ack_skb(local, skb, &info->flags, true, NULL);
 
 	memset(info, 0, sizeof(*info));
 
@@ -4299,7 +4331,7 @@ ieee80211_build_data_template(struct ieee80211_sub_if_data *sdata,
 		goto out;
 	}
 
-	skb = ieee80211_build_hdr(sdata, skb, info_flags, sta, 0);
+	skb = ieee80211_build_hdr(sdata, skb, info_flags, sta, 0, NULL);
 	if (IS_ERR(skb))
 		goto out;
 
@@ -5347,7 +5379,7 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
 	struct sk_buff *skb;
 	struct ethhdr *ehdr;
 	u32 ctrl_flags = 0;
-	u32 flags;
+	u32 flags = 0;
 
 	/* Only accept CONTROL_PORT_PROTOCOL configured in CONNECT/ASSOCIATE
 	 * or Pre-Authentication
@@ -5360,9 +5392,13 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
 		ctrl_flags |= IEEE80211_TX_CTRL_PORT_CTRL_PROTO;
 
 	if (unencrypted)
-		flags = IEEE80211_TX_INTFL_DONT_ENCRYPT;
-	else
-		flags = 0;
+		flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
+
+	if (cookie)
+		ctrl_flags |= IEEE80211_TX_CTL_REQ_TX_STATUS;
+
+	flags |= IEEE80211_TX_INTFL_NL80211_FRAME_TX |
+		 IEEE80211_TX_CTL_INJECTED;
 
 	skb = dev_alloc_skb(local->hw.extra_tx_headroom +
 			    sizeof(struct ethhdr) + len);
@@ -5383,10 +5419,15 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
 	skb_reset_network_header(skb);
 	skb_reset_mac_header(skb);
 
+	/* mutex lock is only needed for incrementing the cookie counter */
+	mutex_lock(&local->mtx);
+
 	local_bh_disable();
-	__ieee80211_subif_start_xmit(skb, skb->dev, flags, ctrl_flags);
+	__ieee80211_subif_start_xmit(skb, skb->dev, flags, ctrl_flags, cookie);
 	local_bh_enable();
 
+	mutex_unlock(&local->mtx);
+
 	return 0;
 }
 
@@ -5413,7 +5454,8 @@ int ieee80211_probe_mesh_link(struct wiphy *wiphy, struct net_device *dev,
 
 	local_bh_disable();
 	__ieee80211_subif_start_xmit(skb, skb->dev, 0,
-				     IEEE80211_TX_CTRL_SKIP_MPATH_LOOKUP);
+				     IEEE80211_TX_CTRL_SKIP_MPATH_LOOKUP,
+				     NULL);
 	local_bh_enable();
 
 	return 0;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4fe232a51078..ad5f43cad1f3 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -16308,8 +16308,9 @@ int nl80211_send_mgmt(struct cfg80211_registered_device *rdev,
 	return -ENOBUFS;
 }
 
-void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie,
-			     const u8 *buf, size_t len, bool ack, gfp_t gfp)
+static void nl80211_frame_tx_status(struct wireless_dev *wdev, u64 cookie,
+				    const u8 *buf, size_t len, bool ack,
+				    gfp_t gfp, enum nl80211_commands command)
 {
 	struct wiphy *wiphy = wdev->wiphy;
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
@@ -16317,13 +16318,16 @@ void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie,
 	struct sk_buff *msg;
 	void *hdr;
 
-	trace_cfg80211_mgmt_tx_status(wdev, cookie, ack);
+	if (command == NL80211_CMD_FRAME_TX_STATUS)
+		trace_cfg80211_mgmt_tx_status(wdev, cookie, ack);
+	else
+		trace_cfg80211_control_port_tx_status(wdev, cookie, ack);
 
 	msg = nlmsg_new(100 + len, gfp);
 	if (!msg)
 		return;
 
-	hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_FRAME_TX_STATUS);
+	hdr = nl80211hdr_put(msg, 0, 0, 0, command);
 	if (!hdr) {
 		nlmsg_free(msg);
 		return;
@@ -16346,9 +16350,25 @@ void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie,
 				NL80211_MCGRP_MLME, gfp);
 	return;
 
- nla_put_failure:
+nla_put_failure:
 	nlmsg_free(msg);
 }
+
+void cfg80211_control_port_tx_status(struct wireless_dev *wdev, u64 cookie,
+				     const u8 *buf, size_t len, bool ack,
+				     gfp_t gfp)
+{
+	nl80211_frame_tx_status(wdev, cookie, buf, len, ack, gfp,
+				NL80211_CMD_CONTROL_PORT_FRAME_TX_STATUS);
+}
+EXPORT_SYMBOL(cfg80211_control_port_tx_status);
+
+void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie,
+			     const u8 *buf, size_t len, bool ack, gfp_t gfp)
+{
+	nl80211_frame_tx_status(wdev, cookie, buf, len, ack, gfp,
+				NL80211_CMD_FRAME_TX_STATUS);
+}
 EXPORT_SYMBOL(cfg80211_mgmt_tx_status);
 
 static int __nl80211_rx_control_port(struct net_device *dev,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 63b5786572d0..950d57494168 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -755,7 +755,10 @@ static inline int rdev_tx_control_port(struct cfg80211_registered_device *rdev,
 				   dest, proto, noencrypt);
 	ret = rdev->ops->tx_control_port(&rdev->wiphy, dev, buf, len,
 					 dest, proto, noencrypt, cookie);
-	trace_rdev_return_int(&rdev->wiphy, ret);
+	if (cookie)
+		trace_rdev_return_int_cookie(&rdev->wiphy, ret, *cookie);
+	else
+		trace_rdev_return_int(&rdev->wiphy, ret);
 	return ret;
 }
 
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 53c887ea67c7..b9b28af1ac71 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2861,6 +2861,23 @@ TRACE_EVENT(cfg80211_mgmt_tx_status,
 		  WDEV_PR_ARG, __entry->cookie, BOOL_TO_STR(__entry->ack))
 );
 
+TRACE_EVENT(cfg80211_control_port_tx_status,
+	TP_PROTO(struct wireless_dev *wdev, u64 cookie, bool ack),
+	TP_ARGS(wdev, cookie, ack),
+	TP_STRUCT__entry(
+		WDEV_ENTRY
+		__field(u64, cookie)
+		__field(bool, ack)
+	),
+	TP_fast_assign(
+		WDEV_ASSIGN;
+		__entry->cookie = cookie;
+		__entry->ack = ack;
+	),
+	TP_printk(WDEV_PR_FMT", cookie: %llu, ack: %s",
+		  WDEV_PR_ARG, __entry->cookie, BOOL_TO_STR(__entry->ack))
+);
+
 TRACE_EVENT(cfg80211_rx_control_port,
 	TP_PROTO(struct net_device *netdev, struct sk_buff *skb,
 		 bool unencrypted),
-- 
2.26.2

Re: [PATCH 2/3] nl80211: add control port tx status method

[Johannes Berg]

>  	struct idr ack_status_frames;
>  	spinlock_t ack_status_lock;
>  
> +	u64 ctrl_port_cookie_counter;

We have a u64 for other things (remain-on-channel), perhaps should just
share? It's not going to overflow even if shared ...

>  	/* disable bottom halves when entering the Tx path */
>  	local_bh_disable();
> -	__ieee80211_subif_start_xmit(skb, dev, flags, 0);
> +	__ieee80211_subif_start_xmit(skb, dev, flags, 0, NULL);

This is a little awkward, any way to avoid that? Probably not ...

> +static u16 ieee80211_store_ack_skb(struct ieee80211_local *local,
>  				   struct sk_buff *skb,
> -				   u32 *info_flags)
> +				   u32 *info_flags,
> +				   bool use_socket,
> +				   u64 *cookie)
>  {
> -	struct sk_buff *ack_skb = skb_clone_sk(skb);
> +	struct sk_buff *ack_skb;
>  	u16 info_id = 0;
>  
> +	if (use_socket)

You can check skb->sk and not need the additional parameter.

>  	if (unlikely(!multicast && skb->sk &&
>  		     skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS))
> -		info_id = ieee80211_store_ack_skb(local, skb, &info_flags);
> +		info_id = ieee80211_store_ack_skb(local, skb, &info_flags,
> +						  true, NULL);
> +
> +	if (unlikely(!multicast && ctrl_flags & IEEE80211_TX_CTL_REQ_TX_STATUS))
> +		info_id = ieee80211_store_ack_skb(local, skb, &info_flags,
> +						  false, cookie);

I think this should be rolled into one condition, since you no longer
need the true/false if you check skb->sk. And 'cookie' is already NULL
in those other cases so you can pass it unconditionally.

> @@ -2765,8 +2795,9 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
>  	if (skb_shared(skb)) {
>  		struct sk_buff *tmp_skb = skb;
>  
> -		/* can't happen -- skb is a clone if info_id != 0 */
> -		WARN_ON(info_id);
> +		if (!(ctrl_flags & IEEE80211_TX_CTL_REQ_TX_STATUS))
> +			/* can't happen -- skb is a clone if info_id != 0 */
> +			WARN_ON(info_id);

This I don't understand, but if it really is needed then you should
adjust the comment and roll it into a single WARN_ON().

Also, please put all the remaining mac80211 changes into one patch - I
already put all the other changes in.

johannes

Re: [PATCH 2/3] nl80211: add control port tx status method

[Markus Theil]

On 5/26/20 3:37 PM, Johannes Berg wrote:
>>  	struct idr ack_status_frames;
>>  	spinlock_t ack_status_lock;
>>  
>> +	u64 ctrl_port_cookie_counter;
> We have a u64 for other things (remain-on-channel), perhaps should just
> share? It's not going to overflow even if shared ...
Sounds fair, I'll consolidate to use the roc cookie variable.
>>  	/* disable bottom halves when entering the Tx path */
>>  	local_bh_disable();
>> -	__ieee80211_subif_start_xmit(skb, dev, flags, 0);
>> +	__ieee80211_subif_start_xmit(skb, dev, flags, 0, NULL);
> This is a little awkward, any way to avoid that? Probably not ...
I first tried to read out the cookie from the skb, in order to avoid
adding this new argument.
Problematic with this approach was, that the skb can be deleted in some
failure cases.
Therefore I went with this additional argument.
>> +static u16 ieee80211_store_ack_skb(struct ieee80211_local *local,
>>  				   struct sk_buff *skb,
>> -				   u32 *info_flags)
>> +				   u32 *info_flags,
>> +				   bool use_socket,
>> +				   u64 *cookie)
>>  {
>> -	struct sk_buff *ack_skb = skb_clone_sk(skb);
>> +	struct sk_buff *ack_skb;
>>  	u16 info_id = 0;
>>  
>> +	if (use_socket)
> You can check skb->sk and not need the additional parameter.
Thanks for the hint!
>
>>  	if (unlikely(!multicast && skb->sk &&
>>  		     skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS))
>> -		info_id = ieee80211_store_ack_skb(local, skb, &info_flags);
>> +		info_id = ieee80211_store_ack_skb(local, skb, &info_flags,
>> +						  true, NULL);
>> +
>> +	if (unlikely(!multicast && ctrl_flags & IEEE80211_TX_CTL_REQ_TX_STATUS))
>> +		info_id = ieee80211_store_ack_skb(local, skb, &info_flags,
>> +						  false, cookie);
> I think this should be rolled into one condition, since you no longer
> need the true/false if you check skb->sk. And 'cookie' is already NULL
> in those other cases so you can pass it unconditionally.
Ok
>> @@ -2765,8 +2795,9 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
>>  	if (skb_shared(skb)) {
>>  		struct sk_buff *tmp_skb = skb;
>>  
>> -		/* can't happen -- skb is a clone if info_id != 0 */
>> -		WARN_ON(info_id);
>> +		if (!(ctrl_flags & IEEE80211_TX_CTL_REQ_TX_STATUS))
>> +			/* can't happen -- skb is a clone if info_id != 0 */
>> +			WARN_ON(info_id);
> This I don't understand, but if it really is needed then you should
> adjust the comment and roll it into a single WARN_ON().
After adapting my patch with the changes lined out above, I have tested
again and the warning
did not occur. Therefore I've ommited changing the warning behavior from
the updated patch.
> Also, please put all the remaining mac80211 changes into one patch - I
> already put all the other changes in.
>
> johannes
>
Thanks for your feedback! I'll send an updated v2 with a single patch.

[PATCH 3/3] nl80211: add feature flag for control port tx status capability

[Markus Theil]

This patch adds a feature flag in order to detect control port tx
status capability.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 include/uapi/linux/nl80211.h | 4 ++++
 net/mac80211/main.c          | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index e3b7a911b35c..02e1aa10f9a2 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -5713,6 +5713,9 @@ enum nl80211_feature_flags {
  * @NL80211_EXT_FEATURE_MULTICAST_REGISTRATIONS: management frame registrations
  *	are possible for multicast frames and those will be reported properly.
  *
+ * @NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211_TX_STATUS: The driver
+ *	can report tx status for control port over nl80211 tx operations.
+ *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
  */
@@ -5766,6 +5769,7 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_DEL_IBSS_STA,
 	NL80211_EXT_FEATURE_MULTICAST_REGISTRATIONS,
 	NL80211_EXT_FEATURE_BEACON_PROTECTION_CLIENT,
+	NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211_TX_STATUS,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index dfcee5e462da..6381e6c1f58b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -596,6 +596,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
 			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211);
 	wiphy_ext_feature_set(wiphy,
 			      NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH);
+	wiphy_ext_feature_set(wiphy,
+			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211_TX_STATUS);
 
 	if (!ops->hw_scan) {
 		wiphy->features |= NL80211_FEATURE_LOW_PRIORITY_SCAN |
-- 
2.26.2

Re: [PATCH 0/3] nl80211/mac80211: add tx status for ctrl port

[Johannes Berg]

Hi Markus,

Generally, this seems fine.

I have applied the cfg80211/nl80211 bits in my tree (currently in
mac80211-next pending branch), and recombined the patches for that.

I've also modified the nl80211 code to return the cookie in the extack
message, rather than a separate message, which is much nicer. That means
you'll have to modify the hostapd changes for this.

The mac80211 changes seem overly complex, I'll reply in detail.

johannes