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

Add support for IP interface loopback action #2307

Merged
merged 10 commits into from
Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
12 changes: 12 additions & 0 deletions cfgmgr/intfmgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,7 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
string grat_arp = "";
string mpls = "";
string ipv6_link_local_mode = "";
string loopback_action = "";

for (auto idx : data)
{
Expand Down Expand Up @@ -750,6 +751,10 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
{
vlanId = value;
}
else if (field == "loopback_action")
{
loopback_action = value;
}
}

if (op == SET_COMMAND)
Expand Down Expand Up @@ -791,6 +796,13 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
data.push_back(fvTuple);
}

/* Set loopback action */
if (!loopback_action.empty())
{
FieldValueTuple fvTuple("loopback_action", loopback_action);
data.push_back(fvTuple);
}

/* Set mpls */
if (!setIntfMpls(alias, mpls))
{
Expand Down
91 changes: 87 additions & 4 deletions orchagent/intfsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ static const vector<sai_router_interface_stat_t> rifStatIds =
SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_OCTETS,
};

// Translation of loopback action from sonic to sai type
const unordered_map<string, sai_packet_action_t> IntfsOrch::m_sai_loopback_action_map =
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can have it a local map variable.. Also please follow the existing naming convention in the file. No need to have it as part of IntfsOrch class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

{
{"drop", SAI_PACKET_ACTION_DROP},
{"forward", SAI_PACKET_ACTION_FORWARD},
};

IntfsOrch::IntfsOrch(DBConnector *db, string tableName, VRFOrch *vrf_orch, DBConnector *chassisAppDb) :
Orch(db, tableName, intfsorch_pri), m_vrfOrch(vrf_orch)
{
Expand Down Expand Up @@ -416,6 +423,37 @@ bool IntfsOrch::setIntfProxyArp(const string &alias, const string &proxy_arp)
return true;
}

bool IntfsOrch::setIntfLoopbackAction(const Port &port, string actionStr)
{
sai_attribute_t attr;
sai_packet_action_t action;

if (!getSaiLoopbackAction(actionStr, action))
{
return false;
}

attr.id = SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION;
attr.value.s32 = action;

sai_status_t status = sai_router_intfs_api->set_router_interface_attribute(port.m_rif_id, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Loopback action [%s] set failed, interface [%s], rc [%d]",
actionStr.c_str(), port.m_alias.c_str(), status);

task_process_status handle_status = handleSaiSetStatus(SAI_API_ROUTER_INTERFACE, status);
if (handle_status != task_success)
{
return parseHandleSaiStatusFailure(handle_status);
}
}

SWSS_LOG_NOTICE("Loopback action [%s] set success, interface [%s]",
actionStr.c_str(), port.m_alias.c_str());
return true;
}

set<IpPrefix> IntfsOrch:: getSubnetRoutes()
{
SWSS_LOG_ENTER();
Expand All @@ -433,7 +471,9 @@ set<IpPrefix> IntfsOrch:: getSubnetRoutes()
return subnet_routes;
}

bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPrefix *ip_prefix, const bool adminUp, const uint32_t mtu)
bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPrefix *ip_prefix,
const bool adminUp, const uint32_t mtu, string loopbackAction)

{
SWSS_LOG_ENTER();

Expand All @@ -443,7 +483,7 @@ bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPre
auto it_intfs = m_syncdIntfses.find(alias);
if (it_intfs == m_syncdIntfses.end())
{
if (!ip_prefix && addRouterIntfs(vrf_id, port))
if (!ip_prefix && addRouterIntfs(vrf_id, port, loopbackAction))
{
gPortsOrch->increasePortRefCount(alias);
IntfsEntry intfs_entry;
Expand Down Expand Up @@ -665,6 +705,7 @@ void IntfsOrch::doTask(Consumer &consumer)
string inband_type = "";
bool mpls = false;
string vlan = "";
string loopbackAction = "";

for (auto idx : data)
{
Expand Down Expand Up @@ -757,6 +798,10 @@ void IntfsOrch::doTask(Consumer &consumer)
{
vlan = value;
}
else if (field == "loopback_action")
{
loopbackAction = value;
}
}

if (alias == "eth0" || alias == "docker0")
Expand Down Expand Up @@ -874,7 +919,8 @@ void IntfsOrch::doTask(Consumer &consumer)
{
adminUp = port.m_admin_state_up;
}
if (!setIntf(alias, vrf_id, ip_prefix_in_key ? &ip_prefix : nullptr, adminUp, mtu))

if (!setIntf(alias, vrf_id, ip_prefix_in_key ? &ip_prefix : nullptr, adminUp, mtu, loopbackAction))
{
it++;
continue;
Expand Down Expand Up @@ -906,6 +952,16 @@ void IntfsOrch::doTask(Consumer &consumer)
setRouterIntfsMpls(port);
gPortsOrch->setPort(alias, port);
}

/* Set loopback action */
if ((!loopbackAction.empty()) && (port.m_loopback_action != loopbackAction))
{
if (setIntfLoopbackAction(port, loopbackAction))
{
port.m_loopback_action = loopbackAction;
gPortsOrch->setPort(alias, port);
}
}
}
}

Expand Down Expand Up @@ -1047,7 +1103,22 @@ void IntfsOrch::doTask(Consumer &consumer)
}
}

bool IntfsOrch::addRouterIntfs(sai_object_id_t vrf_id, Port &port)
bool IntfsOrch::getSaiLoopbackAction(const string &actionStr, sai_packet_action_t &action)
{
auto it = m_sai_loopback_action_map.find(actionStr);
if (it != m_sai_loopback_action_map.end())
{
action = m_sai_loopback_action_map.at(actionStr);
return true;
}
else
{
SWSS_LOG_WARN("Unsupported loopback action [%s]", actionStr.c_str());
return false;
}
}

bool IntfsOrch::addRouterIntfs(sai_object_id_t vrf_id, Port &port, string loopbackActionStr)
{
SWSS_LOG_ENTER();

Expand All @@ -1067,6 +1138,17 @@ bool IntfsOrch::addRouterIntfs(sai_object_id_t vrf_id, Port &port)
attr.value.oid = vrf_id;
attrs.push_back(attr);

if (!loopbackActionStr.empty())
{
sai_packet_action_t loopbackAction;
if (getSaiLoopbackAction(loopbackActionStr, loopbackAction))
{
attr.id = SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION;
attr.value.s32 = loopbackAction;
attrs.push_back(attr);
}
}

attr.id = SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS;
if (port.m_mac)
{
Expand Down Expand Up @@ -1177,6 +1259,7 @@ bool IntfsOrch::addRouterIntfs(sai_object_id_t vrf_id, Port &port)
}

port.m_vr_id = vrf_id;
port.m_loopback_action = loopbackActionStr;

gPortsOrch->setPort(port.m_alias, port);
m_rifsToAdd.push_back(port);
Expand Down
7 changes: 5 additions & 2 deletions orchagent/intfsorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ class IntfsOrch : public Orch
void addRifToFlexCounter(const string&, const string&, const string&);
void removeRifFromFlexCounter(const string&, const string&);

bool setIntf(const string& alias, sai_object_id_t vrf_id = gVirtualRouterId, const IpPrefix *ip_prefix = nullptr, const bool adminUp = true, const uint32_t mtu = 0);
bool setIntfLoopbackAction(const Port &port, string actionStr);
bool getSaiLoopbackAction(const string &actionStr, sai_packet_action_t &action);
bool setIntf(const string& alias, sai_object_id_t vrf_id = gVirtualRouterId, const IpPrefix *ip_prefix = nullptr, const bool adminUp = true, const uint32_t mtu = 0, string loopbackAction = "");
bool removeIntf(const string& alias, sai_object_id_t vrf_id = gVirtualRouterId, const IpPrefix *ip_prefix = nullptr);

void addIp2MeRoute(sai_object_id_t vrf_id, const IpPrefix &ip_prefix);
Expand Down Expand Up @@ -88,10 +90,11 @@ class IntfsOrch : public Orch
unique_ptr<Table> m_vidToRidTable;
unique_ptr<ProducerTable> m_flexCounterTable;
unique_ptr<ProducerTable> m_flexCounterGroupTable;
static const unordered_map<string, sai_packet_action_t> m_sai_loopback_action_map;

std::string getRifFlexCounterTableKey(std::string s);

bool addRouterIntfs(sai_object_id_t vrf_id, Port &port);
bool addRouterIntfs(sai_object_id_t vrf_id, Port &port, string loopbackAction);
bool removeRouterIntfs(Port &port);

void addDirectedBroadcast(const Port &port, const IpPrefix &ip_prefix);
Expand Down
1 change: 1 addition & 0 deletions orchagent/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ class Port
sai_port_interface_type_t m_interface_type;
std::vector<uint32_t> m_adv_interface_types;
bool m_mpls = false;
std::string m_loopback_action;

/*
* Following two bit vectors are used to lock
Expand Down
96 changes: 96 additions & 0 deletions tests/test_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

from swsscommon import swsscommon

VLAN_SUB_INTERFACE_SEPARATOR = '.'

class TestRouterInterface(object):
def setup_db(self, dvs):
self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0)
Expand Down Expand Up @@ -2193,6 +2195,100 @@ def test_VLanInterfaceIpv6LinkLocalOnly(self, dvs, testlog):
# one loopback router interface
assert len(intf_entries) == 1

def set_loopback_action(self, interface, action):
if interface.startswith("PortChannel"):
tbl_name = "PORTCHANNEL_INTERFACE"
elif interface.startswith("Vlan"):
tbl_name = "VLAN_INTERFACE"
else:
sub_intf_sep_idx = interface.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx != -1:
tbl_name = "VLAN_SUB_INTERFACE"
else:
tbl_name = "INTERFACE"

fvs = swsscommon.FieldValuePairs([("loopback_action", action)])
tbl = swsscommon.Table(self.cdb, tbl_name)
tbl.set(interface, fvs)
time.sleep(1)

def loopback_action_test(self, iface, action):
# create interface
self.create_l3_intf(iface, "")

# set interface loopback action in config db
self.set_loopback_action(iface, action)

# check application database
tbl = swsscommon.Table(self.pdb, "INTF_TABLE")
(status, fvs) = tbl.get(iface)
assert status == True

action_found = False
for fv in fvs:
if fv[0] == "loopback_action":
action_found = True
assert fv[1] == action
assert action_found == True

# check asic db
tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE")
intf_entries = tbl.getKeys()

action_map = {"drop": "SAI_PACKET_ACTION_DROP", "forward": "SAI_PACKET_ACTION_FORWARD"}
action_found = False
for key in intf_entries:
(status, fvs) = tbl.get(key)
assert status == True

for fv in fvs:
if fv[0] == "SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION":
action_found = True
assert fv[1] == action_map[action]
assert action_found == True

# remove interface
self.remove_l3_intf(iface)

def test_interfaceLoopbackActionDrop(self, dvs, testlog):
self.setup_db(dvs)
self.loopback_action_test("Ethernet8", "drop")

def test_interfaceLoopbackActionForward(self, dvs, testlog):
self.setup_db(dvs)
self.loopback_action_test("Ethernet8", "forward")

def test_subInterfaceLoopbackActionDrop(self, dvs, testlog):
self.setup_db(dvs)
self.loopback_action_test("Ethernet8.1", "drop")

def test_subInterfaceLoopbackActionForward(self, dvs, testlog):
self.setup_db(dvs)
self.loopback_action_test("Ethernet8.1", "forward")

def test_vlanInterfaceLoopbackActionDrop(self, dvs, testlog):
self.setup_db(dvs)
self.create_vlan("10")
self.loopback_action_test("Vlan10", "drop")
self.remove_vlan("10")

def test_vlanInterfaceLoopbackActionForward(self, dvs, testlog):
self.setup_db(dvs)
self.create_vlan("20")
self.loopback_action_test("Vlan20", "forward")
self.remove_vlan("20")

def test_portChannelInterfaceLoopbackActionDrop(self, dvs, testlog):
self.setup_db(dvs)
self.create_port_channel("PortChannel009")
self.loopback_action_test("PortChannel009", "drop")
self.remove_port_channel("PortChannel009")

def test_portChannelInterfaceLoopbackActionForward(self, dvs, testlog):
self.setup_db(dvs)
self.create_port_channel("PortChannel010")
self.loopback_action_test("PortChannel010", "forward")
self.remove_port_channel("PortChannel010")

# Add Dummy always-pass test at end as workaroud
# for issue when Flaky fail on final test it invokes module tear-down before retrying
Expand Down