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/fib: Added network prefix flag to indicate a network destination #3157

Merged
merged 1 commit into from
Nov 2, 2015

Conversation

BytesGalore
Copy link
Member

This PR introduces a specific flag (FIB_FLAG_NET_PREFIX) for FIB entries used to indicate if a destination of the entry is a host or a network.

edit: The FIB now considers this flag to decide if the given address is a host or network.
So if not set the FIB assumes a host address, even if the address is all (trailing)zeros ::

@BytesGalore BytesGalore added Area: network Area: Networking Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer NSTF labels Jun 3, 2015
@BytesGalore BytesGalore changed the title net/fib: Added network prefix flag to indicate a networ destination net/fib: Added network prefix flag to indicate a network destination Jun 3, 2015
@BytesGalore BytesGalore force-pushed the fib_host_prefix_flags branch 2 times, most recently from 5c161d3 to 9d5fced Compare June 15, 2015 08:05
@BytesGalore BytesGalore force-pushed the fib_host_prefix_flags branch from 9d5fced to 399dfe2 Compare July 27, 2015 12:01
@cgundogan
Copy link
Member

@BytesGalore please rebase

@BytesGalore BytesGalore force-pushed the fib_host_prefix_flags branch from 399dfe2 to 26c93f4 Compare August 18, 2015 08:02
@BytesGalore
Copy link
Member Author

rebased

@cgundogan cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 18, 2015
@cgundogan
Copy link
Member

nit-picking: you forgot the k in the second network in your commit message.

net/fib: Added network prefix flag to indicate a networ destination

@cgundogan
Copy link
Member

travis seems to fail for the mcu groups avr8 and msp430. And I fail to see why

@jnohlgard
Copy link
Member

@cgundogan /home/travis/build/RIOT-OS/RIOT/tests/unittests/tests-fib/tests-fib.c:479:19: error: left shift count >= width of type [-Werror]

there are probably some int variables which should be long or uint32_t

@@ -60,6 +60,11 @@ typedef struct fib_destination_set_entry_t {
#define FIB_LIFETIME_NO_EXPIRE (0xFFFFFFFF)

/**
* @brief flag to identify if the FIB-Entry is a net prefix (MSB == 1)
*/
#define FIB_FLAG_NET_PREFIX (1<<31)
Copy link
Member

Choose a reason for hiding this comment

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

(1ul << 31) or ((uint_32_t)1 << 31) or (0x80000000ul)

Copy link
Member

Choose a reason for hiding this comment

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

@gebart ah, thanks for clarifying

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 change it to (1ul << 31)

@BytesGalore BytesGalore force-pushed the fib_host_prefix_flags branch from 26c93f4 to 5a90bb4 Compare August 18, 2015 12:03
@BytesGalore
Copy link
Member Author

added the k, adjusted the constant width and directly squashed

@cgundogan
Copy link
Member

@BytesGalore I am currently facing a use case where I would hypothetically want to add a network prefix to the fib. However, just marking the prefix with a flag doesn't seem to be enough. How would it be possible to encode the actual prefix length of an ipv6 network prefix in the fib? I assume the address size in the fib would still be IN6ADDRSZ, but then the information about the actual prefix length is lost, isn't it?

@BytesGalore
Copy link
Member Author

@cgundogan right now there is no way to exactly specify the prefix length in bits for a FIB entry :(
The FIB determines the prefix length with by trailing 0 in an entry.
However, this is only lets say "inspired" by IP addresses (and in fact somehow only a workaround).
The FIB is supposed to handle every possible numbering of network endpoints, so the prefix length and more precisely its representation is specific to the address type used.
Probably adding address type specific functions for prefixes would be the way to go.
Like the FIB modularisation I started, but dropped months ago [1]

[1] https://github.com/BytesGalore/RIOT/tree/modular_fib

@cgundogan
Copy link
Member

The FIB determines the prefix length with by trailing 0 in an entry.

I also thought about this, but what if one or more trailing zeros are part of the network prefix? :/ Going the simple route: Just adding a generic extra byte of information for each entry is the wrong way, is it?

@BytesGalore
Copy link
Member Author

@cgundogan its maybe the question if we sacrifice one byte of the flags for address specific information, e.g. prefix-length (I'm just thinking loud)

@cgundogan
Copy link
Member

How many bytes are currently used of the flags entry? 1 out of 4?

@BytesGalore
Copy link
Member Author

This PR introduces the first seriously used bit in the flags, so 1/8 Byte is in use

@BytesGalore
Copy link
Member Author

But the lowest byte is set at several calls in the neighbour discovery

@cgundogan
Copy link
Member

@BytesGalore, could you rebase this again, please?

@OlegHahm OlegHahm modified the milestone: Release 2015.09 Sep 2, 2015
@OlegHahm OlegHahm modified the milestones: Release NEXT MAJOR, Release 2015.09 Sep 2, 2015
@BytesGalore
Copy link
Member Author

sure, rebased :)
(sorry for my long silence I lost momentum during my vacations)

@cgundogan
Copy link
Member

@BytesGalore when I merge this into master then pinging does not work for me on the iotlab testbed with the following setup:

m3-1;ifconfig
1444319155.402302;m3-1;> ifconfig
1444319155.404926;m3-1;Iface  7   HWaddr: ab:12  Channel: 26  NID: 0x23  TX-Power: 0dBm  State: IDLE CSMA Retries: 4
1444319155.406787;m3-1;           Long HWaddr: 36:32:48:33:46:d7:ab:12
1444319155.407772;m3-1;           AUTOACK  CSMA  MTU:1280  6LO  IPHC
1444319155.407880;m3-1;           Source address length: 8
1444319155.408974;m3-1;           Link type: wireless
1444319155.409081;m3-1;           inet6 addr: ff02::1/128  scope: local [multicast]
1444319155.411271;m3-1;           inet6 addr: fe80::3432:4833:46d7:ab12/64  scope: local
1444319155.412411;m3-1;           inet6 addr: ff02::1:ffd7:ab12/128  scope: local [multicast]
1444319155.413270;m3-1;           inet6 addr: abcd::1/64  scope: global
1444319155.415853;m3-1;           inet6 addr: ff02::1:ff00:1/128  scope: local [multicast]
1444319155.415958;m3-1;
m3-2;ifconfig
1444319158.545899;m3-2;> ifconfig
1444319158.548747;m3-2;Iface  7   HWaddr: 7d:2a  Channel: 26  NID: 0x23  TX-Power: 0dBm  State: IDLE CSMA Retries: 4
1444319158.549733;m3-2;           Long HWaddr: 36:32:48:33:46:de:7d:2a
1444319158.550754;m3-2;           AUTOACK  CSMA  MTU:1280  6LO  IPHC
1444319158.551726;m3-2;           Source address length: 8
1444319158.551831;m3-2;           Link type: wireless
1444319158.553714;m3-2;           inet6 addr: ff02::1/128  scope: local [multicast]
1444319158.554715;m3-2;           inet6 addr: fe80::3432:4833:46de:7d2a/64  scope: local
1444319158.555705;m3-2;           inet6 addr: ff02::1:ffde:7d2a/128  scope: local [multicast]
1444319158.556713;m3-2;           inet6 addr: abcd::2/64  scope: global
1444319158.558701;m3-2;           inet6 addr: ff02::1:ff00:2/128  scope: local [multicast]
1444319158.558806;m3-2;
m3-1;fibroute
1444319162.497519;m3-1;> fibroute
1444319162.500435;m3-1;Destination                             Flags  Next Hop                                Flags  Expires          Interface
1444319162.503357;m3-1;abcd::2                                 0x0003 H fe80::3432:4833:46de:7d2a               0x0003 NEVER            7
m3-2;fibroute
1444319165.922716;m3-2;> fibroute
1444319165.926645;m3-2;Destination                             Flags  Next Hop                                Flags  Expires          Interface
1444319165.928215;m3-2;::                                      0x0003 H fe80::3432:4833:46d7:ab12               0x0003 NEVER            7
m3-1;ping6 3 abcd::2 10 10
1444319180.118117;m3-1;> ping6 3 abcd::2 10 10
1444319181.133059;m3-1;ping timeout
1444319182.158023;m3-1;ping timeout
1444319183.183957;m3-1;ping timeout
1444319183.185237;m3-1;--- abcd::2 ping statistics ---
1444319183.185328;m3-1;3 packets transmitted, 0 received, 100% packet loss

It works on native, however.

@cgundogan cgundogan added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Oct 27, 2015
@cgundogan
Copy link
Member

needs a rebase

@BytesGalore BytesGalore force-pushed the fib_host_prefix_flags branch from 01afb92 to d7069e8 Compare November 2, 2015 06:58
@BytesGalore
Copy link
Member Author

rebased (whoa strider is green?)

@cgundogan
Copy link
Member

pinging doesn't seem to work on iotlab-m3 nodes with this PR merged into master

@BytesGalore BytesGalore added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 2, 2015
@BytesGalore BytesGalore force-pushed the fib_host_prefix_flags branch from fad6493 to 1420dde Compare November 2, 2015 14:38
@BytesGalore
Copy link
Member Author

@cgundogan I think I fixed the ping issue :D
The flag indicating a destination as network was missing when adding the unspecified address :: to the FIB.
I added a word in the commit comment on this.

@cgundogan
Copy link
Member

@BytesGalore indeed, you fixed it! squash plz (:

@BytesGalore BytesGalore force-pushed the fib_host_prefix_flags branch from 1420dde to 523d1f8 Compare November 2, 2015 14:58
@BytesGalore BytesGalore added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Nov 2, 2015
@BytesGalore
Copy link
Member Author

squashed

@cgundogan
Copy link
Member

GO

cgundogan added a commit that referenced this pull request Nov 2, 2015
net/fib: Added network prefix flag to indicate a network destination
@cgundogan cgundogan merged commit be7a34b into RIOT-OS:master Nov 2, 2015
@OlegHahm
Copy link
Member

OlegHahm commented Jan 6, 2016

What exactly is the purpose of this flag?

@BytesGalore
Copy link
Member Author

@OlegHahm the purpose is to set if a given destination is a network or a host prefix.
The FIB takes this flag into account when comparing addresses on requests for a next-hop.
It then lets the FIB recognize the prefix length automatically.

#4279 will supersede this behaviour (which is basically not precise)

@OlegHahm
Copy link
Member

OlegHahm commented Jan 6, 2016

I still don't understand. If A performs a next-hop lookup in the FIB for B, the lookup finds the longest prefix match for B, right? Hence, the FIB may return C for the best matching address D. Why should A care if D is a prefix or a full host address?

@BytesGalore
Copy link
Member Author

B: abcd::1234:1010 <- lookup address
C: abcd::1234:1000 <- Host Address
D: abcd::1234:0000 <- Network Address

LPM would always return C, but this makes no sense since C is a Host and wouldn't forward to D

@OlegHahm
Copy link
Member

OlegHahm commented Jan 6, 2016

But the lookup should not return the destination field, but the next-hop field of the FIB, right? And this should always contain a host address.

@BytesGalore
Copy link
Member Author

yes you lookup for the destination address and the returned next-hop is always a host node.
Looking up for destination B always result in returning the next-hop to C with LPM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants