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.

Sign up for GitHub

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

Jump to bottom

VNET and BGP route coexistence #3345

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

VNET and BGP route coexistence #3345

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
06b1bc9
added changes to give vnet route precedence over BGP learnt route.
siqbal1986 Oct 29, 2024
1613524
Merge branch 'master' into bbr_vnet_route_coexistence
siqbal1986 Oct 29, 2024
33ebd25
Merge branch 'master' into bbr_vnet_route_coexistence
siqbal1986 Oct 31, 2024
f1dc4ef
Merge branch 'master' into bbr_vnet_route_coexistence
siqbal1986 Nov 1, 2024
793d739
added some extra logs for debugging.
siqbal1986 Nov 3, 2024
cac52ce
Merge branch 'master' into bbr_vnet_route_coexistence
siqbal1986 Nov 5, 2024
e9ea1bd
bug fixes. added one more test.
siqbal1986 Nov 6, 2024
d73e33a
fix for build
siqbal1986 Nov 6, 2024
7ab637b
fix
siqbal1986 Nov 6, 2024
a2c950f
another fix
siqbal1986 Nov 6, 2024
cba6fc9
compiler
siqbal1986 Nov 6, 2024
21937a7
testfix
siqbal1986 Nov 7, 2024
efce674
more logs for debugging
siqbal1986 Nov 7, 2024
14fdc66
fixed a bug found during testing
siqbal1986 Nov 10, 2024
b756428
improved the logs.
siqbal1986 Nov 16, 2024
6f81581
Merge branch 'master' into bbr_vnet_route_coexistence
siqbal1986 Nov 18, 2024
f671f71
fix a bug.
siqbal1986 Nov 22, 2024
dc7f03a
Merge branch 'master' into bbr_vnet_route_coexistence
siqbal1986 Nov 22, 2024
bbce5e0
cleanup
siqbal1986 Nov 28, 2024
a10fb7b
missed a typo
siqbal1986 Nov 28, 2024
926fbca
alignment fix
siqbal1986 Nov 28, 2024
7a1ef6f
removed extra log
siqbal1986 Nov 29, 2024
b886462
Merge branch 'master' into bbr_vnet_route_coexistence
siqbal1986 Nov 29, 2024
9b23cbc
renamed a function and updated a test
siqbal1986 Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions orchagent/routeorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2639,6 +2639,24 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx)
return true;
}

bool RouteOrch::removeRouteIfExists(const IpPrefix& prefix)
{
// This function removes the route if it exists.

string key = "ROUTE_TABLE:" + prefix.to_string();
RouteBulkContext context(key, false);
context.ip_prefix = prefix;
context.vrf_id = gVirtualRouterId;
if (removeRoute(context))
{
SWSS_LOG_INFO("Could not find the route with prefix %s", prefix.to_string().c_str());
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
gRouteBulker.flush();
return removeRoutePost(context);

}

bool RouteOrch::createRemoteVtep(sai_object_id_t vrf_id, const NextHopKey &nextHop)
{
SWSS_LOG_ENTER();
Expand Down
1 change: 1 addition & 0 deletions orchagent/routeorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ class RouteOrch : public Orch, public Subject
const NextHopGroupKey getSyncdRouteNhgKey(sai_object_id_t vrf_id, const IpPrefix& ipPrefix);
bool createFineGrainedNextHopGroup(sai_object_id_t &next_hop_group_id, vector<sai_attribute_t> &nhg_attrs);
bool removeFineGrainedNextHopGroup(sai_object_id_t &next_hop_group_id);
bool removeRouteIfExists(const IpPrefix& prefix);

void addLinkLocalRouteToMe(sai_object_id_t vrf_id, IpPrefix linklocal_prefix);
void delLinkLocalRouteToMe(sai_object_id_t vrf_id, IpPrefix linklocal_prefix);
Expand Down
102 changes: 91 additions & 11 deletions orchagent/vnetorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,8 @@ bool VNetRouteOrch::addNextHopGroup(const string& vnet, const NextHopGroupKey &n
{
continue;
}
SWSS_LOG_INFO("monitoring(%s), nexthop exists %d", it.ip_address.to_string().c_str(),
nexthop_info_[vnet].find(it.ip_address) != nexthop_info_[vnet].end() );
sai_object_id_t next_hop_id = vrf_obj->getTunnelNextHop(it);
next_hop_ids.push_back(next_hop_id);
nhopgroup_members_set[next_hop_id] = it;
Expand Down Expand Up @@ -926,6 +928,7 @@ bool VNetRouteOrch::createNextHopGroup(const string& vnet,
next_hop_group_entry.ref_count = 0;
if (monitoring == "custom" || nexthop_info_[vnet].find(nexthop.ip_address) == nexthop_info_[vnet].end() || nexthop_info_[vnet][nexthop.ip_address].bfd_state == SAI_BFD_SESSION_STATE_UP)
{
SWSS_LOG_INFO("Adding next hop to the active group %s", nexthop.ip_address.to_string().c_str());
next_hop_group_entry.active_members[nexthop] = SAI_NULL_OBJECT_ID;
}
syncd_nexthop_groups_[vnet][nexthops] = next_hop_group_entry;
Expand Down Expand Up @@ -989,6 +992,7 @@ bool VNetRouteOrch::selectNextHopGroup(const string& vnet,
// This is followed by an attempt to create a NHG which can be subset of nexthops_primary
// depending on the endpoint monitor state. If no NHG from primary is created, we attempt
// the same for secondary.
SWSS_LOG_NOTICE("Route recieved with monitoring %s and secondary NHG %s.\n",monitoring.c_str(), nexthops_secondary.to_string().c_str());
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
if(nexthops_secondary.getSize() != 0 && monitoring == "custom")
{
auto it_route = syncd_tunnel_routes_[vnet].find(ipPrefix);
Expand Down Expand Up @@ -1147,6 +1151,16 @@ bool VNetRouteOrch::doRouteTask<VNetVrfObject>(const string& vnet, IpPrefix& ipP
}
else
{
auto ipPrefixsubnet = ipPrefix.getSubnet();
SWSS_LOG_INFO("Attempting to remove BGP learnt route for prefix : %s\n",
ipPrefixsubnet.to_string().c_str());
if (gRouteOrch && !gRouteOrch->removeRouteIfExists(ipPrefixsubnet))
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
{
SWSS_LOG_ERROR("Couldn't Removed existing bgp route for prefix : %s\n", ipPrefixsubnet.to_string().c_str());
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
SWSS_LOG_INFO("Successfully Removed existing bgp route for prefix : %s \n",
ipPrefixsubnet.to_string().c_str());
if (it_route == syncd_tunnel_routes_[vnet].end())
{
route_status = add_route(vr_id, pfx, nh_id);
Expand Down Expand Up @@ -1295,6 +1309,8 @@ bool VNetRouteOrch::doRouteTask<VNetVrfObject>(const string& vnet, IpPrefix& ipP
SWSS_LOG_ERROR("Route del failed for %s, vr_id '0x%" PRIx64, ipPrefix.to_string().c_str(), vr_id);
return false;
}
SWSS_LOG_INFO("Successfully deleted the route for %s", ipPrefix.to_string().c_str());

}
}

Expand Down Expand Up @@ -1880,6 +1896,7 @@ void VNetRouteOrch::removeBfdSession(const string& vnet, const NextHopKey& endpo
{
SWSS_LOG_ERROR("BFD session for endpoint %s does not exist", endpoint_addr.to_string().c_str());
}
SWSS_LOG_INFO("removing nexthop info for endpoint %s", endpoint_addr.to_string().c_str());
nexthop_info_[vnet].erase(endpoint_addr);

string key = "default:default:" + monitor_addr.to_string();
Expand Down Expand Up @@ -2105,14 +2122,19 @@ void VNetRouteOrch::postRouteState(const string& vnet, IpPrefix& ipPrefix, NextH
auto prefix_to_use = ipPrefix;
if (prefix_to_adv_prefix_.find(ipPrefix) != prefix_to_adv_prefix_.end())
{
route_state = "";
auto adv_pfx = prefix_to_adv_prefix_[ipPrefix];
if (adv_prefix_refcount_[adv_pfx] == 1)
if (route_state == "active" and adv_prefix_refcount_[adv_pfx] == 1)
{
route_state = "active";
prefix_to_use = adv_pfx;
}
}
else
{
route_state = "";
}
}
SWSS_LOG_NOTICE("advertisement of prefix: %s with profile:%s, status: %s via %s\n",
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
ipPrefix.to_string().c_str(), profile.c_str(),
route_state.c_str(), prefix_to_use.to_string().c_str());
if (vnet_orch_->getAdvertisePrefix(vnet))
{
if (route_state == "active")
Expand All @@ -2138,6 +2160,7 @@ void VNetRouteOrch::removeRouteState(const string& vnet, IpPrefix& ipPrefix)
{
const string state_db_key = vnet + state_db_key_delimiter + ipPrefix.to_string();
state_vnet_rt_tunnel_table_->del(state_db_key);
SWSS_LOG_NOTICE("Stopping advertisement of prefix: %s\n", ipPrefix.to_string().c_str());

if(prefix_to_adv_prefix_.find(ipPrefix) !=prefix_to_adv_prefix_.end())
{
Expand Down Expand Up @@ -2270,9 +2293,15 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
{
continue;
}
// when we add the first nexthop to the route, we dont create a nexthop group, we call the updateTunnelRoute with NHG with one member.
// when adding the 2nd, 3rd ... members we create each NH using this create_next_hop_group_member call but give it the reference of next_hop_group_id.
// this way we dont have to update the route, the syncd does it by itself. we only call the updateTunnelRoute to add/remove when adding or removing the
// route fully.

bool failed = false;
if (state == SAI_BFD_SESSION_STATE_UP)
{
SWSS_LOG_INFO("Processing BFD state change to UP.\n");
sai_object_id_t next_hop_group_member_id = SAI_NULL_OBJECT_ID;
if (nexthops.getSize() > 1)
{
Expand Down Expand Up @@ -2322,10 +2351,38 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
{
for (auto ip_pfx : syncd_nexthop_groups_[vnet][nexthops].tunnel_routes)
{
// remove the bgp learnt route first if any exists and then add the tunnel route.
auto ipPrefixsubnet = ip_pfx.getSubnet();
auto prefixStr = ip_pfx.to_string().c_str();
auto nhStr = nexthops.to_string().c_str();
SWSS_LOG_INFO("Attempting to remove BGP learnt route if it exists for prefix: %s\n", ipPrefixsubnet.to_string().c_str());
if (gRouteOrch && !gRouteOrch->removeRouteIfExists(ipPrefixsubnet))
{
SWSS_LOG_NOTICE("Couldnt Removed bgp route for prefix : %s\n", ipPrefixsubnet.to_string().c_str());
failed = true;
break;
}
string op = SET_COMMAND;
updateTunnelRoute(vnet, ip_pfx, nexthops, op);

SWSS_LOG_NOTICE("Adding Vnet route for prefix : %s with nexthops %s\n", prefixStr, nhStr);

if (!updateTunnelRoute(vnet, ip_pfx, nexthops, op))
{
SWSS_LOG_NOTICE("Failed to create tunnel route in hardware for prefix : %s\n", ip_pfx.to_string().c_str());
failed = true;
}
else
{
SWSS_LOG_INFO("Successfully created tunnel route in hardware for prefix : %s\n", prefixStr);
}
}
}
if (failed)
{
// This is an unrecoverable error, Throw a LOG_ERROR and return
SWSS_LOG_ERROR("Inconsistant Hardware State. Failed to create tunnel routes\n");
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
return;
}
}
else
{
Expand All @@ -2334,6 +2391,7 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
}
else
{
SWSS_LOG_INFO("Processing BFD state change to Down.\n");
if (nexthops.getSize() > 1 && nhg_info.active_members.find(endpoint) != nhg_info.active_members.end())
{
sai_object_id_t nexthop_id = nhg_info.active_members[endpoint];
Expand All @@ -2351,6 +2409,7 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
}

vrf_obj->removeTunnelNextHop(endpoint);
SWSS_LOG_INFO("Successfully removed Nexthop: %s\n",endpoint.to_string().c_str() );

gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER);
}
Expand All @@ -2366,19 +2425,22 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
{
for (auto ip_pfx : syncd_nexthop_groups_[vnet][nexthops].tunnel_routes)
{
SWSS_LOG_NOTICE("Removing Vnet route for prefix : %s due to no nexthops.\n",ip_pfx.to_string().c_str());
string op = DEL_COMMAND;
updateTunnelRoute(vnet, ip_pfx, nexthops, op);
}
}
}
}
}

// Post configured in State DB
for (auto ip_pfx : syncd_nexthop_groups_[vnet][nexthops].tunnel_routes)
if (!failed)
{
string profile = vrf_obj->getProfile(ip_pfx);
postRouteState(vnet, ip_pfx, nexthops, profile);
// Post configured in State DB
for (auto ip_pfx : syncd_nexthop_groups_[vnet][nexthops].tunnel_routes)
{
string profile = vrf_obj->getProfile(ip_pfx);
postRouteState(vnet, ip_pfx, nexthops, profile);
}
}
}
}
Expand Down Expand Up @@ -2480,13 +2542,14 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
else
{
//both HHG's are inactive, need to remove the route.
SWSS_LOG_INFO(" Route needs to be removed due ot no active NHG.\n");
updateRoute = true;
}

if (nhg_custom.getSize() == 0)
{
// nhg_custom is empty. we shall create a dummy empty NHG for book keeping.
SWSS_LOG_INFO(" Neither Primary or Secondary endpoints are up.");
SWSS_LOG_INFO(" Neither Primary or Secondary endpoints are up.\n");
if (!hasNextHopGroup(vnet, nhg_custom))
{
NextHopGroupInfo next_hop_group_entry;
Expand All @@ -2504,6 +2567,7 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
{
if (active_nhg_size > 0)
{
SWSS_LOG_INFO(" Removing the route for prefix %s.",prefix.to_string().c_str());
// we need to remove the route
del_route(vr_id, pfx);
}
Expand All @@ -2516,11 +2580,22 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
if (active_nhg_size > 0)
{
// we need to replace the nhg in the route
SWSS_LOG_NOTICE("Replacing Nexthop Group for prefix: %s , nexthop group: %s\n",
prefix.to_string().c_str(), nhg_custom.to_string().c_str());
route_status = update_route(vr_id, pfx, nh_id);
}
else
{
// we need to readd the route.
SWSS_LOG_NOTICE("Adding Custom monitored Route with prefix: %s and Nexthop %s \n",
prefix.to_string().c_str(), nhg_custom.to_string().c_str());
auto ipPrefixsubnet = prefix.getSubnet();
SWSS_LOG_INFO("Attempting to remove BGP learnt route if it exists for prefix: %s\n", ipPrefixsubnet.to_string().c_str());
if (gRouteOrch && !gRouteOrch->removeRouteIfExists(ipPrefixsubnet))
{
SWSS_LOG_NOTICE("Couldnt Removed bgp route for prefix : %s\n", ipPrefixsubnet.to_string().c_str());
route_status = false;
}
route_status = add_route(vr_id, pfx, nh_id);
}
if (!route_status)
Expand Down Expand Up @@ -2549,6 +2624,7 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)

if(--syncd_nexthop_groups_[vnet][active_nhg].ref_count == 0)
{
SWSS_LOG_INFO("refcount for NHG is zero %s\n",active_nhg.to_string().c_str());
if (active_nhg_size > 1)
{
removeNextHopGroup(vnet, active_nhg, vrf_obj);
Expand All @@ -2565,8 +2641,10 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
}
else
{
SWSS_LOG_INFO("Prefix %s no longer references NHG %s \n",prefix.to_string().c_str(), active_nhg.to_string().c_str());
syncd_nexthop_groups_[vnet][active_nhg].tunnel_routes.erase(prefix);
}
SWSS_LOG_INFO("Prefix %s references NHG %s \n",prefix.to_string().c_str(), nhg_custom.to_string().c_str());
syncd_nexthop_groups_[vnet][nhg_custom].tunnel_routes.insert(prefix);
syncd_tunnel_routes_[vnet][prefix].nhg_key = nhg_custom;
if (nhg_custom != active_nhg)
Expand All @@ -2576,6 +2654,7 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
if (nhg_custom.getSize() == 0 && active_nhg_size > 0)
{
vrf_obj->removeRoute(prefix);
SWSS_LOG_NOTICE("Route prefix no longer active: %s \n", prefix.to_string().c_str());
removeRouteState(vnet, prefix);
if (prefix_to_adv_prefix_.find(prefix) != prefix_to_adv_prefix_.end())
{
Expand All @@ -2584,6 +2663,7 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
if (adv_prefix_refcount_[adv_pfx] == 0)
{
adv_prefix_refcount_.erase(adv_pfx);
SWSS_LOG_NOTICE("Stopping advertisement of prefix Range: %s \n",adv_pfx.to_string().c_str());
}
}
}
Expand Down
Loading
Loading