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

[buffermgrd] Move switch-statement outside of if-statement in BufferMgr::doTask #3055

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
35 changes: 17 additions & 18 deletions cfgmgr/buffermgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -549,24 +549,23 @@ void BufferMgr::doTask(Consumer &consumer)
task_status = doSpeedUpdateTask(port);
}
}

switch (task_status)
{
case task_process_status::task_failed:
SWSS_LOG_ERROR("Failed to process table update");
return;
case task_process_status::task_need_retry:
SWSS_LOG_INFO("Unable to process table update. Will retry...");
++it;
break;
case task_process_status::task_invalid_entry:
SWSS_LOG_ERROR("Failed to process invalid entry, drop it");
it = consumer.m_toSync.erase(it);
break;
default:
it = consumer.m_toSync.erase(it);
break;
}
}
switch (task_status)
{
case task_process_status::task_failed:
SWSS_LOG_ERROR("Failed to process table update");
return;
case task_process_status::task_need_retry:
SWSS_LOG_INFO("Unable to process table update. Will retry...");
++it;
break;
case task_process_status::task_invalid_entry:
SWSS_LOG_ERROR("Failed to process invalid entry, drop it");
it = consumer.m_toSync.erase(it);
break;
default:
it = consumer.m_toSync.erase(it);
break;
}
}
}
115 changes: 111 additions & 4 deletions tests/test_buffer_traditional.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,18 @@


class TestBuffer(object):
from conftest import DockerVirtualSwitch
lossless_pgs = []
INTF = "Ethernet0"

def setup_db(self, dvs):
self.app_db = dvs.get_app_db()
self.asic_db = dvs.get_asic_db()
self.config_db = dvs.get_config_db()
self.counter_db = dvs.get_counters_db()
from conftest import ApplDbValidator, AsicDbValidator
from dvslib.dvs_database import DVSDatabase

self.app_db: ApplDbValidator = dvs.get_app_db()
self.asic_db: AsicDbValidator = dvs.get_asic_db()
self.config_db: DVSDatabase = dvs.get_config_db()
self.counter_db: DVSDatabase = dvs.get_counters_db()

# enable PG watermark
self.set_pg_wm_status('enable')
Expand Down Expand Up @@ -74,6 +78,10 @@ def get_pg_name_map(self):
pg_name = "{}:{}".format(self.INTF, pg)
pg_name_map[pg_name] = self.get_pg_oid(pg_name)
return pg_name_map

def check_syslog(self, dvs, marker, err_log, expected_cnt=1):
(exitcode, num) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \"%s\" | wc -l" % (marker, err_log)])
assert num.strip() >= str(expected_cnt)

@pytest.fixture
def setup_teardown_test(self, dvs):
Expand Down Expand Up @@ -246,3 +254,102 @@ def test_buffer_pg_update(self, dvs, setup_teardown_test):
dvs.port_field_set(extra_port, "speed", orig_speed)
dvs.port_admin_set(self.INTF, "down")
dvs.port_admin_set(extra_port, "down")

def test_no_pg_profile_for_speed_and_length(self, dvs: DockerVirtualSwitch, setup_teardown_test):
"""
Test to verify that buffermgrd correctly handles a scenario where no PG profile
is configured for a given speed (10000) and cable length (80m) for Ethernet0 (self.INTF).
"""
orig_cable_len = None
orig_port_speed = None
orig_port_status = None
orig_port_qos_map = None

test_cable_len = "80m" # cable length must not exist for test_speed in
test_speed = "10000"
test_port_status ="down" # can be up or down, but it must exist in port configuration
test_port_pfc_enable = "3,4" # does not matter, but must exist

try:
##################################
## Save original configurations ##
##################################

# Save original cable length
fvs_cable_len = self.config_db.get_entry("CABLE_LENGTH", "AZURE")
orig_cable_len = fvs_cable_len.get(self.INTF) if fvs_cable_len else None

# Save original port speed and admin status
fvs_port = self.config_db.get_entry("PORT", self.INTF)
orig_port_speed = fvs_port.get("speed") if fvs_port else None
orig_port_status = fvs_port.get("admin_status") if fvs_port else None

# Save original port qos map
fvs_qos_map = self.config_db.get_entry("PORT_QOS_MAP", self.INTF)
orig_cable_len = fvs_qos_map.get("pfc_enable") if fvs_qos_map else None

######################################
## Send configurations to CONFIG_DB ##
######################################

# Configure cable length
self.change_cable_len(test_cable_len)

# Configure port speed
dvs.port_field_set(self.INTF, "speed", test_speed)

# Configure PFC enable
self.set_port_qos_table(self.INTF, test_port_pfc_enable)

# Add marker to log to make syslog verification easier
# Set before setting admin status to not miss syslog
marker = dvs.add_log_marker()

# Configure admin status
dvs.port_admin_set(self.INTF, test_port_status)

# Wait for buffermgrd to process the changes
time.sleep(2)

##################
## Verification ##
##################


# Check syslog if this error is present. This is expected.
self.check_syslog(dvs, marker, "Failed to process invalid entry, drop it")

finally:
###############################
## Revert to original values ##
###############################

# Revert values to original values
# If there are none, then assume entry/field never existed and should be deleted

# Revert cable length
if orig_cable_len:
self.change_cable_len(orig_cable_len)
else:
self.config_db.delete_entry("CABLE_LENGTH", "AZURE")

# Revert port speed
if orig_port_speed:
dvs.port_field_set(self.INTF, "speed", orig_port_speed)
else:
self.config_db.delete_field("PORT", self.INTF, "speed")

# Revert admin status
if orig_port_status:
dvs.port_admin_set(self.INTF, orig_port_status)
else:
self.config_db.delete_field("PORT", self.INTF, "admin_status")

# Revert port qos map
if orig_port_qos_map:
self.config_db.update_entry("PORT_QOS_MAP", self.INTF, orig_port_qos_map)
else:
self.config_db.delete_entry("PORT_QOS_MAP", self.INTF)



Loading