[portsorch] Configure hostif tagging for subports by vsenchyshyn · Pull Request #1573 · sonic-net/sonic-swss

vsenchyshyn · 2020-12-30T17:09:29Z

Signed-off-by: Vitaliy Senchyshyn [email protected]

What I did
Configure SAI_HOSTIF_VLAN_TAG_KEEP for the parent port hostif when a first subport is added and restore it to SAI_HOSTIF_VLAN_TAG_STRIP when the last subport is removed.

Why I did it
As the parent port hostif tagging is not modified during subports creation the packets destined to the subport interfaces are forwarded as untagged by the packet driver and therefore never reaching the subport netdevs.

How I verified it

Created two subports using the config below and pinged their IPs with vlan 10 and 20 tagged packets respectively. The ping was successful.

{
    "VLAN_SUB_INTERFACE": {
        "Ethernet1.10": {
            "admin_status" : "up"
        },
        "Ethernet1.10|11.0.10.1/24": {},

        "Ethernet1.20": {
            "admin_status" : "up"
        },
        "Ethernet1.20|11.0.20.1/24": {}
    }
}

Details if related

Signed-off-by: Vitaliy Senchyshyn <[email protected]>

vsenchyshyn · 2021-01-04T12:19:13Z

retest vs please

vsenchyshyn · 2021-01-05T15:49:41Z

@lguohan could you please review?

vsenchyshyn · 2021-01-20T11:04:49Z

@prsunny @stcheng @lguohan Could you please review

vsenchyshyn · 2021-01-20T11:05:50Z

retest vs please

prsunny · 2021-01-21T05:26:03Z

orchagent/portsorch.cpp

+    {
+        if (!parentPort.m_bridge_port_id)
+        {
+            if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_STRIP))


Is it port or parentPort?

Good catch! Fixed.

prsunny · 2021-01-21T05:26:38Z

request @wendani to review

wendani · 2021-01-22T00:16:47Z

orchagent/portsorch.cpp

+        {
+            if (!setHostIntfsStripTag(parentPort, SAI_HOSTIF_VLAN_TAG_STRIP))
+            {
+                SWSS_LOG_WARN("Failed to set %s for hostif of port %s",


What is the thought for WARN instead of ERROR as above?

Copy & paste from the log right above in the function. Didn't notice that there is no return false.

wendani · 2021-01-22T00:22:10Z

orchagent/portsorch.cpp

+    // Restore hostif vlan tag for the parent port when the last subport is removed
+    if (parentPort.m_child_ports.empty())
+    {
+        if (!parentPort.m_bridge_port_id)


== SAI_NULL_OBJECT_ID

wendani · 2021-01-22T00:39:37Z

Can you also validate on the lag parent port case?

wendani · 2021-01-22T01:12:19Z

Looks relevant to sonic-net/sonic-buildimage#3943

vsenchyshyn · 2021-01-22T18:54:38Z

Can you also validate on the lag parent port case?

I'm not sure I can do this. I've applied the following config on a switch running t0 topo config:

{
    "VLAN_SUB_INTERFACE": {
        "PortChannel0001.10": {
            "admin_status" : "up"
        },
        "PortChannel0001.10|11.0.10.1/24": {},

        "PortChannel0001.20": {
            "admin_status" : "up"
        },
        "PortChannel0001.20|11.0.20.1/24": {}
    }
}

But this didn't have any effect - no subinterface PortChannel0001.10 created in linux. The config present in config db only.

wendani · 2021-01-23T01:27:55Z

Can you also validate on the lag parent port case?

I'm not sure I can do this. I've applied the following config on a switch running t0 topo config:
{
    "VLAN_SUB_INTERFACE": {
        "PortChannel0001.10": {
            "admin_status" : "up"
        },
        "PortChannel0001.10|11.0.10.1/24": {},

        "PortChannel0001.20": {
            "admin_status" : "up"
        },
        "PortChannel0001.20|11.0.20.1/24": {}
    }
}
But this didn't have any effect - no subinterface PortChannel0001.10 created in linux. The config present in config db only.

Keep port channel name within 15 characters. Rename to PortChannel1.10

OleksandrKozodoi · 2021-01-27T15:47:58Z

Can you also validate on the lag parent port case?

@wendani I've validated this case.
Steps for validation:

Deploy t0 topology.
Deploy minigrapgh on DUT
Remove member port from Vlan1000 on the DUT

config vlan member del 1000 Ethernet8

Create portchannel on the DUT

config portchannel add PortChannel5 --fallback=true

Add member port to portchannel on the DUT

config portchannel member add PortChannel5 Ethernet8

Setup bond interface on the PTF

ip link add bond2 type bond
ip link set bond2 type bond miimon 100 mode balance-xor
ip link set eth2 down
ip link set eth2 master bond2
ip link set dev bond2 up
ifconfig bond2 hw ether 22:22:22:22:22:21
ifconfig bond2 mtu 9216 up

Setup configuration of sub-ports on the DUT
sub_port_config.j2:

{
	"VLAN_SUB_INTERFACE": {

		"PortChannel5.10": {
			"admin_status" : "up"
		},
		"PortChannel5.10|172.16.0.5/30": {}
	}
}

sonic-cfggen -j sub_port_config.j2 --write-to-db

Setup configuration of sub-ports on the PTF

ip link add link bond2 name bond2.10 type vlan id 10
ip address add 172.16.0.6/30 dev bond2.10
ip link set bond2.10 up

Ping DUT from PTF

ping -I bond2.10 172.16.0.5

Validation results

/# ping -I bond2.10 172.16.0.5
PING 172.16.0.5 (172.16.0.5) from 172.16.0.6 bond2.10: 56(84) bytes of data.
64 bytes from 172.16.0.5: icmp_seq=1 ttl=64 time=0.405 ms
64 bytes from 172.16.0.5: icmp_seq=2 ttl=64 time=0.445 ms
64 bytes from 172.16.0.5: icmp_seq=3 ttl=64 time=0.771 ms
64 bytes from 172.16.0.5: icmp_seq=4 ttl=64 time=0.746 ms

~$ tcpdump -nei PortChannel5.10
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on PortChannel5.10, link-type EN10MB (Ethernet), capture size 262144 bytes
14:55:19.045230 22:22:22:22:22:21 > 74:83:ef:c9:b4:38, ethertype IPv4 (0x0800), length 98: 172.16.0.6 > 172.16.0.5: ICMP echo request, id 911, seq 1862, length 64
14:55:19.045305 74:83:ef:c9:b4:38 > 22:22:22:22:22:21, ethertype IPv4 (0x0800), length 98: 172.16.0.5 > 172.16.0.6: ICMP echo reply, id 911, seq 1862, length 64
14:55:20.069271 22:22:22:22:22:21 > 74:83:ef:c9:b4:38, ethertype IPv4 (0x0800), length 98: 172.16.0.6 > 172.16.0.5: ICMP echo request, id 911, seq 1863, length 64
14:55:20.069359 74:83:ef:c9:b4:38 > 22:22:22:22:22:21, ethertype IPv4 (0x0800), length 98: 172.16.0.5 > 172.16.0.6: ICMP echo reply, id 911, seq 1863, length 64
14:55:21.093466 22:22:22:22:22:21 > 74:83:ef:c9:b4:38, ethertype IPv4 (0x0800), length 98: 172.16.0.6 > 172.16.0.5: ICMP echo request, id 911, seq 1864, length 64
14:55:21.093536 74:83:ef:c9:b4:38 > 22:22:22:22:22:21, ethertype IPv4 (0x0800), length 98: 172.16.0.5 > 172.16.0.6: ICMP echo reply, id 911, seq 1864, length 64

Configure SAI_HOSTIF_VLAN_TAG_KEEP for the parent port hostif when a first subport is added and restore it to SAI_HOSTIF_VLAN_TAG_STRIP when the last subport is removed. Signed-off-by: Vitaliy Senchyshyn <[email protected]>

[portsorch] Configure hostif tagging for subports

67fb633

Signed-off-by: Vitaliy Senchyshyn <[email protected]>

OleksandrKozodoi mentioned this pull request Jan 11, 2021

[Sub-ports] Added new sub-ports test cases sonic-net/sonic-mgmt#2669

Merged

3 tasks

prsunny reviewed Jan 21, 2021

View reviewed changes

Fixed review comment

a58cfbd

wendani reviewed Jan 22, 2021

View reviewed changes

More review comments fix

cd5481e

wendani approved these changes Jan 27, 2021

View reviewed changes

prsunny approved these changes Jan 28, 2021

View reviewed changes

prsunny merged commit 91e231c into sonic-net:master Jan 28, 2021

vsenchyshyn deleted the subport-hostif-tagging-fix branch January 29, 2021 10:00

wendani mentioned this pull request Feb 11, 2021

[sub intf] Update vs tests to validate physical port host interface vlan tag attribute #1634

Merged

daall added Bug 🐛 Included in 202012 Branch Request for 202012 Branch labels Apr 22, 2021

stephenxs mentioned this pull request Apr 23, 2021

[submodule][202012] Advance submodule head for sonic-swss sonic-net/sonic-buildimage#7411

Merged

4 tasks

OleksandrKozodoi mentioned this pull request Jul 2, 2021

[sub intf] test_sub_port_interfaces failed due to vlan tag stripped sonic-net/sonic-buildimage#7941

Closed

Conversation

vsenchyshyn commented Dec 30, 2020

Uh oh!

vsenchyshyn commented Jan 4, 2021

Uh oh!

vsenchyshyn commented Jan 5, 2021

Uh oh!

vsenchyshyn commented Jan 20, 2021

Uh oh!

vsenchyshyn commented Jan 20, 2021

Uh oh!

prsunny Jan 21, 2021

Choose a reason for hiding this comment

Uh oh!

vsenchyshyn Jan 21, 2021

Choose a reason for hiding this comment

Uh oh!

prsunny commented Jan 21, 2021

Uh oh!

wendani Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

vsenchyshyn Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

wendani Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

vsenchyshyn Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

wendani commented Jan 22, 2021

Uh oh!

wendani commented Jan 22, 2021

Uh oh!

vsenchyshyn commented Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wendani commented Jan 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OleksandrKozodoi commented Jan 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vsenchyshyn commented Jan 22, 2021 •

edited

Loading

wendani commented Jan 23, 2021 •

edited

Loading