I am new to XORP and routing protocols so there may be some mis-understand on handle routes.
See my comments below. Jiangxin -----Original Message----- From: Igor Maravic [mailto:[email protected]] Sent: Friday, June 01, 2012 3:18 PM To: Ben Greear Cc: Jiangxin Hu; [email protected] Subject: Re: [Xorp-hackers] XORP enhancement for wireless mesh network routing > First, I believe it would be possible to want this feature enabled on > non-wireless interfaces, so maybe instead of having a 'wireless' > attribute, we could call it something like 'allow-disconnected-routes' or something like that. > Agreed++ @Jiangxin I have few comments on the patch: 1. Style and indetitation should be fixed - consult https://github.com/greearb/xorp.ct/blob/master/xorp/devnotes/coding-style.tx t This is a work related task so I did it in hurry. Will check the style. 2. --- *************** *** 843,848 **** --- 844,857 ---- debug_msg("**directly connected route found for nexthop\n"); break; } + int family = re->net().af(); + IPNextHop<A> *np = (IPNextHop<A> *)re->nexthop(); + if (IPvX::ZERO(family) == np->addr() || + (re->net()).contains(np->addr())) { + tryWireless = true; + //XLOG_INFO("**** find wireless route %s %s %s for %s %s", vif->str().c_str(),(re->net()).str().c_str(),(np->addr()).str().c_str(),net. str().c_str(), nexthop_addr.str().c_str()); + break; + } } // --- Instead of using IPvX::ZERO(family) You can use A::ZERO(). Rib is templatized after all. I am not very faimily with XORP, I just copy IPvX::ZERO(family) from other place and it works. That's the best I can do at that time. When You don't something You should delete it, not comment it out. Also I didn't quite understand what do You wand to do here: + if (IPvX::ZERO(family) == np->addr() || + (re->net()).contains(np->addr())) { + tryWireless = true; Why is this a special case? Didn't you say the problem is when the nexthop route isn't from the same subnet as vif? I think that it's quite ok that route have the nexthop isn't from it's subnet, if this isn't a direct nexthop. The problem is the nexthop is a direct nexthop but not from it's subnet. Here is the guess if such case happens, we need to check wheter it is a route for a wireless interface. The actual check is not here. 3. --- --- 865,916 ---- break; } while (false); ! if (vif == NULL && !tryWireless) { debug_msg("**not directly connected route found for nexthop\n"); // // XXX: If the route came from an IGP, then we must have // a directly-connected interface toward the next-hop router. // if (protocol->protocol_type() == IGP) { ! ! if (tryWireless || net.contains(nexthop_addr)) { ----> You wouldn't be here if tryWireless was true, why do you check it again? Right, I forget reomve it. ! // need find correct vif ! tryWireless = true; ! } ! ! else { ! XLOG_ERROR("Attempting to add IGP route to table \"%s\" " "(prefix %s next-hop %s): no directly connected " "interface toward the next-hop router", tablename.c_str(), net.str().c_str(), nexthop_addr.str().c_str()); ! return XORP_ERROR; ! } } } if (vif != NULL) { nexthop = find_or_create_peer_nexthop(nexthop_addr); } else { ! if (tryWireless) { ! map<string, RibVif*>::iterator iter; ! ! for (iter = _vifs.begin(); iter != _vifs.end(); ++iter) { ! vif = iter->second; ! if (! vif->is_underlying_vif_up()) ! continue; // XXX: ignore vifs that are not up ! if (_rib_manager.is_wireless_if(vif->ifname())) ! break;; --> double ; It is not double, it try to find a wireless vif for the route if addroute() does not specify a vifname. ! } ! nexthop = find_or_create_peer_nexthop(nexthop_addr); --> what if you can find the "wireless" route and you had to make external nexthop? It will lead to an error just like original XORP, the error will be "no direct connected interface ..." I guess. ! } ! else ! nexthop = find_or_create_external_nexthop(nexthop_addr); } --- BR Igor _______________________________________________ Xorp-hackers mailing list [email protected] http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-hackers
