All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 bluetooth-next 0/4] ieee802154: nl802154 SET commands and pib defaults
@ 2015-03-23 15:45 Alexander Aring
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd Alexander Aring
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Alexander Aring @ 2015-03-23 15:45 UTC (permalink / raw)
  To: linux-wpan; +Cc: kernel, mkl, Alexander Aring

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(-)

-- 
2.3.3


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

* [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd
  2015-03-23 15:45 [PATCHv2 bluetooth-next 0/4] ieee802154: nl802154 SET commands and pib defaults Alexander Aring
@ 2015-03-23 15:45 ` Alexander Aring
  2015-03-24  3:41   ` Varka Bhadram
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 2/4] nl802154: add set interface cmd Alexander Aring
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Alexander Aring @ 2015-03-23 15:45 UTC (permalink / raw)
  To: linux-wpan; +Cc: kernel, mkl, 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.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/nl802154.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index a4daf91..c12c07f 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -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,
-- 
2.3.3


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

* [PATCHv2 bluetooth-next 2/4] nl802154: add set interface cmd
  2015-03-23 15:45 [PATCHv2 bluetooth-next 0/4] ieee802154: nl802154 SET commands and pib defaults Alexander Aring
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd Alexander Aring
@ 2015-03-23 15:45 ` Alexander Aring
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 3/4] ieee802154: move mac pib defaults Alexander Aring
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 4/4] ieee802154: set aret handling according to 802.15.4 Alexander Aring
  3 siblings, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2015-03-23 15:45 UTC (permalink / raw)
  To: linux-wpan; +Cc: kernel, mkl, 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.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/nl802154.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index c12c07f..e2f50ba 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -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,
-- 
2.3.3


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

* [PATCHv2 bluetooth-next 3/4] ieee802154: move mac pib defaults
  2015-03-23 15:45 [PATCHv2 bluetooth-next 0/4] ieee802154: nl802154 SET commands and pib defaults Alexander Aring
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd Alexander Aring
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 2/4] nl802154: add set interface cmd Alexander Aring
@ 2015-03-23 15:45 ` Alexander Aring
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 4/4] ieee802154: set aret handling according to 802.15.4 Alexander Aring
  3 siblings, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2015-03-23 15:45 UTC (permalink / raw)
  To: linux-wpan; +Cc: kernel, mkl, 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.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/core.c | 30 ++++++++++++++++++++++++++++++
 net/mac802154/iface.c | 15 ---------------
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 2ee00e8..7a107b0 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -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);
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 6fb6bdf..9b8e264 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -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();
-- 
2.3.3


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

* [PATCHv2 bluetooth-next 4/4] ieee802154: set aret handling according to 802.15.4
  2015-03-23 15:45 [PATCHv2 bluetooth-next 0/4] ieee802154: nl802154 SET commands and pib defaults Alexander Aring
                   ` (2 preceding siblings ...)
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 3/4] ieee802154: move mac pib defaults Alexander Aring
@ 2015-03-23 15:45 ` Alexander Aring
  3 siblings, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2015-03-23 15:45 UTC (permalink / raw)
  To: linux-wpan; +Cc: kernel, mkl, 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.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 7a107b0..619a220 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -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);
-- 
2.3.3


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

* Re: [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd
  2015-03-23 15:45 ` [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd Alexander Aring
@ 2015-03-24  3:41   ` Varka Bhadram
  2015-03-24  8:43     ` Alexander Aring
  0 siblings, 1 reply; 7+ messages in thread
From: Varka Bhadram @ 2015-03-24  3:41 UTC (permalink / raw)
  To: Alexander Aring, linux-wpan; +Cc: kernel, mkl

Hi Alex,

On 03/23/2015 09:15 PM, Alexander Aring wrote:

> 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.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   net/ieee802154/nl802154.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
>
> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> index a4daf91..c12c07f 100644
> --- a/net/ieee802154/nl802154.c
> +++ b/net/ieee802154/nl802154.c
> @@ -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;

Why dont we return directly rdev_set_channel..
Is there any significance to the check on *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,

Thanks

-- 
Varka Bhadram


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

* Re: [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd
  2015-03-24  3:41   ` Varka Bhadram
@ 2015-03-24  8:43     ` Alexander Aring
  0 siblings, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2015-03-24  8:43 UTC (permalink / raw)
  To: Varka Bhadram; +Cc: linux-wpan, kernel, mkl

Hi Varka,

On Tue, Mar 24, 2015 at 09:11:54AM +0530, Varka Bhadram wrote:
> Hi Alex,
> 
> On 03/23/2015 09:15 PM, Alexander Aring wrote:
> 
> >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.
> >
> >Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> >---
> >  net/ieee802154/nl802154.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >
> >diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> >index a4daf91..c12c07f 100644
> >--- a/net/ieee802154/nl802154.c
> >+++ b/net/ieee802154/nl802154.c
> >@@ -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;
> 
> 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.

> >+	}
> >+
> >+	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);
> >+	}

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

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

end of thread, other threads:[~2015-03-24  8:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-23 15:45 [PATCHv2 bluetooth-next 0/4] ieee802154: nl802154 SET commands and pib defaults Alexander Aring
2015-03-23 15:45 ` [PATCHv2 bluetooth-next 1/4] nl802154: add set wpan phy cmd Alexander Aring
2015-03-24  3:41   ` Varka Bhadram
2015-03-24  8:43     ` Alexander Aring
2015-03-23 15:45 ` [PATCHv2 bluetooth-next 2/4] nl802154: add set interface cmd Alexander Aring
2015-03-23 15:45 ` [PATCHv2 bluetooth-next 3/4] ieee802154: move mac pib defaults Alexander Aring
2015-03-23 15:45 ` [PATCHv2 bluetooth-next 4/4] ieee802154: set aret handling according to 802.15.4 Alexander Aring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.