> 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.txt 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. 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. 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? ! // 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 ; ! } ! nexthop = find_or_create_peer_nexthop(nexthop_addr); --> what if you can find the "wireless" route and you had to make external nexthop? ! } ! 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
