kw2xrf: Don't use netdev_ieee802154_t for link layer address #10534

bergzand · 2018-12-01T16:56:57Z

Contribution description

This PR modifies the behaviour of the kw2xrf driver to not propagate the link layer address to the netdev_ieee802154_t struct. This helps the goal of separating the netdev and the ieee802154_t layer by another bit. The end goal is to remove the short and long address from the netdev_ieee802154_t struct and match the behaviour of the ethernet drivers.

The performance impact of this change should be negligible, the NETOPT_ADDRESS(_LONG) call is only used on initialization and by the ifconfig command.

Testing procedure

Please test whether:

ifconfig still shows the correct link layer addresses. These should not have changed between this PR and master.
ping still works. This to verify that the link layer address is also properly configured in the radio, if the ping echo/reply fails, the address filter of the radio might be configured in the wrong endianness.

Issues/PRs references

Another small step for #7736

miri64

I tested the PR according to the testing procedures on two of the firefly boards in lille. During that I encountered the following output, but I'm pretty sure that is unrelated to this PR (just wanted to note it down). It did not lead to the node aborting btw, I could use the node as before after the output:

  RFCORE_ASSERT(rssi_val > CC2538_RF_SENSITIVITY) failed at line 348 in _recv()!
  RFCORE_SFR_RFERRF = 0x00

In addition, I used the default example to check the send/receive behavior under different address configurations (I needed to add the firefly board to the BOARD_PROVIDES_NETIF list for that):

short address to long address
short address to short address
broadcasting from short address
long address to short address
long address to long address
broadcasting from long address

All were successful.

miri64 · 2018-12-02T13:13:19Z

(I did not sniff however while I did the testing. So let me just crosscheck against an M3. You may squash in the meantime)

miri64 · 2018-12-02T13:17:10Z

I was also able to exchange packages both from and to the short and long addresses of the respective nodes between a firefly and an iotlab-m3.

Wait a minute... this isn't a change to the cc2538_rf...

miri64

You still may squash. The code looks alright, I just did not notice that it isn't cc2538_rf code (only noticed because Murdock was complaining about a typo of yours >.<).

miri64 · 2018-12-02T13:21:21Z

drivers/kw2xrf/kw2xrf_getset.c

 #ifdef MODULE_SIXLOWPAN
    /* https://tools.ietf.org/html/rfc4944#section-12 requires the first bit to
     * 0 for unicast addresses */
-    dev->netdev.short_addr[1] &= 0x7F;
+    val_arr[0] &= 0x7F;


Suggested change

val_arr[0] &= 0x7F;

val_ar[0] &= 0x7F;

bergzand · 2018-12-03T07:50:33Z

I just did not notice that it isn't cc2538_rf code

Sorry! 😭 Must have been a copy paste error from earlier PR's

bergzand · 2018-12-03T07:53:31Z

squashed, again sorry for the confusion caused by the subject 😭

miri64 · 2018-12-03T08:14:10Z

I'll have another look today.

miri64 · 2018-12-03T08:56:41Z

I tried to use the phynode in the testbed, but I'm not sure it is just not reachable from one of the iotlab-m3's or if it isn't able to send/receive... The nodes look close on the map, but maybe @aabadie can help. I'll try again with my local phynode once I'm at the office.

bergzand · 2018-12-03T09:01:08Z

I was able to ping between the phynode and saclay,samr21-xpro,1

miri64 · 2018-12-03T09:03:26Z

I was able to ping between the phynode and saclay,samr21-xpro,1

Mh... but with that I can't use the sniffer :-/

aabadie · 2018-12-03T09:07:54Z

but with that I can't use the sniffer :-/

With the phynode you can't use the usual IoT-LAB control node sniffer feature because there's no control node with it (it's plugged on an IoT-LAB RPI3 based gateway). But you can still use the sniffer of samr21-xpro or m3 nodes because they are plugged on a real IoT-LAB gateway, with all the usual tooling it provides (sniffer, power consumption).

miri64 · 2018-12-03T09:13:16Z

With the phynode you can't use the usual IoT-LAB control node sniffer feature because there's no control node with it (it's plugged on an IoT-LAB RPI3 based gateway).

Please read what I wrote. I tried to sniff from an m3 (m3-7 and m3-10 because they seem to be spatially close at least according to the map). I only was able to see the packets from the m3, so I suspect the phynode is out of range from the m3, but I was not sure (since @bergzand was able to ping from a samr21-xpro I guess I was right with that though).

But you can still use the sniffer of samr21-xpro or m3 nodes because they are plugged on a real IoT-LAB gateway, with all the usual tooling it provides (sniffer, power consumption).

Last time I tried to use profiles with the samr21-xpro it didn't work (I'll retry though).

aabadie · 2018-12-03T09:17:06Z

Please read what I wrote

That's what I did, but misinterpret it, sorry ;)

The all samr21-xpro and m3 are in the same room and very close, there's no reason (except maybe radio power setting) for the nodes to no see between each others.

miri64 · 2018-12-03T09:24:59Z

I tried to get the sniffer profile working with the samr21. I tried with two profiles:

$ iotlab-profile get -n sniffer26
{
    "consumption": {
        "average": 64,
        "current": false,
        "period": 1100,
        "power": false,
        "voltage": false
    },
    "nodearch": "m3",
    "power": "dc",
    "profilename": "sniffer26",
    "radio": {
        "channels": [
            26
        ],
        "mode": "sniffer",
        "num_per_channel": null,
        "period": null
    }
}
$ iotlab-profile get -n samr21_sniffer26
{
    "consumption": null,
    "nodearch": "custom",
    "power": "dc",
    "profilename": "samr21_sniffer26",
    "radio": {
        "channels": [
            26
        ],
        "mode": "sniffer",
        "num_per_channel": null,
        "period": null
    }
}

For both profiles I get

$ ssh saclay.iot-lab.info sniffer_aggregator -o - | wireshark -i - -k
SnifferAggregator: Empty nodes list []

miri64 · 2018-12-03T09:25:35Z

(sniffer26 works for the m3 nodes)

miri64 · 2018-12-03T09:28:07Z

I was able to ping between the phynode and saclay,samr21-xpro,1

I was not. But as said before, I'll try at the office again.

miri64 · 2018-12-03T11:34:17Z

I redid the tests as before (also set addresses for the interface). Most things work, however the IPv6 address for the phynode is configured wrong:

2018-12-03 12:31:41,810 - INFO #  ifconfig
2018-12-03 12:31:41,814 - INFO # Iface  7  HWaddr: 03:2A  Channel: 26  NID: 0x23
2018-12-03 12:31:41,818 - INFO #           Long HWaddr: 77:26:6E:76:7D:8A:03:2A
2018-12-03 12:31:41,822 - INFO #            TX-Power: 0dBm  State: IDLE
2018-12-03 12:31:41,825 - INFO #           AUTOACK  ACK_REQ  AUTOCCA
2018-12-03 12:31:41,829 - INFO #           MTU:1280  HL:64  RTR  IPHC
2018-12-03 12:31:41,832 - INFO #           Source address length: 8
2018-12-03 12:31:41,834 - INFO #           Link type: wireless
2018-12-03 12:31:41,839 - INFO #           inet6 addr: fe80::200:0:0:0  scope: local  VAL
2018-12-03 12:31:41,842 - INFO #           inet6 group: ff02::2
2018-12-03 12:31:41,845 - INFO #           inet6 group: ff02::1
2018-12-03 12:31:41,848 - INFO #           inet6 group: ff02::1:ff00:0
2018-12-03 12:31:41,851 - INFO #           inet6 group: ff02::1a
2018-12-03 12:31:41,852 - INFO #
2018-12-03 12:31:41,855 - INFO #           Statistics for Layer 2
2018-12-03 12:31:41,858 - INFO #             RX packets 0  bytes 0
2018-12-03 12:31:41,862 - INFO #             TX packets 3 (Multicast: 3)  bytes 115
2018-12-03 12:31:41,865 - INFO #             TX succeeded 3 errors 0
2018-12-03 12:31:41,868 - INFO #           Statistics for IPv6
2018-12-03 12:31:41,871 - INFO #             RX packets 0  bytes 0
2018-12-03 12:31:41,875 - INFO #             TX packets 3 (Multicast: 3)  bytes 178
2018-12-03 12:31:41,878 - INFO #             TX succeeded 3 errors 0

Weirdly enough, in the sniff the correct IPv6 address is shown as source :-/

miri64 · 2018-12-03T11:37:11Z

If I ping the phynode under the address that the sniff shows as source, it I get a "destination unreachable" error back from the phynode. I think something about getting the IID might be broken ;-).

miri64 · 2018-12-03T12:19:37Z

Arghs, since #10455, the IID from the device is preferred. However, this is currently implemented in netdev_ieee802154 which is now not correctly configured anymore :-/. The source was right because address compression was applied, so wireshark just showed fe80:: + source-based IID (which is the correct hardware address).

miri64 · 2018-12-03T12:20:58Z

I'm willing to revert #10455 but I prefer to have #10524 merged for that first.

miri64 · 2018-12-03T12:22:42Z

Or we make _get_iid in drivers/netdev_ieee802154/netdev_ieee802154.c based on NETOPT calls.

miri64 · 2018-12-03T13:59:17Z

Or we make _get_iid in drivers/netdev_ieee802154/netdev_ieee802154.c based on NETOPT calls.

Done in #10537. With that PR merged this PR works! :-)

jnohlgard · 2018-12-10T14:36:16Z

@miri64 I noticed the same thing as you regarding the IPv6 LL addresses when implementing this same change in PR #7107 for the kw41zrf.

miri64 · 2018-12-10T14:42:04Z

Yes, I think we either should merge #10537 (as a temporary but holistic fix) or #10569 (as a more solid but GNRC-specific fix) soonish.

miri64 · 2018-12-14T00:31:13Z

#10537 got merged into master. Please rebase.

smlng

tested ACK. Used pba-d-01-kw2x and gnrc_networking, ping still works and MAC as well as link-local address stay the same.

smlng · 2018-12-21T12:56:45Z

please squash and rebase as needed

Comments were addressed.

smlng · 2019-01-07T15:54:43Z

ping @bergzand, please rebase and squash

bergzand · 2019-01-12T16:03:44Z

Squashed, sorry for the delay

This change ensures that the endianess returned by the get_addr_long is consistent with the endianess of the get_addr_short

This change modifies the kw2xrf_get_addr_short function to retrieve the short address from the device and not from netdev

bergzand · 2019-01-12T16:04:44Z

And rebased

smlng

tested ACK, used PhyNode and gnrc_networking. Changing addresses works and also shows new address in SLLAO of ND messages.

smlng · 2019-01-14T08:13:09Z

@miri64: good to go here?

miri64

ACK

bergzand added Area: network Area: Networking Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 1, 2018

bergzand requested a review from miri64 December 1, 2018 16:56

miri64 previously approved these changes Dec 2, 2018

View reviewed changes

miri64 changed the title ~~cc2538_rf: Don't use netdev_ieee802154_t for link layer address~~ kw2xrf: Don't use netdev_ieee802154_t for link layer address Dec 2, 2018

miri64 previously requested changes Dec 2, 2018

View reviewed changes

bergzand force-pushed the pr/kw2xrf/undedup_address branch from 27dbea1 to 62ca251 Compare December 3, 2018 07:52

miri64 mentioned this pull request Dec 3, 2018

netdev_ieee802154: fix IID getter #10537

Merged

miri64 added the Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation label Dec 3, 2018

jnohlgard mentioned this pull request Dec 10, 2018

drivers: Add support for KW41Z builtin transceiver #7107

Closed

smlng approved these changes Dec 21, 2018

View reviewed changes

bergzand force-pushed the pr/kw2xrf/undedup_address branch from 62ca251 to 98f2feb Compare January 12, 2019 16:03

bergzand added 5 commits January 12, 2019 17:04

kw2xrf: Change long address get to big endian

b044e83

This change ensures that the endianess returned by the get_addr_long is consistent with the endianess of the get_addr_short

kw2xrf: Fetch short addr from device

666e834

This change modifies the kw2xrf_get_addr_short function to retrieve the short address from the device and not from netdev

kw2xrf: Add NETOPT_ADDRESS{,_LONG} to getters

014901a

kw2xrf: Don't propagate address to 802154 layer

5fa40a0

kw2xrf: Don't set netdev values in addr setters

fcb55c3

bergzand force-pushed the pr/kw2xrf/undedup_address branch from 98f2feb to fcb55c3 Compare January 12, 2019 16:04

smlng approved these changes Jan 14, 2019

View reviewed changes

miri64 approved these changes Jan 14, 2019

View reviewed changes

miri64 merged commit 4ab3d1a into RIOT-OS:master Jan 14, 2019

aabadie added this to the Release 2019.01 milestone Jan 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kw2xrf: Don't use netdev_ieee802154_t for link layer address #10534

kw2xrf: Don't use netdev_ieee802154_t for link layer address #10534

bergzand commented Dec 1, 2018

miri64 left a comment

miri64 commented Dec 2, 2018

miri64 commented Dec 2, 2018

miri64 left a comment

miri64 Dec 2, 2018

bergzand commented Dec 3, 2018

bergzand commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

bergzand commented Dec 3, 2018

miri64 commented Dec 3, 2018

aabadie commented Dec 3, 2018

miri64 commented Dec 3, 2018

aabadie commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

jnohlgard commented Dec 10, 2018

miri64 commented Dec 10, 2018 •

edited

Loading

miri64 commented Dec 14, 2018

smlng left a comment

smlng commented Dec 21, 2018

smlng commented Jan 7, 2019

bergzand commented Jan 12, 2019

bergzand commented Jan 12, 2019

smlng left a comment

smlng commented Jan 14, 2019

miri64 left a comment

kw2xrf: Don't use netdev_ieee802154_t for link layer address #10534

kw2xrf: Don't use netdev_ieee802154_t for link layer address #10534

Conversation

bergzand commented Dec 1, 2018

Contribution description

Testing procedure

Issues/PRs references

miri64 left a comment

Choose a reason for hiding this comment

miri64 commented Dec 2, 2018

miri64 commented Dec 2, 2018

miri64 left a comment

Choose a reason for hiding this comment

miri64 Dec 2, 2018

Choose a reason for hiding this comment

bergzand commented Dec 3, 2018

bergzand commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

bergzand commented Dec 3, 2018

miri64 commented Dec 3, 2018

aabadie commented Dec 3, 2018

miri64 commented Dec 3, 2018

aabadie commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

miri64 commented Dec 3, 2018

jnohlgard commented Dec 10, 2018

miri64 commented Dec 10, 2018 • edited Loading

miri64 commented Dec 14, 2018

smlng left a comment

Choose a reason for hiding this comment

smlng commented Dec 21, 2018

smlng commented Jan 7, 2019

bergzand commented Jan 12, 2019

bergzand commented Jan 12, 2019

smlng left a comment

Choose a reason for hiding this comment

smlng commented Jan 14, 2019

miri64 left a comment

Choose a reason for hiding this comment

miri64 commented Dec 10, 2018 •

edited

Loading