VNet/Vxlan delete handling #766

prsunny · 2019-01-24T01:55:15Z

What I did
Delete handling for VNet Routes, NH tunnels and VNets

Why I did it
To support deletion scenarios

How I verified it
Tested on DUT. VS test case would be added subsequently

Details if related
Based on design sonic-net/SONiC#324

orchagent/intfsorch.cpp

marian-pritsak

As comments

marian-pritsak · 2019-01-30T17:05:45Z

orchagent/vnetorch.cpp


 /*
 * VRF Modeling and VNetVrf class definitions
 */
 std::vector<VR_TYPE> vr_cntxt;

-VNetVrfObject::VNetVrfObject(const std::string& vnet, string& tunnel, set<string>& peer,
-                             vector<sai_attribute_t>& attrs) : VNetObject(tunnel, peer)
+VNetVrfObject::VNetVrfObject(const std::string& vnet, string& tunnel, set<string>& peer, uint32_t vni,


Maybe it is worth considering grouping all these parameters into a single struct VnetInfo?

lguohan · 2019-02-02T02:11:44Z

orchagent/vnetorch.cpp

+
+            if (!vxlan_orch->removeVxlanTunnelMap(vrf_obj->getTunnelName(), vrf_obj->getVni()))
+            {
+                SWSS_LOG_ERROR("VNET '%s' map delete failed", vnet_name.c_str());


SWSS_LOG_ERROR("VNET '%s' map delete failed", vnet_name.c_str()); [](start = 16, length = 65)

why no return false here?

lguohan · 2019-02-02T02:39:33Z

orchagent/vnetorch.cpp

+        return true;
+    }
+
+    bool subnet = (!nh.ips.getSize())?true:false;


subnet [](start = 9, length = 6)

->isSubnet

lguohan · 2019-02-02T02:41:35Z

orchagent/vnetorch.cpp

        {
-            throw std::runtime_error("Route add failed");
+            throw std::runtime_error("Route update failed");
        }
    }


what is else condition? do we need to print warning, log, ???

The else case is handled by the Bitmap implementation. So it is left intentionally.

lguohan · 2019-02-02T02:45:16Z

orchagent/vnetorch.cpp

        {
-            throw std::runtime_error("Route add failed");
+            throw std::runtime_error("Route update failed");


throw std::runtime_error("Route update failed"); [](start = 10, length = 50)

it looks to me it is dangerous to crash here. what if the nexthop interface is not created yet in the doRouteTask.

I checked the routeorch.cpp, it does not throw error, if add or delete route fails.

In reply to: 253250419 [](ancestors = 253250419)

it just retry.

In reply to: 253250430 [](ancestors = 253250430,253250419)

lguohan · 2019-02-02T02:47:41Z

orchagent/vnetorch.cpp

+    return (routes_.size() + tunnels_.size());
+}
+
+bool VNetVrfObject::getRouteNH(IpPrefix& ipPrefix, nextHop& nh)


-> getRouteNexthop

lguohan · 2019-02-02T02:48:50Z

orchagent/vnetorch.cpp

+    return true;
+}
+
+sai_object_id_t VNetVrfObject::getNextHop(tunnelEndpoint& endp)


getNextHop [](start = 31, length = 10)

-> getTunnelNexthop

lguohan · 2019-02-02T02:49:06Z

orchagent/vnetorch.cpp

+    return nh_id;
+}
+
+bool VNetVrfObject::removeNextHop(tunnelEndpoint& endp)


removeTunnelNexthop

lguohan · 2019-02-02T02:54:36Z

orchagent/vnetorch.cpp

+
+    if (!vxlan_orch->removeNextHopTunnel(tun_name, endp.ip, endp.mac, endp.vni))
+    {
+        SWSS_LOG_ERROR("VNET %s NH remove failed for '%s'", vnet_name_.c_str(), endp.ip.to_string().c_str());


NH [](start = 32, length = 2)

-> tunnel nexthop

lguohan · 2019-02-02T02:56:54Z

orchagent/vxlanorch.cpp

+    auto tunnel_obj = getVxlanTunnel(tunnelName);
+
+    //Delete request for the nh tunnel id
+    return tunnel_obj->removeNHTunnel(ipAddr, macAddress, vni);


removeNHTunnel [](start = 23, length = 14)

since it is already a tunnel object, you do not need tunnel in the function name. -> removeNexthop

lguohan · 2019-02-02T02:57:51Z

orchagent/vxlanorch.h

@@ -56,13 +104,23 @@ class VxlanTunnel
        return ids_.tunnel_encap_id;
    }

+    void updateNHTunnel(IpAddress& ipAddr, MacAddress macAddress, uint32_t vni, sai_object_id_t nh_id);


-> updateNexthop

lguohan · 2019-02-02T03:02:50Z

sai_status_t status = sai_route_api->create_route_entry(&route_entry, 1, &route_attr);

we should probably increase the crm counter.

     if (route_entry.destination.addr_family == SAI_IP_ADDR_FAMILY_IPV4)

823 {
824 gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV4_ROUTE);
825 }
826 else
827 {
828 gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE);
829 }

Refers to: orchagent/vnetorch.cpp:484 in 91d77df. [](commit_id = 91d77df, deletion_comment = False)

orchagent/intfsorch.cpp

Added breakout subcommand in config command Signed-off-by: Sangita Maity <[email protected]>

Signed-off-by: Guohan Lu <[email protected]>

prsunny added 4 commits January 24, 2019 01:48

Route/NH and VNet delete handling

1db930a

Merge remote-tracking branch 'upstream/master' into vnet_del

19703a6

Handle VNET Route with Nexthop IP, fix route remove

59f2498

Interface delete fix, base changes for multiple nexthops

e21e202

marian-pritsak reviewed Jan 30, 2019

View reviewed changes

orchagent/intfsorch.cpp Show resolved Hide resolved

marian-pritsak requested changes Jan 30, 2019

View reviewed changes

marian-pritsak reviewed Jan 30, 2019

View reviewed changes

Address review comment, use VnetInfo

91d77df

lguohan reviewed Feb 2, 2019

View reviewed changes

orchagent/vnetorch.cpp Outdated

return nh_id;

}

bool VNetVrfObject::removeNextHop(tunnelEndpoint& endp)

Copy link

Contributor

lguohan Feb 2, 2019

Choose a reason for hiding this comment

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

removeTunnelNexthop

lguohan reviewed Feb 2, 2019

View reviewed changes

marian-pritsak requested changes Feb 5, 2019

View reviewed changes

orchagent/intfsorch.cpp Show resolved Hide resolved

marian-pritsak approved these changes Feb 5, 2019

View reviewed changes

Address review comments, increment CRM counters

23c5fa2

lguohan approved these changes Feb 7, 2019

View reviewed changes

prsunny merged commit 6d5424d into sonic-net:master Feb 7, 2019

prsunny deleted the vnet_del branch March 19, 2020 01:33

EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022

[config] Add Initial draft of Breakout Mode subcommand (sonic-net#766)

aa1b072

Added breakout subcommand in config command Signed-off-by: Sangita Maity <[email protected]>

oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023

[ci]: use sonicbld pool (sonic-net#766)

b10b5dd

Signed-off-by: Guohan Lu <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VNet/Vxlan delete handling #766

VNet/Vxlan delete handling #766

prsunny commented Jan 24, 2019

marian-pritsak left a comment

marian-pritsak Jan 30, 2019

prsunny Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

prsunny Feb 6, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan Feb 2, 2019

lguohan commented Feb 2, 2019

VNet/Vxlan delete handling #766

VNet/Vxlan delete handling #766

Conversation

prsunny commented Jan 24, 2019

marian-pritsak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan commented Feb 2, 2019