[RFC PATCH wpan-next 0/2] ieee802154: hwsim: add hwsim driver

[RFC PATCH wpan-next 0/2] ieee802154: hwsim: add hwsim driver

[Alexander Aring]

Hi,

This patch series introduce a new virtual ieee802154 driver mac802154_hwsim.
The mac802154_hwsim driver offers more flexibility to change topology so
we can use it to test 6LoWPAN routing protocols e.g. RPL.

This driver cleaned up and prepared for mainline now. I showed basic mechanism
in my netdev 2.1 talk [0].

- Alex

[0] https://www.youtube.com/watch?v=iMYCaqHVmS0

Alexander Aring (2):
  ieee802154: hwsim: add replacement for fakelb
  ieee802154: fakelb: add deprecated msg while probe

 drivers/net/ieee802154/Kconfig           |  11 +
 drivers/net/ieee802154/Makefile          |   1 +
 drivers/net/ieee802154/fakelb.c          |   3 +
 drivers/net/ieee802154/mac802154_hwsim.c | 925 +++++++++++++++++++++++++++++++
 drivers/net/ieee802154/mac802154_hwsim.h |  73 +++
 5 files changed, 1013 insertions(+)
 create mode 100644 drivers/net/ieee802154/mac802154_hwsim.c
 create mode 100644 drivers/net/ieee802154/mac802154_hwsim.h

-- 
2.11.0

[RFC PATCH wpan-next 1/2] ieee802154: hwsim: add replacement for fakelb

[Alexander Aring]

This patch adds a new virtual driver mac802154_hwsim which is based on
the fakelb driver.
The fakelb driver will get deprecated and hopefully removed someday.
The main reason for doing this step is to rename the driver to
mac802154_hwsim to have a similar naming scheme as mac80211_hwsim,
which is more popular in the 802.11 wireless word and the idea is the
same behind this driver.

The new features of this driver are to have knowledge about connected
edges, which can be changed during runtime. This offers a testing
environment for routing protocols e.g. RPL.
The default behaviour is still as fakelb: two radios connected to each
other. New added radios during runtime will not be connected to other
wpan_hwsim instances.

The netlink api is not namespace aware on purpose, only the registered
wpan_phy's can be moved to namespaces. The physical layer according to
wiresless "air" communication can be handled across namespaces.

Furthermore the edges can be weighted with the LQI value according IEEE
802.15.4 which offers additional handling to mark bad or good connection
indicators to other connected virtual phys.

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 drivers/net/ieee802154/Kconfig           |  11 +
 drivers/net/ieee802154/Makefile          |   1 +
 drivers/net/ieee802154/mac802154_hwsim.c | 925 +++++++++++++++++++++++++++++++
 drivers/net/ieee802154/mac802154_hwsim.h |  73 +++
 4 files changed, 1010 insertions(+)
 create mode 100644 drivers/net/ieee802154/mac802154_hwsim.c
 create mode 100644 drivers/net/ieee802154/mac802154_hwsim.h

diff --git a/drivers/net/ieee802154/Kconfig b/drivers/net/ieee802154/Kconfig
index 8782f5655e3f..ff3552044679 100644
--- a/drivers/net/ieee802154/Kconfig
+++ b/drivers/net/ieee802154/Kconfig
@@ -115,3 +115,14 @@ config IEEE802154_MCR20A
 
 	  This driver can also be built as a module. To do so, say M here.
 	  the module will be called 'mcr20a'.
+
+config MAC802154_HWSIM
+	depends on IEEE802154_DRIVERS && MAC802154
+	tristate "Simulated radio testing tool for mac802154"
+	---help---
+	  This driver is a developer testing tool that can be used to test
+	  IEEE 802.15.4 networking stack (mac802154) functionality. This is not
+	  needed for normal wireless LAN usage and is only for testing.
+
+	  This driver can also be built as a module. To do so say M here.
+	  The module will be called 'mac802154_hwsim'.
diff --git a/drivers/net/ieee802154/Makefile b/drivers/net/ieee802154/Makefile
index 104744d5a668..3b17aa17d46a 100644
--- a/drivers/net/ieee802154/Makefile
+++ b/drivers/net/ieee802154/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_IEEE802154_ATUSB) += atusb.o
 obj-$(CONFIG_IEEE802154_ADF7242) += adf7242.o
 obj-$(CONFIG_IEEE802154_CA8210) += ca8210.o
 obj-$(CONFIG_IEEE802154_MCR20A) += mcr20a.o
+obj-$(CONFIG_MAC802154_HWSIM) += mac802154_hwsim.o
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
new file mode 100644
index 000000000000..eea0d355bbd8
--- /dev/null
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -0,0 +1,925 @@
+/*
+ * HWSIM IEEE 802.15.4 interface
+ *
+ * (C) 2018 Mojatau, Alexander Aring <aring@mojatau.com>
+ * Copyright 2007-2012 Siemens AG
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Based on fakelb, original Written by:
+ * Sergey Lapin <slapin@ossfans.org>
+ * Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
+ * Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
+ */
+
+#include <linux/module.h>
+#include <linux/timer.h>
+#include <linux/platform_device.h>
+#include <linux/netdevice.h>
+#include <linux/device.h>
+#include <linux/spinlock.h>
+#include <net/mac802154.h>
+#include <net/cfg802154.h>
+#include <net/genetlink.h>
+#include "mac802154_hwsim.h"
+
+MODULE_DESCRIPTION("Software simulator of 802.15.4 radio(s) for mac802154");
+MODULE_LICENSE("GPL");
+
+/* Number of dummy devices to be set up by this module. */
+static int radios = 2;
+module_param(radios, int, 0444);
+MODULE_PARM_DESC(radios, "Number of simulated radios");
+
+static LIST_HEAD(hwsim_phys);
+static DEFINE_MUTEX(hwsim_phys_lock);
+
+static __rcu LIST_HEAD(hwsim_ifup_phys);
+
+static struct platform_device *mac802154hwsim_dev;
+
+/* MAC802154_HWSIM netlink family */
+static struct genl_family hwsim_genl_family;
+
+static int hwsim_radio_idx;
+
+enum hwsim_multicast_groups {
+	HWSIM_MCGRP_CONFIG,
+};
+
+static const struct genl_multicast_group hwsim_mcgrps[] = {
+	[HWSIM_MCGRP_CONFIG] = { .name = "config", },
+};
+
+struct hwsim_pib {
+	u8 page;
+	u8 channel;
+
+	struct rcu_head rcu;
+};
+
+struct hwsim_edge_info {
+	u8 lqi;
+
+	struct rcu_head rcu;
+};
+
+struct hwsim_edge {
+	struct hwsim_phy *endpoint;
+	struct hwsim_edge_info *info;
+
+	struct list_head list;
+	struct rcu_head rcu;
+};
+
+struct hwsim_phy {
+	struct ieee802154_hw *hw;
+	u32 idx;
+
+	struct hwsim_pib __rcu *pib;
+
+	bool suspended;
+	struct list_head __rcu edges;
+
+	struct list_head list;
+	struct list_head list_ifup;
+};
+
+static int hwsim_hw_ed(struct ieee802154_hw *hw, u8 *level)
+{
+	*level = 0xbe;
+
+	return 0;
+}
+
+static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
+{
+	struct hwsim_phy *phy = hw->priv;
+	struct hwsim_pib *pib, *pib_old;
+
+	pib = kzalloc(sizeof(*pib), GFP_KERNEL);
+	if (!pib)
+		return -ENOMEM;
+
+	pib->page = page;
+	pib->channel = channel;
+
+	pib_old = phy->pib;
+	rcu_assign_pointer(phy->pib, pib);
+	kfree_rcu(pib_old, rcu);
+	return 0;
+}
+
+static int hwsim_hw_xmit(struct ieee802154_hw *hw, struct sk_buff *skb)
+{
+	struct hwsim_phy *current_phy = hw->priv;
+	struct hwsim_pib *current_pib, *endpoint_pib;
+	struct hwsim_edge_info *einfo;
+	struct hwsim_edge *e;
+
+	WARN_ON(current_phy->suspended);
+
+	rcu_read_lock();
+	current_pib = rcu_dereference(current_phy->pib);
+	list_for_each_entry_rcu(e, &current_phy->edges, list) {
+		/* Can be changed later in rx_irqsafe, but this is only a
+		 * performance tweak. Received radio should drop the frame
+		 * in mac802154 stack anyway... so we don't need to be
+		 * 100% of locking here to check on suspended
+		 */
+		if (e->endpoint->suspended)
+			continue;
+
+		endpoint_pib = rcu_dereference(e->endpoint->pib);
+		if (current_pib->page == endpoint_pib->page &&
+		    current_pib->channel == endpoint_pib->channel) {
+			struct sk_buff *newskb = pskb_copy(skb, GFP_ATOMIC);
+
+			einfo = rcu_dereference(e->info);
+			if (newskb)
+				ieee802154_rx_irqsafe(e->endpoint->hw, newskb,
+						      einfo->lqi);
+		}
+	}
+	rcu_read_unlock();
+
+	ieee802154_xmit_complete(hw, skb, false);
+	return 0;
+}
+
+static int hwsim_hw_start(struct ieee802154_hw *hw)
+{
+	struct hwsim_phy *phy = hw->priv;
+
+	phy->suspended = false;
+	list_add_rcu(&phy->list_ifup, &hwsim_ifup_phys);
+	synchronize_rcu();
+
+	return 0;
+}
+
+static void hwsim_hw_stop(struct ieee802154_hw *hw)
+{
+	struct hwsim_phy *phy = hw->priv;
+
+	phy->suspended = true;
+	list_del_rcu(&phy->list_ifup);
+	synchronize_rcu();
+}
+
+static int
+hwsim_set_promiscuous_mode(struct ieee802154_hw *hw, const bool on)
+{
+	return 0;
+}
+
+static const struct ieee802154_ops hwsim_ops = {
+	.owner = THIS_MODULE,
+	.xmit_async = hwsim_hw_xmit,
+	.ed = hwsim_hw_ed,
+	.set_channel = hwsim_hw_channel,
+	.start = hwsim_hw_start,
+	.stop = hwsim_hw_stop,
+	.set_promiscuous_mode = hwsim_set_promiscuous_mode,
+};
+
+static int hwsim_add_one(struct genl_info *info, struct device *dev,
+			 bool init);
+static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info)
+{
+	return hwsim_add_one(info, &mac802154hwsim_dev->dev, false);
+}
+
+static void hwsim_del(struct hwsim_phy *phy);
+static int hwsim_del_radio_nl(struct sk_buff *msg, struct genl_info *info)
+{
+	struct hwsim_phy *phy, *tmp;
+	s64 idx = -1;
+
+	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID])
+		return -EINVAL;
+
+	idx = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
+
+	mutex_lock(&hwsim_phys_lock);
+	list_for_each_entry_safe(phy, tmp, &hwsim_phys, list) {
+		if (idx == phy->idx) {
+			hwsim_del(phy);
+			mutex_unlock(&hwsim_phys_lock);
+			return 0;
+		}
+	}
+	mutex_unlock(&hwsim_phys_lock);
+
+	return -ENODEV;
+}
+
+static int append_radio_msg(struct sk_buff *skb, struct hwsim_phy *phy)
+{
+	struct nlattr *nl_edges, *nl_edge;
+	struct hwsim_edge_info *einfo;
+	struct hwsim_edge *e;
+	int ret;
+
+	ret = nla_put_u32(skb, MAC802154_HWSIM_ATTR_RADIO_ID, phy->idx);
+	if (ret < 0)
+		return ret;
+
+	rcu_read_lock();
+	if (list_empty(&phy->edges)) {
+		rcu_read_unlock();
+		return 0;
+	}
+
+	nl_edges = nla_nest_start(skb, MAC802154_HWSIM_ATTR_RADIO_EDGES);
+	if (!nl_edges) {
+		rcu_read_unlock();
+		return -ENOBUFS;
+	}
+
+	list_for_each_entry_rcu(e, &phy->edges, list) {
+		nl_edge = nla_nest_start(skb, MAC802154_HWSIM_ATTR_RADIO_EDGE);
+		if (!nl_edges) {
+			rcu_read_unlock();
+			nla_nest_cancel(skb, nl_edges);
+			return -ENOBUFS;
+		}
+
+		ret = nla_put_u32(skb, MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID,
+				  e->endpoint->idx);
+		if (ret < 0) {
+			rcu_read_unlock();
+			nla_nest_cancel(skb, nl_edge);
+			nla_nest_cancel(skb, nl_edges);
+			return ret;
+		}
+
+		einfo = rcu_dereference(e->info);
+		ret = nla_put_u8(skb, MAC802154_HWSIM_EDGE_ATTR_LQI,
+				 einfo->lqi);
+		if (ret < 0) {
+			rcu_read_unlock();
+			nla_nest_cancel(skb, nl_edge);
+			nla_nest_cancel(skb, nl_edges);
+			return ret;
+		}
+
+		nla_nest_end(skb, nl_edge);
+	}
+	rcu_read_unlock();
+
+	nla_nest_end(skb, nl_edges);
+
+	return 0;
+}
+
+static int hwsim_get_radio(struct sk_buff *skb, struct hwsim_phy *phy,
+			   u32 portid, u32 seq,
+			   struct netlink_callback *cb, int flags)
+{
+	void *hdr;
+	int res = -EMSGSIZE;
+
+	hdr = genlmsg_put(skb, portid, seq, &hwsim_genl_family, flags,
+			  MAC802154_HWSIM_CMD_GET_RADIO);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (cb)
+		genl_dump_check_consistent(cb, hdr);
+
+	res = append_radio_msg(skb, phy);
+	if (res < 0)
+		goto out_err;
+
+	genlmsg_end(skb, hdr);
+	return 0;
+
+out_err:
+	genlmsg_cancel(skb, hdr);
+	return res;
+}
+
+static int hwsim_get_radio_nl(struct sk_buff *msg, struct genl_info *info)
+{
+	struct hwsim_phy *phy;
+	struct sk_buff *skb;
+	int idx, res = -ENODEV;
+
+	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID])
+		return -EINVAL;
+	idx = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
+
+	mutex_lock(&hwsim_phys_lock);
+	list_for_each_entry(phy, &hwsim_phys, list) {
+		if (phy->idx != idx)
+			continue;
+
+		skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+		if (!skb) {
+			res = -ENOMEM;
+			goto out_err;
+		}
+
+		res = hwsim_get_radio(skb, phy, info->snd_portid,
+				      info->snd_seq, NULL, 0);
+		if (res < 0) {
+			nlmsg_free(skb);
+			goto out_err;
+		}
+
+		genlmsg_reply(skb, info);
+		break;
+	}
+
+out_err:
+	mutex_unlock(&hwsim_phys_lock);
+
+	return res;
+}
+
+static int hwsim_dump_radio_nl(struct sk_buff *skb,
+			       struct netlink_callback *cb)
+{
+	int idx = cb->args[0];
+	struct hwsim_phy *phy;
+	int res;
+
+	mutex_lock(&hwsim_phys_lock);
+
+	if (idx == hwsim_radio_idx)
+		goto done;
+
+	list_for_each_entry(phy, &hwsim_phys, list) {
+		if (phy->idx < idx)
+			continue;
+
+		res = hwsim_get_radio(skb, phy, NETLINK_CB(cb->skb).portid,
+				      cb->nlh->nlmsg_seq, cb, NLM_F_MULTI);
+		if (res < 0)
+			break;
+
+		idx = phy->idx + 1;
+	}
+
+	cb->args[0] = idx;
+
+done:
+	mutex_unlock(&hwsim_phys_lock);
+	return skb->len;
+}
+
+/* caller need to held hwsim_phys_lock */
+static struct hwsim_phy *hwsim_get_radio_by_id(uint32_t idx)
+{
+	struct hwsim_phy *phy;
+
+	list_for_each_entry(phy, &hwsim_phys, list) {
+		if (phy->idx == idx)
+			return phy;
+	}
+
+	return NULL;
+}
+
+static const struct nla_policy hwsim_edge_policy[MAC802154_HWSIM_EDGE_ATTR_MAX + 1] = {
+	[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID] = { .type = NLA_U32 },
+	[MAC802154_HWSIM_EDGE_ATTR_LQI] = { .type = NLA_U8 },
+};
+
+static struct hwsim_edge *hwsim_alloc_edge(struct hwsim_phy *endpoint, u8 lqi)
+{
+	struct hwsim_edge_info *einfo;
+	struct hwsim_edge *e;
+
+	e = kzalloc(sizeof(*e), GFP_KERNEL);
+	if (!e)
+		return NULL;
+
+	einfo = kzalloc(sizeof(*einfo), GFP_KERNEL);
+	if (!einfo) {
+		kfree(e);
+		return NULL;
+	}
+
+	einfo->lqi = 0xff;
+	e->info = einfo;
+	e->endpoint = endpoint;
+
+	return e;
+}
+
+static void hwsim_free_edge(struct hwsim_edge *e)
+{
+	kfree_rcu(e->info, rcu);
+	kfree_rcu(e, rcu);
+}
+
+static int hwsim_new_edge_nl(struct sk_buff *msg, struct genl_info *info)
+{
+	struct nlattr *edge_attrs[MAC802154_HWSIM_EDGE_ATTR_MAX + 1];
+	struct hwsim_phy *phy_v0, *phy_v1;
+	struct hwsim_edge *e;
+	u32 v0, v1;
+
+	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID] &&
+	    !info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE])
+		return -EINVAL;
+
+	if (nla_parse_nested(edge_attrs, MAC802154_HWSIM_EDGE_ATTR_MAX,
+			     info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE],
+			     hwsim_edge_policy, NULL))
+		return -EINVAL;
+
+	if (!edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID])
+		return -EINVAL;
+
+	v0 = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
+	v1 = nla_get_u32(edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID]);
+
+	if (v0 == v1)
+		return -EINVAL;
+
+	mutex_lock(&hwsim_phys_lock);
+	phy_v0 = hwsim_get_radio_by_id(v0);
+	if (!phy_v0) {
+		mutex_unlock(&hwsim_phys_lock);
+		return -ENOENT;
+	}
+
+	phy_v1 = hwsim_get_radio_by_id(v1);
+	if (!phy_v1) {
+		mutex_unlock(&hwsim_phys_lock);
+		return -ENOENT;
+	}
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(e, &phy_v0->edges, list) {
+		if (e->endpoint->idx == v1) {
+			mutex_unlock(&hwsim_phys_lock);
+			rcu_read_unlock();
+			return -EEXIST;
+		}
+	}
+	rcu_read_unlock();
+
+	e = hwsim_alloc_edge(phy_v1, 0xff);
+	if (!e) {
+		mutex_unlock(&hwsim_phys_lock);
+		return -ENOMEM;
+	}
+	list_add_rcu(&e->list, &phy_v0->edges);
+	/* wait until changes are done under hwsim_phys_lock lock
+	 * should prevent of calling this function twice while
+	 * edges list has not the changes yet.
+	 */
+	synchronize_rcu();
+	mutex_unlock(&hwsim_phys_lock);
+
+	return 0;
+}
+
+static int hwsim_del_edge_nl(struct sk_buff *msg, struct genl_info *info)
+{
+	struct nlattr *edge_attrs[MAC802154_HWSIM_EDGE_ATTR_MAX + 1];
+	struct hwsim_phy *phy_v0;
+	struct hwsim_edge *e;
+	u32 v0, v1;
+
+	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID] &&
+	    !info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE])
+		return -EINVAL;
+
+	if (nla_parse_nested(edge_attrs, MAC802154_HWSIM_EDGE_ATTR_MAX + 1,
+			     info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE],
+			     hwsim_edge_policy, NULL))
+		return -EINVAL;
+
+	if (!edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID])
+		return -EINVAL;
+
+	v0 = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
+	v1 = nla_get_u32(edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID]);
+
+	mutex_lock(&hwsim_phys_lock);
+	phy_v0 = hwsim_get_radio_by_id(v0);
+	if (!phy_v0) {
+		mutex_unlock(&hwsim_phys_lock);
+		return -ENOENT;
+	}
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(e, &phy_v0->edges, list) {
+		if (e->endpoint->idx == v1) {
+			rcu_read_unlock();
+			list_del_rcu(&e->list);
+			hwsim_free_edge(e);
+			/* same again - wait until list changes are done */
+			synchronize_rcu();
+			mutex_unlock(&hwsim_phys_lock);
+			return 0;
+		}
+	}
+	rcu_read_unlock();
+
+	mutex_unlock(&hwsim_phys_lock);
+
+	return -ENOENT;
+}
+
+static int hwsim_set_edge_lqi(struct sk_buff *msg, struct genl_info *info)
+{
+	struct nlattr *edge_attrs[MAC802154_HWSIM_EDGE_ATTR_MAX + 1];
+	struct hwsim_edge_info *einfo;
+	struct hwsim_phy *phy_v0;
+	struct hwsim_edge *e;
+	u32 v0, v1;
+	u8 lqi;
+
+	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID] &&
+	    !info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE])
+		return -EINVAL;
+
+	if (nla_parse_nested(edge_attrs, MAC802154_HWSIM_EDGE_ATTR_MAX + 1,
+			     info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE],
+			     hwsim_edge_policy, NULL))
+		return -EINVAL;
+
+	if (!edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID] &&
+	    !edge_attrs[MAC802154_HWSIM_EDGE_ATTR_LQI])
+		return -EINVAL;
+
+	v0 = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
+	v1 = nla_get_u32(edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID]);
+	lqi = nla_get_u8(edge_attrs[MAC802154_HWSIM_EDGE_ATTR_LQI]);
+
+	mutex_lock(&hwsim_phys_lock);
+	phy_v0 = hwsim_get_radio_by_id(v0);
+	if (!phy_v0) {
+		mutex_unlock(&hwsim_phys_lock);
+		return -ENOENT;
+	}
+
+	einfo = kzalloc(sizeof(*einfo), GFP_KERNEL);
+	if (!info) {
+		mutex_unlock(&hwsim_phys_lock);
+		return -ENOMEM;
+	}
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(e, &phy_v0->edges, list) {
+		if (e->endpoint->idx == v1) {
+			einfo->lqi = lqi;
+			rcu_assign_pointer(e->info, einfo);
+			rcu_read_unlock();
+			mutex_unlock(&hwsim_phys_lock);
+			return 0;
+		}
+	}
+	rcu_read_unlock();
+
+	kfree(einfo);
+	mutex_unlock(&hwsim_phys_lock);
+
+	return -ENOENT;
+}
+
+/* MAC802154_HWSIM netlink policy */
+
+static const struct nla_policy hwsim_genl_policy[MAC802154_HWSIM_ATTR_MAX + 1] = {
+	[MAC802154_HWSIM_ATTR_RADIO_ID] = { .type = NLA_U32 },
+	[MAC802154_HWSIM_ATTR_RADIO_EDGE] = { .type = NLA_NESTED },
+	[MAC802154_HWSIM_ATTR_RADIO_EDGES] = { .type = NLA_NESTED },
+};
+
+/* Generic Netlink operations array */
+static const struct genl_ops hwsim_nl_ops[] = {
+	{
+		.cmd = MAC802154_HWSIM_CMD_NEW_RADIO,
+		.policy = hwsim_genl_policy,
+		.doit = hwsim_new_radio_nl,
+		.flags = GENL_UNS_ADMIN_PERM,
+	},
+	{
+		.cmd = MAC802154_HWSIM_CMD_DEL_RADIO,
+		.policy = hwsim_genl_policy,
+		.doit = hwsim_del_radio_nl,
+		.flags = GENL_UNS_ADMIN_PERM,
+	},
+	{
+		.cmd = MAC802154_HWSIM_CMD_GET_RADIO,
+		.policy = hwsim_genl_policy,
+		.doit = hwsim_get_radio_nl,
+		.dumpit = hwsim_dump_radio_nl,
+	},
+	{
+		.cmd = MAC802154_HWSIM_CMD_NEW_EDGE,
+		.policy = hwsim_genl_policy,
+		.doit = hwsim_new_edge_nl,
+		.flags = GENL_UNS_ADMIN_PERM,
+	},
+	{
+		.cmd = MAC802154_HWSIM_CMD_DEL_EDGE,
+		.policy = hwsim_genl_policy,
+		.doit = hwsim_del_edge_nl,
+		.flags = GENL_UNS_ADMIN_PERM,
+	},
+	{
+		.cmd = MAC802154_HWSIM_CMD_SET_EDGE,
+		.policy = hwsim_genl_policy,
+		.doit = hwsim_set_edge_lqi,
+		.flags = GENL_UNS_ADMIN_PERM,
+	},
+};
+
+static struct genl_family hwsim_genl_family __ro_after_init = {
+	.name = "MAC802154_HWSIM",
+	.version = 1,
+	.maxattr = MAC802154_HWSIM_ATTR_MAX,
+	.module = THIS_MODULE,
+	.ops = hwsim_nl_ops,
+	.n_ops = ARRAY_SIZE(hwsim_nl_ops),
+	.mcgrps = hwsim_mcgrps,
+	.n_mcgrps = ARRAY_SIZE(hwsim_mcgrps),
+};
+
+static void hwsim_mcast_config_msg(struct sk_buff *mcast_skb,
+				   struct genl_info *info)
+{
+	if (info)
+		genl_notify(&hwsim_genl_family, mcast_skb, info,
+			    HWSIM_MCGRP_CONFIG, GFP_KERNEL);
+	else
+		genlmsg_multicast(&hwsim_genl_family, mcast_skb, 0,
+				  HWSIM_MCGRP_CONFIG, GFP_KERNEL);
+}
+
+static void hwsim_mcast_new_radio(struct genl_info *info, struct hwsim_phy *phy)
+{
+	struct sk_buff *mcast_skb;
+	void *data;
+
+	mcast_skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!mcast_skb)
+		return;
+
+	data = genlmsg_put(mcast_skb, 0, 0, &hwsim_genl_family, 0,
+			   MAC802154_HWSIM_CMD_NEW_RADIO);
+	if (!data)
+		goto out_err;
+
+	if (append_radio_msg(mcast_skb, phy) < 0)
+		goto out_err;
+
+	genlmsg_end(mcast_skb, data);
+
+	hwsim_mcast_config_msg(mcast_skb, info);
+	return;
+
+out_err:
+	genlmsg_cancel(mcast_skb, data);
+	nlmsg_free(mcast_skb);
+}
+
+static void hwsim_edge_unsubscribe_me(struct hwsim_phy *phy)
+{
+	struct hwsim_phy *tmp;
+	struct hwsim_edge *e;
+
+	rcu_read_lock();
+	/* going to all phy edges and remove phy from it */
+	list_for_each_entry(tmp, &hwsim_phys, list) {
+		list_for_each_entry_rcu(e, &tmp->edges, list) {
+			if (e->endpoint->idx == phy->idx) {
+				list_del_rcu(&e->list);
+				hwsim_free_edge(e);
+			}
+		}
+	}
+	rcu_read_unlock();
+}
+
+static int hwsim_subscribe_all_others(struct hwsim_phy *phy)
+{
+	struct hwsim_phy *sub;
+	struct hwsim_edge *e;
+
+	list_for_each_entry(sub, &hwsim_phys, list) {
+		e = hwsim_alloc_edge(sub, 0xff);
+		if (!e)
+			goto me_fail;
+
+		list_add_rcu(&e->list, &phy->edges);
+	}
+
+	list_for_each_entry(sub, &hwsim_phys, list) {
+		e = hwsim_alloc_edge(phy, 0xff);
+		if (!e)
+			goto sub_fail;
+
+		list_add_rcu(&e->list, &sub->edges);
+	}
+
+	return 0;
+
+me_fail:
+	list_for_each_entry(phy, &hwsim_phys, list) {
+		list_del_rcu(&e->list);
+		hwsim_free_edge(e);
+	}
+sub_fail:
+	hwsim_edge_unsubscribe_me(phy);
+	return -ENOMEM;
+}
+
+static int hwsim_add_one(struct genl_info *info, struct device *dev,
+			 bool init)
+{
+	struct ieee802154_hw *hw;
+	struct hwsim_phy *phy;
+	struct hwsim_pib *pib;
+	int idx;
+	int err;
+
+	idx = hwsim_radio_idx++;
+
+	hw = ieee802154_alloc_hw(sizeof(*phy), &hwsim_ops);
+	if (!hw)
+		return -ENOMEM;
+
+	phy = hw->priv;
+	phy->hw = hw;
+
+	/* 868 MHz BPSK	802.15.4-2003 */
+	hw->phy->supported.channels[0] |= 1;
+	/* 915 MHz BPSK	802.15.4-2003 */
+	hw->phy->supported.channels[0] |= 0x7fe;
+	/* 2.4 GHz O-QPSK 802.15.4-2003 */
+	hw->phy->supported.channels[0] |= 0x7FFF800;
+	/* 868 MHz ASK 802.15.4-2006 */
+	hw->phy->supported.channels[1] |= 1;
+	/* 915 MHz ASK 802.15.4-2006 */
+	hw->phy->supported.channels[1] |= 0x7fe;
+	/* 868 MHz O-QPSK 802.15.4-2006 */
+	hw->phy->supported.channels[2] |= 1;
+	/* 915 MHz O-QPSK 802.15.4-2006 */
+	hw->phy->supported.channels[2] |= 0x7fe;
+	/* 2.4 GHz CSS 802.15.4a-2007 */
+	hw->phy->supported.channels[3] |= 0x3fff;
+	/* UWB Sub-gigahertz 802.15.4a-2007 */
+	hw->phy->supported.channels[4] |= 1;
+	/* UWB Low band 802.15.4a-2007 */
+	hw->phy->supported.channels[4] |= 0x1e;
+	/* UWB High band 802.15.4a-2007 */
+	hw->phy->supported.channels[4] |= 0xffe0;
+	/* 750 MHz O-QPSK 802.15.4c-2009 */
+	hw->phy->supported.channels[5] |= 0xf;
+	/* 750 MHz MPSK 802.15.4c-2009 */
+	hw->phy->supported.channels[5] |= 0xf0;
+	/* 950 MHz BPSK 802.15.4d-2009 */
+	hw->phy->supported.channels[6] |= 0x3ff;
+	/* 950 MHz GFSK 802.15.4d-2009 */
+	hw->phy->supported.channels[6] |= 0x3ffc00;
+
+	ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
+
+	/* hwsim phy channel 13 as default */
+	hw->phy->current_channel = 13;
+	pib = kzalloc(sizeof(*pib), GFP_KERNEL);
+	if (!pib) {
+		err = -ENOMEM;
+		goto err_pib;
+	}
+
+	phy->pib = pib;
+	phy->idx = idx;
+	INIT_LIST_HEAD(&phy->edges);
+
+	hw->flags = IEEE802154_HW_PROMISCUOUS;
+	hw->parent = dev;
+
+	err = ieee802154_register_hw(hw);
+	if (err)
+		goto err_reg;
+
+	mutex_lock(&hwsim_phys_lock);
+	if (init) {
+		err = hwsim_subscribe_all_others(phy);
+		if (err < 0)
+			goto err_reg;
+	}
+	list_add_tail(&phy->list, &hwsim_phys);
+	mutex_unlock(&hwsim_phys_lock);
+
+	hwsim_mcast_new_radio(info, phy);
+
+	return idx;
+
+err_reg:
+	kfree(pib);
+err_pib:
+	ieee802154_free_hw(phy->hw);
+	return err;
+}
+
+static void hwsim_del(struct hwsim_phy *phy)
+{
+	hwsim_edge_unsubscribe_me(phy);
+
+	list_del(&phy->list);
+	kfree_rcu(phy->pib, rcu);
+
+	ieee802154_unregister_hw(phy->hw);
+	ieee802154_free_hw(phy->hw);
+}
+
+static int hwsim_probe(struct platform_device *pdev)
+{
+	struct hwsim_phy *phy, *tmp;
+	int err, i;
+
+	for (i = 0; i < radios; i++) {
+		err = hwsim_add_one(NULL, &pdev->dev, true);
+		if (err < 0)
+			goto err_slave;
+	}
+
+	dev_info(&pdev->dev, "Added %i mac802154 hwsim hardware radios\n",
+		 radios);
+	return 0;
+
+err_slave:
+	mutex_lock(&hwsim_phys_lock);
+	list_for_each_entry_safe(phy, tmp, &hwsim_phys, list)
+		hwsim_del(phy);
+	mutex_unlock(&hwsim_phys_lock);
+	return err;
+}
+
+static int hwsim_remove(struct platform_device *pdev)
+{
+	struct hwsim_phy *phy, *tmp;
+
+	mutex_lock(&hwsim_phys_lock);
+	list_for_each_entry_safe(phy, tmp, &hwsim_phys, list)
+		hwsim_del(phy);
+	mutex_unlock(&hwsim_phys_lock);
+
+	return 0;
+}
+
+static struct platform_driver mac802154hwsim_driver = {
+	.probe = hwsim_probe,
+	.remove = hwsim_remove,
+	.driver = {
+			.name = "mac802154_hwsim",
+	},
+};
+
+static __init int hwsim_init_module(void)
+{
+	int rc;
+
+	if (radios < 0 || radios > 100)
+		return -EINVAL;
+
+	rc = genl_register_family(&hwsim_genl_family);
+	if (rc)
+		return rc;
+
+	mac802154hwsim_dev = platform_device_register_simple("mac802154_hwsim",
+							     -1, NULL, 0);
+	if (IS_ERR(mac802154hwsim_dev)) {
+		rc = PTR_ERR(mac802154hwsim_dev);
+		goto platform_dev;
+	}
+
+	rc = platform_driver_register(&mac802154hwsim_driver);
+	if (rc < 0)
+		goto platform_drv;
+
+	return 0;
+
+platform_drv:
+	genl_unregister_family(&hwsim_genl_family);
+platform_dev:
+	platform_device_unregister(mac802154hwsim_dev);
+	return rc;
+}
+
+static __exit void hwsim_remove_module(void)
+{
+	genl_unregister_family(&hwsim_genl_family);
+	platform_driver_unregister(&mac802154hwsim_driver);
+	platform_device_unregister(mac802154hwsim_dev);
+}
+
+module_init(hwsim_init_module);
+module_exit(hwsim_remove_module);
diff --git a/drivers/net/ieee802154/mac802154_hwsim.h b/drivers/net/ieee802154/mac802154_hwsim.h
new file mode 100644
index 000000000000..6009214c0d75
--- /dev/null
+++ b/drivers/net/ieee802154/mac802154_hwsim.h
@@ -0,0 +1,73 @@
+#ifndef __MAC802154_HWSIM_H
+#define __MAC802154_HWSIM_H
+
+/* mac802154 hwsim netlink commands
+ *
+ * @MAC802154_HWSIM_CMD_UNSPEC: unspecified command to catch error
+ * @MAC802154_HWSIM_CMD_GET_RADIO: fetch information about existing radios
+ * @MAC802154_HWSIM_CMD_SEL_RADIO: change radio parameters during runtime
+ * @MAC802154_HWSIM_CMD_NEW_RADIO: create a new radio with the given parameters
+ *	returns the radio ID (>= 0) or negative on errors, if successful
+ *	then multicast the result
+ * @MAC802154_HWSIM_CMD_DEL_RADIO: destroy a radio, reply is multicasted
+ * @MAC802154_HWSIM_CMD_GET_EDGE: fetch information about existing edges
+ * @MAC802154_HWSIM_CMD_SET_EDGE: change edge parameters during runtime
+ * @MAC802154_HWSIM_CMD_DEL_EDGE: delete edges between radios
+ * @MAC802154_HWSIM_CMD_NEW_EDGE: create a new edge between two radios
+ * @__MAC802154_HWSIM_CMD_MAX: enum limit
+ */
+enum {
+	MAC802154_HWSIM_CMD_UNSPEC,
+
+	MAC802154_HWSIM_CMD_GET_RADIO,
+	MAC802154_HWSIM_CMD_SET_RADIO,
+	MAC802154_HWSIM_CMD_NEW_RADIO,
+	MAC802154_HWSIM_CMD_DEL_RADIO,
+
+	MAC802154_HWSIM_CMD_GET_EDGE,
+	MAC802154_HWSIM_CMD_SET_EDGE,
+	MAC802154_HWSIM_CMD_DEL_EDGE,
+	MAC802154_HWSIM_CMD_NEW_EDGE,
+
+	__MAC802154_HWSIM_CMD_MAX,
+};
+
+#define MAC802154_HWSIM_CMD_MAX (__MAC802154_HWSIM_MAX - 1)
+
+/* mac802154 hwsim netlink attributes
+ *
+ * @MAC802154_HWSIM_ATTR_UNSPEC: unspecified attribute to catch error
+ * @MAC802154_HWSIM_ATTR_RADIO_ID: u32 attribute to identify the radio
+ * @MAC802154_HWSIM_ATTR_EDGE: nested attribute of edges
+ * @MAC802154_HWSIM_ATTR_EDGES: list if nested attributes which contains the
+ *	edge information according the radio id
+ * @__MAC802154_HWSIM_ATTR_MAX: enum limit
+ */
+enum {
+	MAC802154_HWSIM_ATTR_UNSPEC,
+	MAC802154_HWSIM_ATTR_RADIO_ID,
+	MAC802154_HWSIM_ATTR_RADIO_EDGE,
+	MAC802154_HWSIM_ATTR_RADIO_EDGES,
+	__MAC802154_HWSIM_ATTR_MAX,
+};
+
+#define MAC802154_HWSIM_ATTR_MAX (__MAC802154_HWSIM_ATTR_MAX - 1)
+
+/* mac802154 hwsim edge netlink attributes
+ *
+ * @MAC802154_HWSIM_EDGE_ATTR_UNSPEC: unspecified attribute to catch error
+ * @MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID: radio id where the edge points to
+ * @MAC802154_HWSIM_EDGE_ATTR_LQI: LQI value which the endpoint radio will
+ *	receive for this edge
+ * @__MAC802154_HWSIM_ATTR_MAX: enum limit
+ */
+enum {
+	MAC802154_HWSIM_EDGE_ATTR_UNSPEC,
+	MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID,
+	MAC802154_HWSIM_EDGE_ATTR_LQI,
+	__MAC802154_HWSIM_EDGE_ATTR_MAX,
+};
+
+#define MAC802154_HWSIM_EDGE_ATTR_MAX (__MAC802154_HWSIM_EDGE_ATTR_MAX - 1)
+
+#endif /* __MAC802154_HWSIM_H */
-- 
2.11.0

Re: [RFC PATCH wpan-next 1/2] ieee802154: hwsim: add replacement for fakelb

[Stefan Schmidt]

Hello.


On 04/27/2018 11:21 PM, Alexander Aring wrote:
> This patch adds a new virtual driver mac802154_hwsim which is based on
> the fakelb driver.
> The fakelb driver will get deprecated and hopefully removed someday.
> The main reason for doing this step is to rename the driver to
> mac802154_hwsim to have a similar naming scheme as mac80211_hwsim,
> which is more popular in the 802.11 wireless word and the idea is the
> same behind this driver.
>
> The new features of this driver are to have knowledge about connected
> edges, which can be changed during runtime. This offers a testing
> environment for routing protocols e.g. RPL.
> The default behaviour is still as fakelb: two radios connected to each
> other. New added radios during runtime will not be connected to other
> wpan_hwsim instances.
>
> The netlink api is not namespace aware on purpose, only the registered
> wpan_phy's can be moved to namespaces. The physical layer according to
> wiresless "air" communication can be handled across namespaces.
>
> Furthermore the edges can be weighted with the LQI value according IEEE
> 802.15.4 which offers additional handling to mark bad or good connection
> indicators to other connected virtual phys.
>
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>  drivers/net/ieee802154/Kconfig           |  11 +
>  drivers/net/ieee802154/Makefile          |   1 +
>  drivers/net/ieee802154/mac802154_hwsim.c | 925 +++++++++++++++++++++++++++++++
>  drivers/net/ieee802154/mac802154_hwsim.h |  73 +++
>  4 files changed, 1010 insertions(+)
>  create mode 100644 drivers/net/ieee802154/mac802154_hwsim.c
>  create mode 100644 drivers/net/ieee802154/mac802154_hwsim.h
>
> diff --git a/drivers/net/ieee802154/Kconfig b/drivers/net/ieee802154/Kconfig
> index 8782f5655e3f..ff3552044679 100644
> --- a/drivers/net/ieee802154/Kconfig
> +++ b/drivers/net/ieee802154/Kconfig
> @@ -115,3 +115,14 @@ config IEEE802154_MCR20A
>  
>  	  This driver can also be built as a module. To do so, say M here.
>  	  the module will be called 'mcr20a'.
> +
> +config MAC802154_HWSIM
> +	depends on IEEE802154_DRIVERS && MAC802154
> +	tristate "Simulated radio testing tool for mac802154"
> +	---help---
> +	  This driver is a developer testing tool that can be used to test
> +	  IEEE 802.15.4 networking stack (mac802154) functionality. This is not
> +	  needed for normal wireless LAN usage and is only for testing.

The wireless LAN wording seems to be a left over :-)


> +	  This driver can also be built as a module. To do so say M here.
> +	  The module will be called 'mac802154_hwsim'.
> diff --git a/drivers/net/ieee802154/Makefile b/drivers/net/ieee802154/Makefile
> index 104744d5a668..3b17aa17d46a 100644
> --- a/drivers/net/ieee802154/Makefile
> +++ b/drivers/net/ieee802154/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_IEEE802154_ATUSB) += atusb.o
>  obj-$(CONFIG_IEEE802154_ADF7242) += adf7242.o
>  obj-$(CONFIG_IEEE802154_CA8210) += ca8210.o
>  obj-$(CONFIG_IEEE802154_MCR20A) += mcr20a.o
> +obj-$(CONFIG_MAC802154_HWSIM) += mac802154_hwsim.o
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> new file mode 100644
> index 000000000000..eea0d355bbd8
> --- /dev/null
> +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> @@ -0,0 +1,925 @@
> +/*
> + * HWSIM IEEE 802.15.4 interface
> + *
> + * (C) 2018 Mojatau, Alexander Aring <aring@mojatau.com>
> + * Copyright 2007-2012 Siemens AG
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.

Not only related to this patch but we might want to think about moving from the license boiler plate text to SPDX for the ieee802154
subsystem at some point.

> + * Based on fakelb, original Written by:
> + * Sergey Lapin <slapin@ossfans.org>
> + * Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> + * Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/timer.h>
> +#include <linux/platform_device.h>
> +#include <linux/netdevice.h>
> +#include <linux/device.h>
> +#include <linux/spinlock.h>
> +#include <net/mac802154.h>
> +#include <net/cfg802154.h>
> +#include <net/genetlink.h>
> +#include "mac802154_hwsim.h"
> +
> +MODULE_DESCRIPTION("Software simulator of 802.15.4 radio(s) for mac802154");

"IEEE 802.15.4", please.

> +MODULE_LICENSE("GPL");

The license text does not have the "or any later version" part which means we should indicate it really is GPL v2 only.

> +
> +/* Number of dummy devices to be set up by this module. */
> +static int radios = 2;

Does this really need to be a global variable?

> +module_param(radios, int, 0444);
> +MODULE_PARM_DESC(radios, "Number of simulated radios");

What happens if more than 2 devices are given as module parameter, are only the first two connected and the rest unconnected?


> +static LIST_HEAD(hwsim_phys);
> +static DEFINE_MUTEX(hwsim_phys_lock);
> +
> +static __rcu LIST_HEAD(hwsim_ifup_phys);
> +
> +static struct platform_device *mac802154hwsim_dev;
> +
> +/* MAC802154_HWSIM netlink family */
> +static struct genl_family hwsim_genl_family;
> +
> +static int hwsim_radio_idx;

Another global variable I would like to avoid, if it make sense.

> +
> +enum hwsim_multicast_groups {
> +	HWSIM_MCGRP_CONFIG,
> +};
> +
> +static const struct genl_multicast_group hwsim_mcgrps[] = {
> +	[HWSIM_MCGRP_CONFIG] = { .name = "config", },
> +};
> +
> +struct hwsim_pib {
> +	u8 page;
> +	u8 channel;
> +
> +	struct rcu_head rcu;
> +};
> +
> +struct hwsim_edge_info {
> +	u8 lqi;
> +
> +	struct rcu_head rcu;
> +};
> +
> +struct hwsim_edge {
> +	struct hwsim_phy *endpoint;
> +	struct hwsim_edge_info *info;
> +
> +	struct list_head list;
> +	struct rcu_head rcu;
> +};
> +
> +struct hwsim_phy {
> +	struct ieee802154_hw *hw;
> +	u32 idx;
> +
> +	struct hwsim_pib __rcu *pib;
> +
> +	bool suspended;
> +	struct list_head __rcu edges;
> +
> +	struct list_head list;
> +	struct list_head list_ifup;
> +};
> +
> +static int hwsim_hw_ed(struct ieee802154_hw *hw, u8 *level)
> +{
> +	*level = 0xbe;
> +
> +	return 0;
> +}
> +
> +static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
> +{
> +	struct hwsim_phy *phy = hw->priv;
> +	struct hwsim_pib *pib, *pib_old;
> +
> +	pib = kzalloc(sizeof(*pib), GFP_KERNEL);
> +	if (!pib)
> +		return -ENOMEM;
> +
> +	pib->page = page;
> +	pib->channel = channel;
> +
> +	pib_old = phy->pib;
> +	rcu_assign_pointer(phy->pib, pib);
> +	kfree_rcu(pib_old, rcu);
> +	return 0;
> +}
> +
> +static int hwsim_hw_xmit(struct ieee802154_hw *hw, struct sk_buff *skb)
> +{
> +	struct hwsim_phy *current_phy = hw->priv;
> +	struct hwsim_pib *current_pib, *endpoint_pib;
> +	struct hwsim_edge_info *einfo;
> +	struct hwsim_edge *e;
> +
> +	WARN_ON(current_phy->suspended);
> +
> +	rcu_read_lock();
> +	current_pib = rcu_dereference(current_phy->pib);
> +	list_for_each_entry_rcu(e, &current_phy->edges, list) {
> +		/* Can be changed later in rx_irqsafe, but this is only a
> +		 * performance tweak. Received radio should drop the frame
> +		 * in mac802154 stack anyway... so we don't need to be
> +		 * 100% of locking here to check on suspended
> +		 */
> +		if (e->endpoint->suspended)
> +			continue;
> +
> +		endpoint_pib = rcu_dereference(e->endpoint->pib);
> +		if (current_pib->page == endpoint_pib->page &&
> +		    current_pib->channel == endpoint_pib->channel) {
> +			struct sk_buff *newskb = pskb_copy(skb, GFP_ATOMIC);
> +
> +			einfo = rcu_dereference(e->info);
> +			if (newskb)
> +				ieee802154_rx_irqsafe(e->endpoint->hw, newskb,
> +						      einfo->lqi);
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	ieee802154_xmit_complete(hw, skb, false);
> +	return 0;
> +}
> +
> +static int hwsim_hw_start(struct ieee802154_hw *hw)
> +{
> +	struct hwsim_phy *phy = hw->priv;
> +
> +	phy->suspended = false;
> +	list_add_rcu(&phy->list_ifup, &hwsim_ifup_phys);
> +	synchronize_rcu();
> +
> +	return 0;
> +}
> +
> +static void hwsim_hw_stop(struct ieee802154_hw *hw)
> +{
> +	struct hwsim_phy *phy = hw->priv;
> +
> +	phy->suspended = true;
> +	list_del_rcu(&phy->list_ifup);
> +	synchronize_rcu();
> +}
> +
> +static int
> +hwsim_set_promiscuous_mode(struct ieee802154_hw *hw, const bool on)
> +{
> +	return 0;
> +}
> +
> +static const struct ieee802154_ops hwsim_ops = {
> +	.owner = THIS_MODULE,
> +	.xmit_async = hwsim_hw_xmit,
> +	.ed = hwsim_hw_ed,
> +	.set_channel = hwsim_hw_channel,
> +	.start = hwsim_hw_start,
> +	.stop = hwsim_hw_stop,
> +	.set_promiscuous_mode = hwsim_set_promiscuous_mode,
> +};
> +
> +static int hwsim_add_one(struct genl_info *info, struct device *dev,
> +			 bool init);

Please re-order the functions so we can avoid forward declarations.

> +static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info)
> +{
> +	return hwsim_add_one(info, &mac802154hwsim_dev->dev, false);
> +}
> +
> +static void hwsim_del(struct hwsim_phy *phy);

Another forward declaration to avoid.

> +static int hwsim_del_radio_nl(struct sk_buff *msg, struct genl_info *info)
> +{
> +	struct hwsim_phy *phy, *tmp;
> +	s64 idx = -1;
> +
> +	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID])
> +		return -EINVAL;
> +
> +	idx = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
> +
> +	mutex_lock(&hwsim_phys_lock);
> +	list_for_each_entry_safe(phy, tmp, &hwsim_phys, list) {
> +		if (idx == phy->idx) {
> +			hwsim_del(phy);
> +			mutex_unlock(&hwsim_phys_lock);
> +			return 0;
> +		}
> +	}
> +	mutex_unlock(&hwsim_phys_lock);
> +
> +	return -ENODEV;
> +}
> +
> +static int append_radio_msg(struct sk_buff *skb, struct hwsim_phy *phy)
> +{
> +	struct nlattr *nl_edges, *nl_edge;
> +	struct hwsim_edge_info *einfo;
> +	struct hwsim_edge *e;
> +	int ret;
> +
> +	ret = nla_put_u32(skb, MAC802154_HWSIM_ATTR_RADIO_ID, phy->idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	rcu_read_lock();
> +	if (list_empty(&phy->edges)) {
> +		rcu_read_unlock();
> +		return 0;
> +	}
> +
> +	nl_edges = nla_nest_start(skb, MAC802154_HWSIM_ATTR_RADIO_EDGES);
> +	if (!nl_edges) {
> +		rcu_read_unlock();
> +		return -ENOBUFS;
> +	}
> +
> +	list_for_each_entry_rcu(e, &phy->edges, list) {
> +		nl_edge = nla_nest_start(skb, MAC802154_HWSIM_ATTR_RADIO_EDGE);
> +		if (!nl_edges) {

You save the return of bla_nest_start in nl_edge but you are checking on nl_edges.

> +			rcu_read_unlock();
> +			nla_nest_cancel(skb, nl_edges);
> +			return -ENOBUFS;
> +		}
> +
> +		ret = nla_put_u32(skb, MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID,
> +				  e->endpoint->idx);
> +		if (ret < 0) {
> +			rcu_read_unlock();
> +			nla_nest_cancel(skb, nl_edge);
> +			nla_nest_cancel(skb, nl_edges);
> +			return ret;
> +		}
> +
> +		einfo = rcu_dereference(e->info);
> +		ret = nla_put_u8(skb, MAC802154_HWSIM_EDGE_ATTR_LQI,
> +				 einfo->lqi);
> +		if (ret < 0) {
> +			rcu_read_unlock();
> +			nla_nest_cancel(skb, nl_edge);
> +			nla_nest_cancel(skb, nl_edges);
> +			return ret;
> +		}
> +
> +		nla_nest_end(skb, nl_edge);
> +	}
> +	rcu_read_unlock();
> +
> +	nla_nest_end(skb, nl_edges);
> +
> +	return 0;
> +}
> +
> +static int hwsim_get_radio(struct sk_buff *skb, struct hwsim_phy *phy,
> +			   u32 portid, u32 seq,
> +			   struct netlink_callback *cb, int flags)
> +{
> +	void *hdr;
> +	int res = -EMSGSIZE;
> +
> +	hdr = genlmsg_put(skb, portid, seq, &hwsim_genl_family, flags,
> +			  MAC802154_HWSIM_CMD_GET_RADIO);
> +	if (!hdr)
> +		return -EMSGSIZE;
> +
> +	if (cb)
> +		genl_dump_check_consistent(cb, hdr);
> +
> +	res = append_radio_msg(skb, phy);
> +	if (res < 0)
> +		goto out_err;
> +
> +	genlmsg_end(skb, hdr);
> +	return 0;
> +
> +out_err:
> +	genlmsg_cancel(skb, hdr);
> +	return res;
> +}
> +
> +static int hwsim_get_radio_nl(struct sk_buff *msg, struct genl_info *info)
> +{
> +	struct hwsim_phy *phy;
> +	struct sk_buff *skb;
> +	int idx, res = -ENODEV;
> +
> +	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID])
> +		return -EINVAL;
> +	idx = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
> +
> +	mutex_lock(&hwsim_phys_lock);
> +	list_for_each_entry(phy, &hwsim_phys, list) {
> +		if (phy->idx != idx)
> +			continue;
> +
> +		skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +		if (!skb) {
> +			res = -ENOMEM;
> +			goto out_err;
> +		}
> +
> +		res = hwsim_get_radio(skb, phy, info->snd_portid,
> +				      info->snd_seq, NULL, 0);
> +		if (res < 0) {
> +			nlmsg_free(skb);
> +			goto out_err;
> +		}
> +
> +		genlmsg_reply(skb, info);
> +		break;
> +	}
> +
> +out_err:
> +	mutex_unlock(&hwsim_phys_lock);
> +
> +	return res;
> +}
> +
> +static int hwsim_dump_radio_nl(struct sk_buff *skb,
> +			       struct netlink_callback *cb)
> +{
> +	int idx = cb->args[0];
> +	struct hwsim_phy *phy;
> +	int res;
> +
> +	mutex_lock(&hwsim_phys_lock);
> +
> +	if (idx == hwsim_radio_idx)
> +		goto done;
> +
> +	list_for_each_entry(phy, &hwsim_phys, list) {
> +		if (phy->idx < idx)
> +			continue;
> +
> +		res = hwsim_get_radio(skb, phy, NETLINK_CB(cb->skb).portid,
> +				      cb->nlh->nlmsg_seq, cb, NLM_F_MULTI);
> +		if (res < 0)
> +			break;
> +
> +		idx = phy->idx + 1;
> +	}
> +
> +	cb->args[0] = idx;
> +
> +done:
> +	mutex_unlock(&hwsim_phys_lock);
> +	return skb->len;
> +}
> +
> +/* caller need to held hwsim_phys_lock */
> +static struct hwsim_phy *hwsim_get_radio_by_id(uint32_t idx)
> +{
> +	struct hwsim_phy *phy;
> +
> +	list_for_each_entry(phy, &hwsim_phys, list) {
> +		if (phy->idx == idx)
> +			return phy;
> +	}
> +
> +	return NULL;
> +}
> +
> +static const struct nla_policy hwsim_edge_policy[MAC802154_HWSIM_EDGE_ATTR_MAX + 1] = {
> +	[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID] = { .type = NLA_U32 },
> +	[MAC802154_HWSIM_EDGE_ATTR_LQI] = { .type = NLA_U8 },
> +};
> +
> +static struct hwsim_edge *hwsim_alloc_edge(struct hwsim_phy *endpoint, u8 lqi)
> +{
> +	struct hwsim_edge_info *einfo;
> +	struct hwsim_edge *e;
> +
> +	e = kzalloc(sizeof(*e), GFP_KERNEL);
> +	if (!e)
> +		return NULL;
> +
> +	einfo = kzalloc(sizeof(*einfo), GFP_KERNEL);
> +	if (!einfo) {
> +		kfree(e);
> +		return NULL;
> +	}
> +
> +	einfo->lqi = 0xff;
> +	e->info = einfo;
> +	e->endpoint = endpoint;
> +
> +	return e;
> +}
> +
> +static void hwsim_free_edge(struct hwsim_edge *e)
> +{
> +	kfree_rcu(e->info, rcu);
> +	kfree_rcu(e, rcu);
> +}
> +
> +static int hwsim_new_edge_nl(struct sk_buff *msg, struct genl_info *info)
> +{
> +	struct nlattr *edge_attrs[MAC802154_HWSIM_EDGE_ATTR_MAX + 1];
> +	struct hwsim_phy *phy_v0, *phy_v1;
> +	struct hwsim_edge *e;
> +	u32 v0, v1;
> +
> +	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID] &&
> +	    !info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE])
> +		return -EINVAL;
> +
> +	if (nla_parse_nested(edge_attrs, MAC802154_HWSIM_EDGE_ATTR_MAX,
> +			     info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE],
> +			     hwsim_edge_policy, NULL))
> +		return -EINVAL;
> +
> +	if (!edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID])
> +		return -EINVAL;
> +
> +	v0 = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
> +	v1 = nla_get_u32(edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID]);
> +
> +	if (v0 == v1)
> +		return -EINVAL;
> +
> +	mutex_lock(&hwsim_phys_lock);
> +	phy_v0 = hwsim_get_radio_by_id(v0);
> +	if (!phy_v0) {
> +		mutex_unlock(&hwsim_phys_lock);
> +		return -ENOENT;
> +	}
> +
> +	phy_v1 = hwsim_get_radio_by_id(v1);
> +	if (!phy_v1) {
> +		mutex_unlock(&hwsim_phys_lock);
> +		return -ENOENT;
> +	}
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(e, &phy_v0->edges, list) {
> +		if (e->endpoint->idx == v1) {
> +			mutex_unlock(&hwsim_phys_lock);
> +			rcu_read_unlock();
> +			return -EEXIST;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	e = hwsim_alloc_edge(phy_v1, 0xff);
> +	if (!e) {
> +		mutex_unlock(&hwsim_phys_lock);
> +		return -ENOMEM;
> +	}
> +	list_add_rcu(&e->list, &phy_v0->edges);
> +	/* wait until changes are done under hwsim_phys_lock lock
> +	 * should prevent of calling this function twice while
> +	 * edges list has not the changes yet.
> +	 */
> +	synchronize_rcu();
> +	mutex_unlock(&hwsim_phys_lock);
> +
> +	return 0;
> +}
> +
> +static int hwsim_del_edge_nl(struct sk_buff *msg, struct genl_info *info)
> +{
> +	struct nlattr *edge_attrs[MAC802154_HWSIM_EDGE_ATTR_MAX + 1];
> +	struct hwsim_phy *phy_v0;
> +	struct hwsim_edge *e;
> +	u32 v0, v1;
> +
> +	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID] &&
> +	    !info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE])
> +		return -EINVAL;
> +
> +	if (nla_parse_nested(edge_attrs, MAC802154_HWSIM_EDGE_ATTR_MAX + 1,
> +			     info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE],
> +			     hwsim_edge_policy, NULL))
> +		return -EINVAL;
> +
> +	if (!edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID])
> +		return -EINVAL;
> +
> +	v0 = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
> +	v1 = nla_get_u32(edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID]);
> +
> +	mutex_lock(&hwsim_phys_lock);
> +	phy_v0 = hwsim_get_radio_by_id(v0);
> +	if (!phy_v0) {
> +		mutex_unlock(&hwsim_phys_lock);
> +		return -ENOENT;
> +	}
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(e, &phy_v0->edges, list) {
> +		if (e->endpoint->idx == v1) {
> +			rcu_read_unlock();
> +			list_del_rcu(&e->list);
> +			hwsim_free_edge(e);
> +			/* same again - wait until list changes are done */
> +			synchronize_rcu();
> +			mutex_unlock(&hwsim_phys_lock);
> +			return 0;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	mutex_unlock(&hwsim_phys_lock);
> +
> +	return -ENOENT;
> +}
> +
> +static int hwsim_set_edge_lqi(struct sk_buff *msg, struct genl_info *info)
> +{
> +	struct nlattr *edge_attrs[MAC802154_HWSIM_EDGE_ATTR_MAX + 1];
> +	struct hwsim_edge_info *einfo;
> +	struct hwsim_phy *phy_v0;
> +	struct hwsim_edge *e;
> +	u32 v0, v1;
> +	u8 lqi;
> +
> +	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID] &&
> +	    !info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE])
> +		return -EINVAL;
> +
> +	if (nla_parse_nested(edge_attrs, MAC802154_HWSIM_EDGE_ATTR_MAX + 1,
> +			     info->attrs[MAC802154_HWSIM_ATTR_RADIO_EDGE],
> +			     hwsim_edge_policy, NULL))
> +		return -EINVAL;
> +
> +	if (!edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID] &&
> +	    !edge_attrs[MAC802154_HWSIM_EDGE_ATTR_LQI])
> +		return -EINVAL;
> +
> +	v0 = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
> +	v1 = nla_get_u32(edge_attrs[MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID]);
> +	lqi = nla_get_u8(edge_attrs[MAC802154_HWSIM_EDGE_ATTR_LQI]);
> +
> +	mutex_lock(&hwsim_phys_lock);
> +	phy_v0 = hwsim_get_radio_by_id(v0);
> +	if (!phy_v0) {
> +		mutex_unlock(&hwsim_phys_lock);
> +		return -ENOENT;
> +	}
> +
> +	einfo = kzalloc(sizeof(*einfo), GFP_KERNEL);
> +	if (!info) {
> +		mutex_unlock(&hwsim_phys_lock);
> +		return -ENOMEM;
> +	}
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(e, &phy_v0->edges, list) {
> +		if (e->endpoint->idx == v1) {
> +			einfo->lqi = lqi;
> +			rcu_assign_pointer(e->info, einfo);
> +			rcu_read_unlock();
> +			mutex_unlock(&hwsim_phys_lock);
> +			return 0;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	kfree(einfo);
> +	mutex_unlock(&hwsim_phys_lock);
> +
> +	return -ENOENT;
> +}
> +
> +/* MAC802154_HWSIM netlink policy */
> +
> +static const struct nla_policy hwsim_genl_policy[MAC802154_HWSIM_ATTR_MAX + 1] = {
> +	[MAC802154_HWSIM_ATTR_RADIO_ID] = { .type = NLA_U32 },
> +	[MAC802154_HWSIM_ATTR_RADIO_EDGE] = { .type = NLA_NESTED },
> +	[MAC802154_HWSIM_ATTR_RADIO_EDGES] = { .type = NLA_NESTED },
> +};
> +
> +/* Generic Netlink operations array */
> +static const struct genl_ops hwsim_nl_ops[] = {
> +	{
> +		.cmd = MAC802154_HWSIM_CMD_NEW_RADIO,
> +		.policy = hwsim_genl_policy,
> +		.doit = hwsim_new_radio_nl,
> +		.flags = GENL_UNS_ADMIN_PERM,
> +	},
> +	{
> +		.cmd = MAC802154_HWSIM_CMD_DEL_RADIO,
> +		.policy = hwsim_genl_policy,
> +		.doit = hwsim_del_radio_nl,
> +		.flags = GENL_UNS_ADMIN_PERM,
> +	},
> +	{
> +		.cmd = MAC802154_HWSIM_CMD_GET_RADIO,
> +		.policy = hwsim_genl_policy,
> +		.doit = hwsim_get_radio_nl,
> +		.dumpit = hwsim_dump_radio_nl,
> +	},
> +	{
> +		.cmd = MAC802154_HWSIM_CMD_NEW_EDGE,
> +		.policy = hwsim_genl_policy,
> +		.doit = hwsim_new_edge_nl,
> +		.flags = GENL_UNS_ADMIN_PERM,
> +	},
> +	{
> +		.cmd = MAC802154_HWSIM_CMD_DEL_EDGE,
> +		.policy = hwsim_genl_policy,
> +		.doit = hwsim_del_edge_nl,
> +		.flags = GENL_UNS_ADMIN_PERM,
> +	},
> +	{
> +		.cmd = MAC802154_HWSIM_CMD_SET_EDGE,
> +		.policy = hwsim_genl_policy,
> +		.doit = hwsim_set_edge_lqi,
> +		.flags = GENL_UNS_ADMIN_PERM,
> +	},
> +};
> +

This answers my questions on why MAC802154_HWSIM_CMD_SET_RADIO and MAC802154_HWSIM_CMD_GET_EDGE are not used in wpan-hwsim.
The kernel does not really expose them. If you think they are not needed I would say we need to remove these enum types from the header below.
> +static struct genl_family hwsim_genl_family __ro_after_init = {
> +	.name = "MAC802154_HWSIM",
> +	.version = 1,
> +	.maxattr = MAC802154_HWSIM_ATTR_MAX,
> +	.module = THIS_MODULE,
> +	.ops = hwsim_nl_ops,
> +	.n_ops = ARRAY_SIZE(hwsim_nl_ops),
> +	.mcgrps = hwsim_mcgrps,
> +	.n_mcgrps = ARRAY_SIZE(hwsim_mcgrps),
> +};
> +
> +static void hwsim_mcast_config_msg(struct sk_buff *mcast_skb,
> +				   struct genl_info *info)
> +{
> +	if (info)
> +		genl_notify(&hwsim_genl_family, mcast_skb, info,
> +			    HWSIM_MCGRP_CONFIG, GFP_KERNEL);
> +	else
> +		genlmsg_multicast(&hwsim_genl_family, mcast_skb, 0,
> +				  HWSIM_MCGRP_CONFIG, GFP_KERNEL);
> +}
> +
> +static void hwsim_mcast_new_radio(struct genl_info *info, struct hwsim_phy *phy)
> +{
> +	struct sk_buff *mcast_skb;
> +	void *data;
> +
> +	mcast_skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +	if (!mcast_skb)
> +		return;
> +
> +	data = genlmsg_put(mcast_skb, 0, 0, &hwsim_genl_family, 0,
> +			   MAC802154_HWSIM_CMD_NEW_RADIO);
> +	if (!data)
> +		goto out_err;
> +
> +	if (append_radio_msg(mcast_skb, phy) < 0)
> +		goto out_err;
> +
> +	genlmsg_end(mcast_skb, data);
> +
> +	hwsim_mcast_config_msg(mcast_skb, info);
> +	return;
> +
> +out_err:
> +	genlmsg_cancel(mcast_skb, data);
> +	nlmsg_free(mcast_skb);
> +}
> +
> +static void hwsim_edge_unsubscribe_me(struct hwsim_phy *phy)
> +{
> +	struct hwsim_phy *tmp;
> +	struct hwsim_edge *e;
> +
> +	rcu_read_lock();
> +	/* going to all phy edges and remove phy from it */
> +	list_for_each_entry(tmp, &hwsim_phys, list) {
> +		list_for_each_entry_rcu(e, &tmp->edges, list) {
> +			if (e->endpoint->idx == phy->idx) {
> +				list_del_rcu(&e->list);
> +				hwsim_free_edge(e);
> +			}
> +		}
> +	}
> +	rcu_read_unlock();
> +}
> +
> +static int hwsim_subscribe_all_others(struct hwsim_phy *phy)
> +{
> +	struct hwsim_phy *sub;
> +	struct hwsim_edge *e;
> +
> +	list_for_each_entry(sub, &hwsim_phys, list) {
> +		e = hwsim_alloc_edge(sub, 0xff);
> +		if (!e)
> +			goto me_fail;
> +
> +		list_add_rcu(&e->list, &phy->edges);
> +	}
> +
> +	list_for_each_entry(sub, &hwsim_phys, list) {
> +		e = hwsim_alloc_edge(phy, 0xff);
> +		if (!e)
> +			goto sub_fail;
> +
> +		list_add_rcu(&e->list, &sub->edges);
> +	}
> +
> +	return 0;
> +
> +me_fail:
> +	list_for_each_entry(phy, &hwsim_phys, list) {
> +		list_del_rcu(&e->list);
> +		hwsim_free_edge(e);
> +	}
> +sub_fail:
> +	hwsim_edge_unsubscribe_me(phy);
> +	return -ENOMEM;
> +}
> +
> +static int hwsim_add_one(struct genl_info *info, struct device *dev,
> +			 bool init)
> +{
> +	struct ieee802154_hw *hw;
> +	struct hwsim_phy *phy;
> +	struct hwsim_pib *pib;
> +	int idx;
> +	int err;
> +
> +	idx = hwsim_radio_idx++;
> +
> +	hw = ieee802154_alloc_hw(sizeof(*phy), &hwsim_ops);
> +	if (!hw)
> +		return -ENOMEM;
> +
> +	phy = hw->priv;
> +	phy->hw = hw;
> +
> +	/* 868 MHz BPSK	802.15.4-2003 */
> +	hw->phy->supported.channels[0] |= 1;
> +	/* 915 MHz BPSK	802.15.4-2003 */
> +	hw->phy->supported.channels[0] |= 0x7fe;
> +	/* 2.4 GHz O-QPSK 802.15.4-2003 */
> +	hw->phy->supported.channels[0] |= 0x7FFF800;
> +	/* 868 MHz ASK 802.15.4-2006 */
> +	hw->phy->supported.channels[1] |= 1;
> +	/* 915 MHz ASK 802.15.4-2006 */
> +	hw->phy->supported.channels[1] |= 0x7fe;
> +	/* 868 MHz O-QPSK 802.15.4-2006 */
> +	hw->phy->supported.channels[2] |= 1;
> +	/* 915 MHz O-QPSK 802.15.4-2006 */
> +	hw->phy->supported.channels[2] |= 0x7fe;
> +	/* 2.4 GHz CSS 802.15.4a-2007 */
> +	hw->phy->supported.channels[3] |= 0x3fff;
> +	/* UWB Sub-gigahertz 802.15.4a-2007 */
> +	hw->phy->supported.channels[4] |= 1;
> +	/* UWB Low band 802.15.4a-2007 */
> +	hw->phy->supported.channels[4] |= 0x1e;
> +	/* UWB High band 802.15.4a-2007 */
> +	hw->phy->supported.channels[4] |= 0xffe0;
> +	/* 750 MHz O-QPSK 802.15.4c-2009 */
> +	hw->phy->supported.channels[5] |= 0xf;
> +	/* 750 MHz MPSK 802.15.4c-2009 */
> +	hw->phy->supported.channels[5] |= 0xf0;
> +	/* 950 MHz BPSK 802.15.4d-2009 */
> +	hw->phy->supported.channels[6] |= 0x3ff;
> +	/* 950 MHz GFSK 802.15.4d-2009 */
> +	hw->phy->supported.channels[6] |= 0x3ffc00;
> +
> +	ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
> +
> +	/* hwsim phy channel 13 as default */
> +	hw->phy->current_channel = 13;
> +	pib = kzalloc(sizeof(*pib), GFP_KERNEL);
> +	if (!pib) {
> +		err = -ENOMEM;
> +		goto err_pib;
> +	}
> +
> +	phy->pib = pib;
> +	phy->idx = idx;
> +	INIT_LIST_HEAD(&phy->edges);
> +
> +	hw->flags = IEEE802154_HW_PROMISCUOUS;
> +	hw->parent = dev;
> +
> +	err = ieee802154_register_hw(hw);
> +	if (err)
> +		goto err_reg;
> +
> +	mutex_lock(&hwsim_phys_lock);
> +	if (init) {
> +		err = hwsim_subscribe_all_others(phy);
> +		if (err < 0)
> +			goto err_reg;
> +	}
> +	list_add_tail(&phy->list, &hwsim_phys);
> +	mutex_unlock(&hwsim_phys_lock);
> +
> +	hwsim_mcast_new_radio(info, phy);
> +
> +	return idx;
> +
> +err_reg:
> +	kfree(pib);
> +err_pib:
> +	ieee802154_free_hw(phy->hw);
> +	return err;
> +}
> +
> +static void hwsim_del(struct hwsim_phy *phy)
> +{
> +	hwsim_edge_unsubscribe_me(phy);
> +
> +	list_del(&phy->list);
> +	kfree_rcu(phy->pib, rcu);
> +
> +	ieee802154_unregister_hw(phy->hw);
> +	ieee802154_free_hw(phy->hw);
> +}
> +
> +static int hwsim_probe(struct platform_device *pdev)
> +{
> +	struct hwsim_phy *phy, *tmp;
> +	int err, i;
> +
> +	for (i = 0; i < radios; i++) {
> +		err = hwsim_add_one(NULL, &pdev->dev, true);
> +		if (err < 0)
> +			goto err_slave;
> +	}
> +
> +	dev_info(&pdev->dev, "Added %i mac802154 hwsim hardware radios\n",
> +		 radios);
> +	return 0;
> +
> +err_slave:
> +	mutex_lock(&hwsim_phys_lock);
> +	list_for_each_entry_safe(phy, tmp, &hwsim_phys, list)
> +		hwsim_del(phy);
> +	mutex_unlock(&hwsim_phys_lock);
> +	return err;
> +}
> +
> +static int hwsim_remove(struct platform_device *pdev)
> +{
> +	struct hwsim_phy *phy, *tmp;
> +
> +	mutex_lock(&hwsim_phys_lock);
> +	list_for_each_entry_safe(phy, tmp, &hwsim_phys, list)
> +		hwsim_del(phy);
> +	mutex_unlock(&hwsim_phys_lock);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mac802154hwsim_driver = {
> +	.probe = hwsim_probe,
> +	.remove = hwsim_remove,
> +	.driver = {
> +			.name = "mac802154_hwsim",
> +	},
> +};
> +
> +static __init int hwsim_init_module(void)
> +{
> +	int rc;
> +
> +	if (radios < 0 || radios > 100)
> +		return -EINVAL;
> +
> +	rc = genl_register_family(&hwsim_genl_family);
> +	if (rc)
> +		return rc;
> +
> +	mac802154hwsim_dev = platform_device_register_simple("mac802154_hwsim",
> +							     -1, NULL, 0);
> +	if (IS_ERR(mac802154hwsim_dev)) {
> +		rc = PTR_ERR(mac802154hwsim_dev);
> +		goto platform_dev;
> +	}
> +
> +	rc = platform_driver_register(&mac802154hwsim_driver);
> +	if (rc < 0)
> +		goto platform_drv;
> +
> +	return 0;
> +
> +platform_drv:
> +	genl_unregister_family(&hwsim_genl_family);
> +platform_dev:
> +	platform_device_unregister(mac802154hwsim_dev);
> +	return rc;
> +}
> +
> +static __exit void hwsim_remove_module(void)
> +{
> +	genl_unregister_family(&hwsim_genl_family);
> +	platform_driver_unregister(&mac802154hwsim_driver);
> +	platform_device_unregister(mac802154hwsim_dev);
> +}
> +
> +module_init(hwsim_init_module);
> +module_exit(hwsim_remove_module);
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.h b/drivers/net/ieee802154/mac802154_hwsim.h
> new file mode 100644
> index 000000000000..6009214c0d75
> --- /dev/null
> +++ b/drivers/net/ieee802154/mac802154_hwsim.h
> @@ -0,0 +1,73 @@
> +#ifndef __MAC802154_HWSIM_H
> +#define __MAC802154_HWSIM_H
> +
> +/* mac802154 hwsim netlink commands
> + *
> + * @MAC802154_HWSIM_CMD_UNSPEC: unspecified command to catch error
> + * @MAC802154_HWSIM_CMD_GET_RADIO: fetch information about existing radios
> + * @MAC802154_HWSIM_CMD_SEL_RADIO: change radio parameters during runtime

Same typo as I mentioned in the wpan-hwsim patch. SET_RADIO.


> + * @MAC802154_HWSIM_CMD_NEW_RADIO: create a new radio with the given parameters
> + *	returns the radio ID (>= 0) or negative on errors, if successful
> + *	then multicast the result
> + * @MAC802154_HWSIM_CMD_DEL_RADIO: destroy a radio, reply is multicasted
> + * @MAC802154_HWSIM_CMD_GET_EDGE: fetch information about existing edges
> + * @MAC802154_HWSIM_CMD_SET_EDGE: change edge parameters during runtime
> + * @MAC802154_HWSIM_CMD_DEL_EDGE: delete edges between radios
> + * @MAC802154_HWSIM_CMD_NEW_EDGE: create a new edge between two radios
> + * @__MAC802154_HWSIM_CMD_MAX: enum limit
> + */
> +enum {
> +	MAC802154_HWSIM_CMD_UNSPEC,
> +
> +	MAC802154_HWSIM_CMD_GET_RADIO,
> +	MAC802154_HWSIM_CMD_SET_RADIO,
> +	MAC802154_HWSIM_CMD_NEW_RADIO,
> +	MAC802154_HWSIM_CMD_DEL_RADIO,
> +
> +	MAC802154_HWSIM_CMD_GET_EDGE,
> +	MAC802154_HWSIM_CMD_SET_EDGE,
> +	MAC802154_HWSIM_CMD_DEL_EDGE,
> +	MAC802154_HWSIM_CMD_NEW_EDGE,
> +
> +	__MAC802154_HWSIM_CMD_MAX,
> +};
> +
> +#define MAC802154_HWSIM_CMD_MAX (__MAC802154_HWSIM_MAX - 1)
> +
> +/* mac802154 hwsim netlink attributes
> + *
> + * @MAC802154_HWSIM_ATTR_UNSPEC: unspecified attribute to catch error
> + * @MAC802154_HWSIM_ATTR_RADIO_ID: u32 attribute to identify the radio
> + * @MAC802154_HWSIM_ATTR_EDGE: nested attribute of edges
> + * @MAC802154_HWSIM_ATTR_EDGES: list if nested attributes which contains the
> + *	edge information according the radio id
> + * @__MAC802154_HWSIM_ATTR_MAX: enum limit
> + */
> +enum {
> +	MAC802154_HWSIM_ATTR_UNSPEC,
> +	MAC802154_HWSIM_ATTR_RADIO_ID,
> +	MAC802154_HWSIM_ATTR_RADIO_EDGE,
> +	MAC802154_HWSIM_ATTR_RADIO_EDGES,
> +	__MAC802154_HWSIM_ATTR_MAX,
> +};
> +
> +#define MAC802154_HWSIM_ATTR_MAX (__MAC802154_HWSIM_ATTR_MAX - 1)
> +
> +/* mac802154 hwsim edge netlink attributes
> + *
> + * @MAC802154_HWSIM_EDGE_ATTR_UNSPEC: unspecified attribute to catch error
> + * @MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID: radio id where the edge points to
> + * @MAC802154_HWSIM_EDGE_ATTR_LQI: LQI value which the endpoint radio will
> + *	receive for this edge
> + * @__MAC802154_HWSIM_ATTR_MAX: enum limit
> + */
> +enum {
> +	MAC802154_HWSIM_EDGE_ATTR_UNSPEC,
> +	MAC802154_HWSIM_EDGE_ATTR_ENDPOINT_ID,
> +	MAC802154_HWSIM_EDGE_ATTR_LQI,
> +	__MAC802154_HWSIM_EDGE_ATTR_MAX,
> +};
> +
> +#define MAC802154_HWSIM_EDGE_ATTR_MAX (__MAC802154_HWSIM_EDGE_ATTR_MAX - 1)
> +
> +#endif /* __MAC802154_HWSIM_H */

In general I am happy with the hwsim approach. I think we can go from RFC to normal patches and see that we get it fully reviewed and applied.

regards
Stefan Schmidt

Re: [RFC PATCH wpan-next 1/2] ieee802154: hwsim: add replacement for fakelb

[Alexander Aring]

Hi,

On Mon, May 07, 2018 at 04:00:49PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> 
> On 04/27/2018 11:21 PM, Alexander Aring wrote:
> > This patch adds a new virtual driver mac802154_hwsim which is based on
> > the fakelb driver.
> > The fakelb driver will get deprecated and hopefully removed someday.
> > The main reason for doing this step is to rename the driver to
> > mac802154_hwsim to have a similar naming scheme as mac80211_hwsim,
> > which is more popular in the 802.11 wireless word and the idea is the
> > same behind this driver.
> >
> > The new features of this driver are to have knowledge about connected
> > edges, which can be changed during runtime. This offers a testing
> > environment for routing protocols e.g. RPL.
> > The default behaviour is still as fakelb: two radios connected to each
> > other. New added radios during runtime will not be connected to other
> > wpan_hwsim instances.
> >
> > The netlink api is not namespace aware on purpose, only the registered
> > wpan_phy's can be moved to namespaces. The physical layer according to
> > wiresless "air" communication can be handled across namespaces.
> >
> > Furthermore the edges can be weighted with the LQI value according IEEE
> > 802.15.4 which offers additional handling to mark bad or good connection
> > indicators to other connected virtual phys.
> >
> > Signed-off-by: Alexander Aring <aring@mojatatu.com>
> > ---
> >  drivers/net/ieee802154/Kconfig           |  11 +
> >  drivers/net/ieee802154/Makefile          |   1 +
> >  drivers/net/ieee802154/mac802154_hwsim.c | 925 +++++++++++++++++++++++++++++++
> >  drivers/net/ieee802154/mac802154_hwsim.h |  73 +++
> >  4 files changed, 1010 insertions(+)
> >  create mode 100644 drivers/net/ieee802154/mac802154_hwsim.c
> >  create mode 100644 drivers/net/ieee802154/mac802154_hwsim.h
> >
> > diff --git a/drivers/net/ieee802154/Kconfig b/drivers/net/ieee802154/Kconfig
> > index 8782f5655e3f..ff3552044679 100644
> > --- a/drivers/net/ieee802154/Kconfig
> > +++ b/drivers/net/ieee802154/Kconfig
> > @@ -115,3 +115,14 @@ config IEEE802154_MCR20A
> >  
> >  	  This driver can also be built as a module. To do so, say M here.
> >  	  the module will be called 'mcr20a'.
> > +
> > +config MAC802154_HWSIM
> > +	depends on IEEE802154_DRIVERS && MAC802154
> > +	tristate "Simulated radio testing tool for mac802154"
> > +	---help---
> > +	  This driver is a developer testing tool that can be used to test
> > +	  IEEE 802.15.4 networking stack (mac802154) functionality. This is not
> > +	  needed for normal wireless LAN usage and is only for testing.
> 
> The wireless LAN wording seems to be a left over :-)
> 
> 

ok.

> > +	  This driver can also be built as a module. To do so say M here.
> > +	  The module will be called 'mac802154_hwsim'.
> > diff --git a/drivers/net/ieee802154/Makefile b/drivers/net/ieee802154/Makefile
> > index 104744d5a668..3b17aa17d46a 100644
> > --- a/drivers/net/ieee802154/Makefile
> > +++ b/drivers/net/ieee802154/Makefile
> > @@ -7,3 +7,4 @@ obj-$(CONFIG_IEEE802154_ATUSB) += atusb.o
> >  obj-$(CONFIG_IEEE802154_ADF7242) += adf7242.o
> >  obj-$(CONFIG_IEEE802154_CA8210) += ca8210.o
> >  obj-$(CONFIG_IEEE802154_MCR20A) += mcr20a.o
> > +obj-$(CONFIG_MAC802154_HWSIM) += mac802154_hwsim.o
> > diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> > new file mode 100644
> > index 000000000000..eea0d355bbd8
> > --- /dev/null
> > +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> > @@ -0,0 +1,925 @@
> > +/*
> > + * HWSIM IEEE 802.15.4 interface
> > + *
> > + * (C) 2018 Mojatau, Alexander Aring <aring@mojatau.com>
> > + * Copyright 2007-2012 Siemens AG
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2
> > + * as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> 
> Not only related to this patch but we might want to think about moving from the license boiler plate text to SPDX for the ieee802154
> subsystem at some point.
> 

ok. Yea I saw that, it's somehow new...

> > + * Based on fakelb, original Written by:
> > + * Sergey Lapin <slapin@ossfans.org>
> > + * Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> > + * Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/timer.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/device.h>
> > +#include <linux/spinlock.h>
> > +#include <net/mac802154.h>
> > +#include <net/cfg802154.h>
> > +#include <net/genetlink.h>
> > +#include "mac802154_hwsim.h"
> > +
> > +MODULE_DESCRIPTION("Software simulator of 802.15.4 radio(s) for mac802154");
> 
> "IEEE 802.15.4", please.
> 

ok.

> > +MODULE_LICENSE("GPL");
> 
> The license text does not have the "or any later version" part which means we should indicate it really is GPL v2 only.
> 

I copied from fakelb but GPLv2 is standard for all kernel code so far I
know (except 'UAPI?' headers). I will change it to GPLv2

> > +
> > +/* Number of dummy devices to be set up by this module. */
> > +static int radios = 2;
> 
> Does this really need to be a global variable?
>

module_param works like that. Question is if we really like to have such
feature as parameter since we can add/del during runtime?

But then the default should be 2. Two nodes are the common case to test
non-big/complex setups.

> > +module_param(radios, int, 0444);
> > +MODULE_PARM_DESC(radios, "Number of simulated radios");
> 
> What happens if more than 2 devices are given as module parameter, are only the first two connected and the rest unconnected?
> 

No, I the current implementation as it is. Everything what you specified
as module parameter "radios" e.g. 2 will end that every radio is
connected to each other except to itself. It's full mesh...

Adding during runtime will occur nothing is connected.

In my feeling with handling of this driver... I usually start with two
radios connected to each other (default behaviour) and then I add
another radios, because I want a bigger/complex setup.

Maybe an argument to let 2 radios always as default and remove this
handling of radios parameters. Will reduce some code handling in init...
and I am always for to make it simple as possible. Okay edge handling is
complex, but so far needed to make experiments in LL networks.

> 
> > +static LIST_HEAD(hwsim_phys);
> > +static DEFINE_MUTEX(hwsim_phys_lock);
> > +
> > +static __rcu LIST_HEAD(hwsim_ifup_phys);
> > +
> > +static struct platform_device *mac802154hwsim_dev;
> > +
> > +/* MAC802154_HWSIM netlink family */
> > +static struct genl_family hwsim_genl_family;
> > +
> > +static int hwsim_radio_idx;
> 
> Another global variable I would like to avoid, if it make sense.
> 

needed to have a global max idx which is currently given and the whole
mess is because dump feature of netlink. We cannot add some private data
pointer when registering netlink ops.

See hwsim_dump_radio_nl() it's our identify when dump is done.

I will see if I can put put all the globals in a hwsim_driver_globals
struct and define this only once... if this is what you want. :-/

> > +
> > +enum hwsim_multicast_groups {
> > +	HWSIM_MCGRP_CONFIG,
> > +};
> > +
> > +static const struct genl_multicast_group hwsim_mcgrps[] = {
> > +	[HWSIM_MCGRP_CONFIG] = { .name = "config", },
> > +};
> > +
> > +struct hwsim_pib {
> > +	u8 page;
> > +	u8 channel;
> > +
> > +	struct rcu_head rcu;
> > +};
> > +
> > +struct hwsim_edge_info {
> > +	u8 lqi;
> > +
> > +	struct rcu_head rcu;
> > +};
> > +
> > +struct hwsim_edge {
> > +	struct hwsim_phy *endpoint;
> > +	struct hwsim_edge_info *info;
> > +
> > +	struct list_head list;
> > +	struct rcu_head rcu;
> > +};
> > +
> > +struct hwsim_phy {
> > +	struct ieee802154_hw *hw;
> > +	u32 idx;
> > +
> > +	struct hwsim_pib __rcu *pib;
> > +
> > +	bool suspended;
> > +	struct list_head __rcu edges;
> > +
> > +	struct list_head list;
> > +	struct list_head list_ifup;
> > +};
> > +
> > +static int hwsim_hw_ed(struct ieee802154_hw *hw, u8 *level)
> > +{
> > +	*level = 0xbe;
> > +
> > +	return 0;
> > +}
> > +
> > +static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
> > +{
> > +	struct hwsim_phy *phy = hw->priv;
> > +	struct hwsim_pib *pib, *pib_old;
> > +
> > +	pib = kzalloc(sizeof(*pib), GFP_KERNEL);
> > +	if (!pib)
> > +		return -ENOMEM;
> > +
> > +	pib->page = page;
> > +	pib->channel = channel;
> > +
> > +	pib_old = phy->pib;
> > +	rcu_assign_pointer(phy->pib, pib);
> > +	kfree_rcu(pib_old, rcu);
> > +	return 0;
> > +}
> > +
> > +static int hwsim_hw_xmit(struct ieee802154_hw *hw, struct sk_buff *skb)
> > +{
> > +	struct hwsim_phy *current_phy = hw->priv;
> > +	struct hwsim_pib *current_pib, *endpoint_pib;
> > +	struct hwsim_edge_info *einfo;
> > +	struct hwsim_edge *e;
> > +
> > +	WARN_ON(current_phy->suspended);
> > +
> > +	rcu_read_lock();
> > +	current_pib = rcu_dereference(current_phy->pib);
> > +	list_for_each_entry_rcu(e, &current_phy->edges, list) {
> > +		/* Can be changed later in rx_irqsafe, but this is only a
> > +		 * performance tweak. Received radio should drop the frame
> > +		 * in mac802154 stack anyway... so we don't need to be
> > +		 * 100% of locking here to check on suspended
> > +		 */
> > +		if (e->endpoint->suspended)
> > +			continue;
> > +
> > +		endpoint_pib = rcu_dereference(e->endpoint->pib);
> > +		if (current_pib->page == endpoint_pib->page &&
> > +		    current_pib->channel == endpoint_pib->channel) {
> > +			struct sk_buff *newskb = pskb_copy(skb, GFP_ATOMIC);
> > +
> > +			einfo = rcu_dereference(e->info);
> > +			if (newskb)
> > +				ieee802154_rx_irqsafe(e->endpoint->hw, newskb,
> > +						      einfo->lqi);
> > +		}
> > +	}
> > +	rcu_read_unlock();
> > +
> > +	ieee802154_xmit_complete(hw, skb, false);
> > +	return 0;
> > +}
> > +
> > +static int hwsim_hw_start(struct ieee802154_hw *hw)
> > +{
> > +	struct hwsim_phy *phy = hw->priv;
> > +
> > +	phy->suspended = false;
> > +	list_add_rcu(&phy->list_ifup, &hwsim_ifup_phys);
> > +	synchronize_rcu();
> > +
> > +	return 0;
> > +}
> > +
> > +static void hwsim_hw_stop(struct ieee802154_hw *hw)
> > +{
> > +	struct hwsim_phy *phy = hw->priv;
> > +
> > +	phy->suspended = true;
> > +	list_del_rcu(&phy->list_ifup);
> > +	synchronize_rcu();
> > +}
> > +
> > +static int
> > +hwsim_set_promiscuous_mode(struct ieee802154_hw *hw, const bool on)
> > +{
> > +	return 0;
> > +}
> > +
> > +static const struct ieee802154_ops hwsim_ops = {
> > +	.owner = THIS_MODULE,
> > +	.xmit_async = hwsim_hw_xmit,
> > +	.ed = hwsim_hw_ed,
> > +	.set_channel = hwsim_hw_channel,
> > +	.start = hwsim_hw_start,
> > +	.stop = hwsim_hw_stop,
> > +	.set_promiscuous_mode = hwsim_set_promiscuous_mode,
> > +};
> > +
> > +static int hwsim_add_one(struct genl_info *info, struct device *dev,
> > +			 bool init);
> 
> Please re-order the functions so we can avoid forward declarations.
> 

What I wanted here is to have netlink handling at one place.
I want to move it:

Then I need a forward declaration for hwsim_new_radio_nl() because
hwsim_nl_ops need to know about that this function. Then I need to move
the whole struct of hwsim_nl_ops under hwsim_add_one() as well and I
stopped thinking about other changes.

Alternative is moving hwsim_add_one() implementation before
hwsim_new_radio_nl(), but then I need a forward declaration of
hwsim_genl_family struct. If I move hwsim_genl_family as well I need a
forward declaration of hwsim_genl_family before it.


I think I will move these forward declaration in begin of this file,
then it's not an eye catcher anymore... but I have no idea how to handle
it and nl_ops should be sit in front of struct genl_ops hwsim_nl_ops[].

> > +static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info)
> > +{
> > +	return hwsim_add_one(info, &mac802154hwsim_dev->dev, false);
> > +}
> > +
> > +static void hwsim_del(struct hwsim_phy *phy);
> 
> Another forward declaration to avoid.

Actually this function has less logic so I can move this above... BUT
the whole reason why this doesn't work at hwsim_add_one is because it
need to specify some global netlink struct foo.

I can move it but then del is not anymore at add_one (del should
cleanup what add_one is doing also, so they depends logically to each other).
... and here comes the another BUT, we don't notify about deleted hwsim
phy's if we do that, then we need to move the function anyway below...

Tell me what to do here. BTW: better name here would be hwsim_del_one()
in contrast to hwsim_add_one.

---

Btw: I can move these functions very easily if we load hwsim without any
phy, so I don't need to use these functions in init()/exit()... but most
common setup is 2 radios connected to each other... if you want complex:
use wpan-hwsim.

> 
> > +static int hwsim_del_radio_nl(struct sk_buff *msg, struct genl_info *info)
> > +{
> > +	struct hwsim_phy *phy, *tmp;
> > +	s64 idx = -1;
> > +
> > +	if (!info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID])
> > +		return -EINVAL;
> > +
> > +	idx = nla_get_u32(info->attrs[MAC802154_HWSIM_ATTR_RADIO_ID]);
> > +
> > +	mutex_lock(&hwsim_phys_lock);
> > +	list_for_each_entry_safe(phy, tmp, &hwsim_phys, list) {
> > +		if (idx == phy->idx) {
> > +			hwsim_del(phy);
> > +			mutex_unlock(&hwsim_phys_lock);
> > +			return 0;
> > +		}
> > +	}
> > +	mutex_unlock(&hwsim_phys_lock);
> > +
> > +	return -ENODEV;
> > +}
> > +
> > +static int append_radio_msg(struct sk_buff *skb, struct hwsim_phy *phy)
> > +{
> > +	struct nlattr *nl_edges, *nl_edge;
> > +	struct hwsim_edge_info *einfo;
> > +	struct hwsim_edge *e;
> > +	int ret;
> > +
> > +	ret = nla_put_u32(skb, MAC802154_HWSIM_ATTR_RADIO_ID, phy->idx);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	rcu_read_lock();
> > +	if (list_empty(&phy->edges)) {
> > +		rcu_read_unlock();
> > +		return 0;
> > +	}
> > +
> > +	nl_edges = nla_nest_start(skb, MAC802154_HWSIM_ATTR_RADIO_EDGES);
> > +	if (!nl_edges) {
> > +		rcu_read_unlock();
> > +		return -ENOBUFS;
> > +	}
> > +
> > +	list_for_each_entry_rcu(e, &phy->edges, list) {
> > +		nl_edge = nla_nest_start(skb, MAC802154_HWSIM_ATTR_RADIO_EDGE);
> > +		if (!nl_edges) {
> 
> You save the return of bla_nest_start in nl_edge but you are checking on nl_edges.
> 

ok. sorry.

> > +			rcu_read_unlock();
> > +			nla_nest_cancel(skb, nl_edges);
> > +			return -ENOBUFS;
...
> > +
> > +/* Generic Netlink operations array */
> > +static const struct genl_ops hwsim_nl_ops[] = {
> > +	{
> > +		.cmd = MAC802154_HWSIM_CMD_NEW_RADIO,
> > +		.policy = hwsim_genl_policy,
> > +		.doit = hwsim_new_radio_nl,
> > +		.flags = GENL_UNS_ADMIN_PERM,
> > +	},
> > +	{
> > +		.cmd = MAC802154_HWSIM_CMD_DEL_RADIO,
> > +		.policy = hwsim_genl_policy,
> > +		.doit = hwsim_del_radio_nl,
> > +		.flags = GENL_UNS_ADMIN_PERM,
> > +	},
> > +	{
> > +		.cmd = MAC802154_HWSIM_CMD_GET_RADIO,
> > +		.policy = hwsim_genl_policy,
> > +		.doit = hwsim_get_radio_nl,
> > +		.dumpit = hwsim_dump_radio_nl,
> > +	},
> > +	{
> > +		.cmd = MAC802154_HWSIM_CMD_NEW_EDGE,
> > +		.policy = hwsim_genl_policy,
> > +		.doit = hwsim_new_edge_nl,
> > +		.flags = GENL_UNS_ADMIN_PERM,
> > +	},
> > +	{
> > +		.cmd = MAC802154_HWSIM_CMD_DEL_EDGE,
> > +		.policy = hwsim_genl_policy,
> > +		.doit = hwsim_del_edge_nl,
> > +		.flags = GENL_UNS_ADMIN_PERM,
> > +	},
> > +	{
> > +		.cmd = MAC802154_HWSIM_CMD_SET_EDGE,
> > +		.policy = hwsim_genl_policy,
> > +		.doit = hwsim_set_edge_lqi,
> > +		.flags = GENL_UNS_ADMIN_PERM,
> > +	},
> > +};
> > +
> 
> This answers my questions on why MAC802154_HWSIM_CMD_SET_RADIO and MAC802154_HWSIM_CMD_GET_EDGE are not used in wpan-hwsim.
> The kernel does not really expose them. If you think they are not needed I would say we need to remove these enum types from the header below.

I wanted to have there because this is general design from wireless to
make something like that (I catch it up for future use).
SET_RADIO could have use later for changing HWFLAGS, I have some ideas
in my mind. GET_EDGE: only dump Edge information with vector specified,
I currently dump radio and it has the list of edges appended.
Use cases for me... HWFLAGS would be to make stuff in driverlayer to
fake that the hardware would do it. e.g. CRC can be set/verified by
softmac or hardware, we could switch that check (ignoring the case of
locking for that yet... should be fine).

Can we rename it to __MAC802154_HWSIM_RESERVED{1...N} ? This offers us to
rename this thing and use it for future use at place it in order. But
then I can ask myself... it's a compile define and would break userspace
(by just renaming) when somebody use that define for whatever the reason
is..., but then it's their problem when they use a reserved define. :-)

> > +static struct genl_family hwsim_genl_family __ro_after_init = {
> > +	.name = "MAC802154_HWSIM",
> > +	.version = 1,
> > +	.maxattr = MAC802154_HWSIM_ATTR_MAX,
> > +	.module = THIS_MODULE,
> > +	.ops = hwsim_nl_ops,
> > +	.n_ops = ARRAY_SIZE(hwsim_nl_ops),
> > +	.mcgrps = hwsim_mcgrps,
> > +	.n_mcgrps = ARRAY_SIZE(hwsim_mcgrps),
> > +};
> > +
...
> > +
> > +module_init(hwsim_init_module);
> > +module_exit(hwsim_remove_module);
> > diff --git a/drivers/net/ieee802154/mac802154_hwsim.h b/drivers/net/ieee802154/mac802154_hwsim.h
> > new file mode 100644
> > index 000000000000..6009214c0d75
> > --- /dev/null
> > +++ b/drivers/net/ieee802154/mac802154_hwsim.h
> > @@ -0,0 +1,73 @@
> > +#ifndef __MAC802154_HWSIM_H
> > +#define __MAC802154_HWSIM_H
> > +
> > +/* mac802154 hwsim netlink commands
> > + *
> > + * @MAC802154_HWSIM_CMD_UNSPEC: unspecified command to catch error
> > + * @MAC802154_HWSIM_CMD_GET_RADIO: fetch information about existing radios
> > + * @MAC802154_HWSIM_CMD_SEL_RADIO: change radio parameters during runtime
> 
> Same typo as I mentioned in the wpan-hwsim patch. SET_RADIO.
> 

ok.

- Alex

Re: [RFC PATCH wpan-next 1/2] ieee802154: hwsim: add replacement for fakelb

[Alexander Aring]

Hi,

On Fri, Apr 27, 2018 at 05:21:53PM -0400, Alexander Aring wrote:
> This patch adds a new virtual driver mac802154_hwsim which is based on
> the fakelb driver.
> The fakelb driver will get deprecated and hopefully removed someday.
> The main reason for doing this step is to rename the driver to
> mac802154_hwsim to have a similar naming scheme as mac80211_hwsim,
> which is more popular in the 802.11 wireless word and the idea is the
> same behind this driver.
> 
> The new features of this driver are to have knowledge about connected
> edges, which can be changed during runtime. This offers a testing
> environment for routing protocols e.g. RPL.
> The default behaviour is still as fakelb: two radios connected to each
> other. New added radios during runtime will not be connected to other
> wpan_hwsim instances.
> 
> The netlink api is not namespace aware on purpose, only the registered
> wpan_phy's can be moved to namespaces. The physical layer according to
> wiresless "air" communication can be handled across namespaces.
> 
> Furthermore the edges can be weighted with the LQI value according IEEE
> 802.15.4 which offers additional handling to mark bad or good connection
> indicators to other connected virtual phys.
> 
> Signed-off-by: Alexander Aring <aring@mojatatu.com>

somebody (don't cc him, I think he don't want to be public) reported
about issues with hwsim privetly. I will check on it and enable more
kernel debugging stuff in kernel hacking feature to see all
locking/stack issues.

TO ALL: never send me private (and absolutely no HTML mails) to me to
report problems. Please do it here.

Thanks.

I will look for this today and change mostly the suggested changes.

- Alex

Re: [RFC PATCH wpan-next 1/2] ieee802154: hwsim: add replacement for fakelb

[Stefan Schmidt]

Hello.


On 05/14/2018 04:29 PM, Alexander Aring wrote:
> Hi,
>
> On Fri, Apr 27, 2018 at 05:21:53PM -0400, Alexander Aring wrote:
>> This patch adds a new virtual driver mac802154_hwsim which is based on
>> the fakelb driver.
>> The fakelb driver will get deprecated and hopefully removed someday.
>> The main reason for doing this step is to rename the driver to
>> mac802154_hwsim to have a similar naming scheme as mac80211_hwsim,
>> which is more popular in the 802.11 wireless word and the idea is the
>> same behind this driver.
>>
>> The new features of this driver are to have knowledge about connected
>> edges, which can be changed during runtime. This offers a testing
>> environment for routing protocols e.g. RPL.
>> The default behaviour is still as fakelb: two radios connected to each
>> other. New added radios during runtime will not be connected to other
>> wpan_hwsim instances.
>>
>> The netlink api is not namespace aware on purpose, only the registered
>> wpan_phy's can be moved to namespaces. The physical layer according to
>> wiresless "air" communication can be handled across namespaces.
>>
>> Furthermore the edges can be weighted with the LQI value according IEEE
>> 802.15.4 which offers additional handling to mark bad or good connection
>> indicators to other connected virtual phys.
>>
>> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> somebody (don't cc him, I think he don't want to be public) reported
> about issues with hwsim privetly. I will check on it and enable more
> kernel debugging stuff in kernel hacking feature to see all
> locking/stack issues.

The positive side is that it got some more testing and you got some problems reported. :-)

>
> TO ALL: never send me private (and absolutely no HTML mails) to me to
> report problems. Please do it here.

Agreed. If you have questions please write to the list. There are often other people who can help as well.

regards
Stefan Schmidt

Re: [RFC PATCH wpan-next 1/2] ieee802154: hwsim: add replacement for fakelb

[Alexander Aring]

Hi,

On Mon, May 14, 2018 at 05:40:26PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> 
> On 05/14/2018 04:29 PM, Alexander Aring wrote:
> > Hi,
> >
> > On Fri, Apr 27, 2018 at 05:21:53PM -0400, Alexander Aring wrote:
> >> This patch adds a new virtual driver mac802154_hwsim which is based on
> >> the fakelb driver.
> >> The fakelb driver will get deprecated and hopefully removed someday.
> >> The main reason for doing this step is to rename the driver to
> >> mac802154_hwsim to have a similar naming scheme as mac80211_hwsim,
> >> which is more popular in the 802.11 wireless word and the idea is the
> >> same behind this driver.
> >>
> >> The new features of this driver are to have knowledge about connected
> >> edges, which can be changed during runtime. This offers a testing
> >> environment for routing protocols e.g. RPL.
> >> The default behaviour is still as fakelb: two radios connected to each
> >> other. New added radios during runtime will not be connected to other
> >> wpan_hwsim instances.
> >>
> >> The netlink api is not namespace aware on purpose, only the registered
> >> wpan_phy's can be moved to namespaces. The physical layer according to
> >> wiresless "air" communication can be handled across namespaces.
> >>
> >> Furthermore the edges can be weighted with the LQI value according IEEE
> >> 802.15.4 which offers additional handling to mark bad or good connection
> >> indicators to other connected virtual phys.
> >>
> >> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> > somebody (don't cc him, I think he don't want to be public) reported
> > about issues with hwsim privetly. I will check on it and enable more
> > kernel debugging stuff in kernel hacking feature to see all
> > locking/stack issues.
> 
> The positive side is that it got some more testing and you got some problems reported. :-)
> 

My test case is:

while true;do wpan-hwsim edge del 0 1; sleep 1; wpan-hwsim edge add 0 1;sleep 1;done

while I am doing:

ping -f fe80::8e6:9f98:edab:f347%lowpan0 -s 4000

to the other node... -f is imporant.

That should occur some race when there is one... I can't see anything
here. I also enabled a lot of kernel hacking features to check stack
corruption/rcu locking issues. Nothing, it runs over 30 minutes without
problems...

I will ignore that issue, I don't know maybe he added some code on his
own...

- Alex

Re: [RFC PATCH wpan-next 1/2] ieee802154: hwsim: add replacement for fakelb

[Alexander Aring]

Hi,

On Thu, May 17, 2018 at 04:29:34PM -0400, Alexander Aring wrote:
> Hi,
> 
> On Mon, May 14, 2018 at 05:40:26PM +0200, Stefan Schmidt wrote:
> > Hello.
> > 
> > 
> > On 05/14/2018 04:29 PM, Alexander Aring wrote:
> > > Hi,
> > >
> > > On Fri, Apr 27, 2018 at 05:21:53PM -0400, Alexander Aring wrote:
> > >> This patch adds a new virtual driver mac802154_hwsim which is based on
> > >> the fakelb driver.
> > >> The fakelb driver will get deprecated and hopefully removed someday.
> > >> The main reason for doing this step is to rename the driver to
> > >> mac802154_hwsim to have a similar naming scheme as mac80211_hwsim,
> > >> which is more popular in the 802.11 wireless word and the idea is the
> > >> same behind this driver.
> > >>
> > >> The new features of this driver are to have knowledge about connected
> > >> edges, which can be changed during runtime. This offers a testing
> > >> environment for routing protocols e.g. RPL.
> > >> The default behaviour is still as fakelb: two radios connected to each
> > >> other. New added radios during runtime will not be connected to other
> > >> wpan_hwsim instances.
> > >>
> > >> The netlink api is not namespace aware on purpose, only the registered
> > >> wpan_phy's can be moved to namespaces. The physical layer according to
> > >> wiresless "air" communication can be handled across namespaces.
> > >>
> > >> Furthermore the edges can be weighted with the LQI value according IEEE
> > >> 802.15.4 which offers additional handling to mark bad or good connection
> > >> indicators to other connected virtual phys.
> > >>
> > >> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> > > somebody (don't cc him, I think he don't want to be public) reported
> > > about issues with hwsim privetly. I will check on it and enable more
> > > kernel debugging stuff in kernel hacking feature to see all
> > > locking/stack issues.
> > 
> > The positive side is that it got some more testing and you got some problems reported. :-)
> > 
> 
> My test case is:
> 
> while true;do wpan-hwsim edge del 0 1; sleep 1; wpan-hwsim edge add 0 1;sleep 1;done
> 

forget the sleep here, better without so add/del edge so fast as
possible while traffic send.

traffic send can be replaced by iperf or netperf.

- Alex

[RFC PATCH wpan-next 2/2] ieee802154: fakelb: add deprecated msg while probe

[Alexander Aring]

Since mac802154_hwsim the fakelb driver will get deprecated. This patch will
notifier all users of fakelb to switch to the new mac802154_hwsim driver.

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 drivers/net/ieee802154/fakelb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ieee802154/fakelb.c b/drivers/net/ieee802154/fakelb.c
index 0d673f7682ee..d44cc27a13c7 100644
--- a/drivers/net/ieee802154/fakelb.c
+++ b/drivers/net/ieee802154/fakelb.c
@@ -254,6 +254,9 @@ static __init int fakelb_init_module(void)
 {
 	ieee802154fake_dev = platform_device_register_simple(
 			     "ieee802154fakelb", -1, NULL, 0);
+
+	pr_warn("fakelb driver is marked as deprecated, please use mac802154_hwsim!");
+
 	return platform_driver_register(&ieee802154fake_driver);
 }
 
-- 
2.11.0

Re: [RFC PATCH wpan-next 2/2] ieee802154: fakelb: add deprecated msg while probe

[Stefan Schmidt]

Hello.


On 04/27/2018 11:21 PM, Alexander Aring wrote:
> Since mac802154_hwsim the fakelb driver will get deprecated. This patch will
> notifier all users of fakelb to switch to the new mac802154_hwsim driver.
>
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>  drivers/net/ieee802154/fakelb.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ieee802154/fakelb.c b/drivers/net/ieee802154/fakelb.c
> index 0d673f7682ee..d44cc27a13c7 100644
> --- a/drivers/net/ieee802154/fakelb.c
> +++ b/drivers/net/ieee802154/fakelb.c
> @@ -254,6 +254,9 @@ static __init int fakelb_init_module(void)
>  {
>  	ieee802154fake_dev = platform_device_register_simple(
>  			     "ieee802154fakelb", -1, NULL, 0);
> +
> +	pr_warn("fakelb driver is marked as deprecated, please use mac802154_hwsim!");
> +
>  	return platform_driver_register(&ieee802154fake_driver);
>  }
>  

I am fine with this patch once the hwsim driver is merged.

regards
Stefan Schmidt

Re: [RFC PATCH wpan-next 2/2] ieee802154: fakelb: add deprecated msg while probe

[Alexander Aring]

Hi,

On Fri, Apr 27, 2018 at 05:21:54PM -0400, Alexander Aring wrote:
> Since mac802154_hwsim the fakelb driver will get deprecated. This patch will
> notifier all users of fakelb to switch to the new mac802154_hwsim driver.
> 
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>  drivers/net/ieee802154/fakelb.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ieee802154/fakelb.c b/drivers/net/ieee802154/fakelb.c
> index 0d673f7682ee..d44cc27a13c7 100644
> --- a/drivers/net/ieee802154/fakelb.c
> +++ b/drivers/net/ieee802154/fakelb.c
> @@ -254,6 +254,9 @@ static __init int fakelb_init_module(void)
>  {
>  	ieee802154fake_dev = platform_device_register_simple(
>  			     "ieee802154fakelb", -1, NULL, 0);
> +
> +	pr_warn("fakelb driver is marked as deprecated, please use mac802154_hwsim!");
> +

I can mess up the smallest patches, there is a missing newline. :-)

- Alex