From b9538666826dbb5459ec311bbb7168706eabc3af Mon Sep 17 00:00:00 2001 From: Andriy Yurkiv <70649192+ayurkiv-nvda@users.noreply.github.com> Date: Sun, 27 Nov 2022 13:24:41 +0200 Subject: [PATCH] [dual-tor] add missing SAI attribte in order to create IPNIP tunnel (#2503) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - What I did add SAI_TUNNEL_ATTR_DECAP_TTL_MODE and SAI_TUNNEL_ATTR_DECAP_DSCP_MODE - Why I did it Need to pass additional SAI attrib. Align encap end decap params 1 SAI_TUNNEL_ATTR_DECAP_DSCP_MODE Problem: Long time ago this attribute was defined as “mandatory on create” in SAI API, but at some point it was changed to “optional”. But MLNX SAI implementation didn’t change it and still expects this attribute to be always provided on tunnel creation. SONiC “dual ToR” implementation, when it creates IP-in-IP tunnel, does not provide this attribute because it is “optional”. So we get SAI error for creating tunnel because as mentioned above MLNX SAI implementation still expects this attribute as “mandatory on create”. Need to always set this attribute in “Dual ToR orchagent” on IP-in-IP tunnel creation 2 SAI_TUNNEL_ATTR_DECAP_TTL_MODE Problem: Long time ago this attribute was defined as “mandatory on create” in SAI API, but at some point it was changed to “optional”. SONiC “dual ToR” implementation, when it creates IP-in-IP tunnel, does not provide this attribute because now it is “optional”. SAI team reported: "We only support pipe, which is not the default API value of uniform, so user must provide explicit pipe This has always been the ip in ip behavior and limitation, which is also documented in our RN" So it is MLNX SAI implementation limitation that we support only “pipe” and since default value for this attribute is “uniform” we need to set it explicitly. And in our case it is becomes “mandatory on create” attribute which must be set to “pipe” value. As a result of this limitation we should update SONiC “Dual ToR” code in order to pass this attribute on IP-in-IP tunnel creation. - How I verified it tunnel successfully created Signed-off-by: Andriy Yurkiv --- orchagent/muxorch.cpp | 8 ++++++++ tests/test_mux.py | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/orchagent/muxorch.cpp b/orchagent/muxorch.cpp index 296d5a3cf3..449778794e 100644 --- a/orchagent/muxorch.cpp +++ b/orchagent/muxorch.cpp @@ -212,6 +212,10 @@ static sai_object_id_t create_tunnel( attr.value.s32 = SAI_TUNNEL_TTL_MODE_PIPE_MODEL; tunnel_attrs.push_back(attr); + attr.id = SAI_TUNNEL_ATTR_DECAP_TTL_MODE; + attr.value.s32 = SAI_TUNNEL_TTL_MODE_PIPE_MODEL; + tunnel_attrs.push_back(attr); + if (dscp_mode_name == "uniform" || dscp_mode_name == "pipe") { sai_tunnel_dscp_mode_t dscp_mode; @@ -226,6 +230,10 @@ static sai_object_id_t create_tunnel( attr.id = SAI_TUNNEL_ATTR_ENCAP_DSCP_MODE; attr.value.s32 = dscp_mode; tunnel_attrs.push_back(attr); + + attr.id = SAI_TUNNEL_ATTR_DECAP_DSCP_MODE; + attr.value.s32 = dscp_mode; + tunnel_attrs.push_back(attr); } attr.id = SAI_TUNNEL_ATTR_LOOPBACK_PACKET_ACTION; diff --git a/tests/test_mux.py b/tests/test_mux.py index 71193735c9..dc739b82a6 100644 --- a/tests/test_mux.py +++ b/tests/test_mux.py @@ -655,6 +655,8 @@ def create_and_test_peer(self, asicdb, tc_to_dscp_map_oid=None, tc_to_queue_map_ assert self.check_interface_exists_in_asicdb(asicdb, value) elif field == "SAI_TUNNEL_ATTR_ENCAP_TTL_MODE": assert value == "SAI_TUNNEL_TTL_MODE_PIPE_MODEL" + elif field == "SAI_TUNNEL_ATTR_DECAP_TTL_MODE": + assert value == "SAI_TUNNEL_TTL_MODE_PIPE_MODEL" elif field == "SAI_TUNNEL_ATTR_LOOPBACK_PACKET_ACTION": assert value == "SAI_PACKET_ACTION_DROP" elif field == "SAI_TUNNEL_ATTR_ENCAP_QOS_TC_AND_COLOR_TO_DSCP_MAP": @@ -663,6 +665,8 @@ def create_and_test_peer(self, asicdb, tc_to_dscp_map_oid=None, tc_to_queue_map_ assert value == tc_to_queue_map_oid elif field == "SAI_TUNNEL_ATTR_ENCAP_DSCP_MODE": assert value == "SAI_TUNNEL_DSCP_MODE_PIPE_MODEL" + elif field == "SAI_TUNNEL_ATTR_DECAP_DSCP_MODE": + assert value == "SAI_TUNNEL_DSCP_MODE_PIPE_MODEL" else: assert False, "Field %s is not tested" % field