[PATCH bluetooth-next 0/3] ieee802154: UAPI changes

[PATCH bluetooth-next 0/3] ieee802154: UAPI changes

[Alexander Aring]

Hi,

I think the stack implementation becomes more popular since the last year.
This patch series contains heavy UAPI changes which will of course breaks
some automated scripts by using iwpan and developed software which checks
on ARPHRD on a lowpan interface and somebody which use sysfs entries for
setting some phy settings.

I sent this series couple months ago and nobody screams, so I think the
changes is okay for everybody. I removed to set the NETDEV type, we can
do this still later and it's more a new feature than a change.

I currently also think about to remove the ioctl callbacks for setting
addresses. Nowadays nobody should use them from userspace to set wpan
addresses. But I want make this later after to be sure that we don't
breaking crypto stuff.

- Alex

Alexander Aring (3):
  ieee802154: 6lowpan: fix ARPHRD to ARPHRD_6LOWPAN
  ieee802154: change wpan-phy name to phy
  ieee802154: remove deprecated sysfs entries

 net/ieee802154/6lowpan/core.c |  2 +-
 net/ieee802154/core.c         |  5 ++++-
 net/ieee802154/sysfs.c        | 49 -------------------------------------------
 3 files changed, 5 insertions(+), 51 deletions(-)

-- 
2.3.0

[PATCH bluetooth-next 1/3] ieee802154: 6lowpan: fix ARPHRD to ARPHRD_6LOWPAN

[Alexander Aring]

Currently there exists two interface types with ARPHRD_IEEE802154. These
are the 802.15.4 interfaces and 802.15.4 6LoWPAN interfaces. This is
more a bug because some userspace applications checks on this value like
wireshark. This occurs that wireshark will always try to parse a lowpan
interface as 802.15.4 frames. With ARPHRD_6LOWPAN wireshark will parse
it as IPv6 frames which is correct.

Much applications checks on this value to readout the EUI64 mac address
which should be the same for ARPHRD_6LOWPAN. BTLE 6LoWPAN and ieee802154
6LoWPAN will share now the same ARPHRD.

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

diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index dfd3c60..0ae5822 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -113,7 +113,7 @@ static void lowpan_setup(struct net_device *dev)
 {
 	dev->addr_len		= IEEE802154_ADDR_LEN;
 	memset(dev->broadcast, 0xff, IEEE802154_ADDR_LEN);
-	dev->type		= ARPHRD_IEEE802154;
+	dev->type		= ARPHRD_6LOWPAN;
 	/* Frame Control + Sequence Number + Address fields + Security Header */
 	dev->hard_header_len	= 2 + 1 + 20 + 14;
 	dev->needed_tailroom	= 2; /* FCS */
-- 
2.3.0

Re: [PATCH bluetooth-next 1/3] ieee802154: 6lowpan: fix ARPHRD to ARPHRD_6LOWPAN

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

On 03/02/2015 03:10 PM, Alexander Aring wrote:
> Currently there exists two interface types with ARPHRD_IEEE802154. These
> are the 802.15.4 interfaces and 802.15.4 6LoWPAN interfaces. This is
> more a bug because some userspace applications checks on this value like
> wireshark. This occurs that wireshark will always try to parse a lowpan
> interface as 802.15.4 frames. With ARPHRD_6LOWPAN wireshark will parse
> it as IPv6 frames which is correct.
> 
> Much applications checks on this value to readout the EUI64 mac address
> which should be the same for ARPHRD_6LOWPAN. BTLE 6LoWPAN and ieee802154
> 6LoWPAN will share now the same ARPHRD.

Does this have effects on libpcap/tcpdump and/or wireshark?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH bluetooth-next 1/3] ieee802154: 6lowpan: fix ARPHRD to ARPHRD_6LOWPAN

[Alexander Aring]

Hi Marc,

On Mon, Mar 02, 2015 at 03:16:50PM +0100, Marc Kleine-Budde wrote:
> On 03/02/2015 03:10 PM, Alexander Aring wrote:
> > Currently there exists two interface types with ARPHRD_IEEE802154. These
> > are the 802.15.4 interfaces and 802.15.4 6LoWPAN interfaces. This is
> > more a bug because some userspace applications checks on this value like
> > wireshark. This occurs that wireshark will always try to parse a lowpan
> > interface as 802.15.4 frames. With ARPHRD_6LOWPAN wireshark will parse
> > it as IPv6 frames which is correct.
> > 
> > Much applications checks on this value to readout the EUI64 mac address
> > which should be the same for ARPHRD_6LOWPAN. BTLE 6LoWPAN and ieee802154
> > 6LoWPAN will share now the same ARPHRD.
> 
> Does this have effects on libpcap/tcpdump and/or wireshark?
> 

The define ARPHRD_6LOWPAN is UAPI. It has effects on every application
which checks on the previous type ARPHRD_IEEE802154, but we can't still
longer use the same type for two interfaces which should be different.

Examples from my previous mail which was marked as RFC:

A little list of applications which need to update afterwards this
patch:

 - radvd [0]
 - unstrung [1]

[0] https://github.com/reubenhwk/radvd/blob/master/device-linux.c#L82
[1] https://github.com/mcr/unstrung/blob/5e7c4060730ab4f81ddcd75657d87ec78da91bd6/lib/libndmgmt/netlink.cpp#L358

These examples checks for EUI64 address which should be the same for
ARPHRD_6LOWPAN.

It would be problematic to check which is the underlaying interface
(It's 802.15.4 or BTLE) but I am sure we can get this information
somewhere else or via NETDEV devtype (SET_NETDEV_DEVTYPE).


For your exactly question, I think libpcap evaluate this. See:

https://github.com/the-tcpdump-group/libpcap/blob/master/pcap-linux.c#L3052

but I am not sure, in this example the change to ARPHRD_6LOWPAN is
correct because the interface doesn't have 802.15.4 frames, it's plain
IPv6 and upper without L2 information.

Somewhere is also an ARPHRD_IEEE802154_MONITOR type, which is also not
necessary by 802.15.4 frame. We should do it like the wireless monitor
implementation which set's some special flags to indenticate monitors.

But this is already UAPI, we can't delete it and now this should life in
UAPI headers forever. :-)

Does this answer your question?

- Alex

[PATCH bluetooth-next 2/3] ieee802154: change wpan-phy name to phy

[Alexander Aring]

Currently the wpan_phy under /sys/class/ieee802154/ is named as
"wpan-phy#", this patch will change the name to phy. This will
introduce the same naming convention like wireless.

Note: wpan-tools users will not type "wpan-phy#" anymore, just a simple
      "phy#" is enough.

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

diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 888d099..2ee00e8 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -25,6 +25,9 @@
 #include "sysfs.h"
 #include "core.h"
 
+/* name for sysfs, %d is appended */
+#define PHY_NAME "phy"
+
 /* RCU-protected (and RTNL for writers) */
 LIST_HEAD(cfg802154_rdev_list);
 int cfg802154_rdev_list_generation;
@@ -122,7 +125,7 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size)
 
 	INIT_LIST_HEAD(&rdev->wpan_dev_list);
 	device_initialize(&rdev->wpan_phy.dev);
-	dev_set_name(&rdev->wpan_phy.dev, "wpan-phy%d", rdev->wpan_phy_idx);
+	dev_set_name(&rdev->wpan_phy.dev, PHY_NAME "%d", rdev->wpan_phy_idx);
 
 	rdev->wpan_phy.dev.class = &wpan_phy_class;
 	rdev->wpan_phy.dev.platform_data = rdev;
-- 
2.3.0

[PATCH bluetooth-next 3/3] ieee802154: remove deprecated sysfs entries

[Alexander Aring]

It's only necessary to offer the name and index, others value are
available over netlink.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/sysfs.c | 49 -------------------------------------------------
 1 file changed, 49 deletions(-)

diff --git a/net/ieee802154/sysfs.c b/net/ieee802154/sysfs.c
index dff55c2..133b428 100644
--- a/net/ieee802154/sysfs.c
+++ b/net/ieee802154/sysfs.c
@@ -48,49 +48,6 @@ static ssize_t name_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(name);
 
-#define MASTER_SHOW_COMPLEX(name, format_string, args...)		\
-static ssize_t name ## _show(struct device *dev,			\
-			    struct device_attribute *attr, char *buf)	\
-{									\
-	struct wpan_phy *phy = container_of(dev, struct wpan_phy, dev);	\
-	int ret;							\
-									\
-	mutex_lock(&phy->pib_lock);					\
-	ret = snprintf(buf, PAGE_SIZE, format_string "\n", args);	\
-	mutex_unlock(&phy->pib_lock);					\
-	return ret;							\
-}									\
-static DEVICE_ATTR_RO(name)
-
-#define MASTER_SHOW(field, format_string)				\
-	MASTER_SHOW_COMPLEX(field, format_string, phy->field)
-
-MASTER_SHOW(current_channel, "%d");
-MASTER_SHOW(current_page, "%d");
-MASTER_SHOW(transmit_power, "%d +- 1 dB");
-MASTER_SHOW_COMPLEX(cca_mode, "%d", phy->cca.mode);
-
-static ssize_t channels_supported_show(struct device *dev,
-				       struct device_attribute *attr,
-				       char *buf)
-{
-	struct wpan_phy *phy = container_of(dev, struct wpan_phy, dev);
-	int ret;
-	int i, len = 0;
-
-	mutex_lock(&phy->pib_lock);
-	for (i = 0; i < 32; i++) {
-		ret = snprintf(buf + len, PAGE_SIZE - len,
-			       "%#09x\n", phy->channels_supported[i]);
-		if (ret < 0)
-			break;
-		len += ret;
-	}
-	mutex_unlock(&phy->pib_lock);
-	return len;
-}
-static DEVICE_ATTR_RO(channels_supported);
-
 static void wpan_phy_release(struct device *dev)
 {
 	struct cfg802154_registered_device *rdev = dev_to_rdev(dev);
@@ -101,12 +58,6 @@ static void wpan_phy_release(struct device *dev)
 static struct attribute *pmib_attrs[] = {
 	&dev_attr_index.attr,
 	&dev_attr_name.attr,
-	/* below will be removed soon */
-	&dev_attr_current_channel.attr,
-	&dev_attr_current_page.attr,
-	&dev_attr_channels_supported.attr,
-	&dev_attr_transmit_power.attr,
-	&dev_attr_cca_mode.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(pmib);
-- 
2.3.0

Re: [PATCH bluetooth-next 0/3] ieee802154: UAPI changes

[Marcel Holtmann]

Hi Alex,

> I think the stack implementation becomes more popular since the last year.
> This patch series contains heavy UAPI changes which will of course breaks
> some automated scripts by using iwpan and developed software which checks
> on ARPHRD on a lowpan interface and somebody which use sysfs entries for
> setting some phy settings.
> 
> I sent this series couple months ago and nobody screams, so I think the
> changes is okay for everybody. I removed to set the NETDEV type, we can
> do this still later and it's more a new feature than a change.
> 
> I currently also think about to remove the ioctl callbacks for setting
> addresses. Nowadays nobody should use them from userspace to set wpan
> addresses. But I want make this later after to be sure that we don't
> breaking crypto stuff.
> 
> - Alex
> 
> Alexander Aring (3):
>  ieee802154: 6lowpan: fix ARPHRD to ARPHRD_6LOWPAN
>  ieee802154: change wpan-phy name to phy
>  ieee802154: remove deprecated sysfs entries
> 
> net/ieee802154/6lowpan/core.c |  2 +-
> net/ieee802154/core.c         |  5 ++++-
> net/ieee802154/sysfs.c        | 49 -------------------------------------------
> 3 files changed, 5 insertions(+), 51 deletions(-)

all 3 patches have been applied to bluetooth-next tree.

Regards

Marcel