[PATCH 1/4] device: Switch to new frame watch API

[PATCH 1/4] device: Switch to new frame watch API

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]

---
 src/device.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/device.c b/src/device.c
index 0ad1ccbf..c24d7ed1 100644
--- a/src/device.c
+++ b/src/device.c
@@ -37,6 +37,7 @@
 #include "src/scan.h"
 #include "src/netdev.h"
 #include "src/dbus.h"
+#include "src/frame-xchg.h"
 #include "src/station.h"
 
 struct device {
@@ -48,16 +49,13 @@ struct device {
 	bool powered : 1;		/* Current IFUP state */
 	bool dbus_powered : 1;		/* Last IFUP state wanted via D-Bus */
 
-	uint32_t ap_roam_watch;
 	uint32_t wiphy_rfkill_watch;
 };
 
 static uint32_t netdev_watch;
 
-static void device_ap_roam_frame_event(struct netdev *netdev,
-		const struct mmpdu_header *hdr,
-		const void *body, size_t body_len,
-		void *user_data)
+static void device_ap_roam_frame_event(const struct mmpdu_header *hdr,
+		const void *body, size_t body_len, int rssi, void *user_data)
 {
 	struct device *device = user_data;
 	struct station *station = station_find(device->index);
@@ -325,9 +323,9 @@ static struct device *device_create(struct wiphy *wiphy, struct netdev *netdev)
 	/*
 	 * register for AP roam transition watch
 	 */
-	device->ap_roam_watch = netdev_frame_watch_add(netdev, 0x00d0,
+	frame_watch_add(netdev_get_wdev_id(netdev), 0, 0x00d0,
 			action_ap_roam_prefix, sizeof(action_ap_roam_prefix),
-			device_ap_roam_frame_event, device);
+			device_ap_roam_frame_event, device, NULL);
 
 	device->powered = netdev_get_is_up(netdev);
 
@@ -343,9 +341,16 @@ static void device_free(struct device *device)
 {
 	l_debug("");
 
-	netdev_frame_watch_remove(device->netdev, device->ap_roam_watch);
 	wiphy_state_watch_remove(device->wiphy, device->wiphy_rfkill_watch);
 
+	/*
+	 * We're triggered on NETDEV_WATCH_EVENT_DEL or device_exit.  The former
+	 * is triggered on NL80211_CMD_DEL_INTERFACE and RTM_DELLINK which
+	 * also cause all frame watches to be unregistered so we don't have
+	 * to do this here.  device_exit is triggered under the same conditions
+	 * as frame_xchg_exit.
+	 */
+
 	l_free(device);
 }
 
-- 
2.20.1

[PATCH 2/4] rrm: Switch to new frame watch API

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

---
 src/rrm.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/rrm.c b/src/rrm.c
index a117dce3..64d4afd3 100644
--- a/src/rrm.c
+++ b/src/rrm.c
@@ -39,6 +39,7 @@
 #include "src/scan.h"
 #include "src/nl80211util.h"
 #include "src/wiphy.h"
+#include "src/frame-xchg.h"
 
 #include "linux/nl80211.h"
 
@@ -658,10 +659,9 @@ static void rrm_station_watch_cb(enum station_state state, void *userdata)
 	}
 }
 
-static void rrm_frame_watch_cb(struct netdev *netdev,
-				const struct mmpdu_header *mpdu,
+static void rrm_frame_watch_cb(const struct mmpdu_header *mpdu,
 				const void *body, size_t body_len,
-				void *user_data)
+				int rssi, void *user_data)
 {
 	struct rrm_state *rrm = user_data;
 	const uint8_t *request = body;
@@ -769,8 +769,8 @@ static void rrm_new_state(struct netdev *netdev)
 	rrm->ifindex = netdev_get_ifindex(netdev);
 	rrm->wdev_id = netdev_get_wdev_id(netdev);
 
-	netdev_frame_watch_add(netdev, frame_type, prefix, sizeof(prefix),
-					rrm_frame_watch_cb, rrm);
+	frame_watch_add(rrm->wdev_id, 0, frame_type, prefix, sizeof(prefix),
+					rrm_frame_watch_cb, rrm, NULL);
 
 	l_queue_push_head(states, rrm);
 }
@@ -794,6 +794,12 @@ static void rrm_netdev_watch(struct netdev *netdev,
 		rrm_new_state(netdev);
 		return;
 	case NETDEV_WATCH_EVENT_DEL:
+		/*
+		 * We're triggered on L80211_CMD_DEL_INTERFACE and RTM_DELLINK which
+		 * also cause all frame watches to be unregistered so we don't have
+		 * to do this here.
+		 */
+
 		rrm = l_queue_remove_if(states, match_ifindex,
 						L_UINT_TO_PTR(ifindex));
 		if (rrm) {
-- 
2.20.1

[PATCH 3/4] netdev: Switch to new frame watch API

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 5048 bytes --]

---
 src/netdev.c | 68 +++++++++++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 32 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index 84ba4c7f..ff56b78f 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -61,6 +61,7 @@
 #include "src/fils.h"
 #include "src/auth-proto.h"
 #include "src/rtnlutil.h"
+#include "src/frame-xchg.h"
 
 #ifndef ENOTSUPP
 #define ENOTSUPP 524
@@ -2814,13 +2815,13 @@ static void netdev_ft_request_cb(struct l_genl_msg *msg, void *user_data)
 	}
 }
 
-static void netdev_ft_response_frame_event(struct netdev *netdev,
-					const struct mmpdu_header *hdr,
+static void netdev_ft_response_frame_event(const struct mmpdu_header *hdr,
 					const void *body, size_t body_len,
-					void *user_data)
+					int rssi, void *user_data)
 {
 	int ret;
 	uint16_t status_code = MMPDU_STATUS_CODE_UNSPECIFIED;
+	struct netdev *netdev = user_data;
 
 	if (!netdev->ap || !netdev->in_ft)
 		return;
@@ -2841,11 +2842,12 @@ ft_error:
 	return;
 }
 
-static void netdev_qos_map_frame_event(struct netdev *netdev,
-					const struct mmpdu_header *hdr,
+static void netdev_qos_map_frame_event(const struct mmpdu_header *hdr,
 					const void *body, size_t body_len,
-					void *user_data)
+					int rssi, void *user_data)
 {
+	struct netdev *netdev = user_data;
+
 	/* No point telling the kernel */
 	if (!netdev->connected)
 		return;
@@ -3090,11 +3092,12 @@ int netdev_neighbor_report_req(struct netdev *netdev,
 	return 0;
 }
 
-static void netdev_neighbor_report_frame_event(struct netdev *netdev,
-					const struct mmpdu_header *hdr,
+static void netdev_neighbor_report_frame_event(const struct mmpdu_header *hdr,
 					const void *body, size_t body_len,
-					void *user_data)
+					int rssi, void *user_data)
 {
+	struct netdev *netdev = user_data;
+
 	if (body_len < 3) {
 		l_debug("Neighbor Report frame too short");
 		return;
@@ -3125,13 +3128,13 @@ static void netdev_sa_query_resp_cb(struct l_genl_msg *msg,
 		l_debug("error sending SA Query request");
 }
 
-static void netdev_sa_query_req_frame_event(struct netdev *netdev,
-		const struct mmpdu_header *hdr,
-		const void *body, size_t body_len,
-		void *user_data)
+static void netdev_sa_query_req_frame_event(const struct mmpdu_header *hdr,
+					const void *body, size_t body_len,
+					int rssi, void *user_data)
 {
 	uint8_t sa_resp[4];
 	uint16_t transaction;
+	struct netdev *netdev = user_data;
 
 	if (body_len < 4) {
 		l_debug("SA Query request too short");
@@ -3163,11 +3166,12 @@ static void netdev_sa_query_req_frame_event(struct netdev *netdev,
 	}
 }
 
-static void netdev_sa_query_resp_frame_event(struct netdev *netdev,
-		const struct mmpdu_header *hdr,
-		const void *body, size_t body_len,
-		void *user_data)
+static void netdev_sa_query_resp_frame_event(const struct mmpdu_header *hdr,
+					const void *body, size_t body_len,
+					int rssi, void *user_data)
 {
+	struct netdev *netdev = user_data;
+
 	if (body_len < 4) {
 		l_debug("SA Query frame too short");
 		return;
@@ -4524,26 +4528,26 @@ struct netdev *netdev_create_from_genl(struct l_genl_msg *msg,
 	l_free(rtmmsg);
 
 	/* Subscribe to Management -> Action -> RM -> Neighbor Report frames */
-	netdev_frame_watch_add(netdev, 0x00d0, action_neighbor_report_prefix,
-				sizeof(action_neighbor_report_prefix),
-				netdev_neighbor_report_frame_event, NULL);
+	frame_watch_add(wdev, 0, 0x00d0, action_neighbor_report_prefix,
+			sizeof(action_neighbor_report_prefix),
+			netdev_neighbor_report_frame_event, netdev, NULL);
 
-	netdev_frame_watch_add(netdev, 0x00d0, action_sa_query_resp_prefix,
-				sizeof(action_sa_query_resp_prefix),
-				netdev_sa_query_resp_frame_event, NULL);
+	frame_watch_add(wdev, 0, 0x00d0, action_sa_query_resp_prefix,
+			sizeof(action_sa_query_resp_prefix),
+			netdev_sa_query_resp_frame_event, netdev, NULL);
 
-	netdev_frame_watch_add(netdev, 0x00d0, action_sa_query_req_prefix,
-				sizeof(action_sa_query_req_prefix),
-				netdev_sa_query_req_frame_event, NULL);
+	frame_watch_add(wdev, 0, 0x00d0, action_sa_query_req_prefix,
+			sizeof(action_sa_query_req_prefix),
+			netdev_sa_query_req_frame_event, netdev, NULL);
 
-	netdev_frame_watch_add(netdev, 0x00d0, action_ft_response_prefix,
-				sizeof(action_ft_response_prefix),
-				netdev_ft_response_frame_event, NULL);
+	frame_watch_add(wdev, 0, 0x00d0, action_ft_response_prefix,
+			sizeof(action_ft_response_prefix),
+			netdev_ft_response_frame_event, netdev, NULL);
 
 	if (wiphy_supports_qos_set_map(netdev->wiphy))
-		netdev_frame_watch_add(netdev, 0x00d0, action_qos_map_prefix,
-					sizeof(action_qos_map_prefix),
-					netdev_qos_map_frame_event, NULL);
+		frame_watch_add(wdev, 0, 0x00d0, action_qos_map_prefix,
+				sizeof(action_qos_map_prefix),
+				netdev_qos_map_frame_event, netdev, NULL);
 
 	/* Set RSSI threshold for CQM notifications */
 	if (netdev->type == NL80211_IFTYPE_STATION)
-- 
2.20.1

[PATCH 4/4] netdev: Drop the netdev_frame_watch API

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 7244 bytes --]

---
 src/netdev.c | 150 ---------------------------------------------------
 src/netdev.h |  10 ----
 2 files changed, 160 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index ff56b78f..0a287a42 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -132,8 +132,6 @@ struct netdev {
 	void *set_powered_user_data;
 	netdev_destroy_func_t set_powered_destroy;
 
-	struct watchlist frame_watches;
-
 	struct watchlist station_watches;
 
 	struct l_io *pae_io;  /* for drivers without EAPoL over NL80211 */
@@ -163,13 +161,6 @@ struct netdev_watch {
 	void *user_data;
 };
 
-struct netdev_frame_watch {
-	uint16_t frame_type;
-	uint8_t *prefix;
-	size_t prefix_len;
-	struct watchlist_item super;
-};
-
 static struct l_netlink *rtnl = NULL;
 static struct l_genl_family *nl80211;
 static struct l_queue *netdev_list;
@@ -629,7 +620,6 @@ static void netdev_free(void *data)
 
 	scan_wdev_remove(netdev->wdev_id);
 
-	watchlist_destroy(&netdev->frame_watches);
 	watchlist_destroy(&netdev->station_watches);
 
 	l_io_destroy(netdev->pae_io);
@@ -3404,74 +3394,6 @@ static void netdev_mlme_notify(struct l_genl_msg *msg, void *user_data)
 	}
 }
 
-struct frame_prefix_info {
-	uint16_t frame_type;
-	const uint8_t *body;
-	size_t body_len;
-};
-
-static bool netdev_frame_watch_match_prefix(const void *a, const void *b)
-{
-	const struct watchlist_item *item = a;
-	const struct netdev_frame_watch *fw =
-		l_container_of(item, struct netdev_frame_watch, super);
-	const struct frame_prefix_info *info = b;
-
-	return fw->frame_type == info->frame_type &&
-		fw->prefix_len <= info->body_len &&
-		(fw->prefix_len == 0 ||
-		 !memcmp(fw->prefix, info->body, fw->prefix_len));
-}
-
-static void netdev_mgmt_frame_event(struct l_genl_msg *msg,
-					struct netdev *netdev)
-{
-	struct l_genl_attr attr;
-	uint16_t type, len, frame_len;
-	const void *data;
-	const struct mmpdu_header *mpdu = NULL;
-	const uint8_t *body;
-	struct frame_prefix_info info;
-
-	if (!l_genl_attr_init(&attr, msg))
-		return;
-
-	while (l_genl_attr_next(&attr, &type, &len, &data)) {
-		switch (type) {
-		case NL80211_ATTR_FRAME:
-			if (mpdu)
-				return;
-
-			mpdu = mpdu_validate(data, len);
-			if (!mpdu)
-				l_error("Frame didn't validate as MMPDU");
-
-			frame_len = len;
-			break;
-		}
-	}
-
-	if (!mpdu)
-		return;
-
-	body = mmpdu_body(mpdu);
-
-	if (memcmp(mpdu->address_1, netdev->addr, 6) &&
-			!util_is_broadcast_address(mpdu->address_1))
-		return;
-
-	/* Only match the frame type and subtype like the kernel does */
-#define FC_FTYPE_STYPE_MASK 0x00fc
-	info.frame_type = l_get_le16(mpdu) & FC_FTYPE_STYPE_MASK;
-	info.body = (const uint8_t *) body;
-	info.body_len = (const uint8_t *) mpdu + frame_len - body;
-
-	WATCHLIST_NOTIFY_MATCHES(&netdev->frame_watches,
-					netdev_frame_watch_match_prefix, &info,
-					netdev_frame_watch_func_t,
-					netdev, mpdu, body, info.body_len);
-}
-
 static void netdev_pae_destroy(void *user_data)
 {
 	struct netdev *netdev = user_data;
@@ -3692,9 +3614,6 @@ static void netdev_unicast_notify(struct l_genl_msg *msg, void *user_data)
 		return;
 
 	switch (cmd) {
-	case NL80211_CMD_FRAME:
-		netdev_mgmt_frame_event(msg, netdev);
-		break;
 	case NL80211_CMD_CONTROL_PORT_FRAME:
 		netdev_control_port_frame_event(msg, netdev);
 		break;
@@ -4313,74 +4232,6 @@ static void netdev_getlink_cb(int error, uint16_t type, const void *data,
 					NULL);
 }
 
-static void netdev_frame_watch_free(struct watchlist_item *item)
-{
-	struct netdev_frame_watch *fw =
-		l_container_of(item, struct netdev_frame_watch, super);
-
-	l_free(fw->prefix);
-	l_free(fw);
-}
-
-static const struct watchlist_ops netdev_frame_watch_ops = {
-	.item_free = netdev_frame_watch_free,
-};
-
-static void netdev_frame_cb(struct l_genl_msg *msg, void *user_data)
-{
-	if (l_genl_msg_get_error(msg) < 0)
-		l_error("Could not register frame watch type %04x: %i",
-			L_PTR_TO_UINT(user_data), l_genl_msg_get_error(msg));
-}
-
-uint32_t netdev_frame_watch_add(struct netdev *netdev, uint16_t frame_type,
-				const uint8_t *prefix, size_t prefix_len,
-				netdev_frame_watch_func_t handler,
-				void *user_data)
-{
-	struct netdev_frame_watch *fw;
-	struct l_genl_msg *msg;
-	struct frame_prefix_info info = { frame_type, prefix, prefix_len };
-	bool registered;
-	uint32_t id;
-
-	registered = l_queue_find(netdev->frame_watches.items,
-					netdev_frame_watch_match_prefix,
-					&info);
-
-	fw = l_new(struct netdev_frame_watch, 1);
-	fw->frame_type = frame_type;
-	fw->prefix = prefix_len ? l_memdup(prefix, prefix_len) : NULL;
-	fw->prefix_len = prefix_len;
-	id = watchlist_link(&netdev->frame_watches, &fw->super,
-						handler, user_data, NULL);
-
-	if (registered)
-		return id;
-
-	msg = l_genl_msg_new_sized(NL80211_CMD_REGISTER_FRAME, 32 + prefix_len);
-
-	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_FRAME_TYPE, 2, &frame_type);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_FRAME_MATCH,
-				prefix_len, prefix);
-
-	l_genl_family_send(nl80211, msg, netdev_frame_cb,
-			L_UINT_TO_PTR(frame_type), NULL);
-
-	return id;
-}
-
-bool netdev_frame_watch_remove(struct netdev *netdev, uint32_t id)
-{
-	/*
-	 * There's no way to unregister from notifications but that's not a
-	 * problem, we leave them active in the kernel but
-	 * netdev_mgmt_frame_event will ignore these events.
-	 */
-	return watchlist_remove(&netdev->frame_watches, id);
-}
-
 static struct l_io *pae_open(uint32_t ifindex)
 {
 	/*
@@ -4506,7 +4357,6 @@ struct netdev *netdev_create_from_genl(struct l_genl_msg *msg,
 							netdev_pae_destroy);
 	}
 
-	watchlist_init(&netdev->frame_watches, &netdev_frame_watch_ops);
 	watchlist_init(&netdev->station_watches, NULL);
 
 	l_queue_push_tail(netdev_list, netdev);
diff --git a/src/netdev.h b/src/netdev.h
index 366dc1ab..f87e7e49 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -112,10 +112,6 @@ typedef void (*netdev_neighbor_report_cb_t)(struct netdev *netdev, int err,
 typedef void (*netdev_preauthenticate_cb_t)(struct netdev *netdev,
 					enum netdev_result result,
 					const uint8_t *pmk, void *user_data);
-typedef void (*netdev_frame_watch_func_t)(struct netdev *netdev,
-					const struct mmpdu_header *frame,
-					const void *body, size_t body_len,
-					void *user_data);
 typedef void (*netdev_station_watch_func_t)(struct netdev *netdev,
 					const uint8_t *mac, bool added,
 					void *user_data);
@@ -180,12 +176,6 @@ int netdev_neighbor_report_req(struct netdev *netdev,
 int netdev_set_rssi_report_levels(struct netdev *netdev, const int8_t *levels,
 					size_t levels_num);
 
-uint32_t netdev_frame_watch_add(struct netdev *netdev, uint16_t frame_type,
-				const uint8_t *prefix, size_t prefix_len,
-				netdev_frame_watch_func_t handler,
-				void *user_data);
-bool netdev_frame_watch_remove(struct netdev *netdev, uint32_t id);
-
 void netdev_handshake_failed(struct handshake_state *hs, uint16_t reason_code);
 
 struct netdev *netdev_find(int ifindex);
-- 
2.20.1

Re: [PATCH 2/4] rrm: Switch to new frame watch API

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 770 bytes --]

Hi Andrew,

On 1/15/20 8:40 PM, Andrew Zaborowski wrote:
> ---
>   src/rrm.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 

<snip>

> @@ -794,6 +794,12 @@ static void rrm_netdev_watch(struct netdev *netdev,
>   		rrm_new_state(netdev);
>   		return;
>   	case NETDEV_WATCH_EVENT_DEL:
> +		/*
> +		 * We're triggered on L80211_CMD_DEL_INTERFACE and RTM_DELLINK which
> +		 * also cause all frame watches to be unregistered so we don't have
> +		 * to do this here.
> +		 */
> +

I reflowed / edited this comment a bit since it was going > 80 chars.

>   		rrm = l_queue_remove_if(states, match_ifindex,
>   						L_UINT_TO_PTR(ifindex));
>   		if (rrm) {
> 

Patches 1-4 all applied, thanks.

Regards,
-Denis