Skip to content

Commit bc4bf12

Browse files
zjswhhha114j0y
authored andcommitted
[ssw][ha] consume new ha_scope fields (sonic-net#3825)
* What I did Address the minor changes in HA HLD: reject ha_scope object createion if ha_set is not created first. get ha_set_id mapping from ha_scope table get vip from ha_scope table Dependency: sonic-net/sonic-dash-api#44
1 parent f205552 commit bc4bf12

2 files changed

Lines changed: 172 additions & 16 deletions

File tree

orchagent/dash/dashhaorch.cpp

Lines changed: 71 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -402,18 +402,22 @@ bool DashHaOrch::addHaScopeEntry(const std::string &key, const dash::ha_scope::H
402402
return success;
403403
}
404404

405-
if (m_ha_set_entries.empty())
405+
std::map<std::string, HaSetEntry>::iterator ha_set_it;
406+
if (!entry.ha_set_id().empty())
406407
{
407-
SWSS_LOG_ERROR("HA Set entry does not exist for %s", key.c_str());
408-
return false;
408+
ha_set_it = m_ha_set_entries.find(entry.ha_set_id());
409+
}
410+
else
411+
{
412+
/* ha_set_id field in ha_scope_table was added as a revision of detailed HLD, adding backward compatibility for ha_set_id mapping. */
413+
ha_set_it = m_ha_set_entries.find(key);
409414
}
410415

411-
auto ha_set_it = m_ha_set_entries.find(key);
412416
if (ha_set_it == m_ha_set_entries.end())
413417
{
414-
// If it's ENI level HA, ha_set id won't map to ha_scope id.
415-
// So we will use the first HA Set entry.
416-
ha_set_it = m_ha_set_entries.begin();
418+
// If there is no HA Set entry, we cannot create HA Scope.
419+
SWSS_LOG_ERROR("HA Set entry does not exist for %s", key.c_str());
420+
return false;
417421
}
418422
sai_object_id_t ha_set_oid = ha_set_it->second.ha_set_id;
419423

@@ -426,18 +430,34 @@ bool DashHaOrch::addHaScopeEntry(const std::string &key, const dash::ha_scope::H
426430
ha_set_attr.value.oid = ha_set_oid;
427431
ha_scope_attrs.push_back(ha_set_attr);
428432

429-
// TODO: add ha_role to attribute value enum
430433
sai_attribute_t ha_role_attr = {};
431434
ha_role_attr.id = SAI_HA_SCOPE_ATTR_DASH_HA_ROLE;
432435
ha_role_attr.value.u16 = to_sai(entry.ha_role());
433436
ha_scope_attrs.push_back(ha_role_attr);
434437

435-
if (ha_set_it->second.metadata.has_vip_v4() && ha_set_it->second.metadata.vip_v4().has_ipv4())
438+
if (entry.has_vip_v4() && entry.vip_v4().has_ipv4())
439+
{
440+
sai_ip_address_t sai_vip_v4 = {};
441+
if(to_sai(entry.vip_v4(), sai_vip_v4))
442+
{
443+
sai_attribute_t vip_v4_attr = {};
444+
vip_v4_attr.id = SAI_HA_SCOPE_ATTR_VIP_V4;
445+
vip_v4_attr.value.ipaddr = sai_vip_v4;
446+
ha_scope_attrs.push_back(vip_v4_attr);
447+
}
448+
else
449+
{
450+
SWSS_LOG_WARN("Failed to convert VIP V4 for HA Scope %s", key.c_str());
451+
}
452+
}
453+
else if (ha_set_it->second.metadata.has_vip_v4() && ha_set_it->second.metadata.vip_v4().has_ipv4())
436454
{
455+
SWSS_LOG_NOTICE("HA Scope entry %s does not have VIP V4, using HA Set metadata", key.c_str());
456+
437457
sai_ip_address_t sai_vip_v4 = {};
438458
if (to_sai(ha_set_it->second.metadata.vip_v4(), sai_vip_v4))
439459
{
440-
sai_attribute_t vip_v4_attr = {}; // Create new attribute for V4
460+
sai_attribute_t vip_v4_attr = {};
441461
vip_v4_attr.id = SAI_HA_SCOPE_ATTR_VIP_V4;
442462
vip_v4_attr.value.ipaddr = sai_vip_v4;
443463
ha_scope_attrs.push_back(vip_v4_attr);
@@ -448,12 +468,27 @@ bool DashHaOrch::addHaScopeEntry(const std::string &key, const dash::ha_scope::H
448468
}
449469
}
450470

451-
if (ha_set_it->second.metadata.has_vip_v6() && !ha_set_it->second.metadata.vip_v6().ipv6().empty())
471+
if (entry.has_vip_v6() && entry.vip_v6().has_ipv6())
472+
{
473+
sai_ip_address_t sai_vip_v6 = {};
474+
if(to_sai(entry.vip_v6(), sai_vip_v6))
475+
{
476+
sai_attribute_t vip_v6_attr = {};
477+
vip_v6_attr.id = SAI_HA_SCOPE_ATTR_VIP_V6;
478+
vip_v6_attr.value.ipaddr = sai_vip_v6;
479+
ha_scope_attrs.push_back(vip_v6_attr);
480+
}
481+
else
482+
{
483+
SWSS_LOG_WARN("Failed to convert VIP V6 for HA Scope %s", key.c_str());
484+
}
485+
}
486+
else if (ha_set_it->second.metadata.has_vip_v6() && !ha_set_it->second.metadata.vip_v6().ipv6().empty())
452487
{
453488
sai_ip_address_t sai_vip_v6 = {};
454489
if (to_sai(ha_set_it->second.metadata.vip_v6(), sai_vip_v6))
455490
{
456-
sai_attribute_t vip_v6_attr = {}; // Create new attribute for V6
491+
sai_attribute_t vip_v6_attr = {};
457492
vip_v6_attr.id = SAI_HA_SCOPE_ATTR_VIP_V6;
458493
vip_v6_attr.value.ipaddr = sai_vip_v6;
459494
ha_scope_attrs.push_back(vip_v6_attr);
@@ -962,7 +997,6 @@ bool DashHaOrch::convertKfvToHaSetPb(const std::vector<FieldValueTuple> &kfv, da
962997
else
963998
{
964999
SWSS_LOG_WARN("Unknown field %s in HA Set entry", field.c_str());
965-
return false;
9661000
}
9671001
}
9681002
return true;
@@ -1002,10 +1036,33 @@ bool DashHaOrch::convertKfvToHaScopePb(const std::vector<FieldValueTuple> &kfv,
10021036
{
10031037
entry.set_activate_role_requested(value == "true" || value == "1");
10041038
}
1039+
else if (field == "vip_v4")
1040+
{
1041+
dash::types::IpAddress temp_ip;
1042+
if (!to_pb(value, temp_ip) || !temp_ip.has_ipv4())
1043+
{
1044+
SWSS_LOG_ERROR("Invalid IPv4 address %s", value.c_str());
1045+
return false;
1046+
}
1047+
entry.mutable_vip_v4()->CopyFrom(temp_ip);
1048+
}
1049+
else if (field == "vip_v6")
1050+
{
1051+
dash::types::IpAddress temp_ip;
1052+
if (!to_pb(value, temp_ip) || !temp_ip.has_ipv6())
1053+
{
1054+
SWSS_LOG_ERROR("Invalid IPv6 address %s", value.c_str());
1055+
return false;
1056+
}
1057+
entry.mutable_vip_v6()->CopyFrom(temp_ip);
1058+
}
1059+
else if (field == "ha_set_id")
1060+
{
1061+
entry.set_ha_set_id(value);
1062+
}
10051063
else
10061064
{
10071065
SWSS_LOG_WARN("Unknown field %s in HA Scope entry", field.c_str());
1008-
return false;
10091066
}
10101067
}
10111068
return true;

tests/mock_tests/dashhaorch_ut.cpp

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,42 @@ namespace dashhaorch_ut
138138
static_cast<Orch *>(m_dashHaOrch)->doTask(*consumer.get());
139139
}
140140

141+
void InvalidField()
142+
{
143+
auto consumer = unique_ptr<Consumer>(new Consumer(
144+
new swss::ConsumerStateTable(m_dpu_app_db.get(), APP_DASH_HA_SET_TABLE_NAME, 1, 1),
145+
m_dashHaOrch, APP_DASH_HA_SET_TABLE_NAME));
146+
147+
consumer->addToSync(
148+
deque<KeyOpFieldsValuesTuple>(
149+
{
150+
{
151+
"HA_SET_1",
152+
SET_COMMAND,
153+
{
154+
{"version", "1"},
155+
{"vip_v4", "10.0.0.1"},
156+
{"vip_v6", "3:2::1:0"},
157+
{"owner", "dpu"},
158+
{"scope", "dpu"},
159+
{"local_npu_ip", "192.168.1.10"},
160+
{"local_ip", "192.168.2.1"},
161+
{"peer_ip", "192.168.2.2"},
162+
{"cp_data_channel_port", "4789"},
163+
{"dp_channel_dst_port", "4790"},
164+
{"dp_channel_src_port_min", "5000"},
165+
{"dp_channel_src_port_max", "6000"},
166+
{"dp_channel_probe_interval_ms", "1000"},
167+
{"dp_channel_probe_fail_threshold", "3"},
168+
{"invalid_field", "invalid_value"}
169+
}
170+
}
171+
}
172+
)
173+
);
174+
static_cast<Orch *>(m_dashHaOrch)->doTask(*consumer.get());
175+
}
176+
141177
void CreateEniScopeHaSet()
142178
{
143179
auto consumer = unique_ptr<Consumer>(new Consumer(
@@ -207,7 +243,33 @@ namespace dashhaorch_ut
207243
SET_COMMAND,
208244
{
209245
{"version", "1"},
210-
{"ha_role", "dead"}
246+
{"ha_role", "dead"},
247+
{"ha_set_id", "HA_SET_1"},
248+
{"vip_v4", "10.0.0.1"},
249+
{"vip_v6", "3:2::1:0"}
250+
}
251+
}
252+
}
253+
)
254+
);
255+
static_cast<Orch *>(m_dashHaOrch)->doTask(*consumer.get());
256+
}
257+
258+
void CreateHaScopeLessFields()
259+
{
260+
auto consumer = unique_ptr<Consumer>(new Consumer(
261+
new swss::ConsumerStateTable(m_dpu_app_db.get(), APP_DASH_HA_SCOPE_TABLE_NAME, 1, 1),
262+
m_dashHaOrch, APP_DASH_HA_SCOPE_TABLE_NAME));
263+
264+
consumer->addToSync(
265+
deque<KeyOpFieldsValuesTuple>(
266+
{
267+
{
268+
"HA_SET_1",
269+
SET_COMMAND,
270+
{
271+
{"version", "1"},
272+
{"ha_role", "dead"},
211273
}
212274
}
213275
}
@@ -523,10 +585,18 @@ namespace dashhaorch_ut
523585

524586
TEST_F(DashHaOrchTest, InvalidIpAddresses)
525587
{
588+
EXPECT_CALL(*mock_sai_dash_ha_api, create_ha_set)
589+
.Times(0);
590+
526591
InvalidIpAddresses();
592+
}
527593

594+
TEST_F(DashHaOrchTest, InvalidField)
595+
{
528596
EXPECT_CALL(*mock_sai_dash_ha_api, create_ha_set)
529-
.Times(0);
597+
.Times(1);
598+
599+
InvalidField();
530600
}
531601

532602
TEST_F(DashHaOrchTest, HaSetAlreadyExists)
@@ -575,6 +645,35 @@ namespace dashhaorch_ut
575645
EXPECT_TRUE(m_dashHaOrch->getHaScopeEntries().size() == 0);
576646
}
577647

648+
TEST_F(DashHaOrchTest, AddRemoveHaScopeLessFields)
649+
{
650+
CreateHaSet();
651+
652+
EXPECT_CALL(*mock_sai_dash_ha_api, create_ha_scope)
653+
.Times(1);
654+
655+
CreateHaScopeLessFields();
656+
657+
EXPECT_TRUE(m_dashHaOrch->getHaScopeEntries().size() == 1);
658+
EXPECT_TRUE(m_dashHaOrch->getHaScopeEntries().find("HA_SET_1") != m_dashHaOrch->getHaScopeEntries().end());
659+
660+
// HA Scope already exists
661+
EXPECT_CALL(*mock_sai_dash_ha_api, create_ha_scope)
662+
.Times(0);
663+
CreateHaScope();
664+
665+
EXPECT_TRUE(m_dashHaOrch->getHaScopeEntries().size() == 1);
666+
EXPECT_TRUE(m_dashHaOrch->getHaScopeEntries().find("HA_SET_1") != m_dashHaOrch->getHaScopeEntries().end());
667+
668+
EXPECT_CALL(*mock_sai_dash_ha_api, remove_ha_scope)
669+
.Times(1);
670+
671+
RemoveHaScope();
672+
673+
EXPECT_TRUE(m_dashHaOrch->getHaScopeEntries().size() == 0);
674+
}
675+
676+
578677
TEST_F(DashHaOrchTest, AddRemoveEniHaScope)
579678
{
580679
CreateEniScopeHaSet();

0 commit comments

Comments
 (0)