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

ng_ndp: cache determined next-hop in FIB #3148

Merged
merged 1 commit into from
Jul 2, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jun 2, 2015

RFC 4861 recommends the store the determined next-hop in the "Destination Cache". For my implementation I use the FIB as such, if it is availabe, so I cache the next hops not fount in the FIB also in there to shorten the determination for later packages.

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation NSTF labels Jun 2, 2015
@miri64 miri64 added this to the Network Stack Task Force milestone Jun 2, 2015
* for later sends */
fib_add_entry(iface, (uint8_t *)dst, sizeof(ng_ipv6_addr_t), 0,
(uint8_t *)next_hop_ip, sizeof(ng_ipv6_addr_t), 0,
UINT32_MAX);
Copy link
Member

Choose a reason for hiding this comment

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

please change to FIB_LIFETIME_NO_EXPIRE (same below)

@BytesGalore
Copy link
Member

The added FIB entries need to be removed if the neighbour node is not reachable anymore, since then a next-hop request (if the FIB is active) will always successfully lead to nirvana.
Probably this can be done here [1]?

[1] https://github.com/authmillenon/RIOT/blob/ng_ndp/enh/cache-next-hop/sys/net/network_layer/ng_ndp/ng_ndp.c#L262

@miri64
Copy link
Member Author

miri64 commented Jun 3, 2015

@BytesGalore does fib_remove_entry() remove the entry too, if dst is the next_hop entry? For l376ff it isn't that problematic if it doesn't, but for 380ff I insert the default router as next hop for dst, not dst itself. Alternative could be to use the default router lifetime there as lifetime.

@miri64
Copy link
Member Author

miri64 commented Jun 3, 2015

Also, can routing protocols cope with the fact, that this would "randomly" remove stuff from the FIB. @Lotterleben @cgundogan?

@BytesGalore
Copy link
Member

@BytesGalore does fib_remove_entry() remove the entry too, if dst is the next_hop entry?

yup, no mercy. The FIB do not determine if an entry is allowed to be deleted.

@authmillenon you're right this is a problem since usually each routing protocol is responsible for a specific prefix.
Changing entries handled by them will result in topology and routing inconsistencies.

@miri64
Copy link
Member Author

miri64 commented Jun 3, 2015

So maybe I should give them a "reasonable lifetime" instead?

I think about

  • next_hop == ip: reachability time, and
  • next_hop == default_router: router lifetime

@BytesGalore
Copy link
Member

+1 for the reasonable lifetimes if they expire the FIB entry is removed automatically.

Also, can routing protocols cope with the fact, that this would "randomly" remove stuff from the FIB.

The FIB can handle access and manipulation from different sources.
If the destination addresses have no collisions then everything is fine :)

@miri64
Copy link
Member Author

miri64 commented Jun 3, 2015

Since the routing protocols probably use prefixes for the next-hop entries and I am using the full addresses, there should be no collision right?

Just one OT question popping-up: How ARE prefixes handled in the FIB? Up until now I assumed this is the dst_len of the add function, but this is in byte, right? Prefixes are usually given in bits (and with IPv4 the thing gets even more complicated with subnet-masks....)

@BytesGalore
Copy link
Member

Since the routing protocols probably use prefixes for the next-hop entries and I am using the full addresses, there should be no collision right?

If this is the case right :)
But I'm not certain if the routing protocols only use prefixes for next-hops.

How ARE prefixes handled in the FIB?

Currently with bytes (I know this is wrong 😊) but,
when #2818 is merged the FIB uses Bits for a prefix. Additionally with #3157 a prefix entry in the FIB is marked with a specific flag.

@BytesGalore
Copy link
Member

oh, and when using IPv4, the subnet mask shouldn't be needed when an entry is flagged as prefix, since the trailing 0-Bits define the size of the subnet.

@miri64
Copy link
Member Author

miri64 commented Jun 3, 2015

oh, and when using IPv4, the subnet mask shouldn't be needed when an entry is flagged as prefix, since the trailing 0-Bits define the size of the subnet.

But weren't subnet masks introduced because there weren't enough prefixes left, making subnets defined by stuff like e.g. 63.255.0.0 possible, too?

@miri64
Copy link
Member Author

miri64 commented Jun 3, 2015

(I know this discussion is going a little bit meta now :D will fix the timeouts soon).

@miri64
Copy link
Member Author

miri64 commented Jun 3, 2015

oh and regarding overwriting entries of RPs: this should be more or less impossible, since in l351 it is checked if there is already something in the FIB. I say "more or less" since I think the IPv6 thread can't be preempted by the routing protocol in this function, but I might be wrong there.

@miri64
Copy link
Member Author

miri64 commented Jun 3, 2015

To recap: I assume that it is safe to just remove the destination if the neighbor becomes unreachable, since it is unlikely that the routing protocol will add it exactly as in this function.

@BytesGalore
Copy link
Member

But weren't subnet masks introduced because there weren't enough prefixes left,

somehow. If I remember right they were extended form Class-Network prefixes only to enable an arbitrary number of significant bits for private networks.

making subnets defined by stuff like e.g. 63.255.0.0 possible, too?

I can't tell for sure, but feels like not (or a proprietary and error-prone solution).

@miri64 miri64 force-pushed the ng_ndp/enh/cache-next-hop branch from ccfeb02 to fa5a408 Compare July 1, 2015 20:33
@miri64 miri64 force-pushed the ng_ndp/enh/cache-next-hop branch from fa5a408 to 0465b83 Compare July 1, 2015 20:34
@miri64
Copy link
Member Author

miri64 commented Jul 1, 2015

Rebased and had to squash, since ng_ndp.c was too different now.

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

ACK, lets see what travis says

OlegHahm added a commit that referenced this pull request Jul 2, 2015
@OlegHahm OlegHahm merged commit 0bf52dd into RIOT-OS:master Jul 2, 2015
@miri64 miri64 deleted the ng_ndp/enh/cache-next-hop branch July 2, 2015 09:39
@miri64 miri64 added the Area: network Area: Networking label Sep 30, 2018
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 Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants