Skip to contentSkip to conversation

ieee802154: nl802154 SET commands and pib defaults

PATCH In review Alexander Aring 3 participants 15 messages Apr 7, 2015

Subsystems (2)
lore
Share & tools
lore

Alexander Aring

author maintainer
#0 lore raw

v3 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.

changes since v3:
- some cleanups in the patch commit msgs, mostly I want to resend this series only.

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.5
author maintainer
#1 lore raw

PATCH v3 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 introduce an easilier wpan phy settings handling in userspace application with 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.5
author maintainer
#2 lore raw

PATCH v3 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. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 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.5
#3 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Quoted 37 lines · On Tue, 7 Apr 2015 13:49:51 +0200 Alexander Aring <[email protected]> wrote:

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 net/ieee802154/nl802154.c | 110

++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)

net/ieee802154/nl802154.c +16 -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]);

net/ieee802154/nl802154.c static int nl802154_get_interface(struct · line 633

Quoted 10 lines
net/ieee802154/nl802154.c +10 -0
+		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;

This is not good. You may have partially applied the requested settings when you return with an error, leaving the device in some intermediate state that is most likely not useful. It'd be better to first check whether all settings can be applied, and then apply them all at once. But even then we have a problem with rolling back some changes if a command fails :/

Quoted 100 lines
net/ieee802154/nl802154.c +5 -0
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		short_addr =

nla_get_le16(info->attrs[NL802154_ATTR_SHORT_ADDR]); +

net/ieee802154/nl802154.c +1 -0
+		ret = rdev_set_short_addr(rdev, wpan_dev,

short_addr);

net/ieee802154/nl802154.c +12 -0
+		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]);

net/ieee802154/nl802154.c +17 -0
+		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]);

net/ieee802154/nl802154.c +1 -0
+		max_be =

nla_get_u8(info->attrs[NL802154_ATTR_MAX_BE]);

net/ieee802154/nl802154.c +4 -0
+		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);

net/ieee802154/nl802154.c +12 -0
+		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]);

net/ieee802154/nl802154.c +16 -0
+		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]);

net/ieee802154/nl802154.c +6 -0
+		return rdev_set_lbt_mode(rdev, wpan_dev, mode);
+	}
+
+	return 0;
+}
+
 static int nl802154_new_interface(struct sk_buff *skb, struct

genl_info *info) {

net/ieee802154/nl802154.c +8 -0
 	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,
author maintainer
#4 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Hi Phoebe,

Quoted 49 lines · Phoebe Buckheister
Quoted 46 lines · On Tue, 7 Apr 2015 13:49:51 +0200 Alexander Aring <[email protected]> wrote:

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 net/ieee802154/nl802154.c | 110

++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)

net/ieee802154/nl802154.c +25 -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)
Quoted 9 lines

net/ieee802154/nl802154.c static int nl802154_get_interface(struct · line 633

Quoted 1 line
net/ieee802154/nl802154.c +1 -0
+			return -EINVAL;

This is not good. You may have partially applied the requested settings when you return with an error, leaving the device in some intermediate state that is most likely not useful. It'd be better to first check whether all settings can be applied, and then apply them all at once. But even then we have a problem with rolling back some changes if a command fails :/

Yes, we need kind of rollback here if the command failed. I also stumble over this issue. [0] Mhh, we don't have it when I do each command in their own CMD SET call. So then I will leave the current behaviour as it is.

Some idea:

Solution would be on MAC settings that we really check if everything is valid. Then we can set everything in the pib values, after an interface up this values will be "really" set, like address filter settings in phy registers.

This works for interface settings (MAC PIB values). But for phy values this is more difficult, because it's directly driver-layer calls. So we can't predigt if an driver layer return some error then.

If nobody complaint then I will send a v4 and remove the nl802154 settings, or maybe somebody has a better idea.

- Alex

[0] http://www.spinics.net/lists/linux-wpan/msg01568.html

#5 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Quoted 79 lines · Alexander Aring

Hi Phoebe,

Quoted 48 lines · Phoebe Buckheister
Quoted 45 lines · On Tue, 7 Apr 2015 13:49:51 +0200 Alexander Aring <[email protected]> wrote:

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 net/ieee802154/nl802154.c | 110

++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)

net/ieee802154/nl802154.c +24 -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;
+
Quoted 9 lines

net/ieee802154/nl802154.c static int nl802154_get_interface(struct · line 633

Quoted 2 lines
net/ieee802154/nl802154.c +2 -0
+		if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+			return -EINVAL;

This is not good. You may have partially applied the requested settings when you return with an error, leaving the device in some intermediate state that is most likely not useful. It'd be better to first check whether all settings can be applied, and then apply them all at once. But even then we have a problem with rolling back some changes if a command fails :/

Yes, we need kind of rollback here if the command failed. I also stumble over this issue. [0] Mhh, we don't have it when I do each command in their own CMD SET call. So then I will leave the current behaviour as it is.

Some idea:

Solution would be on MAC settings that we really check if everything is valid. Then we can set everything in the pib values, after an interface up this values will be "really" set, like address filter settings in phy registers.

This works for interface settings (MAC PIB values). But for phy values this is more difficult, because it's directly driver-layer calls. So we can't predigt if an driver layer return some error then.

If nobody complaint then I will send a v4 and remove the nl802154 settings, or maybe somebody has a better idea.

Quoted 7 lines

- Alex

[0] http://www.spinics.net/lists/linux-wpan/msg01568.html
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Instead of this its better to leave the current behavior as it is.

Why do we need to complicate the process...?

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

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Hi Varka,

Quoted 1 line · Varka Bhadram

On 04/07/2015 05:51 PM, Alexander Aring wrote:

...

Quoted 8 lines
Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Instead of this its better to leave the current behavior as it is.

Why do we need to complicate the process...?

I think more this is easier for userspace application which need to call one CMD_SET for dev or phy only, but yes... it's much complicated to handle error handling then. The most error codes would be -EINVAL and -ENOTSUPP and we should forget some error codes which comming from spi_sync/spi_async calls, these are very unlikely.

To handle the -EINVAL and -ENOTSUPP error we need more stuff to check if the phy supports that value. I will try to introduce at first such behaviour, then we can reconsider if we need something like that or not.

- Alex

#7 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

In reply to Alexander Aring
Quoted 70 lines · On Tue, 7 Apr 2015 14:21:52 +0200 Alexander Aring <[email protected]> wrote:

Hi Phoebe,

Quoted 50 lines · Phoebe Buckheister
Quoted 47 lines · On Tue, 7 Apr 2015 13:49:51 +0200 Alexander Aring <[email protected]> wrote:

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 net/ieee802154/nl802154.c | 110

++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)

net/ieee802154/nl802154.c +25 -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)
Quoted 9 lines

net/ieee802154/nl802154.c static int nl802154_get_interface(struct · line 634

Quoted 1 line
net/ieee802154/nl802154.c +1 -0
+			return -EINVAL;

This is not good. You may have partially applied the requested settings when you return with an error, leaving the device in some intermediate state that is most likely not useful. It'd be better to first check whether all settings can be applied, and then apply them all at once. But even then we have a problem with rolling back some changes if a command fails :/

Yes, we need kind of rollback here if the command failed. I also stumble over this issue. [0] Mhh, we don't have it when I do each command in their own CMD SET call. So then I will leave the current behaviour as it is.

Some idea:

Quoted 8 lines

Solution would be on MAC settings that we really check if everything is valid. Then we can set everything in the pib values, after an interface up this values will be "really" set, like address filter settings in phy registers.

This works for interface settings (MAC PIB values). But for phy values this is more difficult, because it's directly driver-layer calls. So we can't predigt if an driver layer return some error then.

That sounds good. We could add another driver function that checks a PIB/MIB and returns "valid" or "i'm sorry, dave", where "valid" may only happen when the driver *knows* that it can load the *IB into the hardware. Any possible hardware errors are excluded by these checks, but we'll only see those on the actual load anyway. Then we can load the *IB in bulk and be sure that only the hardware can fail.

Quoted 7 lines

If nobody complaint then I will send a v4 and remove the nl802154 settings, or maybe somebody has a better idea.

- Alex

[0] http://www.spinics.net/lists/linux-wpan/msg01568.html

author maintainer
#8 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Quoted 78 lines · Phoebe Buckheister
Quoted 75 lines · On Tue, 7 Apr 2015 14:21:52 +0200 Alexander Aring <[email protected]> wrote:

Hi Phoebe,

Quoted 50 lines · Phoebe Buckheister
Quoted 47 lines · On Tue, 7 Apr 2015 13:49:51 +0200 Alexander Aring <[email protected]> wrote:

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 net/ieee802154/nl802154.c | 110

++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)

net/ieee802154/nl802154.c +25 -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)
Quoted 9 lines

net/ieee802154/nl802154.c static int nl802154_get_interface(struct · line 634

Quoted 1 line
net/ieee802154/nl802154.c +1 -0
+			return -EINVAL;

This is not good. You may have partially applied the requested settings when you return with an error, leaving the device in some intermediate state that is most likely not useful. It'd be better to first check whether all settings can be applied, and then apply them all at once. But even then we have a problem with rolling back some changes if a command fails :/

Yes, we need kind of rollback here if the command failed. I also stumble over this issue. [0] Mhh, we don't have it when I do each command in their own CMD SET call. So then I will leave the current behaviour as it is.

Some idea:

Solution would be on MAC settings that we really check if everything is valid. Then we can set everything in the pib values, after an interface up this values will be "really" set, like address filter settings in phy registers.

Quoted 8 lines
Quoted 3 lines

This works for interface settings (MAC PIB values). But for phy values this is more difficult, because it's directly driver-layer calls. So we can't predigt if an driver layer return some error then.

That sounds good. We could add another driver function that checks a PIB/MIB and returns "valid" or "i'm sorry, dave", where "valid" may only happen when the driver *knows* that it can load the *IB into the hardware. Any possible hardware errors are excluded by these checks,

I think we need such functionality anyway, I wrote in my previous mail that the not _all_ valid 802.15.4 mac pib values are supported by real transceivers. Like the mrf24j40 [0] which can set the MIN_BE value only (I suppose MAX_BE is always 802.15.4 default). But nl802154 checks only on 802.15.4 constraints only, so you can set everything which is allowed by 802.15.4. On a "doing interface up" it will fail because the driver layer will told -EINVAL; then and you don't know why (okay, experts knows why).

We need to check this value on the SoftMAC layer in "net/mac802154/cfg.c".

Now the design question is "a function" or "a array like channels_supported which extends the phy pib by us". I would prefer the array solution and put simple a u32 backoff_exponents_supported in phy_pib [1] and checking over flags if the transceiver supports it's or not.

Then we can add more supported arrays for csma retries, frame retries, tx_pwr (also design question if all bands or current band only), etc...

(For lbt mode, we don't need such functionality... it's only a bool and the HW flag is enough).

Currently we don't have this kind of problem, because most settings can set on an at86rf230 transceiver only which is fully (and beyond) 802.15.4 complaint.

Now the question is again:

It's okay to extend the "supported arrays" in phy pib or we should introduce a driver_ops function? I prefer the array solution, because we have already such thing which is described by 802.15.4 pib.

Quoted 3 lines

but we'll only see those on the actual load anyway. Then we can load the *IB in bulk and be sure that only the hardware can fail.

Then I would add such functionality to check if valid at first which I describe above.

- Alex

[0] http://ww1.microchip.com/downloads/en/DeviceDoc/39776C.pdf
[1] http://lxr.free-electrons.com/source/include/net/cfg802154.h#L77

#9 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Quoted 123 lines · On Tue, 7 Apr 2015 14:59:52 +0200 Alexander Aring <[email protected]> wrote:
Quoted 87 lines · Phoebe Buckheister
Quoted 79 lines · On Tue, 7 Apr 2015 14:21:52 +0200 Alexander Aring <[email protected]> wrote:

Hi Phoebe,

Quoted 53 lines · Phoebe Buckheister
Quoted 50 lines · On Tue, 7 Apr 2015 13:49:51 +0200 Alexander Aring <[email protected]> wrote:

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 net/ieee802154/nl802154.c | 110

++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)

(quoted diff) +0 -0

b/net/ieee802154/nl802154.c index c12c07f..e2f50ba 100644

net/ieee802154/nl802154.c +0 -0
@@ -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) +{

net/ieee802154/nl802154.c +1 -0
+	struct cfg802154_registered_device *rdev =

info->user_ptr[0];

net/ieee802154/nl802154.c +8 -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)

net/ieee802154/nl802154.c +6 -0
+			return -EINVAL;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		pan_id =

nla_get_le16(info->attrs[NL802154_ATTR_PAN_ID]);

net/ieee802154/nl802154.c +1 -0
+		ret = rdev_set_pan_id(rdev, wpan_dev,

pan_id);

net/ieee802154/nl802154.c +8 -0
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_SHORT_ADDR]) {
+		__le16 short_addr;
+
+		if (wpan_dev->iftype ==

NL802154_IFTYPE_MONITOR)

Quoted 9 lines

net/ieee802154/nl802154.c

Quoted 1 line
net/ieee802154/nl802154.c +1 -0
+			return -EINVAL;

This is not good. You may have partially applied the requested settings when you return with an error, leaving the device in some intermediate state that is most likely not useful. It'd be better to first check whether all settings can be applied, and then apply them all at once. But even then we have a problem with rolling back some changes if a command fails :/

Yes, we need kind of rollback here if the command failed. I also stumble over this issue. [0] Mhh, we don't have it when I do each command in their own CMD SET call. So then I will leave the current behaviour as it is.

Some idea:

Solution would be on MAC settings that we really check if everything is valid. Then we can set everything in the pib values, after an interface up this values will be "really" set, like address filter settings in phy registers.

Quoted 4 lines

This works for interface settings (MAC PIB values). But for phy values this is more difficult, because it's directly driver-layer calls. So we can't predigt if an driver layer return some error then.

Quoted 5 lines

That sounds good. We could add another driver function that checks a PIB/MIB and returns "valid" or "i'm sorry, dave", where "valid" may only happen when the driver *knows* that it can load the *IB into the hardware. Any possible hardware errors are excluded by these checks,

I think we need such functionality anyway, I wrote in my previous mail that the not _all_ valid 802.15.4 mac pib values are supported by real transceivers. Like the mrf24j40 [0] which can set the MIN_BE value only (I suppose MAX_BE is always 802.15.4 default). But nl802154 checks only on 802.15.4 constraints only, so you can set everything which is allowed by 802.15.4. On a "doing interface up" it will fail because the driver layer will told -EINVAL; then and you don't know why (okay, experts knows why).

We need to check this value on the SoftMAC layer in "net/mac802154/cfg.c".

Now the design question is "a function" or "a array like channels_supported which extends the phy pib by us". I would prefer the array solution and put simple a u32 backoff_exponents_supported in phy_pib [1] and checking over flags if the transceiver supports it's or not.

Then we can add more supported arrays for csma retries, frame retries, tx_pwr (also design question if all bands or current band only), etc...

(For lbt mode, we don't need such functionality... it's only a bool and the HW flag is enough).

Currently we don't have this kind of problem, because most settings can set on an at86rf230 transceiver only which is fully (and beyond) 802.15.4 complaint.

Quoted 5 lines

Now the question is again:

It's okay to extend the "supported arrays" in phy pib or we should introduce a driver_ops function? I prefer the array solution, because we have already such thing which is described by 802.15.4 pib.

Either is fine. The values permitted by the standard are mostly bitmasks and min/max-pairs, so that actually makes more sense than a check() function in each driver (until some driver doesn't fit that model, but we should worry about that then).

Quoted 12 lines
Quoted 4 lines

but we'll only see those on the actual load anyway. Then we can load the *IB in bulk and be sure that only the hardware can fail.

Then I would add such functionality to check if valid at first which I describe above.

- Alex

[0] http://ww1.microchip.com/downloads/en/DeviceDoc/39776C.pdf
[1] http://lxr.free-electrons.com/source/include/net/cfg802154.h#L77

author maintainer
#10 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Quoted 126 lines · Phoebe Buckheister
Quoted 123 lines · On Tue, 7 Apr 2015 14:59:52 +0200 Alexander Aring <[email protected]> wrote:
Quoted 87 lines · Phoebe Buckheister
Quoted 79 lines · On Tue, 7 Apr 2015 14:21:52 +0200 Alexander Aring <[email protected]> wrote:

Hi Phoebe,

Quoted 53 lines · Phoebe Buckheister
Quoted 50 lines · On Tue, 7 Apr 2015 13:49:51 +0200 Alexander Aring <[email protected]> wrote:

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 net/ieee802154/nl802154.c | 110

++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)

(quoted diff) +0 -0

b/net/ieee802154/nl802154.c index c12c07f..e2f50ba 100644

net/ieee802154/nl802154.c +0 -0
@@ -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) +{

net/ieee802154/nl802154.c +1 -0
+	struct cfg802154_registered_device *rdev =

info->user_ptr[0];

net/ieee802154/nl802154.c +8 -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)

net/ieee802154/nl802154.c +6 -0
+			return -EINVAL;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		pan_id =

nla_get_le16(info->attrs[NL802154_ATTR_PAN_ID]);

net/ieee802154/nl802154.c +1 -0
+		ret = rdev_set_pan_id(rdev, wpan_dev,

pan_id);

net/ieee802154/nl802154.c +8 -0
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_SHORT_ADDR]) {
+		__le16 short_addr;
+
+		if (wpan_dev->iftype ==

NL802154_IFTYPE_MONITOR)

Quoted 9 lines

net/ieee802154/nl802154.c

Quoted 1 line
net/ieee802154/nl802154.c +1 -0
+			return -EINVAL;

This is not good. You may have partially applied the requested settings when you return with an error, leaving the device in some intermediate state that is most likely not useful. It'd be better to first check whether all settings can be applied, and then apply them all at once. But even then we have a problem with rolling back some changes if a command fails :/

Yes, we need kind of rollback here if the command failed. I also stumble over this issue. [0] Mhh, we don't have it when I do each command in their own CMD SET call. So then I will leave the current behaviour as it is.

Some idea:

Solution would be on MAC settings that we really check if everything is valid. Then we can set everything in the pib values, after an interface up this values will be "really" set, like address filter settings in phy registers.

Quoted 4 lines

This works for interface settings (MAC PIB values). But for phy values this is more difficult, because it's directly driver-layer calls. So we can't predigt if an driver layer return some error then.

Quoted 5 lines

That sounds good. We could add another driver function that checks a PIB/MIB and returns "valid" or "i'm sorry, dave", where "valid" may only happen when the driver *knows* that it can load the *IB into the hardware. Any possible hardware errors are excluded by these checks,

I think we need such functionality anyway, I wrote in my previous mail that the not _all_ valid 802.15.4 mac pib values are supported by real transceivers. Like the mrf24j40 [0] which can set the MIN_BE value only (I suppose MAX_BE is always 802.15.4 default). But nl802154 checks only on 802.15.4 constraints only, so you can set everything which is allowed by 802.15.4. On a "doing interface up" it will fail because the driver layer will told -EINVAL; then and you don't know why (okay, experts knows why).

We need to check this value on the SoftMAC layer in "net/mac802154/cfg.c".

Now the design question is "a function" or "a array like channels_supported which extends the phy pib by us". I would prefer the array solution and put simple a u32 backoff_exponents_supported in phy_pib [1] and checking over flags if the transceiver supports it's or not.

Then we can add more supported arrays for csma retries, frame retries, tx_pwr (also design question if all bands or current band only), etc...

(For lbt mode, we don't need such functionality... it's only a bool and the HW flag is enough).

Currently we don't have this kind of problem, because most settings can set on an at86rf230 transceiver only which is fully (and beyond) 802.15.4 complaint.

Quoted 8 lines
Quoted 5 lines

Now the question is again:

It's okay to extend the "supported arrays" in phy pib or we should introduce a driver_ops function? I prefer the array solution, because we have already such thing which is described by 802.15.4 pib.

Either is fine. The values permitted by the standard are mostly bitmasks and min/max-pairs, so that actually makes more sense than a

So we could some union type for it like:

union wpan_phy_support {
	u32	mask;
	union {
		u16	min:16;
		u16	max:16;
	};
};

Don't know if this is good idea.

Quoted 3 lines

check() function in each driver (until some driver doesn't fit that model, but we should worry about that then).

ack.

- Alex

#11 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Quoted 150 lines · On Tue, 7 Apr 2015 15:25:02 +0200 Alexander Aring <[email protected]> wrote:
Quoted 131 lines · Phoebe Buckheister
Quoted 128 lines · On Tue, 7 Apr 2015 14:59:52 +0200 Alexander Aring <[email protected]> wrote:
Quoted 88 lines · Phoebe Buckheister
Quoted 80 lines · On Tue, 7 Apr 2015 14:21:52 +0200 Alexander Aring <[email protected]> wrote:

Hi Phoebe,

Quoted 55 lines · Phoebe Buckheister
Quoted 50 lines · On Tue, 7 Apr 2015 13:49:51 +0200 Alexander Aring <[email protected]> wrote:

This patch introduce the NL802154_CMD_SET_INTERFACE command which handles setting of all wpan interface mac attributes. his will introduce an easilier wpan mac settings handling in userspace application with nl802154.

---
 net/ieee802154/nl802154.c | 110

++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)

(quoted diff) +0 -0

b/net/ieee802154/nl802154.c index c12c07f..e2f50ba 100644

net/ieee802154/nl802154.c +0 -0
@@ -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) +{

net/ieee802154/nl802154.c +1 -0
+	struct cfg802154_registered_device *rdev =

info->user_ptr[0];

net/ieee802154/nl802154.c +8 -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)

net/ieee802154/nl802154.c +6 -0
+			return -EINVAL;
+
+		if (netif_running(dev))
+			return -EBUSY;
+
+		pan_id =

nla_get_le16(info->attrs[NL802154_ATTR_PAN_ID]);

net/ieee802154/nl802154.c +1 -0
+		ret = rdev_set_pan_id(rdev, wpan_dev,

pan_id);

net/ieee802154/nl802154.c +8 -0
+		if (ret < 0)
+			return ret;
+	}
+
+	if (info->attrs[NL802154_ATTR_SHORT_ADDR]) {
+		__le16 short_addr;
+
+		if (wpan_dev->iftype ==

NL802154_IFTYPE_MONITOR)

net/ieee802154/nl802154.c

Quoted 1 line
net/ieee802154/nl802154.c +1 -0
+			return -EINVAL;
Quoted 8 lines

This is not good. You may have partially applied the requested settings when you return with an error, leaving the device in some intermediate state that is most likely not useful. It'd be better to first check whether all settings can be applied, and then apply them all at once. But even then we have a problem with rolling back some changes if a command fails :/

Yes, we need kind of rollback here if the command failed. I also stumble over this issue. [0] Mhh, we don't have it when I do each command in their own CMD SET call. So then I will leave the current behaviour as it is.

Some idea:

Solution would be on MAC settings that we really check if everything is valid. Then we can set everything in the pib values, after an interface up this values will be "really" set, like address filter settings in phy registers.

Quoted 4 lines

This works for interface settings (MAC PIB values). But for phy values this is more difficult, because it's directly driver-layer calls. So we can't predigt if an driver layer return some error then.

Quoted 5 lines

That sounds good. We could add another driver function that checks a PIB/MIB and returns "valid" or "i'm sorry, dave", where "valid" may only happen when the driver *knows* that it can load the *IB into the hardware. Any possible hardware errors are excluded by these checks,

I think we need such functionality anyway, I wrote in my previous mail that the not _all_ valid 802.15.4 mac pib values are supported by real transceivers. Like the mrf24j40 [0] which can set the MIN_BE value only (I suppose MAX_BE is always 802.15.4 default). But nl802154 checks only on 802.15.4 constraints only, so you can set everything which is allowed by 802.15.4. On a "doing interface up" it will fail because the driver layer will told -EINVAL; then and you don't know why (okay, experts knows why).

We need to check this value on the SoftMAC layer in "net/mac802154/cfg.c".

Now the design question is "a function" or "a array like channels_supported which extends the phy pib by us". I would prefer the array solution and put simple a u32 backoff_exponents_supported in phy_pib [1] and checking over flags if the transceiver supports it's or not.

Then we can add more supported arrays for csma retries, frame retries, tx_pwr (also design question if all bands or current band only), etc...

(For lbt mode, we don't need such functionality... it's only a bool and the HW flag is enough).

Currently we don't have this kind of problem, because most settings can set on an at86rf230 transceiver only which is fully (and beyond) 802.15.4 complaint.

Now the question is again:

Quoted 7 lines
Quoted 4 lines

It's okay to extend the "supported arrays" in phy pib or we should introduce a driver_ops function? I prefer the array solution, because we have already such thing which is described by 802.15.4 pib.

Either is fine. The values permitted by the standard are mostly bitmasks and min/max-pairs, so that actually makes more sense than a

So we could some union type for it like:

union wpan_phy_support {
	u32	mask;
	union {
		u16	min:16;
		u16	max:16;
	};
};
Quoted 1 line

Don't know if this is good idea.

Eh? No.

struct wpan_phy_support {
	bool lbt, aack;
	u16 min_be, max_be;
	u32 pages;
	u32 channels[32];
};

or something like that. A complete description of PHY capabilities, we might even export that to userspace somehow?

Quoted 8 lines
Quoted 4 lines

check() function in each driver (until some driver doesn't fit that model, but we should worry about that then).

ack.

- Alex

author maintainer
#12 lore raw

Re: [PATCHv3 bluetooth-next 2/4] nl802154: add set interface cmd

Quoted 3 lines · Phoebe Buckheister

Eh? No.

Okay. :-)

Quoted 2 lines
struct wpan_phy_support {
	bool lbt, aack;

for aack I would use max and min for frame_retries (need to be signed integer) and the same logik like max_frame_retries parameter in driver ops. So if s8 max_frame_retries and min_frame_retries -1 then we don't support aack handling.

Quoted 1 line
	u16 min_be, max_be;

I think for the backoff exponents we need something like:

min_minbe and max_minbe

and

min_maxbe and max_maxbe

sounds a little bit ugly, but some transceivers can't change the maxbe value.

Quoted 2 lines
	u32 pages;
	u32 channels[32];

This is already combined in channels_supported array for pages and channels.

Quoted 5 lines

};

or something like that. A complete description of PHY capabilities, we might even export that to userspace somehow?

yes, of course. I think we should implement then a nested nl attribute for wpan_phy_supported things while wpan_phy dump.

- Alex

author maintainer
#13 lore raw

PATCH v3 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. Currently the SoftMAC layer is the wrong place to do the settings of pib defaults.

---
 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 38b56f9..3f6a3fe 100644
@@ -479,19 +479,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,
@@ -501,7 +488,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);
@@ -511,7 +497,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.5
author maintainer
#14 lore raw

PATCH v3 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.5