[vlan] Add support of VLAN host interface by volodymyrsamotiy · Pull Request #1645 · sonic-net/sonic-swss

volodymyrsamotiy · 2021-02-17T16:42:10Z

Infrastructure needed for the VNET ping tool

Signed-off-by: Volodymyr Samotiy [email protected]

What I did

Add support of VLAN host interface

Why I did it

It is infrastructure needed for the VNET ping tool

How I verified it
Configured new VLAN with "hostif_name" attribute and verified that it created in HW

Details if related
N/A

* Infrastructure needed for the VNET ping tool Signed-off-by: Volodymyr Samotiy <[email protected]>

lguohan · 2021-02-18T07:06:15Z

missing pytest

prsunny · 2021-03-04T05:51:22Z

+                {
+                    if (!createVlanHostIntf(vl, hostif_name))
+                    {
+                        throw runtime_error("Cannot create VLAN host interface");


Lets handle the failure gracefully as this is for monitoring Vlan. We can erase it and continue.

prsunny · 2021-03-04T05:52:27Z

                    mac = fvValue(i);
                    setHostVlanMac(vlan_id, mac);
                }
+                else if (fvField(i) == "hostif_name")


Does this mean, for every Vlan thats in VNET, a corresponding host if shall be created?

Yes, since it is per VLAN attribute and created for specific VLAN.

* Add VS test Signed-off-by: Volodymyr Samotiy <[email protected]>

volodymyrsamotiy · 2021-03-04T16:14:14Z

In order for VS test to pass first need to merge the following PR for "vslib": sonic-net/sonic-sairedis#804

liat-grozovik · 2021-03-29T10:11:28Z

@prsunny can you please review?

liat-grozovik · 2021-04-04T07:26:34Z

In order for VS test to pass first need to merge the following PR for "vslib": Azure/sonic-sairedis#804

@volodymyrsamotiy it was merged. please check that submodule is updated and then retest.

* Handle gracefully faiure for creating monitoring VLAN hostif Signed-off-by: Volodymyr Samotiy <[email protected]>

prsunny · 2021-04-06T00:10:22Z

retest this please

prsunny · 2021-04-06T00:10:57Z

/azp run

azure-pipelines · 2021-04-06T00:11:10Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny · 2021-04-06T19:35:45Z

       return false;
    }

+    if (!removeVlanHostIntf(vlan))


This must be called only if host_intf_id is present, else skip

prsunny · 2021-04-06T19:36:39Z

+                    if (!createVlanHostIntf(vl, hostif_name))
+                    {
+                        // No need to fail in case of error as this is for monitoring VLAN.
+                        // Error message is printed by "createVlanHostIntf" so just handle faiure gracefully.


failure - typo

prsunny · 2021-04-07T16:02:17Z

@volodymyrsamotiy , can you please take a look at the test failures?

liat-grozovik · 2021-04-07T17:40:20Z

/azp run

azure-pipelines · 2021-04-07T17:40:35Z

Azure Pipelines successfully started running 1 pipeline(s).

* Fix review comments Signed-off-by: Volodymyr Samotiy <[email protected]>

volodymyrsamotiy · 2021-04-08T11:09:34Z

Fixed last review comments and test failure

abdosi · 2021-04-08T16:44:38Z

Need pr for 201911. cherry-pick has conflict.

@prsunny @volodymyrsamotiy

* [vlan] Add support of VLAN host interface * Infrastructure needed for the VNET ping tool Signed-off-by: Volodymyr Samotiy <[email protected]>

abdosi · 2021-04-08T22:51:24Z

Included without dvs test. Please create new pr for dvs test in 201911.

abdosi · 2021-04-10T00:28:18Z

@volodymyrsamotiy we should optimize the PR to use addHostIntfs() as this can support VLAN based Host router interface. There should not be need of new set of create/remove API's().

We can just add this check

    if (port.m_type == Port::VLAN)
    {
        attr.value.oid = port.m_vlan_info.vlan_oid;
    }
    else
    {
        attr.value.oid = port.m_port_id;
    }

@prsunny

- Why I did it To include below changes: Set monitoring VLAN hostif up dy default (for VNET ping tool) - How I did it Updated SAI submodule pointer - How to verify it Create VLAN hostif according to changes in PR: sonic-net/sonic-swss#1645 Verify it is admin up by default Signed-off-by: Volodymyr Samotiy <[email protected]>

* [vlan] Add support of VLAN host interface * Infrastructure needed for the VNET ping tool Signed-off-by: Volodymyr Samotiy <[email protected]>

[vlan] Add support of VLAN host interface

125e407

* Infrastructure needed for the VNET ping tool Signed-off-by: Volodymyr Samotiy <[email protected]>

lguohan requested a review from prsunny February 18, 2021 04:07

prsunny reviewed Mar 4, 2021

View reviewed changes

[vlan] Add support of VLAN host interface

3619498

* Add VS test Signed-off-by: Volodymyr Samotiy <[email protected]>

volodymyrsamotiy mentioned this pull request Mar 4, 2021

[vslib] Allow host interface object ID to be "VLAN" type sonic-net/sonic-sairedis#804

Merged

prsunny added Request for 202012 Branch request_for_201911_branch labels Apr 1, 2021

[vlan] Add support of VLAN host interface

c4cdd71

* Handle gracefully faiure for creating monitoring VLAN hostif Signed-off-by: Volodymyr Samotiy <[email protected]>

volodymyrsamotiy force-pushed the vnet_ping_tool branch from c780519 to c4cdd71 Compare April 5, 2021 14:24

prsunny reviewed Apr 6, 2021

View reviewed changes

[vlan] Add support of VLAN host interface

a2fa21e

* Fix review comments Signed-off-by: Volodymyr Samotiy <[email protected]>

prsunny approved these changes Apr 8, 2021

View reviewed changes

prsunny merged commit ca8ba6d into sonic-net:master Apr 8, 2021

yxieca added the Included in 202012 Branch label Apr 8, 2021

yxieca pushed a commit that referenced this pull request Apr 8, 2021

[vlan] Add support of VLAN host interface (#1645)

72ae85d

* [vlan] Add support of VLAN host interface * Infrastructure needed for the VNET ping tool Signed-off-by: Volodymyr Samotiy <[email protected]>

abdosi pushed a commit that referenced this pull request Apr 8, 2021

[vlan] Add support of VLAN host interface (#1645)

5ff3d8c

* [vlan] Add support of VLAN host interface * Infrastructure needed for the VNET ping tool Signed-off-by: Volodymyr Samotiy <[email protected]>

abdosi added the Included in 201911 Branch label Apr 8, 2021

Conversation

volodymyrsamotiy commented Feb 17, 2021

Uh oh!

lguohan commented Feb 18, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

volodymyrsamotiy commented Mar 4, 2021

Uh oh!

liat-grozovik commented Mar 29, 2021

Uh oh!

liat-grozovik commented Apr 4, 2021

Uh oh!

prsunny commented Apr 6, 2021

Uh oh!

prsunny commented Apr 6, 2021

Uh oh!

azure-pipelines bot commented Apr 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prsunny commented Apr 7, 2021

Uh oh!

liat-grozovik commented Apr 7, 2021

Uh oh!

azure-pipelines bot commented Apr 7, 2021

Uh oh!

volodymyrsamotiy commented Apr 8, 2021

Uh oh!

abdosi commented Apr 8, 2021

Uh oh!

abdosi commented Apr 8, 2021

Uh oh!

abdosi commented Apr 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

abdosi commented Apr 10, 2021 •

edited

Loading