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.

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

sonic-swss changes for MPLS #1686

Merged
merged 2 commits into from
Jun 28, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 42 additions & 9 deletions orchagent/label.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,36 @@ typedef uint32_t Label;
#define LABEL_VALUE_MIN 0
#define LABEL_VALUE_MAX 0xFFFFF

class LabelStack
struct LabelStack
{
public:
LabelStack() = default;
std::vector<Label> m_labelstack;
sai_outseg_type_t m_outseg_type;

LabelStack() :
m_outseg_type(SAI_OUTSEG_TYPE_SWAP) {}
// A list of Labels separated by '/'
LabelStack(const std::string &str)
{
auto labels = swss::tokenize(str, LABEL_DELIMITER);
// Expected MPLS format = "<outsegtype><labelstack>+<non-mpls-str>"
// <outsegtype> = "swap" | "push"
// <labelstack> = "<label0>/<label1>/../<labelN>"
// <non-mpls-str> = returned to caller and not parsed here
// Example = "push10100/10101+10.0.0.3@Ethernet4"
if (str.find("swap") == 0)
{
m_outseg_type = SAI_OUTSEG_TYPE_SWAP;
}
else if (str.find("push") == 0)
{
m_outseg_type = SAI_OUTSEG_TYPE_PUSH;
}
else
{
// Malformed string
std::string err = "Error converting " + str + " to MPLS NextHop";
throw std::invalid_argument(err);
}
auto labels = swss::tokenize(str.substr(4), LABEL_DELIMITER);
for (const auto &i : labels)
smaheshm marked this conversation as resolved.
Show resolved Hide resolved
m_labelstack.emplace_back(to_uint<uint32_t>(i, LABEL_VALUE_MIN, LABEL_VALUE_MAX));
}
Expand All @@ -44,12 +66,14 @@ class LabelStack

inline bool operator<(const LabelStack &o) const
{
return m_labelstack < o.m_labelstack;
return tie(m_labelstack, m_outseg_type) <
tie(o.m_labelstack, o.m_outseg_type);
}

inline bool operator==(const LabelStack &o) const
{
return m_labelstack == o.m_labelstack;
return (m_labelstack == o.m_labelstack) &&
(m_outseg_type == o.m_outseg_type);
}

inline bool operator!=(const LabelStack &o) const
Expand All @@ -60,6 +84,18 @@ class LabelStack
const std::string to_string() const
{
std::string str;
smaheshm marked this conversation as resolved.
Show resolved Hide resolved
if (m_labelstack.empty())
{
return str;
}
if (m_outseg_type == SAI_OUTSEG_TYPE_SWAP)
{
str += "swap";
}
else if (m_outseg_type == SAI_OUTSEG_TYPE_PUSH)
{
str += "push";
}
for (auto it = m_labelstack.begin(); it != m_labelstack.end(); ++it)
{
if (it != m_labelstack.begin())
Expand All @@ -70,9 +106,6 @@ class LabelStack
}
return str;
}

private:
std::vector<Label> m_labelstack;
};

}
Expand Down
2 changes: 1 addition & 1 deletion orchagent/neighorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ bool NeighOrch::addNextHop(const NextHopKey &nh)
next_hop_attrs.push_back(next_hop_attr);

next_hop_attr.id = SAI_NEXT_HOP_ATTR_OUTSEG_TYPE;
next_hop_attr.value.s32 = nexthop.outseg_type;
next_hop_attr.value.s32 = nexthop.label_stack.m_outseg_type;
next_hop_attrs.push_back(next_hop_attr);

next_hop_attr.id = SAI_NEXT_HOP_ATTR_LABELSTACK;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add SAI_NEXT_HOP_ATTR_OUTSEG_TYPE with push/swap type. Default operation in SAI is swap

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kperumalbfn: If I program the following two routes in FRR: "ip route 20.20.1.1/32 10.0.0.3 label 2021" and "mpls lsp 2020 10.0.0.3 2021", how many SAI NHs are required? It seems like this OUTSEG_TYPE necessitates the NH to have knowledge of the routes reference it and the NH attr content can change depending on the type of route.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @qbdwlr...in this case, there will be 2 MPLS nexthops in SAI. For IP route, MPLS outseg_nexthop with push operation and label 2021. For label 2020, MPLS outseg_nexthop with swap operation and same label 2021. Based on the lookup, select push/swap mpls nexthop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qbdwlr Can you also add an option to specify uniform/pipe mode for TTL and EXP in the DB which translates to MPLS nexthop's attribute SAI_NEXT_HOP_ATTR_OUTSEG_TTL_MODE? Default mode is uniform, but in some cases pipe mode is useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OUTSEG_TYPE has been added, but not OUTSEG_TTL_MODE since default of uniform is only used for current support.

Expand Down
78 changes: 20 additions & 58 deletions orchagent/nexthopkey.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,21 @@ struct NextHopKey
{
IpAddress ip_address; // neighbor IP address
string alias; // incoming interface alias
sai_next_hop_type_t nh_type; // NH type
uint32_t vni; // Encap VNI overlay nexthop
MacAddress mac_address; // Overlay Nexthop MAC.
LabelStack label_stack; // MPLS label stack
qbdwlr marked this conversation as resolved.
Show resolved Hide resolved
smaheshm marked this conversation as resolved.
Show resolved Hide resolved
sai_outseg_type_t outseg_type; // MPLS outseg type

NextHopKey() :
nh_type(SAI_NEXT_HOP_TYPE_IP),
vni(0), mac_address(),
outseg_type(SAI_OUTSEG_TYPE_SWAP) {}
NextHopKey() = default;
NextHopKey(const std::string &str, const std::string &alias) :
alias(alias), nh_type(SAI_NEXT_HOP_TYPE_IP),
vni(0), mac_address(),
outseg_type(SAI_OUTSEG_TYPE_SWAP)
alias(alias), vni(0), mac_address()
{
std::string ip_str = parseMplsNextHop(str);
ip_address = ip_str;
}
NextHopKey(const IpAddress &ip, const std::string &alias) :
ip_address(ip), alias(alias), nh_type(SAI_NEXT_HOP_TYPE_IP),
vni(0), mac_address(),
outseg_type(SAI_OUTSEG_TYPE_SWAP) {}
ip_address(ip), alias(alias), vni(0), mac_address() {}
NextHopKey(const std::string &str) :
nh_type(SAI_NEXT_HOP_TYPE_IP),
vni(0), mac_address(),
outseg_type(SAI_OUTSEG_TYPE_SWAP)
vni(0), mac_address()
{
if (str.find(NHG_DELIMITER) != string::npos)
{
Expand Down Expand Up @@ -69,9 +58,7 @@ struct NextHopKey
throw std::invalid_argument(err);
}
}
NextHopKey(const std::string &str, bool overlay_nh) :
nh_type(SAI_NEXT_HOP_TYPE_IP),
outseg_type(SAI_OUTSEG_TYPE_SWAP)
NextHopKey(const std::string &str, bool overlay_nh)
{
if (str.find(NHG_DELIMITER) != string::npos)
{
Expand Down Expand Up @@ -108,14 +95,14 @@ struct NextHopKey

bool operator<(const NextHopKey &o) const
{
return tie(ip_address, alias, nh_type, label_stack, outseg_type, vni, mac_address) <
tie(o.ip_address, o.alias, o.nh_type, o.label_stack, o.outseg_type, o.vni, o.mac_address);
return tie(ip_address, alias, label_stack, vni, mac_address) <
tie(o.ip_address, o.alias, o.label_stack, o.vni, o.mac_address);
}

bool operator==(const NextHopKey &o) const
{
return (ip_address == o.ip_address) && (alias == o.alias) && (nh_type == o.nh_type) &&
(label_stack == o.label_stack) && (outseg_type == o.outseg_type) &&
return (ip_address == o.ip_address) && (alias == o.alias) &&
(label_stack == o.label_stack) &&
(vni == o.vni) && (mac_address == o.mac_address);
}

Expand All @@ -131,45 +118,30 @@ struct NextHopKey

bool isMplsNextHop() const
{
return (nh_type == SAI_NEXT_HOP_TYPE_MPLS);
return (!label_stack.empty());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

std::string parseMplsNextHop(const std::string& str)
smaheshm marked this conversation as resolved.
Show resolved Hide resolved
{
// parseMplsNextHop initializes MPLS-related member data of the NextHopKey
// based on content of the input param str.
// outseg_type, nh_type, and label_stack may be updated.
// label_stack may be updated.
std::string ip_str;
auto keys = tokenize(str, LABELSTACK_DELIMITER);
smaheshm marked this conversation as resolved.
Show resolved Hide resolved
if (keys.size() == 1)
{
// No MPLS info to parse
ip_str = str;
}
else if (keys.size() == 3)
else if (keys.size() == 2)
{
// Expected MPLS format = "<outsegtype>+<labelstack>+<non-mpls-str>"
// Expected MPLS format = "<outsegtype><labelstack>+<non-mpls-str>"
// key[0] = <outsegtype> = "swap" | "push"
// key[1] = <labelstack> = "<label0>/<label1>/../<labelN>"
// key[2] = <non-mpls-str> = returned to caller and not parsed here
// Example = "push+10100/10101+10.0.0.3@Ethernet4"
nh_type = SAI_NEXT_HOP_TYPE_MPLS;
if (keys[0] == "swap")
{
outseg_type = SAI_OUTSEG_TYPE_SWAP;
}
else if (keys[0] == "push")
{
outseg_type = SAI_OUTSEG_TYPE_PUSH;
}
else
{
// Malformed string
std::string err = "Error converting " + str + " to MPLS NextHop";
throw std::invalid_argument(err);
}
label_stack = LabelStack(keys[1]);
ip_str = keys[2];
// key[0] = <labelstack> = "<label0>/<label1>/../<labelN>"
// key[1] = <non-mpls-str> = returned to caller and not parsed here
// Example = "push10100/10101+10.0.0.3@Ethernet4"
label_stack = LabelStack(keys[0]);
ip_str = keys[1];
}
else
{
Expand All @@ -183,19 +155,9 @@ struct NextHopKey
std::string formatMplsNextHop() const
{
std::string str;
if (nh_type == SAI_NEXT_HOP_TYPE_MPLS)
if (isMplsNextHop())
{
if (outseg_type == SAI_OUTSEG_TYPE_SWAP)
{
str += "swap";
str += LABELSTACK_DELIMITER;
}
else if (outseg_type == SAI_OUTSEG_TYPE_PUSH)
{
str += "push";
str += LABELSTACK_DELIMITER;
}
str += label_stack.to_string() + LABELSTACK_DELIMITER;
label_stack.to_string() + LABELSTACK_DELIMITER;
}
return str;
}
Expand Down
16 changes: 8 additions & 8 deletions tests/test_mpls.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ def test_RouteAddRemoveIpRoutePush(self, dvs, testlog):
# add route entry
prefix = "2.2.2.0/24"
if self.mpls_appdb_mode():
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "push+201"}
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "push201"}
self.create_route_entry(prefix, fieldValues)
else:
# dvs.runcmd("ip route add 2.2.2.0/24 encap mpls 201 via inet 10.0.0.1 dev Ethernet0")
Expand Down Expand Up @@ -305,7 +305,7 @@ def test_RouteAddRemoveMplsRouteSwap(self, dvs, testlog):
# add route entry
label = "200"
if self.mpls_appdb_mode():
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "swap+201", "mpls_pop": "1"}
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "swap201", "mpls_pop": "1"}
self.create_inseg_entry(label, fieldValues)
else:
# dvs.runcmd("ip -f mpls route add 200 as 201 via inet 10.0.0.1 dev Ethernet0")
Expand Down Expand Up @@ -376,7 +376,7 @@ def test_RouteAddRemoveMplsRouteExplicitNull(self, dvs, testlog):
# add route entry
label = "200"
if self.mpls_appdb_mode():
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "swap+0", "mpls_pop": "1"}
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "swap0", "mpls_pop": "1"}
self.create_inseg_entry(label, fieldValues)
else:
# dvs.runcmd("ip -f mpls route add 200 as 0 via inet 10.0.0.1 dev Ethernet0")
Expand Down Expand Up @@ -411,7 +411,7 @@ def test_RouteAddRemoveIpRoutePushNHG(self, dvs, testlog):
# add route entry
prefix = "2.2.2.0/24"
if self.mpls_appdb_mode():
fieldValues = {"nexthop": "10.0.0.1,10.0.0.5", "ifname": "Ethernet0,Ethernet8", "mpls_nh": "push+200,push+201"}
fieldValues = {"nexthop": "10.0.0.1,10.0.0.5", "ifname": "Ethernet0,Ethernet8", "mpls_nh": "push200,push201"}
self.create_route_entry(prefix, fieldValues)
else:
dvs.runcmd("ip route add 2.2.2.0/24 nexthop encap mpls 200 via inet 10.0.0.1 dev Ethernet0 nexthop encap mpls 201 via inet 10.0.0.5 dev Ethernet8")
Expand Down Expand Up @@ -449,7 +449,7 @@ def test_RouteAddRemoveMplsRouteSwapNHG(self, dvs, testlog):
# add route entry
label = "200"
if self.mpls_appdb_mode():
fieldValues = {"nexthop": "10.0.0.1,10.0.0.5", "ifname": "Ethernet0,Ethernet8", "mpls_nh": "swap+201,swap+202", "mpls_pop": "1"}
fieldValues = {"nexthop": "10.0.0.1,10.0.0.5", "ifname": "Ethernet0,Ethernet8", "mpls_nh": "swap201,swap202", "mpls_pop": "1"}
self.create_inseg_entry(label, fieldValues)
else:
dvs.runcmd("ip -f mpls route add 200 nexthop as 201 via inet 10.0.0.1 dev Ethernet0 nexthop as 202 via inet 10.0.0.5 dev Ethernet8")
Expand Down Expand Up @@ -526,7 +526,7 @@ def test_RouteAddRemoveIpRouteMixedNHG(self, dvs, testlog):
# add route entry
prefix = "2.2.2.0/24"
if self.mpls_appdb_mode():
fieldValues = {"nexthop": "10.0.0.1,10.0.0.5", "ifname": "Ethernet0,Ethernet8", "mpls_nh": "push+200,na"}
fieldValues = {"nexthop": "10.0.0.1,10.0.0.5", "ifname": "Ethernet0,Ethernet8", "mpls_nh": "push200,na"}
self.create_route_entry(prefix, fieldValues)
else:
dvs.runcmd("ip route add 2.2.2.0/24 nexthop encap mpls 200 via inet 10.0.0.1 dev Ethernet0 nexthop via inet 10.0.0.5 dev Ethernet8")
Expand Down Expand Up @@ -565,7 +565,7 @@ def test_RouteAddRemoveMplsRouteMixedNHG(self, dvs, testlog):
# add route entry
label = "200"
if self.mpls_appdb_mode():
fieldValues = {"nexthop": "10.0.0.1,10.0.0.5", "ifname": "Ethernet0,Ethernet8", "mpls_nh": "na,swap+201", "mpls_pop": "1"}
fieldValues = {"nexthop": "10.0.0.1,10.0.0.5", "ifname": "Ethernet0,Ethernet8", "mpls_nh": "na,swap201", "mpls_pop": "1"}
self.create_inseg_entry(label, fieldValues)
else:
dvs.runcmd("ip -f mpls route add 200 nexthop via inet 10.0.0.1 dev Ethernet0 nexthop as 201 via inet 10.0.0.5 dev Ethernet8")
Expand Down Expand Up @@ -604,7 +604,7 @@ def test_RouteUnresolvedMplsRouteSwap(self, dvs, testlog):
# add route entry
label = "200"
if self.mpls_appdb_mode():
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "swap+201", "mpls_pop": "1"}
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "swap201", "mpls_pop": "1"}
self.create_inseg_entry(label, fieldValues)
else:
# dvs.runcmd("ip -f mpls route add 200 as 201 via inet 10.0.0.1 dev Ethernet0")
Expand Down