Skip to content

[intfsorch] Create subport with the entry contains necessary attributes #1650

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

Merged
prsunny merged 1 commit into from
Mar 5, 2021
+3 −3
Merged

[intfsorch] Create subport with the entry contains necessary attributes #1650

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions orchagent/intfsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,8 @@ void IntfsOrch::doTask(Consumer &consumer)
string vrf_name = "", vnet_name = "", nat_zone = "";
MacAddress mac;

uint32_t mtu;
bool adminUp;
uint32_t mtu = 0;
Copy link
Copy Markdown
Contributor

@wendani wendani Feb 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Port object is not supposed to be created by "INTF_TABLE:Ethernet4.20:192.169.1.1/24".

In current fix, sub interface admin_status still faces a similar issue that adminUp get some random init value, most likely non-zero, to cause interface SAI ADMIN_STATE to be "true", ignorant of user config in CONFIG_DB.

Try

"INTF_TABLE:Ethernet4.20": {
    "admin_status": "false"
}

You will still get ADMIN_V4_STATE and ADMIN_V6_STATE to be "true" in ASIC_DB output.

1) "SAI_ROUTER_INTERFACE_ATTR_VIRTUAL_ROUTER_ID"
 2) "oid:0x3000000000022"
 3) "SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS"
 4) "02:42:AC:11:00:04"
 5) "SAI_ROUTER_INTERFACE_ATTR_TYPE"
 6) "SAI_ROUTER_INTERFACE_TYPE_SUB_PORT"
 7) "SAI_ROUTER_INTERFACE_ATTR_PORT_ID"
 8) "oid:0x1000000000012"
 9) "SAI_ROUTER_INTERFACE_ATTR_OUTER_VLAN_ID"
10) "10"
11) "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V4_STATE"
12) "true"
13) "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V6_STATE"
14) "true"
15) "SAI_ROUTER_INTERFACE_ATTR_MTU"
16) "32761"
17) "SAI_ROUTER_INTERFACE_ATTR_NAT_ZONE_ID"
18) "0"

I believe on line 745, we need to amend the condition to create Port object with && !ip_prefix_in_key as root fix, though it does no harm to give mtu an explicit init value. To be consistent, we should do the same treatment to adminUp as mtu. I have reproduced this issue using vs docker, and have crafted a vs test case to validate the root fix.

AssertionError: Expected field/value pairs not found: expected={'SAI_ROUTER_INTERFACE_ATTR_TYPE': 'SAI_ROUTER_INTERFACE_TYPE_SUB_PORT', 'SAI_ROUTER_INTERFACE_ATTR_OUTER_VLAN_ID': '10', 'SAI_ROUTER_INTERFACE_ATTR_ADMIN_V4_STATE': 'false', 'SAI_ROUTER_INTERFACE_ATTR_ADMIN_V6_STATE': 'false', 'SAI_ROUTER_INTERFACE_ATTR_MTU': '9100', 'SAI_ROUTER_INTERFACE_ATTR_VIRTUAL_ROUTER_ID': 'oid:0x3000000000022'}, received={'SAI_ROUTER_INTERFACE_ATTR_VIRTUAL_ROUTER_ID': 'oid:0x3000000000022', 'SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS': '02:42:AC:11:00:02', 'SAI_ROUTER_INTERFACE_ATTR_TYPE': 'SAI_ROUTER_INTERFACE_TYPE_SUB_PORT', 'SAI_ROUTER_INTERFACE_ATTR_PORT_ID': 'oid:0x1000000000012', 'SAI_ROUTER_INTERFACE_ATTR_OUTER_VLAN_ID': '10', 'SAI_ROUTER_INTERFACE_ATTR_ADMIN_V4_STATE': 'true', 'SAI_ROUTER_INTERFACE_ATTR_ADMIN_V6_STATE': 'true', 'SAI_ROUTER_INTERFACE_ATTR_MTU': '32630', 'SAI_ROUTER_INTERFACE_ATTR_NAT_ZONE_ID': '0'}, key="oid:0x60000000005f6", table="ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE"

Copy link
Copy Markdown
Contributor Author

@peter-yu peter-yu Mar 5, 2021

Choose a reason for hiding this comment

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

Thanks. Done.

bool adminUp = false;
uint32_t nat_zone_id = 0;
string proxy_arp = "";
string inband_type = "";
Expand Down Expand Up @@ -742,7 +742,7 @@ void IntfsOrch::doTask(Consumer &consumer)
Port port;
if (!gPortsOrch->getPort(alias, port))
{
if (isSubIntf)
if (!ip_prefix_in_key && isSubIntf)
{
if (!gPortsOrch->addSubPort(port, alias, adminUp, mtu))
{
Expand Down