kamelnetworks / sonic-buildimage

Kamel Networks' pending patches to SONiC
https://dev.azure.com/kamelnetworks/sonic/_build?definitionId=5
Other
3 stars 1 forks source link

orchagent: Creating VXLAN map fails #10

Open bluecmd opened 6 months ago

bluecmd commented 6 months ago

Running sudo config vxlan map add nve1 1234 1234 fails with:

Feb 23 02:57:38.105541 ixp-lab-sw1 ERR swss#orchagent: :- doTask: Logic error: _Map_base::at
Feb 23 02:57:38.127637 ixp-lab-sw1 INFO systemd-udevd[7167]: Using default interface naming scheme 'v247'.
Feb 23 02:57:38.147395 ixp-lab-sw1 INFO kernel: [  318.413520] Bridge: port 2(nve1-1234) entered blocking state
Feb 23 02:57:38.147437 ixp-lab-sw1 INFO kernel: [  318.413528] Bridge: port 2(nve1-1234) entered disabled state
Feb 23 02:57:38.155277 ixp-lab-sw1 INFO kernel: [  318.421538] device nve1-1234 entered promiscuous mode
Feb 23 02:57:38.163820 ixp-lab-sw1 INFO systemd-udevd[7167]: ethtool: autonegotiation is unset or enabled, the speed and duplex are not writable.
Feb 23 02:57:38.207276 ixp-lab-sw1 INFO kernel: [  318.471832] Bridge: port 2(nve1-1234) entered blocking state
Feb 23 02:57:38.207316 ixp-lab-sw1 INFO kernel: [  318.471839] Bridge: port 2(nve1-1234) entered forwarding state
Feb 23 02:57:38.218414 ixp-lab-sw1 WARNING swss#orchagent: :- addOperation: Vxlan tunnel map is not created for vni:1234

Relevant config:

{
    "VXLAN_EVPN_NVO": {
        "nvo1": {
            "source_vtep": "nve1"
        }
    },
    "VXLAN_TUNNEL": {
        "nve1": {
            "src_ip": "100.100.99.3"
        }
    }
}
bluecmd commented 6 months ago

Could be the attempt to fix encap_ttl, if that property doesn't exist maybe this causes an _at error?

EDIT: Trying to set this attribute fails with:

Feb 23 03:10:00.999244 ixp-lab-sw1 ERR swss#orchagent: :- doTask: Parse error: Unknown attribute name: encap_ttl

Doing a new build with SWSS to add this attribute as a supported one.

bluecmd commented 6 months ago

Yes, fixed by adding attribute as well as defining the attribute

bluecmd commented 6 months ago

Patch:

From 5ef42b81d5c4bd2f2a799b542ac375fb765c0eaa Mon Sep 17 00:00:00 2001
From: Christian Svensson <blue@cmd.nu>
Date: Mon, 19 Feb 2024 20:44:50 +0100
Subject: [PATCH 2/2] kamel: Fix for TTL leak from inner to outer VXLAN

See https://github.com/sonic-net/sonic-buildimage/issues/10050 for
details.
---
 orchagent/vxlanorch.cpp | 11 +++++++++--
 orchagent/vxlanorch.h   |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/orchagent/vxlanorch.cpp b/orchagent/vxlanorch.cpp
index 0e93d11a..207884b9 100644
--- a/orchagent/vxlanorch.cpp
+++ b/orchagent/vxlanorch.cpp
@@ -1926,7 +1926,7 @@ bool VxlanTunnelMapOrch::addOperation(const Request& request)
     if (vni_id >= MAX_VNI_ID)
     {
         SWSS_LOG_ERROR("Vxlan tunnel map vni id is too big: %d", vni_id);
-        return true;
+        return false;
     }

     tempPort.m_vnid = (uint32_t) vni_id;
@@ -1951,11 +1951,18 @@ bool VxlanTunnelMapOrch::addOperation(const Request& request)

     if (!tunnel_obj->isActive())
     {
+        auto encap_ttl  = static_cast<sai_uint32_t>(request.getAttrUint("encap_ttl"));
+        if (encap_ttl > 255)
+        {
+            SWSS_LOG_ERROR("Vxlan tunnel map encap TTL is too big: %d", encap_ttl);
+            return false;
+        }
         //@Todo, currently only decap mapper is allowed
         uint8_t mapper_list = 0;
         TUNNELMAP_SET_VLAN(mapper_list);
         TUNNELMAP_SET_VRF(mapper_list);
-        tunnel_obj->createTunnelHw(mapper_list,TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);
+        tunnel_obj->createTunnelHw(mapper_list, TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP,
+                                   /* with_term */ true, static_cast<sai_uint8_t>(encap_ttl));
         if (!tunnel_orch->isDipTunnelsSupported())
         {
             Port tunPort;
diff --git a/orchagent/vxlanorch.h b/orchagent/vxlanorch.h
index 695f7441..cc5e6a88 100644
--- a/orchagent/vxlanorch.h
+++ b/orchagent/vxlanorch.h
@@ -389,6 +389,7 @@ const request_description_t vxlan_tunnel_map_request_description = {
             {
                 { "vni",  REQ_T_UINT },
                 { "vlan", REQ_T_VLAN },
+                { "encap_ttl", REQ_T_UINT },
             },
             { "vni", "vlan" }
 };
-- 
2.42.0

Reverting this patch and trying this instead:

diff --git a/orchagent/vxlanorch.cpp b/orchagent/vxlanorch.cpp
index 0e93d11a..72c89c0b 100644
--- a/orchagent/vxlanorch.cpp
+++ b/orchagent/vxlanorch.cpp
@@ -348,16 +348,13 @@ create_tunnel(
         tunnel_attrs.push_back(attr);
     }

-    if (encap_ttl != 0)
-    {
-        attr.id = SAI_TUNNEL_ATTR_ENCAP_TTL_MODE;
-        attr.value.s32 = SAI_TUNNEL_TTL_MODE_PIPE_MODEL;
-        tunnel_attrs.push_back(attr);
+    attr.id = SAI_TUNNEL_ATTR_ENCAP_TTL_MODE;
+    attr.value.s32 = SAI_TUNNEL_TTL_MODE_PIPE_MODEL;
+    tunnel_attrs.push_back(attr);

-        attr.id = SAI_TUNNEL_ATTR_ENCAP_TTL_VAL;
-        attr.value.u8 = encap_ttl;
-        tunnel_attrs.push_back(attr);
-    }
+    attr.id = SAI_TUNNEL_ATTR_ENCAP_TTL_VAL;
+    attr.value.u8 = encap_ttl != 0 ? encap_ttl : 128;
+    tunnel_attrs.push_back(attr);

     sai_object_id_t tunnel_id;
     sai_status_t status = sai_tunnel_api->create_tunnel(

The idea is that this should cause other call sites to behave better and we never want the TTL mode to be anything else than PIPE anyway.