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.

Sign up for GitHub

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

Jump to bottom

First time config push for AN and FEC #1

Merged
merged 3 commits into from
Apr 6, 2021
Merged

First time config push for AN and FEC #1

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a640d2
First time config push
dgsudharsan Apr 5, 2021
c6e2acb
Addressing comments
dgsudharsan Apr 5, 2021
a171a53
Modifying Unit tests
dgsudharsan Apr 6, 2021
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
2 changes: 2 additions & 0 deletions orchagent/port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ class Port

std::unordered_set<sai_object_id_t> m_ingress_acl_tables_uset;
std::unordered_set<sai_object_id_t> m_egress_acl_tables_uset;
bool m_fec_cfg = false;
bool m_an_cfg = false;
};

}
Expand Down
7 changes: 5 additions & 2 deletions orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2354,12 +2354,13 @@ void PortsOrch::doPortTask(Consumer &consumer)
}
else
{
if (an != -1 && an != p.m_autoneg)
if (an != -1 && (!p.m_an_cfg || an != p.m_autoneg))
{
if (setPortAutoNeg(p.m_port_id, an))
{
SWSS_LOG_NOTICE("Set port %s AutoNeg to %u", alias.c_str(), an);
p.m_autoneg = an;
p.m_an_cfg = true;
m_portList[alias] = p;

// Once AN is changed
Expand Down Expand Up @@ -2492,7 +2493,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
if (fec_mode_map.find(fec_mode) != fec_mode_map.end())
{
/* reset fec mode upon mode change */
if (p.m_fec_mode != fec_mode_map[fec_mode])
if (!p.m_fec_cfg || p.m_fec_mode != fec_mode_map[fec_mode])
{
if (p.m_admin_state_up)
{
Expand All @@ -2506,6 +2507,7 @@ void PortsOrch::doPortTask(Consumer &consumer)

p.m_admin_state_up = false;
p.m_fec_mode = fec_mode_map[fec_mode];
p.m_fec_cfg = true;

if (setPortFec(p, p.m_fec_mode))
{
Expand All @@ -2523,6 +2525,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
{
/* Port is already down, setting fec mode*/
p.m_fec_mode = fec_mode_map[fec_mode];
p.m_fec_cfg = true;
if (setPortFec(p, p.m_fec_mode))
{
m_portList[alias] = p;
Expand Down
23 changes: 23 additions & 0 deletions tests/test_port.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,29 @@ def test_PortNotification(self, dvs, testlog):

assert oper_status == "up"

def test_PortFecForce(self, dvs, testlog):
db = swsscommon.DBConnector(0, dvs.redis_sock, 0)
adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)

tbl = swsscommon.Table(db, "PORT_TABLE")
ptbl = swsscommon.ProducerStateTable(db, "PORT_TABLE")
atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT")

# set fec
fvs = swsscommon.FieldValuePairs([("fec","none")])
ptbl.set("Ethernet0", fvs)

time.sleep(1)
Copy link

Choose a reason for hiding this comment

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

i think on another PR stepan mentioned that tests moved to wait instead of sleep. can you please check?

should we test it on different type of ports? breakout port and non breakout port?
should you test different fec options? only only none?
how this test cover the new push flow?


# get fec
(status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"])
assert status == True

assert "SAI_PORT_ATTR_FEC_MODE" in [fv[0] for fv in fvs]
for fv in fvs:
if fv[0] == "SAI_PORT_ATTR_FEC_MODE":
assert fv[1] == "SAI_PORT_FEC_MODE_NONE"

def test_PortFec(self, dvs, testlog):
dvs.runcmd("config interface startup Ethernet0")
dvs.runcmd("config interface ip add Ethernet0 10.0.0.0/31")
Expand Down
23 changes: 23 additions & 0 deletions tests/test_port_an.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,29 @@


class TestPortAutoNeg(object):
def test_PortAutoNegForce(self, dvs, testlog):

db = swsscommon.DBConnector(0, dvs.redis_sock, 0)

Copy link

Choose a reason for hiding this comment

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

should you prepare the adb now as you did on the prev test for fec? any reason here or there it is not the same flow?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It is initialized in the flow lines below.

Copy link

Choose a reason for hiding this comment

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

I know
I was asking if you should have the 2 DBs unit just after each other like you did in the prev function just to have the same flow

Copy link
Owner Author

Choose a reason for hiding this comment

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

Got you. Will move the initialization to one place.

tbl = swsscommon.ProducerStateTable(db, "PORT_TABLE")

fvs = swsscommon.FieldValuePairs([("autoneg","0"), ("speed", "1000")])

Copy link

Choose a reason for hiding this comment

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

remove extra line

tbl.set("Ethernet0", fvs)

time.sleep(1)
Copy link

Choose a reason for hiding this comment

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

same comment as above: wait instead of sleep


adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)

atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT")
(status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"])
assert status == True

assert "SAI_PORT_ATTR_AUTO_NEG_MODE" in [fv[0] for fv in fvs]
for fv in fvs:
if fv[0] == "SAI_PORT_ATTR_AUTO_NEG_MODE":
assert fv[1] == "false"

def test_PortAutoNegCold(self, dvs, testlog):

db = swsscommon.DBConnector(0, dvs.redis_sock, 0)
Expand Down