Skip to content

Commit

Permalink
address the review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@shuaishang
shuaishang committed Nov 21, 2024
1 parent 8808707 commit a84e9af
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 107 deletions.
55 changes: 10 additions & 45 deletions orchagent/nexthopgroupkey.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,41 +54,6 @@ class NextHopGroupKey
}
}

NextHopGroupKey(const std::string &nexthops, bool overlay_nh, bool srv6_nh, const std::string& weights)
{
auto nhv = tokenize(nexthops, NHG_DELIMITER);
auto wtv = tokenize(weights, NHG_DELIMITER);
bool set_weight = wtv.size() == nhv.size();
if (overlay_nh)
{
m_overlay_nexthops = true;
m_srv6_nexthops = false;
m_srv6_vpn = false;
for (uint32_t i = 0; i < nhv.size(); ++i)
{
auto nh = NextHopKey(nhv[i], overlay_nh, srv6_nh);
nh.weight = set_weight ? (uint32_t)std::stoi(wtv[i]) : 0;
m_nexthops.insert(nh);
}
}
else if (srv6_nh)
{
m_overlay_nexthops = false;
m_srv6_nexthops = true;
m_srv6_vpn = false;
for (uint32_t i = 0; i < nhv.size(); ++i)
{
auto nh = NextHopKey(nhv[i], overlay_nh, srv6_nh);
nh.weight = set_weight ? (uint32_t)std::stoi(wtv[i]) : 0;
m_nexthops.insert(nh);
if (nh.isSrv6Vpn())
{
m_srv6_vpn = true;
}
}
}
}

NextHopGroupKey(const std::string &nexthops, const std::string &weights)
{
m_overlay_nexthops = false;
Expand All @@ -105,16 +70,6 @@ class NextHopGroupKey
}
}

inline bool is_srv6_nexthop() const
{
return m_srv6_nexthops;
}

inline bool is_srv6_vpn() const
{
return m_srv6_vpn;
}

inline const std::set<NextHopKey> &getNextHops() const
{
return m_nexthops;
Expand Down Expand Up @@ -269,6 +224,16 @@ class NextHopGroupKey
return m_overlay_nexthops;
}

inline bool is_srv6_nexthop() const
{
return m_srv6_nexthops;
}

inline bool is_srv6_vpn() const
{
return m_srv6_vpn;
}

void clear()
{
m_nexthops.clear();
Expand Down
62 changes: 18 additions & 44 deletions orchagent/nexthopkey.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,32 +24,17 @@ struct NextHopKey
string srv6_source; // SRV6 source address
string srv6_vpn_sid; // SRV6 vpn sid

NextHopKey() :
weight(0),
srv6_vpn_sid(""),
srv6_source(""),
srv6_segment("")
{}
NextHopKey() : weight(0) {}
NextHopKey(const std::string &str, const std::string &alias) :
alias(alias), vni(0), mac_address(), weight(0),
srv6_vpn_sid(""),
srv6_source(""),
srv6_segment("")
alias(alias), vni(0), mac_address(), weight(0)
{
std::string ip_str = parseMplsNextHop(str);
ip_address = ip_str;
}
NextHopKey(const IpAddress &ip, const std::string &alias) :
ip_address(ip), alias(alias), vni(0), mac_address(), weight(0),
srv6_vpn_sid(""),
srv6_source(""),
srv6_segment("")
{}
ip_address(ip), alias(alias), vni(0), mac_address(), weight(0) {}
NextHopKey(const std::string &str) :
vni(0), mac_address(),
srv6_vpn_sid(""),
srv6_source(""),
srv6_segment("")
vni(0), mac_address()
{
if (str.find(NHG_DELIMITER) != string::npos)
{
Expand Down Expand Up @@ -98,9 +83,9 @@ struct NextHopKey
throw std::invalid_argument(err);
}
ip_address = keys[0];
srv6_vpn_sid = keys[1];
srv6_segment = keys[1];
srv6_source = keys[2];
srv6_segment = keys[3];
srv6_vpn_sid = keys[3];
}
else
{
Expand All @@ -116,18 +101,10 @@ struct NextHopKey
vni = static_cast<uint32_t>(std::stoul(keys[2]));
mac_address = keys[3];
weight = 0;
srv6_vpn_sid = "";
srv6_source = "";
srv6_segment = "";
}
}

NextHopKey(const IpAddress &ip, const MacAddress &mac, const uint32_t &vni, bool overlay_nh) :
ip_address(ip), alias(""), vni(vni), mac_address(mac), weight(0),
srv6_vpn_sid(""),
srv6_source(""),
srv6_segment("")
{}
NextHopKey(const IpAddress &ip, const MacAddress &mac, const uint32_t &vni, bool overlay_nh) : ip_address(ip), alias(""), vni(vni), mac_address(mac), weight(0){}

const std::string to_string() const
{
Expand All @@ -140,10 +117,7 @@ struct NextHopKey
{
if (srv6_nh)
{
return ip_address.to_string() + NH_DELIMITER +
srv6_vpn_sid + NH_DELIMITER +
srv6_source + NH_DELIMITER +
srv6_segment + NH_DELIMITER;
return ip_address.to_string() + NH_DELIMITER + srv6_segment + NH_DELIMITER + srv6_source + NH_DELIMITER + srv6_vpn_sid;
}
std::string str = formatMplsNextHop();
str += (ip_address.to_string() + NH_DELIMITER + alias + NH_DELIMITER +
Expand Down Expand Up @@ -181,6 +155,16 @@ struct NextHopKey
return (!label_stack.empty());
}

bool isSrv6NextHop() const
{
return ((srv6_segment != "") || (srv6_vpn_sid != "") || (srv6_source != ""));
}

bool isSrv6Vpn() const
{
return (srv6_vpn_sid != "");
}

std::string parseMplsNextHop(const std::string& str)
{
// parseMplsNextHop initializes MPLS-related member data of the NextHopKey
Expand Down Expand Up @@ -221,16 +205,6 @@ struct NextHopKey
}
return str;
}

bool isSrv6NextHop() const
{
return ((srv6_segment != "") || (srv6_vpn_sid != "") || (srv6_source != ""));
}

bool isSrv6Vpn() const
{
return (srv6_vpn_sid != "");
}
};

#endif /* SWSS_NEXTHOPKEY_H */
10 changes: 5 additions & 5 deletions orchagent/nhgorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ void NhgOrch::doTask(Consumer& consumer)
}

if (srv6_nh)
nhg_key = NextHopGroupKey(nhg_str, overlay_nh, srv6_nh, weights);
nhg_key = NextHopGroupKey(nhg_str, overlay_nh, srv6_nh);
else
nhg_key = NextHopGroupKey(nhg_str, weights);
}
Expand All @@ -216,12 +216,12 @@ void NhgOrch::doTask(Consumer& consumer)
for (uint32_t i = 0; i < ipv.size(); i++)
{
if (i) nhg_str += NHG_DELIMITER;
nhg_str += ipv[i] + NH_DELIMITER; // ip address
nhg_str += NH_DELIMITER; // srv6 vpn sid
nhg_str += ipv[i] + NH_DELIMITER; // ip address
nhg_str += NH_DELIMITER; // srv6 segment
nhg_str += srv6_srcv[i] + NH_DELIMITER; // srv6 source
nhg_str += NH_DELIMITER; // srv6 segment
nhg_str += NH_DELIMITER; // srv6 vpn sid
}
nhg_key = NextHopGroupKey(nhg_str, overlay_nh, srv6_nh, weights);
nhg_key = NextHopGroupKey(nhg_str, overlay_nh, srv6_nh);
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions orchagent/routeorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -849,9 +849,9 @@ void RouteOrch::doTask(Consumer& consumer)
{
if (i) nhg_str += NHG_DELIMITER;
nhg_str += ipv[i] + NH_DELIMITER; // ip address
nhg_str += (srv6_vpn ? srv6_vpn_sidv[i] : "") + NH_DELIMITER; // srv6 vpn sid
nhg_str += srv6_src[i] + NH_DELIMITER; // srv6 source
nhg_str += (srv6_seg ? srv6_segv[i] : "") + NH_DELIMITER; // srv6 segment
nhg_str += srv6_src[i] + NH_DELIMITER; // srv6 source
nhg_str += (srv6_vpn ? srv6_vpn_sidv[i] : "") + NH_DELIMITER; // srv6 vpn sid
}

nhg = NextHopGroupKey(nhg_str, overlay_nh, srv6_nh);
Expand Down
6 changes: 2 additions & 4 deletions orchagent/srv6orch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ const map<string, sai_my_sid_entry_endpoint_behavior_t> end_behavior_map =

const map<string, sai_my_sid_entry_endpoint_behavior_flavor_t> end_flavor_map =
{
{"end", SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USP},
{"end.x", SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USP},
{"end", SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USD},
{"end.x", SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USD},
{"end.t", SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USD},
{"un", SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USD},
{"ua", SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USD}
Expand Down Expand Up @@ -378,8 +378,6 @@ bool Srv6Orch::srv6Nexthops(const NextHopGroupKey &nhgKey, sai_object_id_t &next
{
SWSS_LOG_ENTER();
set<NextHopKey> nexthops = nhgKey.getNextHops();
string srv6_source;
string srv6_segment;

for (auto nh : nexthops)
{
Expand Down
10 changes: 3 additions & 7 deletions tests/test_srv6.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def test_mysid(self, dvs, testlog):
if fv[0] == "SAI_MY_SID_ENTRY_ATTR_ENDPOINT_BEHAVIOR":
assert fv[1] == "SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_X"
elif fv[0] == "SAI_MY_SID_ENTRY_ATTR_ENDPOINT_BEHAVIOR_FLAVOR":
assert fv[1] == "SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USP"
assert fv[1] == "SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USD"

# create MySID END.DX4
fvs = swsscommon.FieldValuePairs([('action', 'end.dx4'), ('adj', '192.0.2.1')])
Expand Down Expand Up @@ -435,7 +435,7 @@ def test_mysid_l3adj(self, dvs, testlog):
if fv[0] == "SAI_MY_SID_ENTRY_ATTR_ENDPOINT_BEHAVIOR":
assert fv[1] == "SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_X"
elif fv[0] == "SAI_MY_SID_ENTRY_ATTR_ENDPOINT_BEHAVIOR_FLAVOR":
assert fv[1] == "SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USP"
assert fv[1] == "SAI_MY_SID_ENTRY_ENDPOINT_BEHAVIOR_FLAVOR_PSP_AND_USD"

# remove neighbor
self.remove_neighbor("Ethernet104", "2001::1")
Expand Down Expand Up @@ -637,6 +637,7 @@ def test_srv6(self, dvs, testlog):
assert nexthop_entries == get_exist_entries(self.adb.db_connection, "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP")
assert route_entries == get_exist_entries(self.adb.db_connection, "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY")


class TestSrv6MySidFpmsyncd(object):
""" Functionality tests for Srv6 MySid handling in fpmsyncd """

Expand Down Expand Up @@ -1626,7 +1627,6 @@ def remove_pic_context(self, pic_ctx_id):
pictbl = swsscommon.ProducerStateTable(self.pdb.db_connection, "PIC_CONTEXT_TABLE")
pictbl._del(pic_ctx_id)


def check_deleted_route_entries(self, destinations):
def _access_function():
route_entries = self.adb.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY")
Expand Down Expand Up @@ -1892,12 +1892,10 @@ def test_srv6_vpn_with_nhg(self, dvs, testlog):
ifname_list.append('unknown')
ifname_list.append('unknown')


nhg_key = self.create_nhg(nhg_index, nexthop_list, segsrc_list, ifname_list)
pic_ctx_key = self.create_pic_context(pic_ctx_index, nexthop_list, vpn_list)
route_key = self.create_srv6_vpn_route_with_nhg('5000::/64', nhg_index, pic_ctx_index)


nhg_id = get_created_entry(self.adb.db_connection, "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nexthop_group_entries)
prefix_agg_id = "1"

Expand All @@ -1911,7 +1909,6 @@ def test_srv6_vpn_with_nhg(self, dvs, testlog):
if fv[0] == "SAI_ROUTE_ENTRY_ATTR_PREFIX_AGG_ID":
assert fv[1] == prefix_agg_id


route_key_new = self.create_srv6_vpn_route_with_nhg('5001::/64', nhg_index, pic_ctx_index)

# check ASIC SAI_OBJECT_TYPE_ROUTE_ENTRY database
Expand Down Expand Up @@ -2011,7 +2008,6 @@ def test_srv6_vpn_nh_update(self, dvs, testlog):
nhg_key = self.create_nhg(nhg_index, nexthop_list, segsrc_list, ifname_list)
pic_ctx_key = self.create_pic_context(pic_ctx_index, nexthop_list, vpn_list)
self.update_srv6_vpn_route_attribute_with_nhg('5000::/64', nhg_index, pic_ctx_index)


time.sleep(5)
nh_ids = get_created_entries(self.adb.db_connection, "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", nexthop_entries, 2)
Expand Down

0 comments on commit a84e9af

Please sign in to comment.