Skip to content

Commit c78aa1b

Browse files
abdosiabdosi
authored
OA changes to support Ordered ECMP and DVS test for same. (#2092)
What I did:- Orchagent changes to support Ordered ECMP as per design Order ECMP HLD. SONiC#896 While testing Order ECMP for Overlay ECMP use case found that for single nexthop based tunnel we create route without checking BFD state for endpoint. This results in not needed transaction of add_route followed by del_route. Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
1 parent b4b0003 commit c78aa1b

7 files changed

Lines changed: 423 additions & 228 deletions

File tree

‎orchagent/routeorch.cpp‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,13 @@ bool RouteOrch::validnexthopinNextHopGroup(const NextHopKey &nexthop, uint32_t&
376376
nhgm_attrs.push_back(nhgm_attr);
377377
}
378378

379+
if (m_switchOrch->checkOrderedEcmpEnable())
380+
{
381+
nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_SEQUENCE_ID;
382+
nhgm_attr.value.u32 = nhopgroup->second.nhopgroup_members[nexthop].seq_id;
383+
nhgm_attrs.push_back(nhgm_attr);
384+
}
385+
379386
status = sai_next_hop_group_api->create_next_hop_group_member(&nexthop_id, gSwitchId,
380387
(uint32_t)nhgm_attrs.size(),
381388
nhgm_attrs.data());
@@ -393,7 +400,7 @@ bool RouteOrch::validnexthopinNextHopGroup(const NextHopKey &nexthop, uint32_t&
393400

394401
++count;
395402
gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER);
396-
nhopgroup->second.nhopgroup_members[nexthop] = nexthop_id;
403+
nhopgroup->second.nhopgroup_members[nexthop].next_hop_id = nexthop_id;
397404
}
398405

399406
if (!m_fgNhgOrch->validNextHopInNextHopGroup(nexthop))
@@ -421,7 +428,7 @@ bool RouteOrch::invalidnexthopinNextHopGroup(const NextHopKey &nexthop, uint32_t
421428
continue;
422429
}
423430

424-
nexthop_id = nhopgroup->second.nhopgroup_members[nexthop];
431+
nexthop_id = nhopgroup->second.nhopgroup_members[nexthop].next_hop_id;
425432
status = sai_next_hop_group_api->remove_next_hop_group_member(nexthop_id);
426433

427434
if (status != SAI_STATUS_SUCCESS)
@@ -1241,7 +1248,7 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops)
12411248
vector<sai_attribute_t> nhg_attrs;
12421249

12431250
nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_TYPE;
1244-
nhg_attr.value.s32 = SAI_NEXT_HOP_GROUP_TYPE_ECMP;
1251+
nhg_attr.value.s32 = m_switchOrch->checkOrderedEcmpEnable() ? SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_ORDERED_ECMP : SAI_NEXT_HOP_GROUP_TYPE_ECMP;
12451252
nhg_attrs.push_back(nhg_attr);
12461253

12471254
sai_object_id_t next_hop_group_id;
@@ -1295,6 +1302,13 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops)
12951302
nhgm_attrs.push_back(nhgm_attr);
12961303
}
12971304

1305+
if (m_switchOrch->checkOrderedEcmpEnable())
1306+
{
1307+
nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_SEQUENCE_ID;
1308+
nhgm_attr.value.u32 = ((uint32_t)i) + 1; // To make non-zero sequence id
1309+
nhgm_attrs.push_back(nhgm_attr);
1310+
}
1311+
12981312
gNextHopGroupMemberBulker.create_entry(&nhgm_ids[i],
12991313
(uint32_t)nhgm_attrs.size(),
13001314
nhgm_attrs.data());
@@ -1319,7 +1333,8 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops)
13191333
if (nhopgroup_shared_set.find(nhid) != nhopgroup_shared_set.end())
13201334
{
13211335
auto it = nhopgroup_shared_set[nhid].begin();
1322-
next_hop_group_entry.nhopgroup_members[*it] = nhgm_id;
1336+
next_hop_group_entry.nhopgroup_members[*it].next_hop_id = nhgm_id;
1337+
next_hop_group_entry.nhopgroup_members[*it].seq_id = (uint32_t)i + 1;
13231338
nhopgroup_shared_set[nhid].erase(it);
13241339
if (nhopgroup_shared_set[nhid].empty())
13251340
{
@@ -1328,7 +1343,8 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops)
13281343
}
13291344
else
13301345
{
1331-
next_hop_group_entry.nhopgroup_members[nhopgroup_members_set.find(nhid)->second] = nhgm_id;
1346+
next_hop_group_entry.nhopgroup_members[nhopgroup_members_set.find(nhid)->second].next_hop_id = nhgm_id;
1347+
next_hop_group_entry.nhopgroup_members[nhopgroup_members_set.find(nhid)->second].seq_id = ((uint32_t)i) + 1;
13321348
}
13331349
}
13341350

@@ -1373,12 +1389,12 @@ bool RouteOrch::removeNextHopGroup(const NextHopGroupKey &nexthops)
13731389
if (m_neighOrch->isNextHopFlagSet(nhop->first, NHFLAGS_IFDOWN))
13741390
{
13751391
SWSS_LOG_WARN("NHFLAGS_IFDOWN set for next hop group member %s with next_hop_id %" PRIx64,
1376-
nhop->first.to_string().c_str(), nhop->second);
1392+
nhop->first.to_string().c_str(), nhop->second.next_hop_id);
13771393
nhop = nhgm.erase(nhop);
13781394
continue;
13791395
}
13801396

1381-
next_hop_ids.push_back(nhop->second);
1397+
next_hop_ids.push_back(nhop->second.next_hop_id);
13821398
nhop = nhgm.erase(nhop);
13831399
}
13841400

‎orchagent/routeorch.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,13 @@
2424

2525
#define LOOPBACK_PREFIX "Loopback"
2626

27-
typedef std::map<NextHopKey, sai_object_id_t> NextHopGroupMembers;
27+
struct NextHopGroupMemberEntry
28+
{
29+
sai_object_id_t next_hop_id; // next hop sai oid
30+
uint32_t seq_id; // Sequence Id of nexthop in the group
31+
};
32+
33+
typedef std::map<NextHopKey, NextHopGroupMemberEntry> NextHopGroupMembers;
2834

2935
struct NhgBase;
3036

‎orchagent/switchorch.cpp‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <map>
2+
#include <set>
23
#include <inttypes.h>
34

45
#include "switchorch.h"
@@ -44,6 +45,9 @@ const map<string, sai_packet_action_t> packet_action_map =
4445
{"trap", SAI_PACKET_ACTION_TRAP}
4546
};
4647

48+
49+
const std::set<std::string> switch_non_sai_attribute_set = {"ordered_ecmp"};
50+
4751
SwitchOrch::SwitchOrch(DBConnector *db, vector<TableConnector>& connectors, TableConnector switchTable):
4852
Orch(connectors),
4953
m_switchTable(switchTable.first, switchTable.second),
@@ -224,7 +228,51 @@ void SwitchOrch::doCfgSensorsTableTask(Consumer &consumer)
224228
}
225229
}
226230

231+
void SwitchOrch::setSwitchNonSaiAttributes(swss::FieldValueTuple &val)
232+
{
233+
auto attribute = fvField(val);
234+
auto value = fvValue(val);
227235

236+
if (attribute == "ordered_ecmp")
237+
{
238+
vector<FieldValueTuple> fvVector;
239+
if (value == "true")
240+
{
241+
const auto* meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, SAI_NEXT_HOP_GROUP_ATTR_TYPE);
242+
if (meta && meta->isenum)
243+
{
244+
vector<int32_t> values_list(meta->enummetadata->valuescount);
245+
sai_s32_list_t values;
246+
values.count = static_cast<uint32_t>(values_list.size());
247+
values.list = values_list.data();
248+
249+
auto status = sai_query_attribute_enum_values_capability(gSwitchId,
250+
SAI_OBJECT_TYPE_NEXT_HOP_GROUP,
251+
SAI_NEXT_HOP_GROUP_ATTR_TYPE,
252+
&values);
253+
if (status == SAI_STATUS_SUCCESS)
254+
{
255+
for (size_t i = 0; i < values.count; i++)
256+
{
257+
if (values.list[i] == SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_ORDERED_ECMP)
258+
{
259+
m_orderedEcmpEnable = true;
260+
fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_ORDERED_ECMP_CAPABLE, "true");
261+
set_switch_capability(fvVector);
262+
SWSS_LOG_NOTICE("Ordered ECMP/Nexthop-Group is configured");
263+
return;
264+
}
265+
}
266+
}
267+
}
268+
}
269+
m_orderedEcmpEnable = false;
270+
fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_ORDERED_ECMP_CAPABLE, "false");
271+
set_switch_capability(fvVector);
272+
SWSS_LOG_NOTICE("Ordered ECMP/Nexthop-Group is not configured");
273+
return;
274+
}
275+
}
228276
sai_status_t SwitchOrch::setSwitchTunnelVxlanParams(swss::FieldValueTuple &val)
229277
{
230278
auto attribute = fvField(val);
@@ -296,7 +344,12 @@ void SwitchOrch::doAppSwitchTableTask(Consumer &consumer)
296344
{
297345
auto attribute = fvField(i);
298346

299-
if (switch_attribute_map.find(attribute) == switch_attribute_map.end())
347+
if (switch_non_sai_attribute_set.find(attribute) != switch_non_sai_attribute_set.end())
348+
{
349+
setSwitchNonSaiAttributes(i);
350+
continue;
351+
}
352+
else if (switch_attribute_map.find(attribute) == switch_attribute_map.end())
300353
{
301354
// Check additionally 'switch_tunnel_attribute_map' for Switch Tunnel
302355
if (switch_tunnel_attribute_map.find(attribute) == switch_tunnel_attribute_map.end())

‎orchagent/switchorch.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#define SWITCH_CAPABILITY_TABLE_PORT_TPID_CAPABLE "PORT_TPID_CAPABLE"
1212
#define SWITCH_CAPABILITY_TABLE_LAG_TPID_CAPABLE "LAG_TPID_CAPABLE"
13+
#define SWITCH_CAPABILITY_TABLE_ORDERED_ECMP_CAPABLE "ORDERED_ECMP_CAPABLE"
1314

1415
struct WarmRestartCheck
1516
{
@@ -37,6 +38,8 @@ class SwitchOrch : public Orch
3738
// Initialize the ACL groups bind to Switch
3839
void initAclGroupsBindToSwitch();
3940

41+
bool checkOrderedEcmpEnable() { return m_orderedEcmpEnable; }
42+
4043
private:
4144
void doTask(Consumer &consumer);
4245
void doTask(swss::SelectableTimer &timer);
@@ -45,6 +48,8 @@ class SwitchOrch : public Orch
4548
void initSensorsTable();
4649
void querySwitchTpidCapability();
4750
sai_status_t setSwitchTunnelVxlanParams(swss::FieldValueTuple &val);
51+
void setSwitchNonSaiAttributes(swss::FieldValueTuple &val);
52+
4853

4954
// Create the default ACL group for the given stage, bind point is
5055
// SAI_ACL_BIND_POINT_TYPE_SWITCH and group type is
@@ -74,6 +79,7 @@ class SwitchOrch : public Orch
7479
bool m_sensorsMaxTempSupported = true;
7580
bool m_sensorsAvgTempSupported = true;
7681
bool m_vxlanSportUserModeEnabled = false;
82+
bool m_orderedEcmpEnable = false;
7783

7884
// Information contained in the request from
7985
// external program for orchagent pre-shutdown state check

‎orchagent/vnetorch.cpp‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ extern CrmOrch *gCrmOrch;
4040
extern RouteOrch *gRouteOrch;
4141
extern MacAddress gVxlanMacAddress;
4242
extern BfdOrch *gBfdOrch;
43-
43+
extern SwitchOrch *gSwitchOrch;
4444
/*
4545
* VRF Modeling and VNetVrf class definitions
4646
*/
@@ -680,9 +680,12 @@ bool VNetRouteOrch::addNextHopGroup(const string& vnet, const NextHopGroupKey &n
680680
vector<sai_object_id_t> next_hop_ids;
681681
set<NextHopKey> next_hop_set = nexthops.getNextHops();
682682
std::map<sai_object_id_t, NextHopKey> nhopgroup_members_set;
683+
std::map<NextHopKey, uint32_t> nh_seq_id_in_nhgrp;
684+
uint32_t seq_id = 0;
683685

684686
for (auto it : next_hop_set)
685687
{
688+
nh_seq_id_in_nhgrp[it] = ++seq_id;
686689
if (nexthop_info_[vnet].find(it.ip_address) != nexthop_info_[vnet].end() && nexthop_info_[vnet][it.ip_address].bfd_state != SAI_BFD_SESSION_STATE_UP)
687690
{
688691
continue;
@@ -696,7 +699,7 @@ bool VNetRouteOrch::addNextHopGroup(const string& vnet, const NextHopGroupKey &n
696699
vector<sai_attribute_t> nhg_attrs;
697700

698701
nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_TYPE;
699-
nhg_attr.value.s32 = SAI_NEXT_HOP_GROUP_TYPE_ECMP;
702+
nhg_attr.value.s32 = gSwitchOrch->checkOrderedEcmpEnable() ? SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_ORDERED_ECMP : SAI_NEXT_HOP_GROUP_TYPE_ECMP;
700703
nhg_attrs.push_back(nhg_attr);
701704

702705
sai_object_id_t next_hop_group_id;
@@ -733,6 +736,13 @@ bool VNetRouteOrch::addNextHopGroup(const string& vnet, const NextHopGroupKey &n
733736
nhgm_attr.value.oid = nhid;
734737
nhgm_attrs.push_back(nhgm_attr);
735738

739+
if (gSwitchOrch->checkOrderedEcmpEnable())
740+
{
741+
nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_SEQUENCE_ID;
742+
nhgm_attr.value.u32 = nh_seq_id_in_nhgrp[nhopgroup_members_set.find(nhid)->second];
743+
nhgm_attrs.push_back(nhgm_attr);
744+
}
745+
736746
sai_object_id_t next_hop_group_member_id;
737747
status = sai_next_hop_group_api->create_next_hop_group_member(&next_hop_group_member_id,
738748
gSwitchId,
@@ -865,7 +875,10 @@ bool VNetRouteOrch::doRouteTask<VNetVrfObject>(const string& vnet, IpPrefix& ipP
865875
NextHopGroupInfo next_hop_group_entry;
866876
next_hop_group_entry.next_hop_group_id = vrf_obj->getTunnelNextHop(nexthop);
867877
next_hop_group_entry.ref_count = 0;
868-
next_hop_group_entry.active_members[nexthop] = SAI_NULL_OBJECT_ID;
878+
if (nexthop_info_[vnet].find(nexthop.ip_address) == nexthop_info_[vnet].end() || nexthop_info_[vnet][nexthop.ip_address].bfd_state == SAI_BFD_SESSION_STATE_UP)
879+
{
880+
next_hop_group_entry.active_members[nexthop] = SAI_NULL_OBJECT_ID;
881+
}
869882
syncd_nexthop_groups_[vnet][nexthops] = next_hop_group_entry;
870883
}
871884
else
@@ -1680,7 +1693,20 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
16801693
NextHopGroupKey nexthops = nhg_info_pair.first;
16811694
NextHopGroupInfo& nhg_info = nhg_info_pair.second;
16821695

1683-
if (!(nexthops.contains(endpoint)))
1696+
std::set<NextHopKey> next_hop_set = nexthops.getNextHops();
1697+
uint32_t seq_id = 0;
1698+
uint32_t nh_seq_id = 0;
1699+
for (auto nh: next_hop_set)
1700+
{
1701+
seq_id++;
1702+
if (nh == endpoint)
1703+
{
1704+
nh_seq_id = seq_id;
1705+
break;
1706+
}
1707+
}
1708+
1709+
if (!nh_seq_id)
16841710
{
16851711
continue;
16861712
}
@@ -1702,6 +1728,13 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
17021728
nhgm_attr.value.oid = vrf_obj->getTunnelNextHop(endpoint);
17031729
nhgm_attrs.push_back(nhgm_attr);
17041730

1731+
if (gSwitchOrch->checkOrderedEcmpEnable())
1732+
{
1733+
nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_SEQUENCE_ID;
1734+
nhgm_attr.value.u32 = nh_seq_id;
1735+
nhgm_attrs.push_back(nhgm_attr);
1736+
}
1737+
17051738
sai_status_t status = sai_next_hop_group_api->create_next_hop_group_member(&next_hop_group_member_id,
17061739
gSwitchId,
17071740
(uint32_t)nhgm_attrs.size(),

0 commit comments

Comments
 (0)