Bugzilla – Bug 659
Static routing with support for metrics and longest-prefix-match-first support
Last modified: 2009-09-01 02:39:34 EDT
You need to log in before you can comment on or make changes to this bug.
Created an attachment (id=547) [details] Patch to improve static routing This patch uses the GetLength patch from bug #634, http://www.nsnam.org/bugzilla/show_bug.cgi?id=634 I've started to work on better Ipv4StaticRouting support that would allow me to have (most importantly) multiple routes to same destination, and also defining the "destination" as prefix and mask. Attached patch now includes support for metrics (with default parameter =0), and when performing route lookup (LookupStatic), first matches for longest prefix and then based on metrics. If metrics are equal, latest addition is picked. "Default route" and "Host Routes" are no longer considered anything special: Default is simply 0.0.0.0/0 and Host Route is simply a /32. A small change to Ipv4-routing-table-entry now has AddHostRouteTo and IsHost using Ipv4Mask::GetOnes() instead of GetZero - as the route IS to a /32, that's how it should be anyway. API is backwards-compatible. The default metric is zero, and added as last parameter to any function calls. NotifyInterfaceDown (without any changes) should take care of deleting routes appropriately when interface goes down. This *should* take care of e.g. automatically switching to secondary default route. Up to user to reconfigure any routes when interface comes back up. Comments and testing appreciated. For example, I haven't tested multicast.
Created an attachment (id=548) [details] Patch to improve static routing Slightly better logging for lookups.
Created an attachment (id=549) [details] Reworked default route handling Reworked default route handling to completely do away old structures while maintaining backwards compatibility (old system broke down at IfDown).
Created an attachment (id=550) [details] One-liner fix, actually failed the longest-match test case before
(In reply to comment #3) > Created an attachment (id=550) [details] [details] > One-liner fix, actually failed the longest-match test case before > +1. If you already wrote a set of test cases for this new code, can we add them as a unit test to this model? (such as populating the routing table with a number of routes, and then querying for a route)
(In reply to comment #4) > +1. If you already wrote a set of test cases for this new code, can we add > them as a unit test to this model? (such as populating the routing table with > a number of routes, and then querying for a route) I don't have formal test cases limited only to testing this - I'm testing this right now in the scope of my own work. I guess I could cook up something based on examples/static-routing-slash32.cc though.
Created an attachment (id=562) [details] Fix for notifyInterfaceDown Actually fixing bug in *original* NotifyInterfaceDown. The problem is that indexing changes when you RemoveRoute(), thus, if a route is getting removed you should NOT increase the index. Reworked for into a while loop instead.
One place where backwards compatibility right now is broken - it used to be that if you have a default route set, doing a GetRoute(0) would always return the default first. Wondering if such a quirk should be included or not.
(In reply to comment #7) > One place where backwards compatibility right now is broken - it used to be > that if you have a default route set, doing a GetRoute(0) would always return > the default first. Wondering if such a quirk should be included or not. > I'm not in favor of retaining the quirk. It duplicates GetDefaultRoute () and it seems to prevent a user from obtaining the first route stored in m_hostRoutes.
I reviewed this again tonight and plan to push it in the next day or so (along with the patch in bug 614) if there are no other comments.
Created an attachment (id=580) [details] New version aligned with GetPrefixLength() Uses GetPrefixLength instead of GetMaskLength, as per changes in other bug. Also, fix for GetDefaultRoute, now it returns empty Ipv4RoutingTableEntry instead of pure zero when there isn't a default route (just like in the old version).
added as changeset 3dc675bb8b20