Skip to contentSkip to conversation

ieee802154: nl802154 SET commands and pib defaults

PATCH Superseded Alexander Aring 2 participants 7 messages Mar 23, 2015 – Mar 24, 2015

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Alexander Aring

author maintainer
#0 lore raw

v2 cover ieee802154: nl802154 SET commands and pib defaults

Hi,

this patch-series implement the already existed SET commands of an interface and a wpan-phy. Currently we have one CMD for each setting which was a mistake by me to implement it in this way. So we aren't currently a "real" UAPI header for nl802154 and it's still under development. I decide now to change to implement these setting in this way now. Currently I let these commands in coexists with the old commands but _NOTE_ I will remove the other commands from the nl802154 later.

So please use the new commands. When these patches are mainline I also want to release a new wpan-tools release which use this new interface. Then everybody has some time to change to the new wpan-tools release. After this "time" I will remove the obsolete nl802154 commands from the header. This means older wpan-tools releases doesn't work any more with new kernels. Also this means newer wpan-tools releases doesn't work with older kernels. That will end in some kind of chaos and I will note this on the website.

This behaviour to do it in only one command is much easier to handle in userspace, like clicking an "apply button".

Also this patch series contains two patches for move the MAC PIB defaults setting out of SoftMAC layer. Additional we set the max frame retries parameter to 3 which is a real 802.15.4 default. Some transceivers doesn't support ARET settings. If these transceivers doesn't support it, we assume the 802.15.4 default.

- Alex

changes since v2:
- remove ".done = nl802154_dump_wpan_phy_done" of patch 1/2.
- add other movements for mac pib defaults which is needed for cfg802154 crypto layer.

Alexander Aring (4):
nl802154: add set wpan phy cmd
nl802154: add set interface cmd
ieee802154: move mac pib defaults
ieee802154: set aret handling according to 802.15.4

 net/ieee802154/core.c     |  29 +++++++++
 net/ieee802154/nl802154.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++
 net/mac802154/iface.c     |  15 -----
 3 files changed, 190 insertions(+), 15 deletions(-)
Signature · 1 line
2.3.3
author maintainer
#1 lore raw

PATCH v2 1/4 nl802154: add set wpan phy cmd

This patch adds NL802154_CMD_SET_WPAN_PHY command to set all wpan phy attributes instead of doing separate commands. This will improve userspace application handling of nl802154.

---
 net/ieee802154/nl802154.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
net/ieee802154/nl802154.c +51 -0
index a4daf91..c12c07f 100644
@@ -437,6 +437,49 @@ static int nl802154_get_wpan_phy(struct sk_buff *skb, struct genl_info *info)
 	return genlmsg_reply(msg, info);
 }
 
+static int nl802154_set_wpan_phy(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	int ret;
+
+	if (info->attrs[NL802154_ATTR_PAGE] &&
+	    info->attrs[NL802154_ATTR_CHANNEL]) {
+		u8 channel, page;
+
+		page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]);
+		channel = nla_get_u8(info->attrs[NL802154_ATTR_CHANNEL]);
+		if (page > IEEE802154_MAX_PAGE ||
+		    channel > IEEE802154_MAX_CHANNEL)
+			return -EINVAL;
+
+		ret = rdev_set_channel(rdev, page, channel);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_CCA_MODE]) {
+		struct wpan_phy_cca cca;
+
+		cca.mode = nla_get_u32(info->attrs[NL802154_ATTR_CCA_MODE]);
+		if (cca.mode < NL802154_CCA_ENERGY ||
+		    cca.mode > NL802154_CCA_ATTR_MAX)
+			return -EINVAL;
+
+		if (cca.mode == NL802154_CCA_ENERGY_CARRIER) {
+			if (!info->attrs[NL802154_ATTR_CCA_OPT])
+				return -EINVAL;
+
+			cca.opt = nla_get_u32(info->attrs[NL802154_ATTR_CCA_OPT]);
+			if (cca.opt > NL802154_CCA_OPT_ATTR_MAX)
+				return -EINVAL;
+		}
+
+		return rdev_set_cca_mode(rdev, &cca);
+	}
+
+	return 0;
+}
+
 static inline u64 wpan_dev_id(struct wpan_dev *wpan_dev)
 {
 	return (u64)wpan_dev->identifier |
@@ -896,6 +939,14 @@ static const struct genl_ops nl802154_ops[] = {
 				  NL802154_FLAG_NEED_RTNL,
 	},
 	{
+		.cmd = NL802154_CMD_SET_WPAN_PHY,
+		.doit = nl802154_set_wpan_phy,
+		.policy = nl802154_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_WPAN_PHY |
+				  NL802154_FLAG_NEED_RTNL,
+	},
+	{
 		.cmd = NL802154_CMD_GET_INTERFACE,
 		.doit = nl802154_get_interface,
 		.dumpit = nl802154_dump_interface,
Signature · 1 line
2.3.3
#2 lore raw

Re: [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd

Hi Alex,

Quoted 25 lines · Alexander Aring

This patch adds NL802154_CMD_SET_WPAN_PHY command to set all wpan phy attributes instead of doing separate commands. This will improve userspace application handling of nl802154.

---
  net/ieee802154/nl802154.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 51 insertions(+)
net/ieee802154/nl802154.c +8 -0
index a4daf91..c12c07f 100644
@@ -437,6 +437,49 @@ static int nl802154_get_wpan_phy(struct sk_buff *skb, struct genl_info *info)
  	return genlmsg_reply(msg, info);
  }
 
+static int nl802154_set_wpan_phy(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	int ret;
+
+	if (info->attrs[NL802154_ATTR_PAGE] &&
+	    info->attrs[NL802154_ATTR_CHANNEL]) {
+		u8 channel, page;

net/ieee802154/nl802154.c static int nl802154_get_wpan_phy(struct sk_buff *skb, struct genl_info *info) · line 457

Quoted 10 lines
net/ieee802154/nl802154.c +10 -0
+
+		page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]);
+		channel = nla_get_u8(info->attrs[NL802154_ATTR_CHANNEL]);
+		if (page > IEEE802154_MAX_PAGE ||
+		    channel > IEEE802154_MAX_CHANNEL)
+			return -EINVAL;
+
+		ret = rdev_set_channel(rdev, page, channel);
+		if (ret < 0)
+			return ret;

Why dont we return directly rdev_set_channel.. Is there any significance to the check on *ret*...?

Quoted 33 lines
net/ieee802154/nl802154.c +26 -0
+	}
+
+	if (info->attrs[NL802154_ATTR_CCA_MODE]) {
+		struct wpan_phy_cca cca;
+
+		cca.mode = nla_get_u32(info->attrs[NL802154_ATTR_CCA_MODE]);
+		if (cca.mode < NL802154_CCA_ENERGY ||
+		    cca.mode > NL802154_CCA_ATTR_MAX)
+			return -EINVAL;
+
+		if (cca.mode == NL802154_CCA_ENERGY_CARRIER) {
+			if (!info->attrs[NL802154_ATTR_CCA_OPT])
+				return -EINVAL;
+
+			cca.opt = nla_get_u32(info->attrs[NL802154_ATTR_CCA_OPT]);
+			if (cca.opt > NL802154_CCA_OPT_ATTR_MAX)
+				return -EINVAL;
+		}
+
+		return rdev_set_cca_mode(rdev, &cca);
+	}
+
+	return 0;
+}
+
  static inline u64 wpan_dev_id(struct wpan_dev *wpan_dev)
  {
  	return (u64)wpan_dev->identifier |
@@ -896,6 +939,14 @@ static const struct genl_ops nl802154_ops[] = {
  				  NL802154_FLAG_NEED_RTNL,
  	},
  	{
+		.cmd = NL802154_CMD_SET_WPAN_PHY,

net/ieee802154/nl802154.c static const struct genl_ops nl802154_ops[] = { · line 952

Quoted 10 lines
net/ieee802154/nl802154.c +7 -0
+		.doit = nl802154_set_wpan_phy,
+		.policy = nl802154_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_WPAN_PHY |
+				  NL802154_FLAG_NEED_RTNL,
+	},
+	{
  		.cmd = NL802154_CMD_GET_INTERFACE,
  		.doit = nl802154_get_interface,
  		.dumpit = nl802154_dump_interface,

Thanks

Signature · 1 line
Varka Bhadram
author maintainer
#3 lore raw

Re: [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd

Hi Varka,

Quoted 33 lines · Varka Bhadram

Hi Alex,

Quoted 29 lines · Alexander Aring

This patch adds NL802154_CMD_SET_WPAN_PHY command to set all wpan phy attributes instead of doing separate commands. This will improve userspace application handling of nl802154.

---
 net/ieee802154/nl802154.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
net/ieee802154/nl802154.c +13 -0
index a4daf91..c12c07f 100644
@@ -437,6 +437,49 @@ static int nl802154_get_wpan_phy(struct sk_buff *skb, struct genl_info *info)
 	return genlmsg_reply(msg, info);
 }
+static int nl802154_set_wpan_phy(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	int ret;
+
+	if (info->attrs[NL802154_ATTR_PAGE] &&
+	    info->attrs[NL802154_ATTR_CHANNEL]) {
+		u8 channel, page;
+
+		page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]);
+		channel = nla_get_u8(info->attrs[NL802154_ATTR_CHANNEL]);
+		if (page > IEEE802154_MAX_PAGE ||
+		    channel > IEEE802154_MAX_CHANNEL)
Quoted 9 lines

net/ieee802154/nl802154.c static int nl802154_get_wpan_phy(struct sk_buff *skb, struct genl_info *info) · line 457

Quoted 5 lines
net/ieee802154/nl802154.c +5 -0
+			return -EINVAL;
+
+		ret = rdev_set_channel(rdev, page, channel);
+		if (ret < 0)
+			return ret;

Why dont we return directly rdev_set_channel.. Is there any significance to the check on *ret*...?

The current case here is to abort all wpan_phy attributes when one is failing. If we do a directly call of rdev_set_channel here then the function returns also on success.

Quoted 13 lines
Quoted 13 lines
net/ieee802154/nl802154.c +13 -0
+	}
+
+	if (info->attrs[NL802154_ATTR_CCA_MODE]) {
+		struct wpan_phy_cca cca;
+
+		cca.mode = nla_get_u32(info->attrs[NL802154_ATTR_CCA_MODE]);
+		if (cca.mode < NL802154_CCA_ENERGY ||
+		    cca.mode > NL802154_CCA_ATTR_MAX)
+			return -EINVAL;
+
+		if (cca.mode == NL802154_CCA_ENERGY_CARRIER) {
+			if (!info->attrs[NL802154_ATTR_CCA_OPT])
+				return -EINVAL;
Quoted 8 lines

net/ieee802154/nl802154.c static int nl802154_get_wpan_phy(struct sk_buff *skb, struct genl_info *info) · line 478

Quoted 8 lines
net/ieee802154/nl802154.c +8 -0
+
+			cca.opt = nla_get_u32(info->attrs[NL802154_ATTR_CCA_OPT]);
+			if (cca.opt > NL802154_CCA_OPT_ATTR_MAX)
+				return -EINVAL;
+		}
+
+		return rdev_set_cca_mode(rdev, &cca);
+	}

This branch need also running on success. Then applications can set multiple phy pib attributes at one CMD call.

That's why I told in my "advantage/disadvantage mail" on 100% correctness we need to handle a "rollback" on previous failure. On the other hand this doesn't matter because setting with only ONE attribute will keep the old behaviour. If we have more "capabilities" flags for asking the phy what it supports we can mostly avoid "-EINVAL, -EOPNOTSUPP" errors, otherwise the user application makes something wrong. To return on previous attr setting while failure it's for stop doing other things in kernelspace if any error occurs. But the userspace application knows then that the setting of all phy attributes are in a "unknown" state and need to dump PIB values again if an error occurs.

Handling the rollback in kernelspace is also not really possible since IO access can always fail, also on rollback handling.

- Alex

author maintainer
#4 lore raw

PATCH v2 2/4 nl802154: add set interface cmd

In reply to Alexander Aring

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. This will reduce the complexity of several netlink commands for each attributes and userspace applications are easier to implement.

---
 net/ieee802154/nl802154.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)
net/ieee802154/nl802154.c +110 -0
index c12c07f..e2f50ba 100644
@@ -603,6 +603,108 @@ static int nl802154_get_interface(struct sk_buff *skb, struct genl_info *info)
 	return genlmsg_reply(msg, info);
 }
 
+static int nl802154_set_interface(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
+	int ret;
+
+	if (info->attrs[NL802154_ATTR_PAN_ID]) {
+		__le16 pan_id;
+
+		if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+			return -EINVAL;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		pan_id = nla_get_le16(info->attrs[NL802154_ATTR_PAN_ID]);
+		ret = rdev_set_pan_id(rdev, wpan_dev, pan_id);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_SHORT_ADDR]) {
+		__le16 short_addr;
+
+		if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+			return -EINVAL;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		short_addr = nla_get_le16(info->attrs[NL802154_ATTR_SHORT_ADDR]);
+
+		ret = rdev_set_short_addr(rdev, wpan_dev, short_addr);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_MAX_FRAME_RETRIES]) {
+		s8 max_frame_retries;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		max_frame_retries = nla_get_s8(
+				info->attrs[NL802154_ATTR_MAX_FRAME_RETRIES]);
+		if (max_frame_retries < -1 || max_frame_retries > 7)
+			return -EINVAL;
+
+		ret = rdev_set_max_frame_retries(rdev, wpan_dev,
+						 max_frame_retries);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_MIN_BE] &&
+	    info->attrs[NL802154_ATTR_MAX_BE]) {
+		u8 min_be, max_be;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		min_be = nla_get_u8(info->attrs[NL802154_ATTR_MIN_BE]);
+		max_be = nla_get_u8(info->attrs[NL802154_ATTR_MAX_BE]);
+		if (max_be < 3 || max_be > 8 || min_be > max_be)
+			return -EINVAL;
+
+		ret = rdev_set_backoff_exponent(rdev, wpan_dev, min_be, max_be);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_MAX_CSMA_BACKOFFS]) {
+		u8 max_csma_backoffs;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		max_csma_backoffs = nla_get_u8(
+				info->attrs[NL802154_ATTR_MAX_CSMA_BACKOFFS]);
+		if (max_csma_backoffs > 5)
+			return -EINVAL;
+
+		ret = rdev_set_max_csma_backoffs(rdev, wpan_dev,
+						 max_csma_backoffs);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_LBT_MODE]) {
+		bool mode;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		mode = !!nla_get_u8(info->attrs[NL802154_ATTR_LBT_MODE]);
+		return rdev_set_lbt_mode(rdev, wpan_dev, mode);
+	}
+
+	return 0;
+}
+
 static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg802154_registered_device *rdev = info->user_ptr[0];
@@ -964,6 +1066,14 @@ static const struct genl_ops nl802154_ops[] = {
 				  NL802154_FLAG_NEED_RTNL,
 	},
 	{
+		.cmd = NL802154_CMD_SET_INTERFACE,
+		.doit = nl802154_set_interface,
+		.policy = nl802154_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_NEED_RTNL,
+	},
+	{
 		.cmd = NL802154_CMD_DEL_INTERFACE,
 		.doit = nl802154_del_interface,
 		.policy = nl802154_policy,
Signature · 1 line
2.3.3
author maintainer
#5 lore raw

PATCH v2 3/4 ieee802154: move mac pib defaults

In reply to Alexander Aring

This patch moves the mac pib defaults setting out of SoftMAC layer. This will prepare for possible HardMAC drivers that these values are 802.15.4 defaults while netdev registration.

---
 net/ieee802154/core.c | 30 ++++++++++++++++++++++++++++++
 net/mac802154/iface.c | 15 ---------------
 2 files changed, 30 insertions(+), 15 deletions(-)
net/ieee802154/core.c +30 -0
index 2ee00e8..7a107b0 100644
@@ -211,6 +211,35 @@ cfg802154_update_iface_num(struct cfg802154_registered_device *rdev,
 	rdev->num_running_ifaces += num;
 }
 
+static void
+cfg802154_reset_mac_pib(struct wpan_dev *wpan_dev)
+{
+	get_random_bytes(&wpan_dev->bsn, 1);
+	get_random_bytes(&wpan_dev->dsn, 1);
+
+	/* defaults per 802.15.4-2011 */
+	wpan_dev->min_be = 3;
+	wpan_dev->max_be = 5;
+	wpan_dev->csma_retries = 4;
+	/* for compatibility, actual default is 3 */
+	wpan_dev->frame_retries = -1;
+
+	wpan_dev->pan_id = cpu_to_le16(IEEE802154_PAN_ID_BROADCAST);
+	wpan_dev->short_addr = cpu_to_le16(IEEE802154_ADDR_SHORT_BROADCAST);
+
+	/* normally false, but monitors are always in promiscuous_mode */
+	switch (wpan_dev->iftype) {
+	case NL802154_IFTYPE_NODE:
+		wpan_dev->promiscuous_mode = false;
+		break;
+	case NL802154_IFTYPE_MONITOR:
+		wpan_dev->promiscuous_mode = true;
+		break;
+	default:
+		BUG();
+	}
+}
+
 static int cfg802154_netdev_notifier_call(struct notifier_block *nb,
 					  unsigned long state, void *ptr)
 {
@@ -234,6 +263,7 @@ static int cfg802154_netdev_notifier_call(struct notifier_block *nb,
 		rdev->devlist_generation++;
 
 		wpan_dev->netdev = dev;
+		cfg802154_reset_mac_pib(wpan_dev);
 		break;
 	case NETDEV_DOWN:
 		cfg802154_update_iface_num(rdev, wpan_dev->iftype, -1);
net/mac802154/iface.c +0 -15
index 6fb6bdf..9b8e264 100644
@@ -487,19 +487,6 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
 	sdata->vif.type = type;
 	sdata->wpan_dev.iftype = type;
 
-	get_random_bytes(&wpan_dev->bsn, 1);
-	get_random_bytes(&wpan_dev->dsn, 1);
-
-	/* defaults per 802.15.4-2011 */
-	wpan_dev->min_be = 3;
-	wpan_dev->max_be = 5;
-	wpan_dev->csma_retries = 4;
-	/* for compatibility, actual default is 3 */
-	wpan_dev->frame_retries = -1;
-
-	wpan_dev->pan_id = cpu_to_le16(IEEE802154_PANID_BROADCAST);
-	wpan_dev->short_addr = cpu_to_le16(IEEE802154_ADDR_BROADCAST);
-
 	switch (type) {
 	case NL802154_IFTYPE_NODE:
 		ieee802154_be64_to_le64(&wpan_dev->extended_addr,
@@ -509,7 +496,6 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
 		sdata->dev->destructor = mac802154_wpan_free;
 		sdata->dev->netdev_ops = &mac802154_wpan_ops;
 		sdata->dev->ml_priv = &mac802154_mlme_wpan;
-		wpan_dev->promiscuous_mode = false;
 
 		spin_lock_init(&sdata->mib_lock);
 		mutex_init(&sdata->sec_mtx);
@@ -519,7 +505,6 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
 	case NL802154_IFTYPE_MONITOR:
 		sdata->dev->destructor = free_netdev;
 		sdata->dev->netdev_ops = &mac802154_monitor_ops;
-		wpan_dev->promiscuous_mode = true;
 		break;
 	default:
 		BUG();
Signature · 1 line
2.3.3
author maintainer
#6 lore raw

PATCH v2 4/4 ieee802154: set aret handling according to 802.15.4

In reply to Alexander Aring

The 802.15.4 standard describes a max frame retries parameter 3 as default. Currently this parameter affects only the transceivers which supports automatic retransmission. This at the moment the at86rf230 driver, all other drivers which don't support setting of max frame retries parameter we assume the 802.15.4 default now.

This change will introduce that the at86rf230 driver will go into ARET mode (max frame retries parameter above or equal zero) per default.

If somebody experience that other nodes receives the same frame three times repeatly, then your receiving node doesn't support AACK handling for receiving. For all users they should set the max frame retries parameter to "-1" which turns off the ARET mode and restore the old behaviour.

---
 net/ieee802154/core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
net/ieee802154/core.c +1 -2
index 7a107b0..619a220 100644
@@ -221,8 +221,7 @@ cfg802154_reset_mac_pib(struct wpan_dev *wpan_dev)
 	wpan_dev->min_be = 3;
 	wpan_dev->max_be = 5;
 	wpan_dev->csma_retries = 4;
-	/* for compatibility, actual default is 3 */
-	wpan_dev->frame_retries = -1;
+	wpan_dev->frame_retries = 3;
 
 	wpan_dev->pan_id = cpu_to_le16(IEEE802154_PAN_ID_BROADCAST);
 	wpan_dev->short_addr = cpu_to_le16(IEEE802154_ADDR_SHORT_BROADCAST);
Signature · 1 line
2.3.3