Skip to content

Commit

Permalink
Revert "Add support for IP interface loopback action (#2307)"
Browse files Browse the repository at this point in the history 
This reverts commit 5043701.

Revert "[VS Test] Skip failing subport tests (#2370)"

This reverts commit 7175245.
  • Loading branch information
@prabhataravind
prabhataravind committed Jul 20, 2022
1 parent 24a0797 commit 40e2d4b
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 203 deletions.
12 changes: 0 additions & 12 deletions cfgmgr/intfmgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,6 @@ 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 @@ -771,10 +770,6 @@ 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 @@ -816,13 +811,6 @@ 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
85 changes: 4 additions & 81 deletions orchagent/intfsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,37 +416,6 @@ 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 @@ -464,9 +433,7 @@ 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, string loopbackAction)

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

Expand All @@ -476,7 +443,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, loopbackAction))
if (!ip_prefix && addRouterIntfs(vrf_id, port))
{
gPortsOrch->increasePortRefCount(alias);
IntfsEntry intfs_entry;
Expand Down Expand Up @@ -698,7 +665,6 @@ void IntfsOrch::doTask(Consumer &consumer)
string inband_type = "";
bool mpls = false;
string vlan = "";
string loopbackAction = "";

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

if (alias == "eth0" || alias == "docker0")
Expand Down Expand Up @@ -912,8 +874,7 @@ 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, loopbackAction))
if (!setIntf(alias, vrf_id, ip_prefix_in_key ? &ip_prefix : nullptr, adminUp, mtu))
{
it++;
continue;
Expand Down Expand Up @@ -945,12 +906,6 @@ void IntfsOrch::doTask(Consumer &consumer)
setRouterIntfsMpls(port);
gPortsOrch->setPort(alias, port);
}

/* Set loopback action */
if (!loopbackAction.empty())
{
setIntfLoopbackAction(port, loopbackAction);
}
}
}

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

bool IntfsOrch::getSaiLoopbackAction(const string &actionStr, sai_packet_action_t &action)
{
const unordered_map<string, sai_packet_action_t> loopbackActionMap =
{
{"drop", SAI_PACKET_ACTION_DROP},
{"forward", SAI_PACKET_ACTION_FORWARD},
};

auto it = loopbackActionMap.find(actionStr);
if (it != loopbackActionMap.end())
{
action = loopbackActionMap.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)
bool IntfsOrch::addRouterIntfs(sai_object_id_t vrf_id, Port &port)
{
SWSS_LOG_ENTER();

Expand All @@ -1133,17 +1067,6 @@ bool IntfsOrch::addRouterIntfs(sai_object_id_t vrf_id, Port &port, string loopba
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
6 changes: 2 additions & 4 deletions orchagent/intfsorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ class IntfsOrch : public Orch
void addRifToFlexCounter(const string&, const string&, const string&);
void removeRifFromFlexCounter(const string&, const string&);

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 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 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 @@ -97,7 +95,7 @@ class IntfsOrch : public Orch
std::string getRifRateInitTableKey(std::string s);
void cleanUpRifFromCounterDb(const string &id, const string &name);

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

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

/*
* Following bit vector is used to lock
* the queue from being changed in BufferOrch.
Expand Down
96 changes: 0 additions & 96 deletions tests/test_interface.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

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 @@ -2195,100 +2193,6 @@ 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
10 changes: 0 additions & 10 deletions tests/test_sub_port_intf.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import json
import time
import pytest

from dvslib.dvs_common import wait_for_result
from swsscommon import swsscommon
Expand Down Expand Up @@ -582,7 +581,6 @@ def _test_sub_port_intf_creation(self, dvs, sub_port_intf_name, vrf_name=None):
self.remove_lag(parent_port)
self.check_lag_removal(parent_port_oid)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_creation(self, dvs):
self.connect_dbs(dvs)

Expand Down Expand Up @@ -669,7 +667,6 @@ def _test_sub_port_intf_add_ip_addrs(self, dvs, sub_port_intf_name, vrf_name=Non
self.remove_lag(parent_port)
self.asic_db.wait_for_n_keys(ASIC_LAG_TABLE, 0)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_add_ip_addrs(self, dvs):
self.connect_dbs(dvs)

Expand Down Expand Up @@ -745,7 +742,6 @@ def _test_sub_port_intf_appl_db_proc_seq(self, dvs, sub_port_intf_name, admin_up
self.remove_lag(parent_port)
self.check_lag_removal(parent_port_oid)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_appl_db_proc_seq(self, dvs):
self.connect_dbs(dvs)

Expand Down Expand Up @@ -870,7 +866,6 @@ def _test_sub_port_intf_admin_status_change(self, dvs, sub_port_intf_name, vrf_n
self.remove_lag(parent_port)
self.asic_db.wait_for_n_keys(ASIC_LAG_TABLE, 0)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_admin_status_change(self, dvs):
self.connect_dbs(dvs)

Expand Down Expand Up @@ -955,7 +950,6 @@ def _test_sub_port_intf_remove_ip_addrs(self, dvs, sub_port_intf_name, vrf_name=
self.remove_lag(parent_port)
self.asic_db.wait_for_n_keys(ASIC_LAG_TABLE, 0)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_remove_ip_addrs(self, dvs):
self.connect_dbs(dvs)

Expand Down Expand Up @@ -1147,7 +1141,6 @@ def _test_sub_port_intf_removal(self, dvs, sub_port_intf_name, removal_seq_test=
self.remove_lag(parent_port)
self.check_lag_removal(parent_port_oid)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_removal(self, dvs):
self.connect_dbs(dvs)

Expand Down Expand Up @@ -1223,7 +1216,6 @@ def _test_sub_port_intf_mtu(self, dvs, sub_port_intf_name, vrf_name=None):
self.remove_lag(parent_port)
self.asic_db.wait_for_n_keys(ASIC_LAG_TABLE, 0)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_mtu(self, dvs):
self.connect_dbs(dvs)

Expand Down Expand Up @@ -1452,7 +1444,6 @@ def _test_sub_port_intf_nhg_accel(self, dvs, sub_port_intf_name, nhop_num=3, cre

parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_nhg_accel(self, dvs):
self.connect_dbs(dvs)

Expand Down Expand Up @@ -1593,7 +1584,6 @@ def _test_sub_port_intf_oper_down_with_pending_neigh_route_tasks(self, dvs, sub_

parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1)

@pytest.mark.skip(reason="Failing. Under investigation")
def test_sub_port_intf_oper_down_with_pending_neigh_route_tasks(self, dvs):
self.connect_dbs(dvs)

Expand Down

1 comment on commit 40e2d4b

@prabhataravind
Copy link
Contributor Author

Choose a reason for hiding this comment

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

/azp run

Please sign in to comment.