Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net_if: proper u/l-bit inversion for short addresses #2518

Closed
wants to merge 3 commits into from

Conversation

cgundogan
Copy link
Member

This is an alternative to #2517 and does not introduce the pan_id to the IID/EUI-64 generation from short addresses

@cgundogan cgundogan added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking labels Mar 2, 2015
@cgundogan
Copy link
Member Author

@authmillenon @gebart please look at this alternative

@@ -122,7 +122,7 @@ void udp_send(int argc, char **argv)
memset(&sa, 0, sizeof(sa));

if (address) {
ipv6_addr_init(&ipaddr, 0xabcd, 0x0, 0x0, 0x0, 0x0, 0x00ff, 0xfe00, (uint16_t)address);
ipv6_addr_init(&ipaddr, 0xabcd, 0x0, 0x0, 0x0, 0x0200, 0x00ff, 0xfe00, (uint16_t)address);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this happening in UDP? IP chooses the best fitting source address. Also s/0xabcd/0x0000/ as it is done everywhere else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well somehow you have to define the destination address to the socket, before going down to ip

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the setting of abcd explains a lot, why we have so many checksum errors with UDP. Why abcd? Even if we use the method in RFC 4944, who is saying that the PAN is abcd for the destination?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think abcd is representing the pan id here. The device is configured with an ip address prefixed with abcd in rpl_udp/rpl.c . that's why the destination address also has to start with abcd

@cgundogan cgundogan added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 2, 2015
@cgundogan
Copy link
Member Author

I updated this to prevent u/l inversion for short addresses.
But the rpl_udp example is broken again with this commit.
The actual problem lies within lowpan decoding/encoding.
All my nodes are configured with fe80::ff:fe00:X, but they send to fe80:200:ff:fe00:X.
This 200 results from u/l inversion somewhere in lowpan enc/dec..

@@ -1295,9 +1295,6 @@ void lowpan_iphc_decoding(uint8_t *data, uint8_t length, net_if_eui64_t *s_addr,
memcpy(&(ipv6_buf->srcaddr.uint8[0]), &ll_prefix[0], 2);
memset(&(ipv6_buf->srcaddr.uint8[2]), 0, 6);
memcpy(&(ipv6_buf->srcaddr.uint8[8]), &s_addr->uint8[0], 8);
/* Invert Universal/local bit as specified in
* RFC4291, section 2.5.1 "Interface Identifiers" */
ipv6_buf->srcaddr.uint8[8] ^= 0x02;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gebart Without these two lines the example works on native again.. but I fear this undoes your PR introducing the u/l bit inversion.. Can you test this on your setup?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any time to test this right now, but I guess it would work again since it reverts the change in #2497

@jnohlgard
Copy link
Member

The IPHC en-/decoding will also need to handle short addresses differently in order for it to work. If we always "translate" short addresses to extended and flipping the annoying bit (new name, same old 0x02) I guess it would work. It may cause troubles in other parts using the hardware address though..

@jnohlgard
Copy link
Member

@cgundogan please test #2499 again, I have added two more commits for EUI64 handling for short addresses.

@jnohlgard
Copy link
Member

Sorry, I split the PR, please test #2523 instead.

@jnohlgard
Copy link
Member

Can this be closed now that #2523 has been merged?

@cgundogan
Copy link
Member Author

@gebart ofcourse, I will close this one

@cgundogan cgundogan closed this Mar 17, 2015
@cgundogan cgundogan deleted the fix_ul_bit_inversion branch March 17, 2015 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants