linux-wpan.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH wpan-next 0/6] net: ieee802154: PAN management
@ 2022-06-03 18:21 Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type Miquel Raynal
                   ` (5 more replies)
  0 siblings, 6 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-03 18:21 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
	David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, Miquel Raynal

Hello,

Last step before adding scan support, we need to introduce a proper PAN
description (with its main properties) and PAN management helpers.

This series provides generic code to do simple operations on PANs and
PAN coordinators.

Thanks,
Miquèl

David Girault (1):
  net: ieee802154: Trace the registration of new PANs

Miquel Raynal (5):
  net: ieee802154: Drop coordinator interface type
  net: ieee802154: Add support for internal PAN management
  net: ieee802154: Create a node type
  net: ieee802154: Add the PAN coordinator information
  net: ieee802154: Full PAN management

 include/net/cfg802154.h   |  31 +++++
 include/net/nl802154.h    |  61 +++++++++-
 net/ieee802154/Makefile   |   2 +-
 net/ieee802154/core.c     |   2 +
 net/ieee802154/core.h     |  42 +++++++
 net/ieee802154/nl802154.c | 232 ++++++++++++++++++++++++++++++++++-
 net/ieee802154/pan.c      | 246 ++++++++++++++++++++++++++++++++++++++
 net/ieee802154/trace.h    |  25 ++++
 8 files changed, 636 insertions(+), 5 deletions(-)
 create mode 100644 net/ieee802154/pan.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-03 18:21 [PATCH wpan-next 0/6] net: ieee802154: PAN management Miquel Raynal
@ 2022-06-03 18:21 ` Miquel Raynal
  2022-06-04  2:01   ` Alexander Aring
  2022-06-03 18:21 ` [PATCH wpan-next 2/6] net: ieee802154: Add support for internal PAN management Miquel Raynal
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Miquel Raynal @ 2022-06-03 18:21 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
	David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, Miquel Raynal

The current enum is wrong. A device can either be an RFD, an RFD-RX, an
RFD-TX or an FFD. If it is an FFD, it can also be a coordinator. While
defining a node type might make sense from a strict software point of
view, opposing node and coordinator seems meaningless in the ieee
802.15.4 world. As this enumeration is not used anywhere, let's just
drop it. We will in a second time add a new "node type" enumeration
which apply only to nodes, and does differentiates the type of devices
mentioned above.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/nl802154.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index 145acb8f2509..0f508aaae126 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -156,7 +156,6 @@ enum nl802154_iftype {
 
 	NL802154_IFTYPE_NODE = 0,
 	NL802154_IFTYPE_MONITOR,
-	NL802154_IFTYPE_COORD,
 
 	/* keep last */
 	NUM_NL802154_IFTYPES,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH wpan-next 2/6] net: ieee802154: Add support for internal PAN management
  2022-06-03 18:21 [PATCH wpan-next 0/6] net: ieee802154: PAN management Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type Miquel Raynal
@ 2022-06-03 18:21 ` Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 3/6] net: ieee802154: Create a node type Miquel Raynal
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-03 18:21 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
	David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, Miquel Raynal

Let's introduce the basics of PAN management:
- structures defining PAN
- helpers for PAN registration
- helpers discarding old PAN

Co-developed-by: David Girault <david.girault@qorvo.com>
Signed-off-by: David Girault <david.girault@qorvo.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/cfg802154.h |  31 ++++++
 net/ieee802154/Makefile |   2 +-
 net/ieee802154/core.c   |   2 +
 net/ieee802154/core.h   |  26 +++++
 net/ieee802154/pan.c    | 231 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 291 insertions(+), 1 deletion(-)
 create mode 100644 net/ieee802154/pan.c

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 04b996895fc1..9838eca3c41e 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -255,6 +255,24 @@ struct ieee802154_addr {
 	};
 };
 
+/**
+ * struct ieee802154_pan_desc - PAN descriptor information
+ * @coord: PAN ID and coordinator address
+ * @page: page this PAN is on
+ * @channel: channel this PAN is on
+ * @superframe_spec: SuperFrame specification as received
+ * @link_quality: link quality indicator at which the beacon was received
+ * @gts_permit: the PAN coordinator accepts GTS requests
+ */
+struct ieee802154_pan_desc {
+	struct ieee802154_addr *coord;
+	u8 page;
+	u8 channel;
+	u16 superframe_spec;
+	u8 link_quality;
+	bool gts_permit;
+};
+
 struct ieee802154_llsec_key_id {
 	u8 mode;
 	u8 id;
@@ -426,4 +444,17 @@ static inline const char *wpan_phy_name(struct wpan_phy *phy)
 
 void ieee802154_configure_durations(struct wpan_phy *phy);
 
+/**
+ * cfg802154_record_pan - Advertize a new PAN following a beacon's reception
+ * @wpan_phy: PHY receiving the beacon
+ * @pan: PAN descriptor
+ *
+ * Tells the internal pan management layer to either register this PAN if it is
+ * new or at least update its entry if already discovered.
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ */
+int cfg802154_record_pan(struct wpan_phy *wpan_phy,
+			 struct ieee802154_pan_desc *pan);
+
 #endif /* __NET_CFG802154_H */
diff --git a/net/ieee802154/Makefile b/net/ieee802154/Makefile
index f05b7bdae2aa..6b7c66de730d 100644
--- a/net/ieee802154/Makefile
+++ b/net/ieee802154/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_IEEE802154_SOCKET) += ieee802154_socket.o
 obj-y += 6lowpan/
 
 ieee802154-y := netlink.o nl-mac.o nl-phy.o nl_policy.o core.o \
-                header_ops.o sysfs.o nl802154.o trace.o
+                header_ops.o sysfs.o nl802154.o pan.o trace.o
 ieee802154_socket-y := socket.o
 
 CFLAGS_trace.o := -I$(src)
diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 57546e07e06a..f642db35d62e 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -115,6 +115,8 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size)
 		kfree(rdev);
 		return NULL;
 	}
+	spin_lock_init(&rdev->pan_lock);
+	INIT_LIST_HEAD(&rdev->pan_list);
 
 	/* atomic_inc_return makes it start at 1, make it start at 0 */
 	rdev->wpan_phy_idx--;
diff --git a/net/ieee802154/core.h b/net/ieee802154/core.h
index 1c19f575d574..cae071bede37 100644
--- a/net/ieee802154/core.h
+++ b/net/ieee802154/core.h
@@ -22,6 +22,14 @@ struct cfg802154_registered_device {
 	struct list_head wpan_dev_list;
 	int devlist_generation, wpan_dev_id;
 
+	/* PAN management */
+	spinlock_t pan_lock;
+	struct list_head pan_list;
+	unsigned int max_pan_entries;
+	unsigned int pan_expiration;
+	unsigned int pan_entries;
+	unsigned int pan_generation;
+
 	/* must be last because of the way we do wpan_phy_priv(),
 	 * and it should at least be aligned to NETDEV_ALIGN
 	 */
@@ -39,6 +47,17 @@ wpan_phy_to_rdev(struct wpan_phy *wpan_phy)
 extern struct list_head cfg802154_rdev_list;
 extern int cfg802154_rdev_list_generation;
 
+struct cfg802154_internal_pan {
+	struct list_head list;
+	unsigned long discovery_ts;
+	struct ieee802154_pan_desc desc;
+};
+
+/* Always update the list by dropping the expired PANs before iterating */
+#define ieee802154_for_each_pan(pan, rdev)				\
+	cfg802154_expire_pans(rdev);					\
+	list_for_each_entry((pan), &(rdev)->pan_list, list)
+
 int cfg802154_switch_netns(struct cfg802154_registered_device *rdev,
 			   struct net *net);
 /* free object */
@@ -47,4 +66,11 @@ struct cfg802154_registered_device *
 cfg802154_rdev_by_wpan_phy_idx(int wpan_phy_idx);
 struct wpan_phy *wpan_phy_idx_to_wpan_phy(int wpan_phy_idx);
 
+void cfg802154_set_max_pan_entries(struct cfg802154_registered_device *rdev,
+				   unsigned int max);
+void cfg802154_set_pans_expiration(struct cfg802154_registered_device *rdev,
+				   unsigned int exp_time_s);
+void cfg802154_expire_pans(struct cfg802154_registered_device *rdev);
+void cfg802154_flush_pans(struct cfg802154_registered_device *rdev);
+
 #endif /* __IEEE802154_CORE_H */
diff --git a/net/ieee802154/pan.c b/net/ieee802154/pan.c
new file mode 100644
index 000000000000..b9f50f785960
--- /dev/null
+++ b/net/ieee802154/pan.c
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * IEEE 802.15.4 PAN management
+ *
+ * Copyright (C) Qorvo, 2021
+ * Authors:
+ *   - David Girault <david.girault@qorvo.com>
+ *   - Miquel Raynal <miquel.raynal@bootlin.com>
+ */
+
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+
+#include <net/cfg802154.h>
+#include <net/af_ieee802154.h>
+
+#include "ieee802154.h"
+#include "core.h"
+
+static struct cfg802154_internal_pan *
+cfg802154_alloc_pan(struct ieee802154_pan_desc *desc)
+{
+	struct cfg802154_internal_pan *new;
+	struct ieee802154_addr *coord;
+
+	new = kzalloc(sizeof(*new), GFP_ATOMIC);
+	if (!new)
+		return ERR_PTR(-ENOMEM);
+
+	coord = kzalloc(sizeof(*coord), GFP_ATOMIC);
+	if (!coord) {
+		kfree(new);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	new->discovery_ts = jiffies;
+	new->desc = *desc;
+
+	*coord = *desc->coord;
+	new->desc.coord = coord;
+
+	return new;
+}
+
+static void cfg802154_free_pan(struct cfg802154_internal_pan *pan)
+{
+	kfree(pan->desc.coord);
+	kfree(pan);
+}
+
+static void cfg802154_unlink_pan(struct cfg802154_registered_device *rdev,
+				 struct cfg802154_internal_pan *pan)
+{
+	lockdep_assert_held(&rdev->pan_lock);
+
+	list_del(&pan->list);
+	cfg802154_free_pan(pan);
+	rdev->pan_entries--;
+	rdev->pan_generation++;
+}
+
+static void cfg802154_link_pan(struct cfg802154_registered_device *rdev,
+			       struct cfg802154_internal_pan *pan)
+{
+	lockdep_assert_held(&rdev->pan_lock);
+
+	list_add_tail(&pan->list, &rdev->pan_list);
+	rdev->pan_entries++;
+	rdev->pan_generation++;
+}
+
+void cfg802154_set_max_pan_entries(struct cfg802154_registered_device *rdev,
+				   unsigned int max)
+{
+	lockdep_assert_held(&rdev->pan_lock);
+
+	rdev->max_pan_entries = max;
+}
+EXPORT_SYMBOL(cfg802154_set_max_pan_entries);
+
+static bool
+cfg802154_need_to_expire_pans(struct cfg802154_registered_device *rdev)
+{
+	if (!rdev->max_pan_entries)
+		return false;
+
+	if (rdev->pan_entries > rdev->max_pan_entries)
+		return true;
+
+	return false;
+}
+
+void cfg802154_set_pans_expiration(struct cfg802154_registered_device *rdev,
+				   unsigned int exp_time_s)
+{
+	lockdep_assert_held(&rdev->pan_lock);
+
+	rdev->pan_expiration = exp_time_s * HZ;
+}
+EXPORT_SYMBOL(cfg802154_set_pans_expiration);
+
+void cfg802154_expire_pans(struct cfg802154_registered_device *rdev)
+{
+	struct cfg802154_internal_pan *pan, *tmp;
+	unsigned long expiration_time;
+
+	lockdep_assert_held(&rdev->pan_lock);
+
+	if (!rdev->pan_expiration)
+		return;
+
+	expiration_time = jiffies - rdev->pan_expiration;
+	list_for_each_entry_safe(pan, tmp, &rdev->pan_list, list) {
+		if (!time_after(expiration_time, pan->discovery_ts))
+			continue;
+
+		cfg802154_unlink_pan(rdev, pan);
+	}
+}
+EXPORT_SYMBOL(cfg802154_expire_pans);
+
+static void cfg802154_expire_oldest_pan(struct cfg802154_registered_device *rdev)
+{
+	struct cfg802154_internal_pan *pan, *oldest;
+
+	lockdep_assert_held(&rdev->pan_lock);
+
+	if (WARN_ON(list_empty(&rdev->pan_list)))
+		return;
+
+	oldest = list_first_entry(&rdev->pan_list,
+				  struct cfg802154_internal_pan, list);
+
+	list_for_each_entry(pan, &rdev->pan_list, list) {
+		if (!time_before(oldest->discovery_ts, pan->discovery_ts))
+			oldest = pan;
+	}
+
+	cfg802154_unlink_pan(rdev, oldest);
+}
+
+void cfg802154_flush_pans(struct cfg802154_registered_device *rdev)
+{
+	struct cfg802154_internal_pan *pan, *tmp;
+
+	lockdep_assert_held(&rdev->pan_lock);
+
+	list_for_each_entry_safe(pan, tmp, &rdev->pan_list, list)
+		cfg802154_unlink_pan(rdev, pan);
+}
+EXPORT_SYMBOL(cfg802154_flush_pans);
+
+static bool cfg802154_same_pan(struct ieee802154_pan_desc *a,
+			       struct ieee802154_pan_desc *b)
+{
+	int ret;
+
+	if (a->page != b->page)
+		return false;
+
+	if (a->channel != b->channel)
+		return false;
+
+	ret = memcmp(&a->coord->pan_id, &b->coord->pan_id,
+		     sizeof(a->coord->pan_id));
+	if (ret)
+		return false;
+
+	if (a->coord->mode != b->coord->mode)
+		return false;
+
+	if (a->coord->mode == IEEE802154_ADDR_SHORT)
+		ret = memcmp(&a->coord->short_addr, &b->coord->short_addr,
+			     IEEE802154_SHORT_ADDR_LEN);
+	else
+		ret = memcmp(&a->coord->extended_addr, &b->coord->extended_addr,
+			     IEEE802154_EXTENDED_ADDR_LEN);
+
+	return true;
+}
+
+static struct cfg802154_internal_pan *
+cfg802154_find_matching_pan(struct cfg802154_registered_device *rdev,
+			    struct cfg802154_internal_pan *tmp)
+{
+	struct cfg802154_internal_pan *pan;
+
+	list_for_each_entry(pan, &rdev->pan_list, list) {
+		if (cfg802154_same_pan(&pan->desc, &tmp->desc))
+			return pan;
+	}
+
+	return NULL;
+}
+
+static void cfg802154_pan_update(struct cfg802154_registered_device *rdev,
+				 struct cfg802154_internal_pan *new)
+{
+	struct cfg802154_internal_pan *found;
+
+	spin_lock_bh(&rdev->pan_lock);
+
+	found = cfg802154_find_matching_pan(rdev, new);
+	if (found)
+		cfg802154_unlink_pan(rdev, found);
+
+	if (unlikely(cfg802154_need_to_expire_pans(rdev)))
+		cfg802154_expire_oldest_pan(rdev);
+
+	cfg802154_link_pan(rdev, new);
+
+	spin_unlock_bh(&rdev->pan_lock);
+}
+
+int cfg802154_record_pan(struct wpan_phy *wpan_phy,
+			 struct ieee802154_pan_desc *desc)
+{
+	struct cfg802154_registered_device *rdev = wpan_phy_to_rdev(wpan_phy);
+	struct cfg802154_internal_pan *new;
+
+	new = cfg802154_alloc_pan(desc);
+	if (IS_ERR(new))
+		return (PTR_ERR(new));
+
+	cfg802154_pan_update(rdev, new);
+
+	return 0;
+}
+EXPORT_SYMBOL(cfg802154_record_pan);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH wpan-next 3/6] net: ieee802154: Create a node type
  2022-06-03 18:21 [PATCH wpan-next 0/6] net: ieee802154: PAN management Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 2/6] net: ieee802154: Add support for internal PAN management Miquel Raynal
@ 2022-06-03 18:21 ` Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 4/6] net: ieee802154: Add the PAN coordinator information Miquel Raynal
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-03 18:21 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
	David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, Miquel Raynal

A device can be either a fully functioning device or a kind of reduced
functioning device. Let's create a node type member. Drivers will be in
charge of setting this value if they handle non-FFD devices, which can
be considered the default for now.

Provide this information in the interface get netlink command.

Create a helper just to check if an rdev is a FFD or not, which will
then be useful when bringing scan support.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/nl802154.h    | 9 +++++++++
 net/ieee802154/core.h     | 7 +++++++
 net/ieee802154/nl802154.c | 6 +++++-
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index 0f508aaae126..8e4d8b7a6e24 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -133,6 +133,8 @@ enum nl802154_attrs {
 	NL802154_ATTR_PID,
 	NL802154_ATTR_NETNS_FD,
 
+	NL802154_ATTR_NODE_TYPE,
+
 	/* add attributes here, update the policy in nl802154.c */
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
@@ -162,6 +164,13 @@ enum nl802154_iftype {
 	NL802154_IFTYPE_MAX = NUM_NL802154_IFTYPES - 1
 };
 
+enum nl802154_node_type {
+	NL802154_NODE_TYPE_FFD,
+	NL802154_NODE_TYPE_RFD,
+	NL802154_NODE_TYPE_RFD_RX,
+	NL802154_NODE_TYPE_RFD_TX,
+};
+
 /**
  * enum nl802154_wpan_phy_capability_attr - wpan phy capability attributes
  *
diff --git a/net/ieee802154/core.h b/net/ieee802154/core.h
index cae071bede37..019309d6a3bb 100644
--- a/net/ieee802154/core.h
+++ b/net/ieee802154/core.h
@@ -23,6 +23,7 @@ struct cfg802154_registered_device {
 	int devlist_generation, wpan_dev_id;
 
 	/* PAN management */
+	enum nl802154_node_type node_type;
 	spinlock_t pan_lock;
 	struct list_head pan_list;
 	unsigned int max_pan_entries;
@@ -73,4 +74,10 @@ void cfg802154_set_pans_expiration(struct cfg802154_registered_device *rdev,
 void cfg802154_expire_pans(struct cfg802154_registered_device *rdev);
 void cfg802154_flush_pans(struct cfg802154_registered_device *rdev);
 
+static inline bool
+cfg802154_is_ffd(struct cfg802154_registered_device *rdev)
+{
+	return rdev->node_type == NL802154_NODE_TYPE_FFD;
+}
+
 #endif /* __IEEE802154_CORE_H */
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index e0b072aecf0f..10767c3b25d7 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -216,6 +216,9 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 
 	[NL802154_ATTR_PID] = { .type = NLA_U32 },
 	[NL802154_ATTR_NETNS_FD] = { .type = NLA_U32 },
+
+	[NL802154_ATTR_NODE_TYPE] = { .type = NLA_U8 },
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	[NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
 	[NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
@@ -790,7 +793,8 @@ nl802154_send_iface(struct sk_buff *msg, u32 portid, u32 seq, int flags,
 			      wpan_dev_id(wpan_dev), NL802154_ATTR_PAD) ||
 	    nla_put_u32(msg, NL802154_ATTR_GENERATION,
 			rdev->devlist_generation ^
-			(cfg802154_rdev_list_generation << 2)))
+			(cfg802154_rdev_list_generation << 2)) ||
+	    nla_put_u8(msg, NL802154_ATTR_NODE_TYPE, rdev->node_type))
 		goto nla_put_failure;
 
 	/* address settings */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH wpan-next 4/6] net: ieee802154: Add the PAN coordinator information
  2022-06-03 18:21 [PATCH wpan-next 0/6] net: ieee802154: PAN management Miquel Raynal
                   ` (2 preceding siblings ...)
  2022-06-03 18:21 ` [PATCH wpan-next 3/6] net: ieee802154: Create a node type Miquel Raynal
@ 2022-06-03 18:21 ` Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 5/6] net: ieee802154: Full PAN management Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 6/6] net: ieee802154: Trace the registration of new PANs Miquel Raynal
  5 siblings, 0 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-03 18:21 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
	David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, Miquel Raynal

We need to be able to differentiate between an FFD which acts as PAN
coordinator or not. For instance, in the former case, the device can
send beacons, otherwise not.

As no proper PAN creation exist yet, introduce a netlink command to
force that parameter. This could be dropped in the future when proper
PAN creation support gets added.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/nl802154.h    |  3 +++
 net/ieee802154/core.h     |  9 +++++++++
 net/ieee802154/nl802154.c | 31 ++++++++++++++++++++++++++++++-
 net/ieee802154/pan.c      | 12 ++++++++++++
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index 8e4d8b7a6e24..bc545d1f6f13 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -58,6 +58,8 @@ enum nl802154_commands {
 
 	NL802154_CMD_SET_WPAN_PHY_NETNS,
 
+	NL802154_CMD_SET_PAN_COORDINATOR_ROLE,
+
 	/* add new commands above here */
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
@@ -134,6 +136,7 @@ enum nl802154_attrs {
 	NL802154_ATTR_NETNS_FD,
 
 	NL802154_ATTR_NODE_TYPE,
+	NL802154_ATTR_PAN_COORDINATOR,
 
 	/* add attributes here, update the policy in nl802154.c */
 
diff --git a/net/ieee802154/core.h b/net/ieee802154/core.h
index 019309d6a3bb..dc240a9f830d 100644
--- a/net/ieee802154/core.h
+++ b/net/ieee802154/core.h
@@ -24,6 +24,7 @@ struct cfg802154_registered_device {
 
 	/* PAN management */
 	enum nl802154_node_type node_type;
+	bool is_pan_coordinator;
 	spinlock_t pan_lock;
 	struct list_head pan_list;
 	unsigned int max_pan_entries;
@@ -73,6 +74,8 @@ void cfg802154_set_pans_expiration(struct cfg802154_registered_device *rdev,
 				   unsigned int exp_time_s);
 void cfg802154_expire_pans(struct cfg802154_registered_device *rdev);
 void cfg802154_flush_pans(struct cfg802154_registered_device *rdev);
+int cfg802154_set_pan_coordinator_role(struct cfg802154_registered_device *rdev,
+				       bool is_pan_coordinator);
 
 static inline bool
 cfg802154_is_ffd(struct cfg802154_registered_device *rdev)
@@ -80,4 +83,10 @@ cfg802154_is_ffd(struct cfg802154_registered_device *rdev)
 	return rdev->node_type == NL802154_NODE_TYPE_FFD;
 }
 
+static inline bool
+cfg802154_is_pan_coordinator(struct cfg802154_registered_device *rdev)
+{
+	return cfg802154_is_ffd(rdev) && rdev->is_pan_coordinator;
+}
+
 #endif /* __IEEE802154_CORE_H */
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 10767c3b25d7..1e4f9b1e7362 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -218,6 +218,7 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 	[NL802154_ATTR_NETNS_FD] = { .type = NLA_U32 },
 
 	[NL802154_ATTR_NODE_TYPE] = { .type = NLA_U8 },
+	[NL802154_ATTR_PAN_COORDINATOR] = { .type = NLA_U8 },
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	[NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
@@ -794,7 +795,9 @@ nl802154_send_iface(struct sk_buff *msg, u32 portid, u32 seq, int flags,
 	    nla_put_u32(msg, NL802154_ATTR_GENERATION,
 			rdev->devlist_generation ^
 			(cfg802154_rdev_list_generation << 2)) ||
-	    nla_put_u8(msg, NL802154_ATTR_NODE_TYPE, rdev->node_type))
+	    nla_put_u8(msg, NL802154_ATTR_NODE_TYPE, rdev->node_type) ||
+	    nla_put_u8(msg, NL802154_ATTR_PAN_COORDINATOR,
+		       rdev->is_pan_coordinator))
 		goto nla_put_failure;
 
 	/* address settings */
@@ -1254,6 +1257,25 @@ nl802154_set_ackreq_default(struct sk_buff *skb, struct genl_info *info)
 	return rdev_set_ackreq_default(rdev, wpan_dev, ackreq);
 }
 
+static int nl802154_set_pan_coordinator_role(struct sk_buff *skb,
+					     struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	bool is_pan_coordinator;
+	int ret;
+
+	if (!info->attrs[NL802154_ATTR_PAN_COORDINATOR])
+		return -EINVAL;
+
+	is_pan_coordinator = nla_get_u8(info->attrs[NL802154_ATTR_PAN_COORDINATOR]);
+
+	spin_lock_bh(&rdev->pan_lock);
+	ret = cfg802154_set_pan_coordinator_role(rdev, is_pan_coordinator);
+	spin_unlock_bh(&rdev->pan_lock);
+
+	return ret;
+}
+
 static int nl802154_wpan_phy_netns(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg802154_registered_device *rdev = info->user_ptr[0];
@@ -2373,6 +2395,13 @@ static const struct genl_ops nl802154_ops[] = {
 		.internal_flags = NL802154_FLAG_NEED_NETDEV |
 				  NL802154_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL802154_CMD_SET_PAN_COORDINATOR_ROLE,
+		.doit = nl802154_set_pan_coordinator_role,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_NEED_RTNL,
+	},
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	{
 		.cmd = NL802154_CMD_SET_SEC_PARAMS,
diff --git a/net/ieee802154/pan.c b/net/ieee802154/pan.c
index b9f50f785960..7f48e3547b2f 100644
--- a/net/ieee802154/pan.c
+++ b/net/ieee802154/pan.c
@@ -19,6 +19,18 @@
 #include "ieee802154.h"
 #include "core.h"
 
+int cfg802154_set_pan_coordinator_role(struct cfg802154_registered_device *rdev,
+				       bool is_pan_coordinator)
+{
+	if (rdev->node_type != NL802154_NODE_TYPE_FFD)
+		return -EOPNOTSUPP;
+
+	rdev->is_pan_coordinator = is_pan_coordinator;
+
+	return 0;
+}
+EXPORT_SYMBOL(cfg802154_set_pan_coordinator_role);
+
 static struct cfg802154_internal_pan *
 cfg802154_alloc_pan(struct ieee802154_pan_desc *desc)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH wpan-next 5/6] net: ieee802154: Full PAN management
  2022-06-03 18:21 [PATCH wpan-next 0/6] net: ieee802154: PAN management Miquel Raynal
                   ` (3 preceding siblings ...)
  2022-06-03 18:21 ` [PATCH wpan-next 4/6] net: ieee802154: Add the PAN coordinator information Miquel Raynal
@ 2022-06-03 18:21 ` Miquel Raynal
  2022-06-03 18:21 ` [PATCH wpan-next 6/6] net: ieee802154: Trace the registration of new PANs Miquel Raynal
  5 siblings, 0 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-03 18:21 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
	David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, Miquel Raynal

Now that we have support for registering PANs, give certain rights to
the user, such as listing asynchronously the PANs as well as flushing
the list.

The maximum number of PANs to list and their delay before expiration can
be configured. By default there is no limit. When these parameters are
set, PANs are automatically dropped from the list.

This change has the side effect of moving the following helpers out of
the experimental zone as they are now used by non-experimental security
functions:
- nl802154_prepare_wpan_dev_dump()
- nl802154_finish_wpan_dev_dump()

Co-developed-by: David Girault <david.girault@qorvo.com>
Signed-off-by: David Girault <david.girault@qorvo.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/nl802154.h    |  48 ++++++++++
 net/ieee802154/nl802154.c | 197 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 243 insertions(+), 2 deletions(-)

diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index bc545d1f6f13..e4f2e7ee3ade 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -60,6 +60,11 @@ enum nl802154_commands {
 
 	NL802154_CMD_SET_PAN_COORDINATOR_ROLE,
 
+	NL802154_CMD_DUMP_PANS,
+	NL802154_CMD_FLUSH_PANS,
+	NL802154_CMD_SET_MAX_PAN_ENTRIES,
+	NL802154_CMD_SET_PANS_EXPIRATION,
+
 	/* add new commands above here */
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
@@ -137,6 +142,9 @@ enum nl802154_attrs {
 
 	NL802154_ATTR_NODE_TYPE,
 	NL802154_ATTR_PAN_COORDINATOR,
+	NL802154_ATTR_PAN,
+	NL802154_ATTR_MAX_PAN_ENTRIES,
+	NL802154_ATTR_PANS_EXPIRATION,
 
 	/* add attributes here, update the policy in nl802154.c */
 
@@ -229,6 +237,46 @@ enum nl802154_wpan_phy_capability_attr {
 	NL802154_CAP_ATTR_MAX = __NL802154_CAP_ATTR_AFTER_LAST - 1
 };
 
+/**
+ * enum nl802154_pan - Netlink attributes for a PAN
+ *
+ * @__NL802154_PAN_INVALID: invalid
+ * @NL802154_PAN_PANID: PANID of the PAN (2 bytes)
+ * @NL802154_PAN_COORD_ADDR: Coordinator address, (8 bytes or 2 bytes)
+ * @NL802154_PAN_CHANNEL: channel number, related to @NL802154_PAN_PAGE (u8)
+ * @NL802154_PAN_PAGE: channel page, related to @NL802154_PAN_CHANNEL (u8)
+ * @NL802154_PAN_PREAMBLE_CODE: Preamble code while the beacon was received,
+ *	this is PHY dependent and optional (4 bytes)
+ * @NL802154_PAN_SUPERFRAME_SPEC: superframe specification of the PAN (u16)
+ * @NL802154_PAN_LINK_QUALITY: signal quality of beacon in unspecified units,
+ *	scaled to 0..255 (u8)
+ * @NL802154_PAN_GTS_PERMIT: set to true if GTS is permitted on this PAN
+ * @NL802154_PAN_PAYLOAD_DATA: binary data containing the raw data from the
+ *	frame payload, (only if beacon or probe response had data)
+ * @NL802154_PAN_STATUS: status, if this PAN is "used"
+ * @NL802154_PAN_SEEN_MS_AGO: age of this PAN entry in ms
+ * @NL802154_PAN_PAD: attribute used for padding for 64-bit alignment
+ * @NL802154_PAN_MAX: highest PAN attribute
+ */
+enum nl802154_pan {
+	__NL802154_PAN_INVALID,
+	NL802154_PAN_PANID,
+	NL802154_PAN_COORD_ADDR,
+	NL802154_PAN_CHANNEL,
+	NL802154_PAN_PAGE,
+	NL802154_PAN_PREAMBLE_CODE,
+	NL802154_PAN_SUPERFRAME_SPEC,
+	NL802154_PAN_LINK_QUALITY,
+	NL802154_PAN_GTS_PERMIT,
+	NL802154_PAN_PAYLOAD_DATA,
+	NL802154_PAN_STATUS,
+	NL802154_PAN_SEEN_MS_AGO,
+	NL802154_PAN_PAD,
+
+	/* keep last */
+	NL802154_PAN_MAX,
+};
+
 /**
  * enum nl802154_cca_modes - cca modes
  *
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 1e4f9b1e7362..61c167cebe4b 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -219,6 +219,9 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 
 	[NL802154_ATTR_NODE_TYPE] = { .type = NLA_U8 },
 	[NL802154_ATTR_PAN_COORDINATOR] = { .type = NLA_U8 },
+	[NL802154_ATTR_PAN] = { .type = NLA_NESTED },
+	[NL802154_ATTR_MAX_PAN_ENTRIES] = { .type = NLA_U32 },
+	[NL802154_ATTR_PANS_EXPIRATION] = { .type = NLA_U32 },
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	[NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
@@ -233,7 +236,6 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */
 };
 
-#ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 static int
 nl802154_prepare_wpan_dev_dump(struct sk_buff *skb,
 			       struct netlink_callback *cb,
@@ -292,7 +294,6 @@ nl802154_finish_wpan_dev_dump(struct cfg802154_registered_device *rdev)
 {
 	rtnl_unlock();
 }
-#endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */
 
 /* message building helper */
 static inline void *nl802154hdr_put(struct sk_buff *skb, u32 portid, u32 seq,
@@ -1307,6 +1308,172 @@ static int nl802154_wpan_phy_netns(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+static int nl802154_send_pan_info(struct sk_buff *msg,
+				  struct netlink_callback *cb,
+				  u32 seq, int flags,
+				  struct cfg802154_registered_device *rdev,
+				  struct wpan_dev *wpan_dev,
+				  struct cfg802154_internal_pan *intpan)
+{
+	struct ieee802154_pan_desc *pan = &intpan->desc;
+	struct nlattr *nla;
+	void *hdr;
+
+	ASSERT_RTNL();
+
+	hdr = nl802154hdr_put(msg, NETLINK_CB(cb->skb).portid, seq, flags,
+			      NL802154_CMD_DUMP_PANS);
+	if (!hdr)
+		return -ENOBUFS;
+
+	genl_dump_check_consistent(cb, hdr);
+
+	if (nla_put_u32(msg, NL802154_ATTR_GENERATION, rdev->pan_generation))
+		goto nla_put_failure;
+
+	if (wpan_dev->netdev &&
+	    nla_put_u32(msg, NL802154_ATTR_IFINDEX, wpan_dev->netdev->ifindex))
+		goto nla_put_failure;
+
+	if (nla_put_u64_64bit(msg, NL802154_ATTR_WPAN_DEV, wpan_dev_id(wpan_dev),
+			      NL802154_ATTR_PAD))
+		goto nla_put_failure;
+
+	nla = nla_nest_start_noflag(msg, NL802154_ATTR_PAN);
+	if (!nla)
+		goto nla_put_failure;
+
+	if (nla_put(msg, NL802154_PAN_PANID, IEEE802154_PAN_ID_LEN,
+		    &pan->coord->pan_id))
+		goto nla_put_failure;
+
+	if (pan->coord->mode == IEEE802154_ADDR_SHORT) {
+		if (nla_put(msg, NL802154_PAN_COORD_ADDR,
+			    IEEE802154_SHORT_ADDR_LEN,
+			    &pan->coord->short_addr))
+			goto nla_put_failure;
+	} else {
+		if (nla_put(msg, NL802154_PAN_COORD_ADDR,
+			    IEEE802154_EXTENDED_ADDR_LEN,
+			    &pan->coord->extended_addr))
+			goto nla_put_failure;
+	}
+
+	if (nla_put_u8(msg, NL802154_PAN_CHANNEL, pan->channel))
+		goto nla_put_failure;
+
+	if (nla_put_u8(msg, NL802154_PAN_PAGE, pan->page))
+		goto nla_put_failure;
+
+	if (nla_put_u16(msg, NL802154_PAN_SUPERFRAME_SPEC,
+			pan->superframe_spec))
+		goto nla_put_failure;
+
+	if (nla_put_u8(msg, NL802154_PAN_LINK_QUALITY, pan->link_quality))
+		goto nla_put_failure;
+
+	if (nla_put_u32(msg, NL802154_PAN_SEEN_MS_AGO,
+			jiffies_to_msecs(jiffies - intpan->discovery_ts)))
+		goto nla_put_failure;
+
+	if (pan->gts_permit && nla_put_flag(msg, NL802154_PAN_GTS_PERMIT))
+		goto nla_put_failure;
+
+	/* TODO: NL802154_PAN_PAYLOAD_DATA if any */
+
+	nla_nest_end(msg, nla);
+	genlmsg_end(msg, hdr);
+
+	return 0;
+
+ nla_put_failure:
+	genlmsg_cancel(msg, hdr);
+	return -EMSGSIZE;
+}
+
+static int nl802154_dump_pans(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct cfg802154_registered_device *rdev;
+	struct cfg802154_internal_pan *pan;
+	struct wpan_dev *wpan_dev;
+	int err;
+
+	err = nl802154_prepare_wpan_dev_dump(skb, cb, &rdev, &wpan_dev);
+	if (err)
+		return err;
+
+	spin_lock_bh(&rdev->pan_lock);
+
+	if (cb->args[2])
+		goto out;
+
+	cb->seq = rdev->pan_generation;
+
+	ieee802154_for_each_pan(pan, rdev) {
+		err = nl802154_send_pan_info(skb, cb, cb->nlh->nlmsg_seq,
+					     NLM_F_MULTI, rdev, wpan_dev, pan);
+		if (err < 0)
+			goto out_err;
+	}
+
+	cb->args[2] = 1;
+out:
+	err = skb->len;
+out_err:
+	spin_unlock_bh(&rdev->pan_lock);
+
+	nl802154_finish_wpan_dev_dump(rdev);
+
+	return err;
+}
+
+static int nl802154_flush_pans(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+
+	spin_lock_bh(&rdev->pan_lock);
+	cfg802154_flush_pans(rdev);
+	spin_unlock_bh(&rdev->pan_lock);
+
+	return 0;
+}
+
+static int nl802154_set_max_pan_entries(struct sk_buff *skb,
+					struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	unsigned int max_entries;
+
+	if (!info->attrs[NL802154_ATTR_MAX_PAN_ENTRIES])
+		return -EINVAL;
+
+	max_entries = nla_get_u32(info->attrs[NL802154_ATTR_MAX_PAN_ENTRIES]);
+
+	spin_lock_bh(&rdev->pan_lock);
+	cfg802154_set_max_pan_entries(rdev, max_entries);
+	spin_unlock_bh(&rdev->pan_lock);
+
+	return 0;
+}
+
+static int nl802154_set_pans_expiration(struct sk_buff *skb,
+					struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	unsigned int exp_time_s;
+
+	if (!info->attrs[NL802154_ATTR_PANS_EXPIRATION])
+		return -EINVAL;
+
+	exp_time_s = nla_get_u32(info->attrs[NL802154_ATTR_PANS_EXPIRATION]);
+
+	spin_lock_bh(&rdev->pan_lock);
+	cfg802154_set_pans_expiration(rdev, exp_time_s);
+	spin_unlock_bh(&rdev->pan_lock);
+
+	return 0;
+}
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 static const struct nla_policy nl802154_dev_addr_policy[NL802154_DEV_ADDR_ATTR_MAX + 1] = {
 	[NL802154_DEV_ADDR_ATTR_PAN_ID] = { .type = NLA_U16 },
@@ -2402,6 +2569,32 @@ static const struct genl_ops nl802154_ops[] = {
 		.internal_flags = NL802154_FLAG_NEED_NETDEV |
 				  NL802154_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL802154_CMD_DUMP_PANS,
+		.dumpit = nl802154_dump_pans,
+		/* can be retrieved by unprivileged users */
+	},
+	{
+		.cmd = NL802154_CMD_FLUSH_PANS,
+		.doit = nl802154_flush_pans,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_NEED_RTNL,
+	},
+	{
+		.cmd = NL802154_CMD_SET_MAX_PAN_ENTRIES,
+		.doit = nl802154_set_max_pan_entries,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_NEED_RTNL,
+	},
+	{
+		.cmd = NL802154_CMD_SET_PANS_EXPIRATION,
+		.doit = nl802154_set_pans_expiration,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_NEED_RTNL,
+	},
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	{
 		.cmd = NL802154_CMD_SET_SEC_PARAMS,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH wpan-next 6/6] net: ieee802154: Trace the registration of new PANs
  2022-06-03 18:21 [PATCH wpan-next 0/6] net: ieee802154: PAN management Miquel Raynal
                   ` (4 preceding siblings ...)
  2022-06-03 18:21 ` [PATCH wpan-next 5/6] net: ieee802154: Full PAN management Miquel Raynal
@ 2022-06-03 18:21 ` Miquel Raynal
  5 siblings, 0 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-03 18:21 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
	David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, Miquel Raynal

From: David Girault <david.girault@qorvo.com>

Add an internal trace when new PANs get discovered.

Signed-off-by: David Girault <david.girault@qorvo.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 net/ieee802154/pan.c   |  3 +++
 net/ieee802154/trace.h | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/net/ieee802154/pan.c b/net/ieee802154/pan.c
index 7f48e3547b2f..abdb5bcf40ff 100644
--- a/net/ieee802154/pan.c
+++ b/net/ieee802154/pan.c
@@ -18,6 +18,7 @@
 
 #include "ieee802154.h"
 #include "core.h"
+#include "trace.h"
 
 int cfg802154_set_pan_coordinator_role(struct cfg802154_registered_device *rdev,
 				       bool is_pan_coordinator)
@@ -217,6 +218,8 @@ static void cfg802154_pan_update(struct cfg802154_registered_device *rdev,
 	found = cfg802154_find_matching_pan(rdev, new);
 	if (found)
 		cfg802154_unlink_pan(rdev, found);
+	else
+		trace_802154_new_pan(&new->desc);
 
 	if (unlikely(cfg802154_need_to_expire_pans(rdev)))
 		cfg802154_expire_oldest_pan(rdev);
diff --git a/net/ieee802154/trace.h b/net/ieee802154/trace.h
index 19c2e5d60e76..fa989dac090d 100644
--- a/net/ieee802154/trace.h
+++ b/net/ieee802154/trace.h
@@ -295,6 +295,31 @@ TRACE_EVENT(802154_rdev_set_ackreq_default,
 		WPAN_DEV_PR_ARG, BOOL_TO_STR(__entry->ackreq))
 );
 
+DECLARE_EVENT_CLASS(802154_pan_evt,
+	TP_PROTO(struct ieee802154_pan_desc *desc),
+	TP_ARGS(desc),
+	TP_STRUCT__entry(
+		__field(u16, pan_id)
+		__field(__le64, coord_addr)
+		__field(u8, channel)
+		__field(u8, page)
+	),
+	TP_fast_assign(
+		__entry->page = desc->page;
+		__entry->channel = desc->channel;
+		memcpy(&__entry->pan_id, &desc->coord->pan_id, 2);
+		memcpy(&__entry->coord_addr, &desc->coord->extended_addr, 8);
+	),
+	TP_printk("panid: %u, coord_addr: 0x%llx, page: %u, channel: %u",
+		  __entry->pan_id, __le64_to_cpu(__entry->coord_addr),
+		  __entry->page, __entry->channel)
+);
+
+DEFINE_EVENT(802154_pan_evt, 802154_new_pan,
+	TP_PROTO(struct ieee802154_pan_desc *desc),
+	TP_ARGS(desc)
+);
+
 TRACE_EVENT(802154_rdev_return_int,
 	TP_PROTO(struct wpan_phy *wpan_phy, int ret),
 	TP_ARGS(wpan_phy, ret),
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-03 18:21 ` [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type Miquel Raynal
@ 2022-06-04  2:01   ` Alexander Aring
  2022-06-06 15:43     ` Miquel Raynal
  0 siblings, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-04  2:01 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi,

On Fri, Jun 3, 2022 at 2:34 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> The current enum is wrong. A device can either be an RFD, an RFD-RX, an
> RFD-TX or an FFD. If it is an FFD, it can also be a coordinator. While
> defining a node type might make sense from a strict software point of
> view, opposing node and coordinator seems meaningless in the ieee
> 802.15.4 world. As this enumeration is not used anywhere, let's just
> drop it. We will in a second time add a new "node type" enumeration
> which apply only to nodes, and does differentiates the type of devices
> mentioned above.
>

First you cannot say if this is not used anywhere else. Second I have
a different opinion here that you cannot just "switch" the role from
RFD, FFD, whatever.
You are mixing things here with "role in the network" and what the
transceiver capability (RFD, FFD) is, which are two different things.

You should use those defines and the user needs to create a new
interface type and probably have a different extended address to act
as a coordinator.

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-04  2:01   ` Alexander Aring
@ 2022-06-06 15:43     ` Miquel Raynal
  2022-06-07  3:04       ` Alexander Aring
  0 siblings, 1 reply; 29+ messages in thread
From: Miquel Raynal @ 2022-06-06 15:43 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi Alexander,

aahringo@redhat.com wrote on Fri, 3 Jun 2022 22:01:38 -0400:

> Hi,
> 
> On Fri, Jun 3, 2022 at 2:34 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > The current enum is wrong. A device can either be an RFD, an RFD-RX, an
> > RFD-TX or an FFD. If it is an FFD, it can also be a coordinator. While
> > defining a node type might make sense from a strict software point of
> > view, opposing node and coordinator seems meaningless in the ieee
> > 802.15.4 world. As this enumeration is not used anywhere, let's just
> > drop it. We will in a second time add a new "node type" enumeration
> > which apply only to nodes, and does differentiates the type of devices
> > mentioned above.
> >  
> 
> First you cannot say if this is not used anywhere else.

Mmmh, that's tricky, I really don't see how that might be a
problem because there is literally nowhere in the kernel that uses this
type, besides ieee802154_setup_sdata() which would just BUG() if this
type was to be used. So I assumed it was safe to be removed.

> Second I have
> a different opinion here that you cannot just "switch" the role from
> RFD, FFD, whatever.

I agree with this, and that's why I don't understand this enum.

A device can either be a NODE (an active device) or a MONITOR (a
passive device) at a time. We can certainly switch from one to
another at run time.

A NODE can be either an RFD or an FFD. That is a static property which
cannot change.

However being a coordinator is just an additional property of a NODE
which is of type FFD, and this can change over time.

So I don't get what having a coordinator interface would bring. What
was the idea behind its introduction then?

> You are mixing things here with "role in the network" and what the
> transceiver capability (RFD, FFD) is, which are two different things.

I don't think I am, however maybe our vision differ on what an
interface should be.

> You should use those defines and the user needs to create a new
> interface type and probably have a different extended address to act
> as a coordinator.

Can't we just simply switch from coordinator to !coordinator (that's
what I currently implemented)? Why would we need the user to create a
new interface type *and* to provide a new address?

Note that these are real questions that I am asking myself. I'm fine
adapting my implementation, as long as I get the main idea.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-06 15:43     ` Miquel Raynal
@ 2022-06-07  3:04       ` Alexander Aring
  2022-06-07 16:16         ` Miquel Raynal
  0 siblings, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-07  3:04 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi,

On Mon, Jun 6, 2022 at 11:43 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Fri, 3 Jun 2022 22:01:38 -0400:
>
> > Hi,
> >
> > On Fri, Jun 3, 2022 at 2:34 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > The current enum is wrong. A device can either be an RFD, an RFD-RX, an
> > > RFD-TX or an FFD. If it is an FFD, it can also be a coordinator. While
> > > defining a node type might make sense from a strict software point of
> > > view, opposing node and coordinator seems meaningless in the ieee
> > > 802.15.4 world. As this enumeration is not used anywhere, let's just
> > > drop it. We will in a second time add a new "node type" enumeration
> > > which apply only to nodes, and does differentiates the type of devices
> > > mentioned above.
> > >
> >
> > First you cannot say if this is not used anywhere else.
>
> Mmmh, that's tricky, I really don't see how that might be a
> problem because there is literally nowhere in the kernel that uses this
> type, besides ieee802154_setup_sdata() which would just BUG() if this
> type was to be used. So I assumed it was safe to be removed.
>

this header is somehow half uapi where we copy it into some other
software e.g. wpan-tools as you noticed.

> > Second I have
> > a different opinion here that you cannot just "switch" the role from
> > RFD, FFD, whatever.
>
> I agree with this, and that's why I don't understand this enum.
>
> A device can either be a NODE (an active device) or a MONITOR (a
> passive device) at a time. We can certainly switch from one to
> another at run time.
>
> A NODE can be either an RFD or an FFD. That is a static property which
> cannot change.
>
> However being a coordinator is just an additional property of a NODE
> which is of type FFD, and this can change over time.
>
> So I don't get what having a coordinator interface would bring. What
> was the idea behind its introduction then?
>

There exists arguments which I have in my mind right now:

1. frame parsing/address filter (which is somehow missing in your patches)

The parsing of frames is different from other types (just as monitor
interfaces). You will notice that setting up the address filter will
require a parameter if coordinator or not. Changing the address
filterung during runtime of an interface is somehow _not_ supported.
The reason is that the datasheets tell you to first set up an address
filter and then switch into receiving mode. Changing the address
filter during receive mode (start()/stop()) is not a specified
behaviour. Due to bus communication it also cannot be done atomically.
This might be avoidable but is a pain to synchronize if you really
depend on hw address filtering which we might do in future. It should
end in a different receiving path e.g. node_rx/monitor_rx.

2. HardMAC transceivers

The add_interface() callback will be directly forwarded to the driver
and the driver will during the lifetime of this interface act as a
coordinator and not a mixed mode which can be disabled and enabled
anytime. I am not even sure if this can ever be handled in such a way
from hardmac transceivers, it might depend on the transceiver
interface but we should assume some strict "static" handling. Static
handling means here that the transceiver is unable to switch from
coordinator and vice versa after some initialization state.

3. coordinator (any $TYPE specific) userspace software

May the main argument. Some coordinator specific user space daemon
does specific type handling (e.g. hostapd) maybe because some library
is required. It is a pain to deal with changing roles during the
lifetime of an interface and synchronize user space software with it.
We should keep in mind that some of those handlings will maybe be
moved to user space instead of doing it in the kernel. I am fine with
the solution now, but keep in mind to offer such a possibility.

I think the above arguments are probably the same why wireless is
doing something similar and I would avoid running into issues or it's
really difficult to handle because you need to solve other Linux net
architecture handling at first.

> > You are mixing things here with "role in the network" and what the
> > transceiver capability (RFD, FFD) is, which are two different things.
>
> I don't think I am, however maybe our vision differ on what an
> interface should be.
>
> > You should use those defines and the user needs to create a new
> > interface type and probably have a different extended address to act
> > as a coordinator.
>
> Can't we just simply switch from coordinator to !coordinator (that's
> what I currently implemented)? Why would we need the user to create a
> new interface type *and* to provide a new address?
>
> Note that these are real questions that I am asking myself. I'm fine
> adapting my implementation, as long as I get the main idea.
>

See above.

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-07  3:04       ` Alexander Aring
@ 2022-06-07 16:16         ` Miquel Raynal
  2022-06-08 13:47           ` Miquel Raynal
  2022-06-09  1:42           ` Alexander Aring
  0 siblings, 2 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-07 16:16 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi Alex,

aahringo@redhat.com wrote on Mon, 6 Jun 2022 23:04:06 -0400:

> Hi,
> 
> On Mon, Jun 6, 2022 at 11:43 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Fri, 3 Jun 2022 22:01:38 -0400:
> >  
> > > Hi,
> > >
> > > On Fri, Jun 3, 2022 at 2:34 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > The current enum is wrong. A device can either be an RFD, an RFD-RX, an
> > > > RFD-TX or an FFD. If it is an FFD, it can also be a coordinator. While
> > > > defining a node type might make sense from a strict software point of
> > > > view, opposing node and coordinator seems meaningless in the ieee
> > > > 802.15.4 world. As this enumeration is not used anywhere, let's just
> > > > drop it. We will in a second time add a new "node type" enumeration
> > > > which apply only to nodes, and does differentiates the type of devices
> > > > mentioned above.
> > > >  
> > >
> > > First you cannot say if this is not used anywhere else.  
> >
> > Mmmh, that's tricky, I really don't see how that might be a
> > problem because there is literally nowhere in the kernel that uses this
> > type, besides ieee802154_setup_sdata() which would just BUG() if this
> > type was to be used. So I assumed it was safe to be removed.
> >  
> 
> this header is somehow half uapi where we copy it into some other
> software e.g. wpan-tools as you noticed.
> 
> > > Second I have
> > > a different opinion here that you cannot just "switch" the role from
> > > RFD, FFD, whatever.  
> >
> > I agree with this, and that's why I don't understand this enum.
> >
> > A device can either be a NODE (an active device) or a MONITOR (a
> > passive device) at a time. We can certainly switch from one to
> > another at run time.
> >
> > A NODE can be either an RFD or an FFD. That is a static property which
> > cannot change.
> >
> > However being a coordinator is just an additional property of a NODE
> > which is of type FFD, and this can change over time.
> >
> > So I don't get what having a coordinator interface would bring. What
> > was the idea behind its introduction then?
> >  
> 
> There exists arguments which I have in my mind right now:
> 
> 1. frame parsing/address filter (which is somehow missing in your patches)
> 
> The parsing of frames is different from other types (just as monitor
> interfaces). You will notice that setting up the address filter will
> require a parameter if coordinator or not.

I think this is something that I completely missed until now, can you
point me to where/how this is expected to be done? I don't see anything
wpan specific filtering implementation. What is expected on this area
and is there code that I missed already?

> Changing the address
> filterung during runtime of an interface is somehow _not_ supported.
> The reason is that the datasheets tell you to first set up an address
> filter and then switch into receiving mode. Changing the address
> filter during receive mode (start()/stop()) is not a specified
> behaviour. Due to bus communication it also cannot be done atomically.
> This might be avoidable but is a pain to synchronize if you really
> depend on hw address filtering which we might do in future. It should
> end in a different receiving path e.g. node_rx/monitor_rx.

Got it.

> 
> 2. HardMAC transceivers
> 
> The add_interface() callback will be directly forwarded to the driver
> and the driver will during the lifetime of this interface act as a
> coordinator and not a mixed mode which can be disabled and enabled
> anytime. I am not even sure if this can ever be handled in such a way
> from hardmac transceivers, it might depend on the transceiver
> interface but we should assume some strict "static" handling. Static
> handling means here that the transceiver is unable to switch from
> coordinator and vice versa after some initialization state.

Okay. I just completely missed the "interface add" command. So your
advice is to treat the "PAN coordinator" property as a static property
for a given interface, which seems reasonable.

For now I will assume the same treatment when adding the interface is
required compared to a NODE, but if something comes to your mind,
please let me know.

By the way, is there a mechanism limiting the number of interfaces on a
device? Should we prevent the introduction of a coordinator iface if
there is a node iface active?

> 3. coordinator (any $TYPE specific) userspace software
> 
> May the main argument. Some coordinator specific user space daemon
> does specific type handling (e.g. hostapd) maybe because some library
> is required. It is a pain to deal with changing roles during the
> lifetime of an interface and synchronize user space software with it.
> We should keep in mind that some of those handlings will maybe be
> moved to user space instead of doing it in the kernel. I am fine with
> the solution now, but keep in mind to offer such a possibility.
> 
> I think the above arguments are probably the same why wireless is
> doing something similar and I would avoid running into issues or it's
> really difficult to handle because you need to solve other Linux net
> architecture handling at first.

Yep.

> > > You are mixing things here with "role in the network" and what the
> > > transceiver capability (RFD, FFD) is, which are two different things.  
> >
> > I don't think I am, however maybe our vision differ on what an
> > interface should be.
> >  
> > > You should use those defines and the user needs to create a new
> > > interface type and probably have a different extended address to act
> > > as a coordinator.  
> >
> > Can't we just simply switch from coordinator to !coordinator (that's
> > what I currently implemented)? Why would we need the user to create a
> > new interface type *and* to provide a new address?
> >
> > Note that these are real questions that I am asking myself. I'm fine
> > adapting my implementation, as long as I get the main idea.
> >  
> 
> See above.

That's okay for me. I will adapt my implementation to use the interface
thing. In the mean time additional details about what a coordinator
interface should do differently (above question) is welcome because
this is not something I am really comfortable with.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-07 16:16         ` Miquel Raynal
@ 2022-06-08 13:47           ` Miquel Raynal
  2022-06-08 14:37             ` Miquel Raynal
  2022-06-09  1:56             ` Alexander Aring
  2022-06-09  1:42           ` Alexander Aring
  1 sibling, 2 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-08 13:47 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi Alex,

> > 3. coordinator (any $TYPE specific) userspace software
> > 
> > May the main argument. Some coordinator specific user space daemon
> > does specific type handling (e.g. hostapd) maybe because some library
> > is required. It is a pain to deal with changing roles during the
> > lifetime of an interface and synchronize user space software with it.
> > We should keep in mind that some of those handlings will maybe be
> > moved to user space instead of doing it in the kernel. I am fine with
> > the solution now, but keep in mind to offer such a possibility.
> > 
> > I think the above arguments are probably the same why wireless is
> > doing something similar and I would avoid running into issues or it's
> > really difficult to handle because you need to solve other Linux net
> > architecture handling at first.  
> 
> Yep.

The spec makes a difference between "coordinator" and "PAN
coordinator", which one is the "coordinator" interface type supposed to
picture? I believe we are talking about being a "PAN coordinator", but
I want to be sure that we are aligned on the terms.

> > > > You are mixing things here with "role in the network" and what
> > > > the transceiver capability (RFD, FFD) is, which are two
> > > > different things.    
> > >
> > > I don't think I am, however maybe our vision differ on what an
> > > interface should be.
> > >    
> > > > You should use those defines and the user needs to create a new
> > > > interface type and probably have a different extended address
> > > > to act as a coordinator.    
> > >
> > > Can't we just simply switch from coordinator to !coordinator
> > > (that's what I currently implemented)? Why would we need the user
> > > to create a new interface type *and* to provide a new address?
> > >
> > > Note that these are real questions that I am asking myself. I'm
> > > fine adapting my implementation, as long as I get the main idea.
> > >    
> > 
> > See above.  
> 
> That's okay for me. I will adapt my implementation to use the
> interface thing. In the mean time additional details about what a
> coordinator interface should do differently (above question) is
> welcome because this is not something I am really comfortable with.

I've updated the implementation to use the IFACE_COORD interface and it
works fine, besides one question below.

Also, I read the spec once again (soon I'll sleep with it) and
actually what I extracted is that:

* A FFD, when turned on, will perform a scan, then associate to any PAN
  it found (algorithm is beyond the spec) or otherwise create a PAN ID
  and start its own PAN. In both cases, it finishes its setup by
  starting to send beacons.

* A RFD will behave more or less the same, without the PAN creation
  possibility of course. RFD-RX and RFD-TX are not required to support
  any of that, I'll assume none of the scanning features is suitable
  for them.

I have a couple of questions however:

- Creating an interface (let's call it wpancoord) out of wpan0 means
  that two interfaces can be used in different ways and one can use
  wpan0 as a node while using wpancoord as a PAN coordinator. Is that
  really allowed? How should we prevent this from happening?

- Should the device always wait for the user(space) to provide the PAN
  to associate to after the scan procedure right after the
  add_interface()? (like an information that must be provided prior to
  set the interface up?)

- How does an orphan FFD should pick the PAN ID for a PAN creation?
  Should we use a random number? Start from 0 upwards? Start from
  0xfffd downwards? Should the user always provide it?

- Should an FFD be able to create its own PAN on demand? Shall we
  allow to do that at the creation of the new interface?

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-08 13:47           ` Miquel Raynal
@ 2022-06-08 14:37             ` Miquel Raynal
  2022-06-09  2:06               ` Alexander Aring
  2022-06-09  1:56             ` Alexander Aring
  1 sibling, 1 reply; 29+ messages in thread
From: Miquel Raynal @ 2022-06-08 14:37 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni


miquel.raynal@bootlin.com wrote on Wed, 8 Jun 2022 15:47:49 +0200:

> Hi Alex,
> 
> > > 3. coordinator (any $TYPE specific) userspace software
> > > 
> > > May the main argument. Some coordinator specific user space daemon
> > > does specific type handling (e.g. hostapd) maybe because some library
> > > is required. It is a pain to deal with changing roles during the
> > > lifetime of an interface and synchronize user space software with it.
> > > We should keep in mind that some of those handlings will maybe be
> > > moved to user space instead of doing it in the kernel. I am fine with
> > > the solution now, but keep in mind to offer such a possibility.
> > > 
> > > I think the above arguments are probably the same why wireless is
> > > doing something similar and I would avoid running into issues or it's
> > > really difficult to handle because you need to solve other Linux net
> > > architecture handling at first.    
> > 
> > Yep.  
> 
> The spec makes a difference between "coordinator" and "PAN
> coordinator", which one is the "coordinator" interface type supposed to
> picture? I believe we are talking about being a "PAN coordinator", but
> I want to be sure that we are aligned on the terms.
> 
> > > > > You are mixing things here with "role in the network" and what
> > > > > the transceiver capability (RFD, FFD) is, which are two
> > > > > different things.      
> > > >
> > > > I don't think I am, however maybe our vision differ on what an
> > > > interface should be.
> > > >      
> > > > > You should use those defines and the user needs to create a new
> > > > > interface type and probably have a different extended address
> > > > > to act as a coordinator.      
> > > >
> > > > Can't we just simply switch from coordinator to !coordinator
> > > > (that's what I currently implemented)? Why would we need the user
> > > > to create a new interface type *and* to provide a new address?
> > > >
> > > > Note that these are real questions that I am asking myself. I'm
> > > > fine adapting my implementation, as long as I get the main idea.
> > > >      
> > > 
> > > See above.    
> > 
> > That's okay for me. I will adapt my implementation to use the
> > interface thing. In the mean time additional details about what a
> > coordinator interface should do differently (above question) is
> > welcome because this is not something I am really comfortable with.  
> 
> I've updated the implementation to use the IFACE_COORD interface and it
> works fine, besides one question below.
> 
> Also, I read the spec once again (soon I'll sleep with it) and
> actually what I extracted is that:
> 
> * A FFD, when turned on, will perform a scan, then associate to any PAN
>   it found (algorithm is beyond the spec) or otherwise create a PAN ID
>   and start its own PAN. In both cases, it finishes its setup by
>   starting to send beacons.
> 
> * A RFD will behave more or less the same, without the PAN creation
>   possibility of course. RFD-RX and RFD-TX are not required to support
>   any of that, I'll assume none of the scanning features is suitable
>   for them.

Acutally, RFDs do not send beacons, AFAIU.

> I have a couple of questions however:
> 
> - Creating an interface (let's call it wpancoord) out of wpan0 means
>   that two interfaces can be used in different ways and one can use
>   wpan0 as a node while using wpancoord as a PAN coordinator. Is that
>   really allowed? How should we prevent this from happening?
> 
> - Should the device always wait for the user(space) to provide the PAN
>   to associate to after the scan procedure right after the
>   add_interface()? (like an information that must be provided prior to
>   set the interface up?)
> 
> - How does an orphan FFD should pick the PAN ID for a PAN creation?
>   Should we use a random number? Start from 0 upwards? Start from
>   0xfffd downwards? Should the user always provide it?
> 
> - Should an FFD be able to create its own PAN on demand? Shall we
>   allow to do that at the creation of the new interface?

Additional questions:

  - How is chosen the beacon order? Should we have a default value?
    Should we default to 15 (not on a beacon enabled PAN)? Should we be
    able to update this value during the lifetime of the PAN?

  - The spec talks about the cluster topology, where a coordinator that
    just associated to a PAN starts emitting beacons, which may enable
    other devices in its range to ask to join the PAN (increased area
    coverage). But then, there is no information about how the newly
    added device should do to join the PAN coordinator which is anyway
    out of range to require the association, transmit data, etc. Any
    idea how this is supposed to work?

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-07 16:16         ` Miquel Raynal
  2022-06-08 13:47           ` Miquel Raynal
@ 2022-06-09  1:42           ` Alexander Aring
  2022-06-09 14:42             ` Miquel Raynal
  1 sibling, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-09  1:42 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi,

On Tue, Jun 7, 2022 at 12:16 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alex,
>
> aahringo@redhat.com wrote on Mon, 6 Jun 2022 23:04:06 -0400:
>
> > Hi,
> >
> > On Mon, Jun 6, 2022 at 11:43 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > aahringo@redhat.com wrote on Fri, 3 Jun 2022 22:01:38 -0400:
> > >
> > > > Hi,
> > > >
> > > > On Fri, Jun 3, 2022 at 2:34 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > >
> > > > > The current enum is wrong. A device can either be an RFD, an RFD-RX, an
> > > > > RFD-TX or an FFD. If it is an FFD, it can also be a coordinator. While
> > > > > defining a node type might make sense from a strict software point of
> > > > > view, opposing node and coordinator seems meaningless in the ieee
> > > > > 802.15.4 world. As this enumeration is not used anywhere, let's just
> > > > > drop it. We will in a second time add a new "node type" enumeration
> > > > > which apply only to nodes, and does differentiates the type of devices
> > > > > mentioned above.
> > > > >
> > > >
> > > > First you cannot say if this is not used anywhere else.
> > >
> > > Mmmh, that's tricky, I really don't see how that might be a
> > > problem because there is literally nowhere in the kernel that uses this
> > > type, besides ieee802154_setup_sdata() which would just BUG() if this
> > > type was to be used. So I assumed it was safe to be removed.
> > >
> >
> > this header is somehow half uapi where we copy it into some other
> > software e.g. wpan-tools as you noticed.
> >
> > > > Second I have
> > > > a different opinion here that you cannot just "switch" the role from
> > > > RFD, FFD, whatever.
> > >
> > > I agree with this, and that's why I don't understand this enum.
> > >
> > > A device can either be a NODE (an active device) or a MONITOR (a
> > > passive device) at a time. We can certainly switch from one to
> > > another at run time.
> > >
> > > A NODE can be either an RFD or an FFD. That is a static property which
> > > cannot change.
> > >
> > > However being a coordinator is just an additional property of a NODE
> > > which is of type FFD, and this can change over time.
> > >
> > > So I don't get what having a coordinator interface would bring. What
> > > was the idea behind its introduction then?
> > >
> >
> > There exists arguments which I have in my mind right now:
> >
> > 1. frame parsing/address filter (which is somehow missing in your patches)
> >
> > The parsing of frames is different from other types (just as monitor
> > interfaces). You will notice that setting up the address filter will
> > require a parameter if coordinator or not.
>
> I think this is something that I completely missed until now, can you
> point me to where/how this is expected to be done? I don't see anything
> wpan specific filtering implementation. What is expected on this area
> and is there code that I missed already?
>

https://elixir.bootlin.com/linux/v5.19-rc1/source/net/mac802154/rx.c#L284

> > Changing the address
> > filterung during runtime of an interface is somehow _not_ supported.
> > The reason is that the datasheets tell you to first set up an address
> > filter and then switch into receiving mode. Changing the address
> > filter during receive mode (start()/stop()) is not a specified
> > behaviour. Due to bus communication it also cannot be done atomically.
> > This might be avoidable but is a pain to synchronize if you really
> > depend on hw address filtering which we might do in future. It should
> > end in a different receiving path e.g. node_rx/monitor_rx.
>
> Got it.
>

I had some thoughts about this as well when going to promiscuous mode
while in "receiving mode"... this is "normally" not supported...

> >
> > 2. HardMAC transceivers
> >
> > The add_interface() callback will be directly forwarded to the driver
> > and the driver will during the lifetime of this interface act as a
> > coordinator and not a mixed mode which can be disabled and enabled
> > anytime. I am not even sure if this can ever be handled in such a way
> > from hardmac transceivers, it might depend on the transceiver
> > interface but we should assume some strict "static" handling. Static
> > handling means here that the transceiver is unable to switch from
> > coordinator and vice versa after some initialization state.
>
> Okay. I just completely missed the "interface add" command. So your
> advice is to treat the "PAN coordinator" property as a static property
> for a given interface, which seems reasonable.
>
> For now I will assume the same treatment when adding the interface is
> required compared to a NODE, but if something comes to your mind,
> please let me know.
>
> By the way, is there a mechanism limiting the number of interfaces on a
> device? Should we prevent the introduction of a coordinator iface if
> there is a node iface active?
>

such a mechanism already exists, look at the code when trying to ifup
an interface in mac802154. You cannot simply have a monitor and node
up at the same time. Hardware could have multiple address filters and
run multiple mac stack instances on one phy, which is in my opinion
not different than macvlan and in wireless running multiple access
points on the same phy.

> > 3. coordinator (any $TYPE specific) userspace software
> >
> > May the main argument. Some coordinator specific user space daemon
> > does specific type handling (e.g. hostapd) maybe because some library
> > is required. It is a pain to deal with changing roles during the
> > lifetime of an interface and synchronize user space software with it.
> > We should keep in mind that some of those handlings will maybe be
> > moved to user space instead of doing it in the kernel. I am fine with
> > the solution now, but keep in mind to offer such a possibility.
> >
> > I think the above arguments are probably the same why wireless is
> > doing something similar and I would avoid running into issues or it's
> > really difficult to handle because you need to solve other Linux net
> > architecture handling at first.
>
> Yep.
>
> > > > You are mixing things here with "role in the network" and what the
> > > > transceiver capability (RFD, FFD) is, which are two different things.
> > >
> > > I don't think I am, however maybe our vision differ on what an
> > > interface should be.
> > >
> > > > You should use those defines and the user needs to create a new
> > > > interface type and probably have a different extended address to act
> > > > as a coordinator.
> > >
> > > Can't we just simply switch from coordinator to !coordinator (that's
> > > what I currently implemented)? Why would we need the user to create a
> > > new interface type *and* to provide a new address?
> > >
> > > Note that these are real questions that I am asking myself. I'm fine
> > > adapting my implementation, as long as I get the main idea.
> > >
> >
> > See above.
>
> That's okay for me. I will adapt my implementation to use the interface
> thing. In the mean time additional details about what a coordinator
> interface should do differently (above question) is welcome because
> this is not something I am really comfortable with.
>

I think we need to figure this out...

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-08 13:47           ` Miquel Raynal
  2022-06-08 14:37             ` Miquel Raynal
@ 2022-06-09  1:56             ` Alexander Aring
  2022-06-09 15:52               ` Miquel Raynal
  1 sibling, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-09  1:56 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi,

On Wed, Jun 8, 2022 at 9:47 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alex,
>
> > > 3. coordinator (any $TYPE specific) userspace software
> > >
> > > May the main argument. Some coordinator specific user space daemon
> > > does specific type handling (e.g. hostapd) maybe because some library
> > > is required. It is a pain to deal with changing roles during the
> > > lifetime of an interface and synchronize user space software with it.
> > > We should keep in mind that some of those handlings will maybe be
> > > moved to user space instead of doing it in the kernel. I am fine with
> > > the solution now, but keep in mind to offer such a possibility.
> > >
> > > I think the above arguments are probably the same why wireless is
> > > doing something similar and I would avoid running into issues or it's
> > > really difficult to handle because you need to solve other Linux net
> > > architecture handling at first.
> >
> > Yep.
>
> The spec makes a difference between "coordinator" and "PAN
> coordinator", which one is the "coordinator" interface type supposed to
> picture? I believe we are talking about being a "PAN coordinator", but
> I want to be sure that we are aligned on the terms.
>

I think it depends what exactly the difference is. So far I see for
address filtering it should be the same. Maybe this is an interface
option then?

> > > > > You are mixing things here with "role in the network" and what
> > > > > the transceiver capability (RFD, FFD) is, which are two
> > > > > different things.
> > > >
> > > > I don't think I am, however maybe our vision differ on what an
> > > > interface should be.
> > > >
> > > > > You should use those defines and the user needs to create a new
> > > > > interface type and probably have a different extended address
> > > > > to act as a coordinator.
> > > >
> > > > Can't we just simply switch from coordinator to !coordinator
> > > > (that's what I currently implemented)? Why would we need the user
> > > > to create a new interface type *and* to provide a new address?
> > > >
> > > > Note that these are real questions that I am asking myself. I'm
> > > > fine adapting my implementation, as long as I get the main idea.
> > > >
> > >
> > > See above.
> >
> > That's okay for me. I will adapt my implementation to use the
> > interface thing. In the mean time additional details about what a
> > coordinator interface should do differently (above question) is
> > welcome because this is not something I am really comfortable with.
>
> I've updated the implementation to use the IFACE_COORD interface and it
> works fine, besides one question below.
>
> Also, I read the spec once again (soon I'll sleep with it) and
> actually what I extracted is that:
>
> * A FFD, when turned on, will perform a scan, then associate to any PAN
>   it found (algorithm is beyond the spec) or otherwise create a PAN ID
>   and start its own PAN. In both cases, it finishes its setup by
>   starting to send beacons.
>

What does it mean "algorithm is beyond the spec" - build your own?

> * A RFD will behave more or less the same, without the PAN creation
>   possibility of course. RFD-RX and RFD-TX are not required to support
>   any of that, I'll assume none of the scanning features is suitable
>   for them.
>
> I have a couple of questions however:
>
> - Creating an interface (let's call it wpancoord) out of wpan0 means
>   that two interfaces can be used in different ways and one can use
>   wpan0 as a node while using wpancoord as a PAN coordinator. Is that
>   really allowed? How should we prevent this from happening?
>

When the hardware does not support it, it should be forbidden. As most
transceivers have only one address filter it should be forbidden
then... but there exists a way to indeed have such a setup (which you
probably don't need to think about). It's better to forbid something
now, with the possibility later allowing it. So it should not break
any existing behaviour.

> - Should the device always wait for the user(space) to provide the PAN
>   to associate to after the scan procedure right after the
>   add_interface()? (like an information that must be provided prior to
>   set the interface up?)
>
> - How does an orphan FFD should pick the PAN ID for a PAN creation?
>   Should we use a random number? Start from 0 upwards? Start from
>   0xfffd downwards? Should the user always provide it?
>

I think this can be done all with some "fallback strategies" (build
your own) if it's not given as a parameter.

> - Should an FFD be able to create its own PAN on demand? Shall we
>   allow to do that at the creation of the new interface?
>

I thought the spec said "or otherwise"? That means if nothing can be
found, create one?

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-08 14:37             ` Miquel Raynal
@ 2022-06-09  2:06               ` Alexander Aring
  2022-06-09  2:23                 ` Alexander Aring
  0 siblings, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-09  2:06 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi,

On Wed, Jun 8, 2022 at 10:37 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
>
> miquel.raynal@bootlin.com wrote on Wed, 8 Jun 2022 15:47:49 +0200:
>
> > Hi Alex,
> >
> > > > 3. coordinator (any $TYPE specific) userspace software
> > > >
> > > > May the main argument. Some coordinator specific user space daemon
> > > > does specific type handling (e.g. hostapd) maybe because some library
> > > > is required. It is a pain to deal with changing roles during the
> > > > lifetime of an interface and synchronize user space software with it.
> > > > We should keep in mind that some of those handlings will maybe be
> > > > moved to user space instead of doing it in the kernel. I am fine with
> > > > the solution now, but keep in mind to offer such a possibility.
> > > >
> > > > I think the above arguments are probably the same why wireless is
> > > > doing something similar and I would avoid running into issues or it's
> > > > really difficult to handle because you need to solve other Linux net
> > > > architecture handling at first.
> > >
> > > Yep.
> >
> > The spec makes a difference between "coordinator" and "PAN
> > coordinator", which one is the "coordinator" interface type supposed to
> > picture? I believe we are talking about being a "PAN coordinator", but
> > I want to be sure that we are aligned on the terms.
> >
> > > > > > You are mixing things here with "role in the network" and what
> > > > > > the transceiver capability (RFD, FFD) is, which are two
> > > > > > different things.
> > > > >
> > > > > I don't think I am, however maybe our vision differ on what an
> > > > > interface should be.
> > > > >
> > > > > > You should use those defines and the user needs to create a new
> > > > > > interface type and probably have a different extended address
> > > > > > to act as a coordinator.
> > > > >
> > > > > Can't we just simply switch from coordinator to !coordinator
> > > > > (that's what I currently implemented)? Why would we need the user
> > > > > to create a new interface type *and* to provide a new address?
> > > > >
> > > > > Note that these are real questions that I am asking myself. I'm
> > > > > fine adapting my implementation, as long as I get the main idea.
> > > > >
> > > >
> > > > See above.
> > >
> > > That's okay for me. I will adapt my implementation to use the
> > > interface thing. In the mean time additional details about what a
> > > coordinator interface should do differently (above question) is
> > > welcome because this is not something I am really comfortable with.
> >
> > I've updated the implementation to use the IFACE_COORD interface and it
> > works fine, besides one question below.
> >
> > Also, I read the spec once again (soon I'll sleep with it) and
> > actually what I extracted is that:
> >
> > * A FFD, when turned on, will perform a scan, then associate to any PAN
> >   it found (algorithm is beyond the spec) or otherwise create a PAN ID
> >   and start its own PAN. In both cases, it finishes its setup by
> >   starting to send beacons.
> >
> > * A RFD will behave more or less the same, without the PAN creation
> >   possibility of course. RFD-RX and RFD-TX are not required to support
> >   any of that, I'll assume none of the scanning features is suitable
> >   for them.
>
> Acutally, RFDs do not send beacons, AFAIU.
>
> > I have a couple of questions however:
> >
> > - Creating an interface (let's call it wpancoord) out of wpan0 means
> >   that two interfaces can be used in different ways and one can use
> >   wpan0 as a node while using wpancoord as a PAN coordinator. Is that
> >   really allowed? How should we prevent this from happening?
> >
> > - Should the device always wait for the user(space) to provide the PAN
> >   to associate to after the scan procedure right after the
> >   add_interface()? (like an information that must be provided prior to
> >   set the interface up?)
> >
> > - How does an orphan FFD should pick the PAN ID for a PAN creation?
> >   Should we use a random number? Start from 0 upwards? Start from
> >   0xfffd downwards? Should the user always provide it?
> >
> > - Should an FFD be able to create its own PAN on demand? Shall we
> >   allow to do that at the creation of the new interface?
>
> Additional questions:
>
>   - How is chosen the beacon order? Should we have a default value?
>     Should we default to 15 (not on a beacon enabled PAN)? Should we be
>     able to update this value during the lifetime of the PAN?
>

Is there no mib default value for this?

>   - The spec talks about the cluster topology, where a coordinator that
>     just associated to a PAN starts emitting beacons, which may enable
>     other devices in its range to ask to join the PAN (increased area
>     coverage). But then, there is no information about how the newly
>     added device should do to join the PAN coordinator which is anyway
>     out of range to require the association, transmit data, etc. Any
>     idea how this is supposed to work?
>

I think we should maybe add a feature for this later if we don't know
how it is supposed to work or there are still open questions and first
introduce the manual setup. After that, maybe things will become
clearer and we can add support for this part. Is this okay?

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-09  2:06               ` Alexander Aring
@ 2022-06-09  2:23                 ` Alexander Aring
  2022-06-09 15:43                   ` Miquel Raynal
  0 siblings, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-09  2:23 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi,

On Wed, Jun 8, 2022 at 10:06 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Wed, Jun 8, 2022 at 10:37 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> >
> > miquel.raynal@bootlin.com wrote on Wed, 8 Jun 2022 15:47:49 +0200:
> >
> > > Hi Alex,
> > >
> > > > > 3. coordinator (any $TYPE specific) userspace software
> > > > >
> > > > > May the main argument. Some coordinator specific user space daemon
> > > > > does specific type handling (e.g. hostapd) maybe because some library
> > > > > is required. It is a pain to deal with changing roles during the
> > > > > lifetime of an interface and synchronize user space software with it.
> > > > > We should keep in mind that some of those handlings will maybe be
> > > > > moved to user space instead of doing it in the kernel. I am fine with
> > > > > the solution now, but keep in mind to offer such a possibility.
> > > > >
> > > > > I think the above arguments are probably the same why wireless is
> > > > > doing something similar and I would avoid running into issues or it's
> > > > > really difficult to handle because you need to solve other Linux net
> > > > > architecture handling at first.
> > > >
> > > > Yep.
> > >
> > > The spec makes a difference between "coordinator" and "PAN
> > > coordinator", which one is the "coordinator" interface type supposed to
> > > picture? I believe we are talking about being a "PAN coordinator", but
> > > I want to be sure that we are aligned on the terms.
> > >
> > > > > > > You are mixing things here with "role in the network" and what
> > > > > > > the transceiver capability (RFD, FFD) is, which are two
> > > > > > > different things.
> > > > > >
> > > > > > I don't think I am, however maybe our vision differ on what an
> > > > > > interface should be.
> > > > > >
> > > > > > > You should use those defines and the user needs to create a new
> > > > > > > interface type and probably have a different extended address
> > > > > > > to act as a coordinator.
> > > > > >
> > > > > > Can't we just simply switch from coordinator to !coordinator
> > > > > > (that's what I currently implemented)? Why would we need the user
> > > > > > to create a new interface type *and* to provide a new address?
> > > > > >
> > > > > > Note that these are real questions that I am asking myself. I'm
> > > > > > fine adapting my implementation, as long as I get the main idea.
> > > > > >
> > > > >
> > > > > See above.
> > > >
> > > > That's okay for me. I will adapt my implementation to use the
> > > > interface thing. In the mean time additional details about what a
> > > > coordinator interface should do differently (above question) is
> > > > welcome because this is not something I am really comfortable with.
> > >
> > > I've updated the implementation to use the IFACE_COORD interface and it
> > > works fine, besides one question below.
> > >
> > > Also, I read the spec once again (soon I'll sleep with it) and
> > > actually what I extracted is that:
> > >
> > > * A FFD, when turned on, will perform a scan, then associate to any PAN
> > >   it found (algorithm is beyond the spec) or otherwise create a PAN ID
> > >   and start its own PAN. In both cases, it finishes its setup by
> > >   starting to send beacons.
> > >
> > > * A RFD will behave more or less the same, without the PAN creation
> > >   possibility of course. RFD-RX and RFD-TX are not required to support
> > >   any of that, I'll assume none of the scanning features is suitable
> > >   for them.
> >
> > Acutally, RFDs do not send beacons, AFAIU.
> >
> > > I have a couple of questions however:
> > >
> > > - Creating an interface (let's call it wpancoord) out of wpan0 means
> > >   that two interfaces can be used in different ways and one can use
> > >   wpan0 as a node while using wpancoord as a PAN coordinator. Is that
> > >   really allowed? How should we prevent this from happening?
> > >
> > > - Should the device always wait for the user(space) to provide the PAN
> > >   to associate to after the scan procedure right after the
> > >   add_interface()? (like an information that must be provided prior to
> > >   set the interface up?)
> > >
> > > - How does an orphan FFD should pick the PAN ID for a PAN creation?
> > >   Should we use a random number? Start from 0 upwards? Start from
> > >   0xfffd downwards? Should the user always provide it?
> > >
> > > - Should an FFD be able to create its own PAN on demand? Shall we
> > >   allow to do that at the creation of the new interface?
> >
> > Additional questions:
> >
> >   - How is chosen the beacon order? Should we have a default value?
> >     Should we default to 15 (not on a beacon enabled PAN)? Should we be
> >     able to update this value during the lifetime of the PAN?
> >
>
> Is there no mib default value for this?
>
> >   - The spec talks about the cluster topology, where a coordinator that
> >     just associated to a PAN starts emitting beacons, which may enable
> >     other devices in its range to ask to join the PAN (increased area
> >     coverage). But then, there is no information about how the newly
> >     added device should do to join the PAN coordinator which is anyway
> >     out of range to require the association, transmit data, etc. Any
> >     idea how this is supposed to work?
> >
>
> I think we should maybe add a feature for this later if we don't know
> how it is supposed to work or there are still open questions and first
> introduce the manual setup. After that, maybe things will become
> clearer and we can add support for this part. Is this okay?

*I also think that this can be done in user space by a daemon by
triggering netlink commands for scan/assoc/etc. (manual setup) and
providing such functionality as mentioned by the spec (auto creation
of pan, assoc with pan). Things which are unclear here are then moved
to the user as the operations for scan/assoc/etc. will not be
different or at least parameterized. The point here is that providing
the minimum basic functionality should be done at first, then we can
look at how to realize such handling (either in kernel or user space).

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-09  1:42           ` Alexander Aring
@ 2022-06-09 14:42             ` Miquel Raynal
  0 siblings, 0 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-09 14:42 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi Alex,

> > > > > Second I have
> > > > > a different opinion here that you cannot just "switch" the role from
> > > > > RFD, FFD, whatever.  
> > > >
> > > > I agree with this, and that's why I don't understand this enum.
> > > >
> > > > A device can either be a NODE (an active device) or a MONITOR (a
> > > > passive device) at a time. We can certainly switch from one to
> > > > another at run time.
> > > >
> > > > A NODE can be either an RFD or an FFD. That is a static property which
> > > > cannot change.
> > > >
> > > > However being a coordinator is just an additional property of a NODE
> > > > which is of type FFD, and this can change over time.
> > > >
> > > > So I don't get what having a coordinator interface would bring. What
> > > > was the idea behind its introduction then?
> > > >  
> > >
> > > There exists arguments which I have in my mind right now:
> > >
> > > 1. frame parsing/address filter (which is somehow missing in your patches)
> > >
> > > The parsing of frames is different from other types (just as monitor
> > > interfaces). You will notice that setting up the address filter will
> > > require a parameter if coordinator or not.  
> >
> > I think this is something that I completely missed until now, can you
> > point me to where/how this is expected to be done? I don't see anything
> > wpan specific filtering implementation. What is expected on this area
> > and is there code that I missed already?
> >  
> 
> https://elixir.bootlin.com/linux/v5.19-rc1/source/net/mac802154/rx.c#L284

Oh okay now I get what you mean. Indeed, I had to look into this
function to allow coordinators to receive packets with the IFACE_COORD
implementation, but so far the filtering is "the same" as for a node.
We can improve that later if needed.
 
> > > Changing the address
> > > filterung during runtime of an interface is somehow _not_ supported.
> > > The reason is that the datasheets tell you to first set up an address
> > > filter and then switch into receiving mode. Changing the address
> > > filter during receive mode (start()/stop()) is not a specified
> > > behaviour. Due to bus communication it also cannot be done atomically.
> > > This might be avoidable but is a pain to synchronize if you really
> > > depend on hw address filtering which we might do in future. It should
> > > end in a different receiving path e.g. node_rx/monitor_rx.  
> >
> > Got it.
> >  
> 
> I had some thoughts about this as well when going to promiscuous mode
> while in "receiving mode"... this is "normally" not supported...
> 
> > >
> > > 2. HardMAC transceivers
> > >
> > > The add_interface() callback will be directly forwarded to the driver
> > > and the driver will during the lifetime of this interface act as a
> > > coordinator and not a mixed mode which can be disabled and enabled
> > > anytime. I am not even sure if this can ever be handled in such a way
> > > from hardmac transceivers, it might depend on the transceiver
> > > interface but we should assume some strict "static" handling. Static
> > > handling means here that the transceiver is unable to switch from
> > > coordinator and vice versa after some initialization state.  
> >
> > Okay. I just completely missed the "interface add" command. So your
> > advice is to treat the "PAN coordinator" property as a static property
> > for a given interface, which seems reasonable.
> >
> > For now I will assume the same treatment when adding the interface is
> > required compared to a NODE, but if something comes to your mind,
> > please let me know.
> >
> > By the way, is there a mechanism limiting the number of interfaces on a
> > device? Should we prevent the introduction of a coordinator iface if
> > there is a node iface active?
> >  
> 
> such a mechanism already exists, look at the code when trying to ifup
> an interface in mac802154. You cannot simply have a monitor and node
> up at the same time. Hardware could have multiple address filters and
> run multiple mac stack instances on one phy, which is in my opinion
> not different than macvlan and in wireless running multiple access
> points on the same phy.

Oh nice, I didn't pay enough attention to figure out that this was
executed during ifup. So I already changed that code to refuse two node
*and* coordinators to be up at the same time, we should be on the safe
side.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-09  2:23                 ` Alexander Aring
@ 2022-06-09 15:43                   ` Miquel Raynal
  2022-06-11 12:05                     ` Alexander Aring
  0 siblings, 1 reply; 29+ messages in thread
From: Miquel Raynal @ 2022-06-09 15:43 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi Alex,

> > >
> > >   - How is chosen the beacon order? Should we have a default value?
> > >     Should we default to 15 (not on a beacon enabled PAN)? Should we be
> > >     able to update this value during the lifetime of the PAN?
> > >  
> >
> > Is there no mib default value for this?

I didn't find anything. I suppose we can ask for that parameter at PAN
creation, but otherwise I'll keep a backward compatible value: 15,
which means that the PAN is not beacon enabled (like today, basically).

> >  
> > >   - The spec talks about the cluster topology, where a coordinator that
> > >     just associated to a PAN starts emitting beacons, which may enable
> > >     other devices in its range to ask to join the PAN (increased area
> > >     coverage). But then, there is no information about how the newly
> > >     added device should do to join the PAN coordinator which is anyway
> > >     out of range to require the association, transmit data, etc. Any
> > >     idea how this is supposed to work?
> > >  
> >
> > I think we should maybe add a feature for this later if we don't know
> > how it is supposed to work or there are still open questions and first
> > introduce the manual setup. After that, maybe things will become
> > clearer and we can add support for this part. Is this okay?  
> 
> *I also think that this can be done in user space by a daemon by
> triggering netlink commands for scan/assoc/etc. (manual setup) and
> providing such functionality as mentioned by the spec (auto creation
> of pan, assoc with pan). Things which are unclear here are then moved
> to the user as the operations for scan/assoc/etc. will not be
> different or at least parameterized. The point here is that providing
> the minimum basic functionality should be done at first, then we can
> look at how to realize such handling (either in kernel or user space).

Actually this is none of the 802.15.4 MAC layer business. I believe
this is the upper layer duty to make this interoperability work,
namely, 6lowpan?

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-09  1:56             ` Alexander Aring
@ 2022-06-09 15:52               ` Miquel Raynal
       [not found]                 ` <CAK-6q+jchHcge2_hMznO6fwx=xoUEpmoZTFYLAUwqM2Ue4Lx-A@mail.gmail.com>
  0 siblings, 1 reply; 29+ messages in thread
From: Miquel Raynal @ 2022-06-09 15:52 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi Alexander,

aahringo@redhat.com wrote on Wed, 8 Jun 2022 21:56:53 -0400:

> Hi,
> 
> On Wed, Jun 8, 2022 at 9:47 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alex,
> >  
> > > > 3. coordinator (any $TYPE specific) userspace software
> > > >
> > > > May the main argument. Some coordinator specific user space daemon
> > > > does specific type handling (e.g. hostapd) maybe because some library
> > > > is required. It is a pain to deal with changing roles during the
> > > > lifetime of an interface and synchronize user space software with it.
> > > > We should keep in mind that some of those handlings will maybe be
> > > > moved to user space instead of doing it in the kernel. I am fine with
> > > > the solution now, but keep in mind to offer such a possibility.
> > > >
> > > > I think the above arguments are probably the same why wireless is
> > > > doing something similar and I would avoid running into issues or it's
> > > > really difficult to handle because you need to solve other Linux net
> > > > architecture handling at first.  
> > >
> > > Yep.  
> >
> > The spec makes a difference between "coordinator" and "PAN
> > coordinator", which one is the "coordinator" interface type supposed to
> > picture? I believe we are talking about being a "PAN coordinator", but
> > I want to be sure that we are aligned on the terms.
> >  
> 
> I think it depends what exactly the difference is. So far I see for
> address filtering it should be the same. Maybe this is an interface
> option then?

The difference is that the PAN coordinator can decide to eg. refuse an
association, while the other coordinators, are just FFDs with no
specific decision making capability wrt the PAN itself, but have some
routing capabilities available for the upper layers.

The most I look into this, the less likely it is that the Linux stack
will drive an RFD. Do you think it's worth supporting them? Because if
we don't:
* NODE == FFD which acts as coordinator
* COORD == FFD which acts as the PAN coordinator

> > > > > > You are mixing things here with "role in the network" and what
> > > > > > the transceiver capability (RFD, FFD) is, which are two
> > > > > > different things.  
> > > > >
> > > > > I don't think I am, however maybe our vision differ on what an
> > > > > interface should be.
> > > > >  
> > > > > > You should use those defines and the user needs to create a new
> > > > > > interface type and probably have a different extended address
> > > > > > to act as a coordinator.  
> > > > >
> > > > > Can't we just simply switch from coordinator to !coordinator
> > > > > (that's what I currently implemented)? Why would we need the user
> > > > > to create a new interface type *and* to provide a new address?
> > > > >
> > > > > Note that these are real questions that I am asking myself. I'm
> > > > > fine adapting my implementation, as long as I get the main idea.
> > > > >  
> > > >
> > > > See above.  
> > >
> > > That's okay for me. I will adapt my implementation to use the
> > > interface thing. In the mean time additional details about what a
> > > coordinator interface should do differently (above question) is
> > > welcome because this is not something I am really comfortable with.  
> >
> > I've updated the implementation to use the IFACE_COORD interface and it
> > works fine, besides one question below.
> >
> > Also, I read the spec once again (soon I'll sleep with it) and
> > actually what I extracted is that:
> >
> > * A FFD, when turned on, will perform a scan, then associate to any PAN
> >   it found (algorithm is beyond the spec) or otherwise create a PAN ID
> >   and start its own PAN. In both cases, it finishes its setup by
> >   starting to send beacons.
> >  
> 
> What does it mean "algorithm is beyond the spec" - build your own?

This is really what is in the spec, I suppose it means "do what you
want in your use case".

What I have in mind: when a device is powered on and detects two PANs,
well, it can join whichever it wants, but perhaps we should make the
decision based on the LQI information we have (the closer the better).

> > * A RFD will behave more or less the same, without the PAN creation
> >   possibility of course. RFD-RX and RFD-TX are not required to support
> >   any of that, I'll assume none of the scanning features is suitable
> >   for them.
> >
> > I have a couple of questions however:
> >
> > - Creating an interface (let's call it wpancoord) out of wpan0 means
> >   that two interfaces can be used in different ways and one can use
> >   wpan0 as a node while using wpancoord as a PAN coordinator. Is that
> >   really allowed? How should we prevent this from happening?
> >  
> 
> When the hardware does not support it, it should be forbidden. As most
> transceivers have only one address filter it should be forbidden
> then... but there exists a way to indeed have such a setup (which you
> probably don't need to think about). It's better to forbid something
> now, with the possibility later allowing it. So it should not break
> any existing behaviour.

Done, thanks to the pointer you gave in the other mail.

> 
> > - Should the device always wait for the user(space) to provide the PAN
> >   to associate to after the scan procedure right after the
> >   add_interface()? (like an information that must be provided prior to
> >   set the interface up?)
> >
> > - How does an orphan FFD should pick the PAN ID for a PAN creation?
> >   Should we use a random number? Start from 0 upwards? Start from
> >   0xfffd downwards? Should the user always provide it?
> >  
> 
> I think this can be done all with some "fallback strategies" (build
> your own) if it's not given as a parameter.

Ok, In case no PAN is found, and at creation no PAN ID is provided, we
can default to 0.

> > - Should an FFD be able to create its own PAN on demand? Shall we
> >   allow to do that at the creation of the new interface?
> >  
> 
> I thought the spec said "or otherwise"? That means if nothing can be
> found, create one?

Ok, so we assume this is only at startup, fine. But then how to handle
the set_pan_id() call? I believe we can forbid any set_pan_id() command
to be run while the interface is up. That would ease the handling.
Unless I am missing something?

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-09 15:43                   ` Miquel Raynal
@ 2022-06-11 12:05                     ` Alexander Aring
  2022-06-15  9:15                       ` Miquel Raynal
  0 siblings, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-11 12:05 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi,

On Thu, Jun 9, 2022 at 11:44 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alex,
>
> > > >
> > > >   - How is chosen the beacon order? Should we have a default value?
> > > >     Should we default to 15 (not on a beacon enabled PAN)? Should we be
> > > >     able to update this value during the lifetime of the PAN?
> > > >
> > >
> > > Is there no mib default value for this?
>
> I didn't find anything. I suppose we can ask for that parameter at PAN
> creation, but otherwise I'll keep a backward compatible value: 15,
> which means that the PAN is not beacon enabled (like today, basically).
>

I hope it is not necessary to answer this question, see below.

> > >
> > > >   - The spec talks about the cluster topology, where a coordinator that
> > > >     just associated to a PAN starts emitting beacons, which may enable
> > > >     other devices in its range to ask to join the PAN (increased area
> > > >     coverage). But then, there is no information about how the newly
> > > >     added device should do to join the PAN coordinator which is anyway
> > > >     out of range to require the association, transmit data, etc. Any
> > > >     idea how this is supposed to work?
> > > >
> > >
> > > I think we should maybe add a feature for this later if we don't know
> > > how it is supposed to work or there are still open questions and first
> > > introduce the manual setup. After that, maybe things will become
> > > clearer and we can add support for this part. Is this okay?
> >
> > *I also think that this can be done in user space by a daemon by
> > triggering netlink commands for scan/assoc/etc. (manual setup) and
> > providing such functionality as mentioned by the spec (auto creation
> > of pan, assoc with pan). Things which are unclear here are then moved
> > to the user as the operations for scan/assoc/etc. will not be
> > different or at least parameterized. The point here is that providing
> > the minimum basic functionality should be done at first, then we can
> > look at how to realize such handling (either in kernel or user space).
>
> Actually this is none of the 802.15.4 MAC layer business. I believe
> this is the upper layer duty to make this interoperability work,
> namely, 6lowpan?

I am not sure if I understand your answer, I meant that if
"coordinator" or "PAN coordinator" depends on whatever, if somebody is
running a "coordinator" software in the background on top of a coord
interface.
The kernel offers the functionality for scan/assoc/etc. (offers link
quality, etc. _statistics_ and not _heuristic_) which will be used by
this software to whatever the user defines to realize this behaviour
as it is user specific.

Sure linux-wpan, should then provide at least a standard piece of
software for it.

This has in my opinion nothing to do with 6lowpan.

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-11 12:05                     ` Alexander Aring
@ 2022-06-15  9:15                       ` Miquel Raynal
  0 siblings, 0 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-15  9:15 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development, David Girault, Romuald Despres,
	Frederic Blain, Nicolas Schodet, Thomas Petazzoni

Hi Alexander,

aahringo@redhat.com wrote on Sat, 11 Jun 2022 08:05:31 -0400:

> Hi,
> 
> On Thu, Jun 9, 2022 at 11:44 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alex,
> >  
> > > > >
> > > > >   - How is chosen the beacon order? Should we have a default value?
> > > > >     Should we default to 15 (not on a beacon enabled PAN)? Should we be
> > > > >     able to update this value during the lifetime of the PAN?
> > > > >  
> > > >
> > > > Is there no mib default value for this?  
> >
> > I didn't find anything. I suppose we can ask for that parameter at PAN
> > creation, but otherwise I'll keep a backward compatible value: 15,
> > which means that the PAN is not beacon enabled (like today, basically).
> >  
> 
> I hope it is not necessary to answer this question, see below.
> 
> > > >  
> > > > >   - The spec talks about the cluster topology, where a coordinator that
> > > > >     just associated to a PAN starts emitting beacons, which may enable
> > > > >     other devices in its range to ask to join the PAN (increased area
> > > > >     coverage). But then, there is no information about how the newly
> > > > >     added device should do to join the PAN coordinator which is anyway
> > > > >     out of range to require the association, transmit data, etc. Any
> > > > >     idea how this is supposed to work?
> > > > >  
> > > >
> > > > I think we should maybe add a feature for this later if we don't know
> > > > how it is supposed to work or there are still open questions and first
> > > > introduce the manual setup. After that, maybe things will become
> > > > clearer and we can add support for this part. Is this okay?  
> > >
> > > *I also think that this can be done in user space by a daemon by
> > > triggering netlink commands for scan/assoc/etc. (manual setup) and
> > > providing such functionality as mentioned by the spec (auto creation
> > > of pan, assoc with pan). Things which are unclear here are then moved
> > > to the user as the operations for scan/assoc/etc. will not be
> > > different or at least parameterized. The point here is that providing
> > > the minimum basic functionality should be done at first, then we can
> > > look at how to realize such handling (either in kernel or user space).  
> >
> > Actually this is none of the 802.15.4 MAC layer business. I believe
> > this is the upper layer duty to make this interoperability work,
> > namely, 6lowpan?  
> 
> I am not sure if I understand your answer, I meant that if
> "coordinator" or "PAN coordinator" depends on whatever, if somebody is
> running a "coordinator" software in the background on top of a coord
> interface.
> The kernel offers the functionality for scan/assoc/etc. (offers link
> quality, etc. _statistics_ and not _heuristic_) which will be used by
> this software to whatever the user defines to realize this behaviour
> as it is user specific.

Yes.

> Sure linux-wpan, should then provide at least a standard piece of
> software for it.
> 
> This has in my opinion nothing to do with 6lowpan.

I was referring to the cluster topology routing logic. The routing
logic to reach a device in a PAN that is not directly reachable by the
PAN coordinator is the responsibility of the layer 3 in the OSI model,
so I believe it's either 6lowpan's duty or even above.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
       [not found]                 ` <CAK-6q+jchHcge2_hMznO6fwx=xoUEpmoZTFYLAUwqM2Ue4Lx-A@mail.gmail.com>
@ 2022-06-17 15:12                   ` Miquel Raynal
  2022-06-20  0:13                     ` Alexander Aring
  0 siblings, 1 reply; 29+ messages in thread
From: Miquel Raynal @ 2022-06-17 15:12 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, Alexander Aring, linux-wpan - ML, David Girault,
	Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development

Hi Alex,

aahringo@redhat.com wrote on Sat, 11 Jun 2022 08:23:41 -0400:

> Hi,
> 
> On Thu, Jun 9, 2022 at 11:52 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Wed, 8 Jun 2022 21:56:53 -0400:
> >  
> > > Hi,
> > >
> > > On Wed, Jun 8, 2022 at 9:47 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > Hi Alex,
> > > >  
> > > > > > 3. coordinator (any $TYPE specific) userspace software
> > > > > >
> > > > > > May the main argument. Some coordinator specific user space daemon
> > > > > > does specific type handling (e.g. hostapd) maybe because some library
> > > > > > is required. It is a pain to deal with changing roles during the
> > > > > > lifetime of an interface and synchronize user space software with it.
> > > > > > We should keep in mind that some of those handlings will maybe be
> > > > > > moved to user space instead of doing it in the kernel. I am fine with
> > > > > > the solution now, but keep in mind to offer such a possibility.
> > > > > >
> > > > > > I think the above arguments are probably the same why wireless is
> > > > > > doing something similar and I would avoid running into issues or it's
> > > > > > really difficult to handle because you need to solve other Linux net
> > > > > > architecture handling at first.  
> > > > >
> > > > > Yep.  
> > > >
> > > > The spec makes a difference between "coordinator" and "PAN
> > > > coordinator", which one is the "coordinator" interface type supposed to
> > > > picture? I believe we are talking about being a "PAN coordinator", but
> > > > I want to be sure that we are aligned on the terms.
> > > >  
> > >
> > > I think it depends what exactly the difference is. So far I see for
> > > address filtering it should be the same. Maybe this is an interface
> > > option then?  
> >
> > The difference is that the PAN coordinator can decide to eg. refuse an
> > association, while the other coordinators, are just FFDs with no
> > specific decision making capability wrt the PAN itself, but have some
> > routing capabilities available for the upper layers.
> >  
> 
> As I said, if there is a behaviour "it can do xxx, but the spec
> doesn't give more information about it" this smells for me like things
> moving into the user space. This can also be done e.g. by a filtering
> mechanism, _just_ the user will configure how this filtering will look
> like.
> 
> > The most I look into this, the less likely it is that the Linux stack
> > will drive an RFD. Do you think it's worth supporting them? Because if
> > we don't:
> > * NODE == FFD which acts as coordinator
> > * COORD == FFD which acts as the PAN coordinator
> >  
> 
> I thought that this is a kind of "transceiver type capability " e.g. I
> can imagine if it's only a "RFD" transceiver then you would be e.g.
> not able to set the address filter to coordinator capability. However
> I think that will never happen for a SoftMAC transceiver because why
> not adding a little bit silicon to provide that? People also can
> always have a co-processor and run the transceiver in promiscuous
> mode. E.g. atusb (which makes this transceiver poweful, because we
> have control over the firmware).
> 
> For me node != coord, because the address filtering is different. As I
> mentioned in another mail "coordinator" vs "PAN coordinator" as
> described is what the user is doing here on top of it.
> 
> > > > > > > > You are mixing things here with "role in the network" and what
> > > > > > > > the transceiver capability (RFD, FFD) is, which are two
> > > > > > > > different things.  
> > > > > > >
> > > > > > > I don't think I am, however maybe our vision differ on what an
> > > > > > > interface should be.
> > > > > > >  
> > > > > > > > You should use those defines and the user needs to create a new
> > > > > > > > interface type and probably have a different extended address
> > > > > > > > to act as a coordinator.  
> > > > > > >
> > > > > > > Can't we just simply switch from coordinator to !coordinator
> > > > > > > (that's what I currently implemented)? Why would we need the user
> > > > > > > to create a new interface type *and* to provide a new address?
> > > > > > >
> > > > > > > Note that these are real questions that I am asking myself. I'm
> > > > > > > fine adapting my implementation, as long as I get the main idea.
> > > > > > >  
> > > > > >
> > > > > > See above.  
> > > > >
> > > > > That's okay for me. I will adapt my implementation to use the
> > > > > interface thing. In the mean time additional details about what a
> > > > > coordinator interface should do differently (above question) is
> > > > > welcome because this is not something I am really comfortable with.  
> > > >
> > > > I've updated the implementation to use the IFACE_COORD interface and it
> > > > works fine, besides one question below.
> > > >
> > > > Also, I read the spec once again (soon I'll sleep with it) and
> > > > actually what I extracted is that:
> > > >
> > > > * A FFD, when turned on, will perform a scan, then associate to any PAN
> > > >   it found (algorithm is beyond the spec) or otherwise create a PAN ID
> > > >   and start its own PAN. In both cases, it finishes its setup by
> > > >   starting to send beacons.
> > > >  
> > >
> > > What does it mean "algorithm is beyond the spec" - build your own?  
> >
> > This is really what is in the spec, I suppose it means "do what you
> > want in your use case".
> >
> > What I have in mind: when a device is powered on and detects two PANs,
> > well, it can join whichever it wants, but perhaps we should make the
> > decision based on the LQI information we have (the closer the better).
> >  
> 
> As I said in the other mail, this smells more and more for me to move
> this handling to user space. The kernel therefore supports operations
> to trigger the necessary steps (scan/assoc/etc.)
> 
> > > > * A RFD will behave more or less the same, without the PAN creation
> > > >   possibility of course. RFD-RX and RFD-TX are not required to support
> > > >   any of that, I'll assume none of the scanning features is suitable
> > > >   for them.
> > > >
> > > > I have a couple of questions however:
> > > >
> > > > - Creating an interface (let's call it wpancoord) out of wpan0 means
> > > >   that two interfaces can be used in different ways and one can use
> > > >   wpan0 as a node while using wpancoord as a PAN coordinator. Is that
> > > >   really allowed? How should we prevent this from happening?
> > > >  
> > >
> > > When the hardware does not support it, it should be forbidden. As most
> > > transceivers have only one address filter it should be forbidden
> > > then... but there exists a way to indeed have such a setup (which you
> > > probably don't need to think about). It's better to forbid something
> > > now, with the possibility later allowing it. So it should not break
> > > any existing behaviour.  
> >
> > Done, thanks to the pointer you gave in the other mail.
> >  
> > >  
> > > > - Should the device always wait for the user(space) to provide the PAN
> > > >   to associate to after the scan procedure right after the
> > > >   add_interface()? (like an information that must be provided prior to
> > > >   set the interface up?)
> > > >
> > > > - How does an orphan FFD should pick the PAN ID for a PAN creation?
> > > >   Should we use a random number? Start from 0 upwards? Start from
> > > >   0xfffd downwards? Should the user always provide it?
> > > >  
> > >
> > > I think this can be done all with some "fallback strategies" (build
> > > your own) if it's not given as a parameter.  
> >
> > Ok, In case no PAN is found, and at creation no PAN ID is provided, we
> > can default to 0.
> >  
> 
> See me for other mails. (user space job)
> 
> > > > - Should an FFD be able to create its own PAN on demand? Shall we
> > > >   allow to do that at the creation of the new interface?
> > > >  
> > >
> > > I thought the spec said "or otherwise"? That means if nothing can be
> > > found, create one?  
> >
> > Ok, so we assume this is only at startup, fine. But then how to handle
> > the set_pan_id() call? I believe we can forbid any set_pan_id() command
> > to be run while the interface is up. That would ease the handling.
> > Unless I am missing something?
> >  
> 
> See my other mails (user space job).

Ok then, I'll go with the following constraints in mind:

SCAN (passive/active) (all devices)
- All devices are allowed to perform scans.
- The user decides when a scan must be performed, there is no
  limitation on when to do a scan (but the interface must be up for
  physical reasons).
PAN ID
- The user is responsible to set the PAN ID.
- Like several other parameters, the PAN ID can only be changed if the
  iface is down. Which means the user might need to do:
	link up > scan > link down > set params > link up 
BEACON
- Coordinator interfaces only can send beacons.
- Beacons can only be sent when part of a PAN (PAN ID != 0xffff).
- The choice of the beacon interval is up to the user, at any moment.
OTHER PARAMETERS
- The choice of the channel (page, etc) is free until the device is
  associated to another, then it becomes fixed.

ASSOCIATION (to be done)
- Device association/disassociation procedure is requested by the
  user.
- Accepting new associations is up to the user (coordinator only).
- If the device has no parent (was not associated to any device) it is
  PAN coordinator and has additional rights regarding associations.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-17 15:12                   ` Miquel Raynal
@ 2022-06-20  0:13                     ` Alexander Aring
  2022-06-20  9:19                       ` Miquel Raynal
  0 siblings, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-20  0:13 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Stefan Schmidt, Alexander Aring, linux-wpan - ML, David Girault,
	Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development

Hi,

On Fri, Jun 17, 2022 at 11:13 AM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> Hi Alex,
>
> aahringo@redhat.com wrote on Sat, 11 Jun 2022 08:23:41 -0400:
>
> > Hi,
> >
> > On Thu, Jun 9, 2022 at 11:52 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > aahringo@redhat.com wrote on Wed, 8 Jun 2022 21:56:53 -0400:
> > >
> > > > Hi,
> > > >
> > > > On Wed, Jun 8, 2022 at 9:47 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > >
> > > > > Hi Alex,
> > > > >
> > > > > > > 3. coordinator (any $TYPE specific) userspace software
> > > > > > >
> > > > > > > May the main argument. Some coordinator specific user space daemon
> > > > > > > does specific type handling (e.g. hostapd) maybe because some library
> > > > > > > is required. It is a pain to deal with changing roles during the
> > > > > > > lifetime of an interface and synchronize user space software with it.
> > > > > > > We should keep in mind that some of those handlings will maybe be
> > > > > > > moved to user space instead of doing it in the kernel. I am fine with
> > > > > > > the solution now, but keep in mind to offer such a possibility.
> > > > > > >
> > > > > > > I think the above arguments are probably the same why wireless is
> > > > > > > doing something similar and I would avoid running into issues or it's
> > > > > > > really difficult to handle because you need to solve other Linux net
> > > > > > > architecture handling at first.
> > > > > >
> > > > > > Yep.
> > > > >
> > > > > The spec makes a difference between "coordinator" and "PAN
> > > > > coordinator", which one is the "coordinator" interface type supposed to
> > > > > picture? I believe we are talking about being a "PAN coordinator", but
> > > > > I want to be sure that we are aligned on the terms.
> > > > >
> > > >
> > > > I think it depends what exactly the difference is. So far I see for
> > > > address filtering it should be the same. Maybe this is an interface
> > > > option then?
> > >
> > > The difference is that the PAN coordinator can decide to eg. refuse an
> > > association, while the other coordinators, are just FFDs with no
> > > specific decision making capability wrt the PAN itself, but have some
> > > routing capabilities available for the upper layers.
> > >
> >
> > As I said, if there is a behaviour "it can do xxx, but the spec
> > doesn't give more information about it" this smells for me like things
> > moving into the user space. This can also be done e.g. by a filtering
> > mechanism, _just_ the user will configure how this filtering will look
> > like.
> >
> > > The most I look into this, the less likely it is that the Linux stack
> > > will drive an RFD. Do you think it's worth supporting them? Because if
> > > we don't:
> > > * NODE == FFD which acts as coordinator
> > > * COORD == FFD which acts as the PAN coordinator
> > >
> >
> > I thought that this is a kind of "transceiver type capability " e.g. I
> > can imagine if it's only a "RFD" transceiver then you would be e.g.
> > not able to set the address filter to coordinator capability. However
> > I think that will never happen for a SoftMAC transceiver because why
> > not adding a little bit silicon to provide that? People also can
> > always have a co-processor and run the transceiver in promiscuous
> > mode. E.g. atusb (which makes this transceiver poweful, because we
> > have control over the firmware).
> >
> > For me node != coord, because the address filtering is different. As I
> > mentioned in another mail "coordinator" vs "PAN coordinator" as
> > described is what the user is doing here on top of it.
> >
> > > > > > > > > You are mixing things here with "role in the network" and what
> > > > > > > > > the transceiver capability (RFD, FFD) is, which are two
> > > > > > > > > different things.
> > > > > > > >
> > > > > > > > I don't think I am, however maybe our vision differ on what an
> > > > > > > > interface should be.
> > > > > > > >
> > > > > > > > > You should use those defines and the user needs to create a new
> > > > > > > > > interface type and probably have a different extended address
> > > > > > > > > to act as a coordinator.
> > > > > > > >
> > > > > > > > Can't we just simply switch from coordinator to !coordinator
> > > > > > > > (that's what I currently implemented)? Why would we need the user
> > > > > > > > to create a new interface type *and* to provide a new address?
> > > > > > > >
> > > > > > > > Note that these are real questions that I am asking myself. I'm
> > > > > > > > fine adapting my implementation, as long as I get the main idea.
> > > > > > > >
> > > > > > >
> > > > > > > See above.
> > > > > >
> > > > > > That's okay for me. I will adapt my implementation to use the
> > > > > > interface thing. In the mean time additional details about what a
> > > > > > coordinator interface should do differently (above question) is
> > > > > > welcome because this is not something I am really comfortable with.
> > > > >
> > > > > I've updated the implementation to use the IFACE_COORD interface and it
> > > > > works fine, besides one question below.
> > > > >
> > > > > Also, I read the spec once again (soon I'll sleep with it) and
> > > > > actually what I extracted is that:
> > > > >
> > > > > * A FFD, when turned on, will perform a scan, then associate to any PAN
> > > > >   it found (algorithm is beyond the spec) or otherwise create a PAN ID
> > > > >   and start its own PAN. In both cases, it finishes its setup by
> > > > >   starting to send beacons.
> > > > >
> > > >
> > > > What does it mean "algorithm is beyond the spec" - build your own?
> > >
> > > This is really what is in the spec, I suppose it means "do what you
> > > want in your use case".
> > >
> > > What I have in mind: when a device is powered on and detects two PANs,
> > > well, it can join whichever it wants, but perhaps we should make the
> > > decision based on the LQI information we have (the closer the better).
> > >
> >
> > As I said in the other mail, this smells more and more for me to move
> > this handling to user space. The kernel therefore supports operations
> > to trigger the necessary steps (scan/assoc/etc.)
> >
> > > > > * A RFD will behave more or less the same, without the PAN creation
> > > > >   possibility of course. RFD-RX and RFD-TX are not required to support
> > > > >   any of that, I'll assume none of the scanning features is suitable
> > > > >   for them.
> > > > >
> > > > > I have a couple of questions however:
> > > > >
> > > > > - Creating an interface (let's call it wpancoord) out of wpan0 means
> > > > >   that two interfaces can be used in different ways and one can use
> > > > >   wpan0 as a node while using wpancoord as a PAN coordinator. Is that
> > > > >   really allowed? How should we prevent this from happening?
> > > > >
> > > >
> > > > When the hardware does not support it, it should be forbidden. As most
> > > > transceivers have only one address filter it should be forbidden
> > > > then... but there exists a way to indeed have such a setup (which you
> > > > probably don't need to think about). It's better to forbid something
> > > > now, with the possibility later allowing it. So it should not break
> > > > any existing behaviour.
> > >
> > > Done, thanks to the pointer you gave in the other mail.
> > >
> > > >
> > > > > - Should the device always wait for the user(space) to provide the PAN
> > > > >   to associate to after the scan procedure right after the
> > > > >   add_interface()? (like an information that must be provided prior to
> > > > >   set the interface up?)
> > > > >
> > > > > - How does an orphan FFD should pick the PAN ID for a PAN creation?
> > > > >   Should we use a random number? Start from 0 upwards? Start from
> > > > >   0xfffd downwards? Should the user always provide it?
> > > > >
> > > >
> > > > I think this can be done all with some "fallback strategies" (build
> > > > your own) if it's not given as a parameter.
> > >
> > > Ok, In case no PAN is found, and at creation no PAN ID is provided, we
> > > can default to 0.
> > >
> >
> > See me for other mails. (user space job)
> >
> > > > > - Should an FFD be able to create its own PAN on demand? Shall we
> > > > >   allow to do that at the creation of the new interface?
> > > > >
> > > >
> > > > I thought the spec said "or otherwise"? That means if nothing can be
> > > > found, create one?
> > >
> > > Ok, so we assume this is only at startup, fine. But then how to handle
> > > the set_pan_id() call? I believe we can forbid any set_pan_id() command
> > > to be run while the interface is up. That would ease the handling.
> > > Unless I am missing something?
> > >
> >
> > See my other mails (user space job).
>
> Ok then, I'll go with the following constraints in mind:
>
> SCAN (passive/active) (all devices)
> - All devices are allowed to perform scans.
> - The user decides when a scan must be performed, there is no
>   limitation on when to do a scan (but the interface must be up for
>   physical reasons).

Yes, I think it should not have anything to do with interface
limitation.... it needs to have an operating phy. However I can say
more to this when I see code (but please don't provide me with any
github repository, I mean here on the mailing list and not a more than
15 patches stack, Thanks.) You probably want to say on an user level
"run scan for iface $FOO" but this is just to make it simpler.

> PAN ID
> - The user is responsible to set the PAN ID.

This is currently the case and I don't see a reason to change it.

> - Like several other parameters, the PAN ID can only be changed if the
>   iface is down. Which means the user might need to do:
>         link up > scan > link down > set params > link up

Yes, changing this behaviour will break other things.

> BEACON
> - Coordinator interfaces only can send beacons.

okay.

> - Beacons can only be sent when part of a PAN (PAN ID != 0xffff).

I guess that 0xffff means no pan is set and if no pan is set there is no pan?

> - The choice of the beacon interval is up to the user, at any moment.
> OTHER PARAMETERS

I would say "okay", there might be an implementation detail about when
it's effective.
But is this not only required if doing such "passive" mode?

> - The choice of the channel (page, etc) is free until the device is
>   associated to another, then it becomes fixed.
>

I would say no here, because if the user changes it it's their
problem... it's required to be root for doing it and that should be
enough to do idiot things?

> ASSOCIATION (to be done)
> - Device association/disassociation procedure is requested by the
>   user.

This is similar like wireless is doing with assoc/deassoc to ap.

> - Accepting new associations is up to the user (coordinator only).

Again implementation details how this should be realized.

> - If the device has no parent (was not associated to any device) it is
>   PAN coordinator and has additional rights regarding associations.
>

No idea what a "device' here is, did we not made a difference between
"coordinator" vs "PAN coordinator" before and PAN is that thing which
does some automatically scan/assoc operation and the other one not? I
really have no idea what "device" here means.

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-20  0:13                     ` Alexander Aring
@ 2022-06-20  9:19                       ` Miquel Raynal
  2022-06-21  1:54                         ` Alexander Aring
  0 siblings, 1 reply; 29+ messages in thread
From: Miquel Raynal @ 2022-06-20  9:19 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, Alexander Aring, linux-wpan - ML, David Girault,
	Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development

Hi Alexander,

aahringo@redhat.com wrote on Sun, 19 Jun 2022 20:13:08 -0400:

> Hi,
> 
> On Fri, Jun 17, 2022 at 11:13 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alex,
> >
> > aahringo@redhat.com wrote on Sat, 11 Jun 2022 08:23:41 -0400:
> >  
> > > Hi,
> > >
> > > On Thu, Jun 9, 2022 at 11:52 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > Hi Alexander,
> > > >
> > > > aahringo@redhat.com wrote on Wed, 8 Jun 2022 21:56:53 -0400:
> > > >  
> > > > > Hi,
> > > > >
> > > > > On Wed, Jun 8, 2022 at 9:47 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > > > >
> > > > > > Hi Alex,
> > > > > >  
> > > > > > > > 3. coordinator (any $TYPE specific) userspace software
> > > > > > > >
> > > > > > > > May the main argument. Some coordinator specific user space daemon
> > > > > > > > does specific type handling (e.g. hostapd) maybe because some library
> > > > > > > > is required. It is a pain to deal with changing roles during the
> > > > > > > > lifetime of an interface and synchronize user space software with it.
> > > > > > > > We should keep in mind that some of those handlings will maybe be
> > > > > > > > moved to user space instead of doing it in the kernel. I am fine with
> > > > > > > > the solution now, but keep in mind to offer such a possibility.
> > > > > > > >
> > > > > > > > I think the above arguments are probably the same why wireless is
> > > > > > > > doing something similar and I would avoid running into issues or it's
> > > > > > > > really difficult to handle because you need to solve other Linux net
> > > > > > > > architecture handling at first.  
> > > > > > >
> > > > > > > Yep.  
> > > > > >
> > > > > > The spec makes a difference between "coordinator" and "PAN
> > > > > > coordinator", which one is the "coordinator" interface type supposed to
> > > > > > picture? I believe we are talking about being a "PAN coordinator", but
> > > > > > I want to be sure that we are aligned on the terms.
> > > > > >  
> > > > >
> > > > > I think it depends what exactly the difference is. So far I see for
> > > > > address filtering it should be the same. Maybe this is an interface
> > > > > option then?  
> > > >
> > > > The difference is that the PAN coordinator can decide to eg. refuse an
> > > > association, while the other coordinators, are just FFDs with no
> > > > specific decision making capability wrt the PAN itself, but have some
> > > > routing capabilities available for the upper layers.
> > > >  
> > >
> > > As I said, if there is a behaviour "it can do xxx, but the spec
> > > doesn't give more information about it" this smells for me like things
> > > moving into the user space. This can also be done e.g. by a filtering
> > > mechanism, _just_ the user will configure how this filtering will look
> > > like.
> > >  
> > > > The most I look into this, the less likely it is that the Linux stack
> > > > will drive an RFD. Do you think it's worth supporting them? Because if
> > > > we don't:
> > > > * NODE == FFD which acts as coordinator
> > > > * COORD == FFD which acts as the PAN coordinator
> > > >  
> > >
> > > I thought that this is a kind of "transceiver type capability " e.g. I
> > > can imagine if it's only a "RFD" transceiver then you would be e.g.
> > > not able to set the address filter to coordinator capability. However
> > > I think that will never happen for a SoftMAC transceiver because why
> > > not adding a little bit silicon to provide that? People also can
> > > always have a co-processor and run the transceiver in promiscuous
> > > mode. E.g. atusb (which makes this transceiver poweful, because we
> > > have control over the firmware).
> > >
> > > For me node != coord, because the address filtering is different. As I
> > > mentioned in another mail "coordinator" vs "PAN coordinator" as
> > > described is what the user is doing here on top of it.
> > >  
> > > > > > > > > > You are mixing things here with "role in the network" and what
> > > > > > > > > > the transceiver capability (RFD, FFD) is, which are two
> > > > > > > > > > different things.  
> > > > > > > > >
> > > > > > > > > I don't think I am, however maybe our vision differ on what an
> > > > > > > > > interface should be.
> > > > > > > > >  
> > > > > > > > > > You should use those defines and the user needs to create a new
> > > > > > > > > > interface type and probably have a different extended address
> > > > > > > > > > to act as a coordinator.  
> > > > > > > > >
> > > > > > > > > Can't we just simply switch from coordinator to !coordinator
> > > > > > > > > (that's what I currently implemented)? Why would we need the user
> > > > > > > > > to create a new interface type *and* to provide a new address?
> > > > > > > > >
> > > > > > > > > Note that these are real questions that I am asking myself. I'm
> > > > > > > > > fine adapting my implementation, as long as I get the main idea.
> > > > > > > > >  
> > > > > > > >
> > > > > > > > See above.  
> > > > > > >
> > > > > > > That's okay for me. I will adapt my implementation to use the
> > > > > > > interface thing. In the mean time additional details about what a
> > > > > > > coordinator interface should do differently (above question) is
> > > > > > > welcome because this is not something I am really comfortable with.  
> > > > > >
> > > > > > I've updated the implementation to use the IFACE_COORD interface and it
> > > > > > works fine, besides one question below.
> > > > > >
> > > > > > Also, I read the spec once again (soon I'll sleep with it) and
> > > > > > actually what I extracted is that:
> > > > > >
> > > > > > * A FFD, when turned on, will perform a scan, then associate to any PAN
> > > > > >   it found (algorithm is beyond the spec) or otherwise create a PAN ID
> > > > > >   and start its own PAN. In both cases, it finishes its setup by
> > > > > >   starting to send beacons.
> > > > > >  
> > > > >
> > > > > What does it mean "algorithm is beyond the spec" - build your own?  
> > > >
> > > > This is really what is in the spec, I suppose it means "do what you
> > > > want in your use case".
> > > >
> > > > What I have in mind: when a device is powered on and detects two PANs,
> > > > well, it can join whichever it wants, but perhaps we should make the
> > > > decision based on the LQI information we have (the closer the better).
> > > >  
> > >
> > > As I said in the other mail, this smells more and more for me to move
> > > this handling to user space. The kernel therefore supports operations
> > > to trigger the necessary steps (scan/assoc/etc.)
> > >  
> > > > > > * A RFD will behave more or less the same, without the PAN creation
> > > > > >   possibility of course. RFD-RX and RFD-TX are not required to support
> > > > > >   any of that, I'll assume none of the scanning features is suitable
> > > > > >   for them.
> > > > > >
> > > > > > I have a couple of questions however:
> > > > > >
> > > > > > - Creating an interface (let's call it wpancoord) out of wpan0 means
> > > > > >   that two interfaces can be used in different ways and one can use
> > > > > >   wpan0 as a node while using wpancoord as a PAN coordinator. Is that
> > > > > >   really allowed? How should we prevent this from happening?
> > > > > >  
> > > > >
> > > > > When the hardware does not support it, it should be forbidden. As most
> > > > > transceivers have only one address filter it should be forbidden
> > > > > then... but there exists a way to indeed have such a setup (which you
> > > > > probably don't need to think about). It's better to forbid something
> > > > > now, with the possibility later allowing it. So it should not break
> > > > > any existing behaviour.  
> > > >
> > > > Done, thanks to the pointer you gave in the other mail.
> > > >  
> > > > >  
> > > > > > - Should the device always wait for the user(space) to provide the PAN
> > > > > >   to associate to after the scan procedure right after the
> > > > > >   add_interface()? (like an information that must be provided prior to
> > > > > >   set the interface up?)
> > > > > >
> > > > > > - How does an orphan FFD should pick the PAN ID for a PAN creation?
> > > > > >   Should we use a random number? Start from 0 upwards? Start from
> > > > > >   0xfffd downwards? Should the user always provide it?
> > > > > >  
> > > > >
> > > > > I think this can be done all with some "fallback strategies" (build
> > > > > your own) if it's not given as a parameter.  
> > > >
> > > > Ok, In case no PAN is found, and at creation no PAN ID is provided, we
> > > > can default to 0.
> > > >  
> > >
> > > See me for other mails. (user space job)
> > >  
> > > > > > - Should an FFD be able to create its own PAN on demand? Shall we
> > > > > >   allow to do that at the creation of the new interface?
> > > > > >  
> > > > >
> > > > > I thought the spec said "or otherwise"? That means if nothing can be
> > > > > found, create one?  
> > > >
> > > > Ok, so we assume this is only at startup, fine. But then how to handle
> > > > the set_pan_id() call? I believe we can forbid any set_pan_id() command
> > > > to be run while the interface is up. That would ease the handling.
> > > > Unless I am missing something?
> > > >  
> > >
> > > See my other mails (user space job).  
> >
> > Ok then, I'll go with the following constraints in mind:
> >
> > SCAN (passive/active) (all devices)
> > - All devices are allowed to perform scans.
> > - The user decides when a scan must be performed, there is no
> >   limitation on when to do a scan (but the interface must be up for
> >   physical reasons).  
> 
> Yes, I think it should not have anything to do with interface
> limitation.... it needs to have an operating phy.

Yes

> However I can say
> more to this when I see code (but please don't provide me with any
> github repository, I mean here on the mailing list and not a more than
> 15 patches stack, Thanks.) You probably want to say on an user level
> "run scan for iface $FOO" but this is just to make it simpler.
> 
> > PAN ID
> > - The user is responsible to set the PAN ID.  
> 
> This is currently the case and I don't see a reason to change it.
> 
> > - Like several other parameters, the PAN ID can only be changed if the
> >   iface is down. Which means the user might need to do:
> >         link up > scan > link down > set params > link up  
> 
> Yes, changing this behaviour will break other things.
> 
> > BEACON
> > - Coordinator interfaces only can send beacons.  
> 
> okay.
> 
> > - Beacons can only be sent when part of a PAN (PAN ID != 0xffff).  
> 
> I guess that 0xffff means no pan is set and if no pan is set there is no pan?

Yes, Table 8-94—MAC PIB attributes states:

	"The identifier of the PAN on which the device is operating. If
	this value is 0xffff, the device is not associated."

> > - The choice of the beacon interval is up to the user, at any moment.
> > OTHER PARAMETERS  
> 
> I would say "okay", there might be an implementation detail about when
> it's effective.
> But is this not only required if doing such "passive" mode?

The spec states that a coordinator can be in one of these 3 states:
- Not associated/not in a PAN yet: it cannot send beacons nor answer
  beacon requests
- Associated/in a PAN and in this case:
    - It can be configured to answer beacon requests (for other
      devices performing active scans)
    - It can be configured to send beacons "passively" (for other
      devices performing passive scans)

In practice we will let to the user the choice of sending beacons
passively or answering to beacon requests or doing nothing by adding a
fourth possibility:
    - The device is not configured, it does not send beacons, even when
      receiving a beacon request, no matter its association status.

> > - The choice of the channel (page, etc) is free until the device is
> >   associated to another, then it becomes fixed.
> >  
> 
> I would say no here, because if the user changes it it's their
> problem... it's required to be root for doing it and that should be
> enough to do idiot things?

That was a proposal to match the spec, but I do agree we can let the
user handle this, so I won't add any checks regarding channel changes.

> > ASSOCIATION (to be done)
> > - Device association/disassociation procedure is requested by the
> >   user.  
> 
> This is similar like wireless is doing with assoc/deassoc to ap.

Kind of, yes.

> > - Accepting new associations is up to the user (coordinator only).  
> 
> Again implementation details how this should be realized.

Any coordinator can decide whether new associations are possible or
not. There is no real use case besides this option besides the memory
consumption on limited devices. So either we say "we don't care about
that possible limitation on Linux systems" or "let's add a user
parameter which tells eg. the number of devices allowed to associate".

What's your favorite?

> > - If the device has no parent (was not associated to any device) it is
> >   PAN coordinator and has additional rights regarding associations.
> >  
> 
> No idea what a "device' here is, did we not made a difference between
> "coordinator" vs "PAN coordinator" before and PAN is that thing which
> does some automatically scan/assoc operation and the other one not? I
> really have no idea what "device" here means.

When implementing association, we need to keep track of the
parent/child relationship because we may expect coordinators to
propagate messages from leaf node up to their parent. A device without
parent is then the PAN coordinator. Any coordinator may advertise the
PAN and subsequent devices may attach to it, this is creating a tree of
nodes.

The sentence about the additional rights is wrong, however, the spec
does not state anything about it, it was a misinterpretation on my side.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-20  9:19                       ` Miquel Raynal
@ 2022-06-21  1:54                         ` Alexander Aring
  2022-06-21  6:27                           ` Miquel Raynal
  0 siblings, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-21  1:54 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Stefan Schmidt, Alexander Aring, linux-wpan - ML, David Girault,
	Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development

Hi,

On Mon, Jun 20, 2022 at 5:19 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
...
> >
> > > - Beacons can only be sent when part of a PAN (PAN ID != 0xffff).
> >
> > I guess that 0xffff means no pan is set and if no pan is set there is no pan?
>
> Yes, Table 8-94—MAC PIB attributes states:
>
>         "The identifier of the PAN on which the device is operating. If
>         this value is 0xffff, the device is not associated."
>

I am not sure if I understand this correctly but for me sending
beacons means something like "here is a pan which I broadcast around"
and then there is "'device' is not associated". Is when "associated"
(doesn't matter if set manual or due scan/assoc) does this behaviour
implies "I am broadcasting my pan around, because my panid is !=
0xffff" ?

> > > - The choice of the beacon interval is up to the user, at any moment.
> > > OTHER PARAMETERS
> >
> > I would say "okay", there might be an implementation detail about when
> > it's effective.
> > But is this not only required if doing such "passive" mode?
>
> The spec states that a coordinator can be in one of these 3 states:
> - Not associated/not in a PAN yet: it cannot send beacons nor answer
>   beacon requests

so this will confirm, it should send beacons if panid != 0xffff (as my
question above)?

> - Associated/in a PAN and in this case:
>     - It can be configured to answer beacon requests (for other
>       devices performing active scans)
>     - It can be configured to send beacons "passively" (for other
>       devices performing passive scans)
>
> In practice we will let to the user the choice of sending beacons
> passively or answering to beacon requests or doing nothing by adding a
> fourth possibility:
>     - The device is not configured, it does not send beacons, even when
>       receiving a beacon request, no matter its association status.
>

Where is this "user choice"? I mean you handle those answers for
beacon requests in the kernel?

> > > - The choice of the channel (page, etc) is free until the device is
> > >   associated to another, then it becomes fixed.
> > >
> >
> > I would say no here, because if the user changes it it's their
> > problem... it's required to be root for doing it and that should be
> > enough to do idiot things?
>
> That was a proposal to match the spec, but I do agree we can let the
> user handle this, so I won't add any checks regarding channel changes.
>

okay.

> > > ASSOCIATION (to be done)
> > > - Device association/disassociation procedure is requested by the
> > >   user.
> >
> > This is similar like wireless is doing with assoc/deassoc to ap.
>
> Kind of, yes.
>

In the sense of "by the user" you don't mean putting this logic into
user space you want to do it in kernel and implement it as a
netlink-op, the same as wireless is doing? I just want to confirm
that. Of course everything else is different, but from this
perspective it should be realized.

> > > - Accepting new associations is up to the user (coordinator only).
> >
> > Again implementation details how this should be realized.
>
> Any coordinator can decide whether new associations are possible or
> not. There is no real use case besides this option besides the memory
> consumption on limited devices. So either we say "we don't care about
> that possible limitation on Linux systems" or "let's add a user
> parameter which tells eg. the number of devices allowed to associate".
>
> What's your favorite?
>

Sure there should be a limitation about how many pans should be
allowed, that is somehow the bare minimum which should be there.
I was not quite sure how the user can decide of denied assoc or not,
but this seems out of scope for right now...

> > > - If the device has no parent (was not associated to any device) it is
> > >   PAN coordinator and has additional rights regarding associations.
> > >
> >
> > No idea what a "device' here is, did we not made a difference between
> > "coordinator" vs "PAN coordinator" before and PAN is that thing which
> > does some automatically scan/assoc operation and the other one not? I
> > really have no idea what "device" here means.
>
> When implementing association, we need to keep track of the
> parent/child relationship because we may expect coordinators to
> propagate messages from leaf node up to their parent. A device without
> parent is then the PAN coordinator. Any coordinator may advertise the
> PAN and subsequent devices may attach to it, this is creating a tree of
> nodes.
>


Who is keeping track of this relationship? So we store pans which we
found with a whole "subtree" attached to it?

btw: that sounds similar to me to what RPL is doing...,

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-21  1:54                         ` Alexander Aring
@ 2022-06-21  6:27                           ` Miquel Raynal
  2022-06-26  1:36                             ` Alexander Aring
  0 siblings, 1 reply; 29+ messages in thread
From: Miquel Raynal @ 2022-06-21  6:27 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, Alexander Aring, linux-wpan - ML, David Girault,
	Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development

Hi Alexander,

aahringo@redhat.com wrote on Mon, 20 Jun 2022 21:54:33 -0400:

> Hi,
> 
> On Mon, Jun 20, 2022 at 5:19 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> ...
> > >  
> > > > - Beacons can only be sent when part of a PAN (PAN ID != 0xffff).  
> > >
> > > I guess that 0xffff means no pan is set and if no pan is set there is no pan?  
> >
> > Yes, Table 8-94—MAC PIB attributes states:
> >
> >         "The identifier of the PAN on which the device is operating. If
> >         this value is 0xffff, the device is not associated."
> >  
> 
> I am not sure if I understand this correctly but for me sending
> beacons means something like "here is a pan which I broadcast around"

Yes, and any coordinator in a beacon enabled PAN is required to send
beacons to advertise the PAN after it associated.

> and then there is "'device' is not associated".

FFDs are not supposed to send any beacon if they are not part of a PAN.

> Is when "associated"
> (doesn't matter if set manual or due scan/assoc) does this behaviour
> implies "I am broadcasting my pan around, because my panid is !=
> 0xffff" ?

I think so, yes. To summarize:

Associated:
* PAN is != 0xffff
* FFD can send beacons
Not associated:
* PAN is == 0xffff
* FFD cannot send beacons

> > > > - The choice of the beacon interval is up to the user, at any moment.
> > > > OTHER PARAMETERS  
> > >
> > > I would say "okay", there might be an implementation detail about when
> > > it's effective.
> > > But is this not only required if doing such "passive" mode?  
> >
> > The spec states that a coordinator can be in one of these 3 states:
> > - Not associated/not in a PAN yet: it cannot send beacons nor answer
> >   beacon requests  
> 
> so this will confirm, it should send beacons if panid != 0xffff (as my
> question above)?

It should only send beacons if in a beacon-enabled PAN. The spec is
not very clear about if this is mandatory or not.

> > - Associated/in a PAN and in this case:
> >     - It can be configured to answer beacon requests (for other
> >       devices performing active scans)
> >     - It can be configured to send beacons "passively" (for other
> >       devices performing passive scans)
> >
> > In practice we will let to the user the choice of sending beacons
> > passively or answering to beacon requests or doing nothing by adding a
> > fourth possibility:
> >     - The device is not configured, it does not send beacons, even when
> >       receiving a beacon request, no matter its association status.
> >  
> 
> Where is this "user choice"? I mean you handle those answers for
> beacon requests in the kernel?

Without user input, so the default state remains the same as today: do
not send beacons + do not answer beacon requests. Then, we created a:

	iwpan dev xxx beacons send ...

command which, depending on the beacon interval will either send
beacons at a given pace (interval < 15) or answer beacon requests
otherwise.

If the user want's to get back to the initial state (silent device):

	iwpan dev xxx beacons stop

> > > > - The choice of the channel (page, etc) is free until the device is
> > > >   associated to another, then it becomes fixed.
> > > >  
> > >
> > > I would say no here, because if the user changes it it's their
> > > problem... it's required to be root for doing it and that should be
> > > enough to do idiot things?  
> >
> > That was a proposal to match the spec, but I do agree we can let the
> > user handle this, so I won't add any checks regarding channel changes.
> >  
> 
> okay.
> 
> > > > ASSOCIATION (to be done)
> > > > - Device association/disassociation procedure is requested by the
> > > >   user.  
> > >
> > > This is similar like wireless is doing with assoc/deassoc to ap.  
> >
> > Kind of, yes.
> >  
> 
> In the sense of "by the user" you don't mean putting this logic into
> user space you want to do it in kernel and implement it as a
> netlink-op, the same as wireless is doing? I just want to confirm
> that. Of course everything else is different, but from this
> perspective it should be realized.

Yes absolutely, the user would have a command (which I am currently
writing) like:

iwpan dev xxx associate pan_id yyy coord zzz
iwpan dev xxx disassociate device zzz

Mind the disassociate command which works for parents (you are
associated to a device and want to leave the PAN) and also for children
(a device is associated to you, and you request it to leave the PAN).

> > > > - Accepting new associations is up to the user (coordinator only).  
> > >
> > > Again implementation details how this should be realized.  
> >
> > Any coordinator can decide whether new associations are possible or
> > not. There is no real use case besides this option besides the memory
> > consumption on limited devices. So either we say "we don't care about
> > that possible limitation on Linux systems" or "let's add a user
> > parameter which tells eg. the number of devices allowed to associate".
> >
> > What's your favorite?
> >  
> 
> Sure there should be a limitation about how many pans should be
> allowed, that is somehow the bare minimum which should be there.
> I was not quite sure how the user can decide of denied assoc or not,
> but this seems out of scope for right now...

I thought as well about this, I think in the future we might find a way
to whitelist or blacklist devices and have these answers being sent
automatically by the kernel based on user lists, but this is really an
advanced feature and there is currently no use case yet, so I'll go for
the netlink op which changes the default number of devices which may
associate to a coordinator.
> 
> > > > - If the device has no parent (was not associated to any device) it is
> > > >   PAN coordinator and has additional rights regarding associations.
> > > >  
> > >
> > > No idea what a "device' here is, did we not made a difference between
> > > "coordinator" vs "PAN coordinator" before and PAN is that thing which
> > > does some automatically scan/assoc operation and the other one not? I
> > > really have no idea what "device" here means.  
> >
> > When implementing association, we need to keep track of the
> > parent/child relationship because we may expect coordinators to
> > propagate messages from leaf node up to their parent. A device without
> > parent is then the PAN coordinator. Any coordinator may advertise the
> > PAN and subsequent devices may attach to it, this is creating a tree of
> > nodes.
> >  
> 
> 
> Who is keeping track of this relationship?

Each device keeps track of:
- the parent (the coordinator it associated with, NULL if PAN
  coordinator)
- a list of devices (FFD or RFD) which successfully associated

> So we store pans which we
> found with a whole "subtree" attached to it?

This is different, we need basically three lists:
- the parent in the PAN
- the children (as in the logic of association) in the PAN
- the coordinators around which advertise their PAN with beacons (the
  PAN can be the same as ours, or different)

> btw: that sounds similar to me to what RPL is doing...,

I think RPL stands one level above in the OSI layers? Anyway, my
understanding (and this is really something which I grasped by reading
papers because the spec lacks a lot of information about it) is that:
- the list of PANs is mainly useful for our own initial association
- the list of immediate parent/children is to be used by the upper
  layer to create routes, but as in the 802.15.4 layer, we are not
  supposed to propagate this information besides giving it to the
  "next upper layer".

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-21  6:27                           ` Miquel Raynal
@ 2022-06-26  1:36                             ` Alexander Aring
  2022-06-27  8:17                               ` Miquel Raynal
  0 siblings, 1 reply; 29+ messages in thread
From: Alexander Aring @ 2022-06-26  1:36 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Stefan Schmidt, Alexander Aring, linux-wpan - ML, David Girault,
	Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development

Hi,

On Tue, Jun 21, 2022 at 2:27 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Mon, 20 Jun 2022 21:54:33 -0400:
>
> > Hi,
> >
> > On Mon, Jun 20, 2022 at 5:19 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > ...
> > > >
> > > > > - Beacons can only be sent when part of a PAN (PAN ID != 0xffff).
> > > >
> > > > I guess that 0xffff means no pan is set and if no pan is set there is no pan?
> > >
> > > Yes, Table 8-94—MAC PIB attributes states:
> > >
> > >         "The identifier of the PAN on which the device is operating. If
> > >         this value is 0xffff, the device is not associated."
> > >
> >
> > I am not sure if I understand this correctly but for me sending
> > beacons means something like "here is a pan which I broadcast around"
>
> Yes, and any coordinator in a beacon enabled PAN is required to send
> beacons to advertise the PAN after it associated.
>
> > and then there is "'device' is not associated".
>

I think there are several misunderstandings in the used terms in this
discussion.

A "associated" device does not imply an association by using the mac
commands, it can also be by setting a pan id manually without doing
anything of communication.
But this would for me just end in a term "has valid PAN id",
association is using the mac commands.

> FFDs are not supposed to send any beacon if they are not part of a PAN.
>

FFD is a device capability (it can run as pan coordinator, which is a
node with more functions), it is not a term which can be used in
combination with an instance of a living role in the network
(coordinator). That's for me what I understand in linux-wpan terms ->
it's a coordinator interface.

> > Is when "associated"
> > (doesn't matter if set manual or due scan/assoc) does this behaviour
> > implies "I am broadcasting my pan around, because my panid is !=
> > 0xffff" ?
>
> I think so, yes. To summarize:
>
> Associated:
> * PAN is != 0xffff
> * FFD can send beacons
> Not associated:
> * PAN is == 0xffff
> * FFD cannot send beacons
>

But the user need to explicit enable the "reacting/sending (probably
depends on active/passive) beacon feature" which indeed sounds fine to
me.

> > > > > - The choice of the beacon interval is up to the user, at any moment.
> > > > > OTHER PARAMETERS
> > > >
> > > > I would say "okay", there might be an implementation detail about when
> > > > it's effective.
> > > > But is this not only required if doing such "passive" mode?
> > >
> > > The spec states that a coordinator can be in one of these 3 states:
> > > - Not associated/not in a PAN yet: it cannot send beacons nor answer
> > >   beacon requests
> >
> > so this will confirm, it should send beacons if panid != 0xffff (as my
> > question above)?
>
> It should only send beacons if in a beacon-enabled PAN. The spec is
> not very clear about if this is mandatory or not.
>

as above. Sounds fine to me to have a setting start/stop for that.

> > > - Associated/in a PAN and in this case:
> > >     - It can be configured to answer beacon requests (for other
> > >       devices performing active scans)
> > >     - It can be configured to send beacons "passively" (for other
> > >       devices performing passive scans)
> > >
> > > In practice we will let to the user the choice of sending beacons
> > > passively or answering to beacon requests or doing nothing by adding a
> > > fourth possibility:
> > >     - The device is not configured, it does not send beacons, even when
> > >       receiving a beacon request, no matter its association status.
> > >
> >
> > Where is this "user choice"? I mean you handle those answers for
> > beacon requests in the kernel?
>
> Without user input, so the default state remains the same as today: do
> not send beacons + do not answer beacon requests. Then, we created a:
>
>         iwpan dev xxx beacons send ...
>
> command which, depending on the beacon interval will either send
> beacons at a given pace (interval < 15) or answer beacon requests
> otherwise.
>
> If the user want's to get back to the initial state (silent device):
>
>         iwpan dev xxx beacons stop
>

sounds fine.

> > > > > - The choice of the channel (page, etc) is free until the device is
> > > > >   associated to another, then it becomes fixed.
> > > > >
> > > >
> > > > I would say no here, because if the user changes it it's their
> > > > problem... it's required to be root for doing it and that should be
> > > > enough to do idiot things?
> > >
> > > That was a proposal to match the spec, but I do agree we can let the
> > > user handle this, so I won't add any checks regarding channel changes.
> > >
> >
> > okay.
> >
> > > > > ASSOCIATION (to be done)
> > > > > - Device association/disassociation procedure is requested by the
> > > > >   user.
> > > >
> > > > This is similar like wireless is doing with assoc/deassoc to ap.
> > >
> > > Kind of, yes.
> > >
> >
> > In the sense of "by the user" you don't mean putting this logic into
> > user space you want to do it in kernel and implement it as a
> > netlink-op, the same as wireless is doing? I just want to confirm
> > that. Of course everything else is different, but from this
> > perspective it should be realized.
>
> Yes absolutely, the user would have a command (which I am currently
> writing) like:
>
> iwpan dev xxx associate pan_id yyy coord zzz
> iwpan dev xxx disassociate device zzz
>

Can say more about that if I am seeing code.

> Mind the disassociate command which works for parents (you are
> associated to a device and want to leave the PAN) and also for children
> (a device is associated to you, and you request it to leave the PAN).
>
> > > > > - Accepting new associations is up to the user (coordinator only).
> > > >
> > > > Again implementation details how this should be realized.
> > >
> > > Any coordinator can decide whether new associations are possible or
> > > not. There is no real use case besides this option besides the memory
> > > consumption on limited devices. So either we say "we don't care about
> > > that possible limitation on Linux systems" or "let's add a user
> > > parameter which tells eg. the number of devices allowed to associate".
> > >
> > > What's your favorite?
> > >
> >
> > Sure there should be a limitation about how many pans should be
> > allowed, that is somehow the bare minimum which should be there.
> > I was not quite sure how the user can decide of denied assoc or not,
> > but this seems out of scope for right now...
>
> I thought as well about this, I think in the future we might find a way
> to whitelist or blacklist devices and have these answers being sent
> automatically by the kernel based on user lists, but this is really an
> advanced feature and there is currently no use case yet, so I'll go for
> the netlink op which changes the default number of devices which may
> associate to a coordinator.

yes.

> >
> > > > > - If the device has no parent (was not associated to any device) it is
> > > > >   PAN coordinator and has additional rights regarding associations.
> > > > >
> > > >
> > > > No idea what a "device' here is, did we not made a difference between
> > > > "coordinator" vs "PAN coordinator" before and PAN is that thing which
> > > > does some automatically scan/assoc operation and the other one not? I
> > > > really have no idea what "device" here means.
> > >
> > > When implementing association, we need to keep track of the
> > > parent/child relationship because we may expect coordinators to
> > > propagate messages from leaf node up to their parent. A device without
> > > parent is then the PAN coordinator. Any coordinator may advertise the
> > > PAN and subsequent devices may attach to it, this is creating a tree of
> > > nodes.
> > >
> >
> >
> > Who is keeping track of this relationship?
>
> Each device keeps track of:
> - the parent (the coordinator it associated with, NULL if PAN
>   coordinator)
> - a list of devices (FFD or RFD) which successfully associated
>
> > So we store pans which we
> > found with a whole "subtree" attached to it?
>
> This is different, we need basically three lists:
> - the parent in the PAN
> - the children (as in the logic of association) in the PAN
> - the coordinators around which advertise their PAN with beacons (the
>   PAN can be the same as ours, or different)
>
> > btw: that sounds similar to me to what RPL is doing...,
>
> I think RPL stands one level above in the OSI layers? Anyway, my
> understanding (and this is really something which I grasped by reading
> papers because the spec lacks a lot of information about it) is that:
> - the list of PANs is mainly useful for our own initial association
> - the list of immediate parent/children is to be used by the upper
>   layer to create routes, but as in the 802.15.4 layer, we are not
>   supposed to propagate this information besides giving it to the
>   "next upper layer".
>

I am asking the question who stores that, because I don't think this
should be stored in kernel space. A user space daemon will trigger the
mac commands scan/assoc/deassoc/etc. (kernel job) and get the results
of those commands and store it in however the user space daemon is
handling it. This is from my point of view no kernel job, triggering
the mac commands in a way that we can add all children information,
parents, etc, whatever you need to fill information in mac commands.
Yes, this is a kernel task.

Also I heard that association with a coordinator will allocate in the
pan a short address, the associated device (which triggered the
association command) will then set the allocated short address by the
coordinator, did you take a look into that?

- Alex


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
  2022-06-26  1:36                             ` Alexander Aring
@ 2022-06-27  8:17                               ` Miquel Raynal
  0 siblings, 0 replies; 29+ messages in thread
From: Miquel Raynal @ 2022-06-27  8:17 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, Alexander Aring, linux-wpan - ML, David Girault,
	Romuald Despres, Frederic Blain, Nicolas Schodet,
	Thomas Petazzoni, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Network Development

Hi Alexander,

aahringo@redhat.com wrote on Sat, 25 Jun 2022 21:36:05 -0400:

> Hi,
> 
> On Tue, Jun 21, 2022 at 2:27 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Mon, 20 Jun 2022 21:54:33 -0400:
> >  
> > > Hi,
> > >
> > > On Mon, Jun 20, 2022 at 5:19 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > ...  
> > > > >  
> > > > > > - Beacons can only be sent when part of a PAN (PAN ID != 0xffff).  
> > > > >
> > > > > I guess that 0xffff means no pan is set and if no pan is set there is no pan?  
> > > >
> > > > Yes, Table 8-94—MAC PIB attributes states:
> > > >
> > > >         "The identifier of the PAN on which the device is operating. If
> > > >         this value is 0xffff, the device is not associated."
> > > >  
> > >
> > > I am not sure if I understand this correctly but for me sending
> > > beacons means something like "here is a pan which I broadcast around"  
> >
> > Yes, and any coordinator in a beacon enabled PAN is required to send
> > beacons to advertise the PAN after it associated.
> >  
> > > and then there is "'device' is not associated".  
> >  
> 
> I think there are several misunderstandings in the used terms in this
> discussion.
> 
> A "associated" device does not imply an association by using the mac
> commands, it can also be by setting a pan id manually without doing
> anything of communication.

Yes, I am referring to the association procedure here, but I agree it
can be done "statically", that's how it's been done in Linux so far
anyway.

> But this would for me just end in a term "has valid PAN id",
> association is using the mac commands.
> 
> > FFDs are not supposed to send any beacon if they are not part of a PAN.
> >  
> 
> FFD is a device capability (it can run as pan coordinator, which is a
> node with more functions), it is not a term which can be used in
> combination with an instance of a living role in the network
> (coordinator). That's for me what I understand in linux-wpan terms ->
> it's a coordinator interface.

I think we are aligned on the terms.

"Coordinators are not supposed to send any beacon if they are not part
of a PAN." if you prefer. But then while your are not connected to any
network, can we really talk about coordinator? A coordinator, as you
rightfully said, is a living role, so talking about a coordinator that
is not "associated" does not really make sense. Hence the generic term
"device", which I stretched a bit towards "FFD" because RFDs cannot
become coordinators on the network.

Anyway, I think we mean the same thing :)

> > > Is when "associated"
> > > (doesn't matter if set manual or due scan/assoc) does this behaviour
> > > implies "I am broadcasting my pan around, because my panid is !=
> > > 0xffff" ?  
> >
> > I think so, yes. To summarize:
> >
> > Associated:
> > * PAN is != 0xffff
> > * FFD can send beacons
> > Not associated:
> > * PAN is == 0xffff
> > * FFD cannot send beacons
> >  
> 
> But the user need to explicit enable the "reacting/sending (probably
> depends on active/passive) beacon feature" which indeed sounds fine to
> me.

Absolutely

> > > > > > - The choice of the beacon interval is up to the user, at any moment.
> > > > > > OTHER PARAMETERS  
> > > > >
> > > > > I would say "okay", there might be an implementation detail about when
> > > > > it's effective.
> > > > > But is this not only required if doing such "passive" mode?  
> > > >
> > > > The spec states that a coordinator can be in one of these 3 states:
> > > > - Not associated/not in a PAN yet: it cannot send beacons nor answer
> > > >   beacon requests  
> > >
> > > so this will confirm, it should send beacons if panid != 0xffff (as my
> > > question above)?  
> >
> > It should only send beacons if in a beacon-enabled PAN. The spec is
> > not very clear about if this is mandatory or not.
> >  
> 
> as above. Sounds fine to me to have a setting start/stop for that.

Yes

> > > > - Associated/in a PAN and in this case:
> > > >     - It can be configured to answer beacon requests (for other
> > > >       devices performing active scans)
> > > >     - It can be configured to send beacons "passively" (for other
> > > >       devices performing passive scans)
> > > >
> > > > In practice we will let to the user the choice of sending beacons
> > > > passively or answering to beacon requests or doing nothing by adding a
> > > > fourth possibility:
> > > >     - The device is not configured, it does not send beacons, even when
> > > >       receiving a beacon request, no matter its association status.
> > > >  
> > >
> > > Where is this "user choice"? I mean you handle those answers for
> > > beacon requests in the kernel?  
> >
> > Without user input, so the default state remains the same as today: do
> > not send beacons + do not answer beacon requests. Then, we created a:
> >
> >         iwpan dev xxx beacons send ...
> >
> > command which, depending on the beacon interval will either send
> > beacons at a given pace (interval < 15) or answer beacon requests
> > otherwise.
> >
> > If the user want's to get back to the initial state (silent device):
> >
> >         iwpan dev xxx beacons stop
> >  
> 
> sounds fine.
> 
> > > > > > - The choice of the channel (page, etc) is free until the device is
> > > > > >   associated to another, then it becomes fixed.
> > > > > >  
> > > > >
> > > > > I would say no here, because if the user changes it it's their
> > > > > problem... it's required to be root for doing it and that should be
> > > > > enough to do idiot things?  
> > > >
> > > > That was a proposal to match the spec, but I do agree we can let the
> > > > user handle this, so I won't add any checks regarding channel changes.
> > > >  
> > >
> > > okay.
> > >  
> > > > > > ASSOCIATION (to be done)
> > > > > > - Device association/disassociation procedure is requested by the
> > > > > >   user.  
> > > > >
> > > > > This is similar like wireless is doing with assoc/deassoc to ap.  
> > > >
> > > > Kind of, yes.
> > > >  
> > >
> > > In the sense of "by the user" you don't mean putting this logic into
> > > user space you want to do it in kernel and implement it as a
> > > netlink-op, the same as wireless is doing? I just want to confirm
> > > that. Of course everything else is different, but from this
> > > perspective it should be realized.  
> >
> > Yes absolutely, the user would have a command (which I am currently
> > writing) like:
> >
> > iwpan dev xxx associate pan_id yyy coord zzz
> > iwpan dev xxx disassociate device zzz
> >  
> 
> Can say more about that if I am seeing code.
> 
> > Mind the disassociate command which works for parents (you are
> > associated to a device and want to leave the PAN) and also for children
> > (a device is associated to you, and you request it to leave the PAN).
> >  
> > > > > > - Accepting new associations is up to the user (coordinator only).  
> > > > >
> > > > > Again implementation details how this should be realized.  
> > > >
> > > > Any coordinator can decide whether new associations are possible or
> > > > not. There is no real use case besides this option besides the memory
> > > > consumption on limited devices. So either we say "we don't care about
> > > > that possible limitation on Linux systems" or "let's add a user
> > > > parameter which tells eg. the number of devices allowed to associate".
> > > >
> > > > What's your favorite?
> > > >  
> > >
> > > Sure there should be a limitation about how many pans should be
> > > allowed, that is somehow the bare minimum which should be there.
> > > I was not quite sure how the user can decide of denied assoc or not,
> > > but this seems out of scope for right now...  
> >
> > I thought as well about this, I think in the future we might find a way
> > to whitelist or blacklist devices and have these answers being sent
> > automatically by the kernel based on user lists, but this is really an
> > advanced feature and there is currently no use case yet, so I'll go for
> > the netlink op which changes the default number of devices which may
> > associate to a coordinator.  
> 
> yes.
> 
> > >  
> > > > > > - If the device has no parent (was not associated to any device) it is
> > > > > >   PAN coordinator and has additional rights regarding associations.
> > > > > >  
> > > > >
> > > > > No idea what a "device' here is, did we not made a difference between
> > > > > "coordinator" vs "PAN coordinator" before and PAN is that thing which
> > > > > does some automatically scan/assoc operation and the other one not? I
> > > > > really have no idea what "device" here means.  
> > > >
> > > > When implementing association, we need to keep track of the
> > > > parent/child relationship because we may expect coordinators to
> > > > propagate messages from leaf node up to their parent. A device without
> > > > parent is then the PAN coordinator. Any coordinator may advertise the
> > > > PAN and subsequent devices may attach to it, this is creating a tree of
> > > > nodes.
> > > >  
> > >
> > >
> > > Who is keeping track of this relationship?  
> >
> > Each device keeps track of:
> > - the parent (the coordinator it associated with, NULL if PAN
> >   coordinator)
> > - a list of devices (FFD or RFD) which successfully associated
> >  
> > > So we store pans which we
> > > found with a whole "subtree" attached to it?  
> >
> > This is different, we need basically three lists:
> > - the parent in the PAN
> > - the children (as in the logic of association) in the PAN
> > - the coordinators around which advertise their PAN with beacons (the
> >   PAN can be the same as ours, or different)
> >  
> > > btw: that sounds similar to me to what RPL is doing...,  
> >
> > I think RPL stands one level above in the OSI layers? Anyway, my
> > understanding (and this is really something which I grasped by reading
> > papers because the spec lacks a lot of information about it) is that:
> > - the list of PANs is mainly useful for our own initial association
> > - the list of immediate parent/children is to be used by the upper
> >   layer to create routes, but as in the 802.15.4 layer, we are not
> >   supposed to propagate this information besides giving it to the
> >   "next upper layer".
> >  
> 
> I am asking the question who stores that, because I don't think this
> should be stored in kernel space.

I think the kernel needs to keep track of the associated devices,
just to be able to give this information in an asynchronous manner to
the user ("next upper layer", whatever) and do very basic checks on
the user inputs to avoid confusing the neighboring devices. Note that
there is no way to "check" with mac commands if an association is
active or not. In practice I agree the kernel itself should not make any
other use of these information.

> A user space daemon will trigger the
> mac commands scan/assoc/deassoc/etc.

Yes

> (kernel job) and get the results of those commands and store it in
> however the user space daemon is handling it. This is from my point
> of view no kernel job, triggering the mac commands in a way that we
> can add all children information, parents, etc, whatever you need to
> fill information in mac commands. Yes, this is a kernel task.

So far I only focused on performing the mac commands and updating the
parent/children list, perhaps that should be improved to also give the
user the opportunity to add an association in the list that has been
performed "statically" (without mac commands, just by setting the PAN
ID to the right value).

> Also I heard that association with a coordinator will allocate in the
> pan a short address, the associated device (which triggered the
> association command) will then set the allocated short address by the
> coordinator, did you take a look into that?

Yes, the association requests contains a bit which means "please give
me a short address". The coordinator that receives the request should
then, if it decides to successfully associate with the device, get a
short address (randomly) and give it to the device by responding with an
association response which contains this address in the payload. The
associating device will then configure itself with this short address,
until it disassociates from the PAN. If the association request "give
me a short address" bit is unset, it means the device does not
want/supports short addressing, the coordinator should answer with the
specific value 0xfffe to notify the device that it will continue using
its extended address only as part of the communications happening
within the PAN. Typically, constrained RFDs might not accept short
addresses.

There is one caveat though, and this is not covered by the spec at all:
the coordinator has no knowledge of the surrounding devices out of its
own range, which could be part of the PAN through another coordinator.
In this case, if there is a short address conflict, there is no
procedure to detect nor correct the situation.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2022-06-27  8:18 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-03 18:21 [PATCH wpan-next 0/6] net: ieee802154: PAN management Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type Miquel Raynal
2022-06-04  2:01   ` Alexander Aring
2022-06-06 15:43     ` Miquel Raynal
2022-06-07  3:04       ` Alexander Aring
2022-06-07 16:16         ` Miquel Raynal
2022-06-08 13:47           ` Miquel Raynal
2022-06-08 14:37             ` Miquel Raynal
2022-06-09  2:06               ` Alexander Aring
2022-06-09  2:23                 ` Alexander Aring
2022-06-09 15:43                   ` Miquel Raynal
2022-06-11 12:05                     ` Alexander Aring
2022-06-15  9:15                       ` Miquel Raynal
2022-06-09  1:56             ` Alexander Aring
2022-06-09 15:52               ` Miquel Raynal
     [not found]                 ` <CAK-6q+jchHcge2_hMznO6fwx=xoUEpmoZTFYLAUwqM2Ue4Lx-A@mail.gmail.com>
2022-06-17 15:12                   ` Miquel Raynal
2022-06-20  0:13                     ` Alexander Aring
2022-06-20  9:19                       ` Miquel Raynal
2022-06-21  1:54                         ` Alexander Aring
2022-06-21  6:27                           ` Miquel Raynal
2022-06-26  1:36                             ` Alexander Aring
2022-06-27  8:17                               ` Miquel Raynal
2022-06-09  1:42           ` Alexander Aring
2022-06-09 14:42             ` Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 2/6] net: ieee802154: Add support for internal PAN management Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 3/6] net: ieee802154: Create a node type Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 4/6] net: ieee802154: Add the PAN coordinator information Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 5/6] net: ieee802154: Full PAN management Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 6/6] net: ieee802154: Trace the registration of new PANs Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).