Conversation
OlegHahm
added
Type: enhancement
|
Don't know what you did, but the diff looks very chaotic for an addition of new features oO |
sys/include/net/ng_ipv6/addr.h
Outdated
There was a problem hiding this comment.
ffc0 would fall through the cracks, too. Also, since the link-local equivalent is checking the link-local multicast prefix too, this one should do it too.
There was a problem hiding this comment.
ffc0 would fall through the cracks, too
Yes, but according to RFC4291 site-local has the first 10 bits set to 1111111011. Hence, ffc0 would not be site-local.
Also, since the link-local equivalent is checking the link-local multicast prefix too, this one should do it too.
Will add.
There was a problem hiding this comment.
You misunderstood: ffc0 will be registered as site-local by this function, since the & shadows the lsb of the first byte.
There was a problem hiding this comment.
Also: I think the compiler has a better chance of optimization if you swap the byteorder of the constants (as I did in the rest of the file)
There was a problem hiding this comment.
Why should 0b1111111011000000 result in smaller size than 0b0000001101111111?
Or did I get you wrong?
There was a problem hiding this comment.
Because the code to convert it like that isn't generated.
There was a problem hiding this comment.
wanted to make a build size comparison:
/home/oleg/git/RIOT/sys/include/net/ng_ipv6/addr.h: In function ‘ng_ipv6_addr_is_site_local’
/home/oleg/git/RIOT/sys/include/net/ng_ipv6/addr.h:366:46: error: incompatible type for argument 1 of ‘byteorder_ntohs’
return (((addr->u16[0] & byteorder_ntohs(0xFFC0)) ==
There was a problem hiding this comment.
I tried several combinations, none of them was compilable. If you can provide the exact code I should test, I will run a buildsize comparison.
There was a problem hiding this comment.
Will do in the current diff, but I see the error on spot: you use byteorder_ntohs() with an integer (thus an hosts ordered value) you should have used `byteorder_htons().
I move one or two checks to order them according to their order in RFC4291. |
|
I think if you play around with the diff options it looks less chaotic. |
OlegHahm
added
the
CI: needs squashing
|
updated |
sys/include/net/ng_ipv6/addr.h
Outdated
There was a problem hiding this comment.
No byteorder conversion necessary:
addr->u16[5].u16 == 0xffffsince 0xffff is symmetrical.
miri64
added
the
CI: ready for build
|
Apart from a minor copy-pasta error in doc: ACK |
|
(please squash immediately) |
Added checks for: * Global Unicast Addresses * IPv6 Addresses with Embedded IPv4 Addresses * IPv4-Compatible IPv6 Address * IPv4-Mapped IPv6 Address * Site-Local IPv6 Unicast Addresses
OlegHahm
force-pushed
the
ipv6_more_address_types
branch
from
836b16d to
f9417ba
Compare
|
Addressed comment, fixed a minor glitch, and squashed without conflicts. Thanks for the review. |
OlegHahm
removed
the
CI: needs squashing
2 participants
No description provided.