From 1a0be1b89cca9d9958a3514c8a27e9ca965e8846 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Thu, 5 Feb 2026 23:52:40 +0000 Subject: [PATCH] [DASH] Validate ca to pa SAI attributes **What I did** Only set certain CA to PA attributes if they are valid for a given mapping type **Why I did it** Some SAI attributes are only valid for privatelink and some are only valid for non-privatelink. This is enforced by the SAI metadata layer, so add this check in orchagent to prevent crashes. **How I verified it** **Details if related** --- orchagent/dash/dashvnetorch.cpp | 69 +++++++++++++++--------- tests/dash/test_dash_vnet.py | 1 - tests/mock_tests/dashvnetorch_ut.cpp | 12 +++-- tests/mock_tests/mock_dash_orch_test.cpp | 6 +-- 4 files changed, 55 insertions(+), 33 deletions(-) diff --git a/orchagent/dash/dashvnetorch.cpp b/orchagent/dash/dashvnetorch.cpp index adc6e7e6..4c8922f8 100644 --- a/orchagent/dash/dashvnetorch.cpp +++ b/orchagent/dash/dashvnetorch.cpp @@ -318,29 +318,30 @@ bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt return false; } + uint32_t routing_type_tunnel_key = 0; + sai_dash_encapsulation_t encap_type = SAI_DASH_ENCAPSULATION_INVALID; for (auto action: route_type_actions.items()) { if (action.action_type() == dash::route_type::ACTION_TYPE_STATICENCAP) { - outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_DASH_ENCAPSULATION; if (action.encap_type() == dash::route_type::ENCAP_TYPE_VXLAN) { - outbound_ca_to_pa_attr.value.u32 = SAI_DASH_ENCAPSULATION_VXLAN; + encap_type = SAI_DASH_ENCAPSULATION_VXLAN; } else if (action.encap_type() == dash::route_type::ENCAP_TYPE_NVGRE) { - outbound_ca_to_pa_attr.value.u32 = SAI_DASH_ENCAPSULATION_NVGRE; + encap_type = SAI_DASH_ENCAPSULATION_NVGRE; } else { SWSS_LOG_ERROR("Invalid encap type %d for %s", action.encap_type(), key.c_str()); return true; } - outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); - outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_TUNNEL_KEY; - outbound_ca_to_pa_attr.value.u32 = action.vni(); - outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + if (action.has_vni()) + { + routing_type_tunnel_key = action.vni(); + } outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_UNDERLAY_DIP; to_sai(ctxt.metadata.underlay_ip(), outbound_ca_to_pa_attr.value.ipaddr); @@ -349,6 +350,7 @@ bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt } } + // Setting SAI attributes that are valid for all values of SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_ACTION if (ctxt.metadata.has_tunnel()) { auto tunnel_oid = gDirectory.get()->getTunnelOid(ctxt.metadata.tunnel()); @@ -362,12 +364,25 @@ bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); } + if (ctxt.metadata.has_metering_class_or()) + { + outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_METER_CLASS_OR; + outbound_ca_to_pa_attr.value.u32 = ctxt.metadata.metering_class_or(); + outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + } + if (ctxt.metadata.routing_type() == dash::route_type::ROUTING_TYPE_PRIVATELINK) { + SWSS_LOG_DEBUG("Creating private link outbound CA to PA entry for %s", key.c_str()); + // Setting SAI attributes specific to private link routing type outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_ACTION; outbound_ca_to_pa_attr.value.u32 = SAI_OUTBOUND_CA_TO_PA_ENTRY_ACTION_SET_PRIVATE_LINK_MAPPING; outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_DASH_ENCAPSULATION; + outbound_ca_to_pa_attr.value.u32 = encap_type; + outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DIP; to_sai(ctxt.metadata.overlay_dip_prefix().ip(), outbound_ca_to_pa_attr.value.ipaddr); outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); @@ -376,7 +391,6 @@ bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt to_sai(ctxt.metadata.overlay_dip_prefix().mask(), outbound_ca_to_pa_attr.value.ipaddr); outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); - outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_SIP; to_sai(ctxt.metadata.overlay_sip_prefix().ip(), outbound_ca_to_pa_attr.value.ipaddr); outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); @@ -385,6 +399,13 @@ bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt to_sai(ctxt.metadata.overlay_sip_prefix().mask(), outbound_ca_to_pa_attr.value.ipaddr); outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + if (routing_type_tunnel_key != 0) + { + outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_TUNNEL_KEY; + outbound_ca_to_pa_attr.value.u32 = routing_type_tunnel_key; + outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + } + if (ctxt.metadata.has_port_map()) { auto port_map_oid = @@ -400,26 +421,22 @@ bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); } } - - if (ctxt.metadata.has_metering_class_or()) - { - outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_METER_CLASS_OR; - outbound_ca_to_pa_attr.value.u32 = ctxt.metadata.metering_class_or(); - outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); - } - - if (ctxt.metadata.has_mac_address()) + else { - outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DMAC; - memcpy(outbound_ca_to_pa_attr.value.mac, ctxt.metadata.mac_address().c_str(), sizeof(sai_mac_t)); - outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); - } + // Setting SAI attributes specific to non-private link routing types + if (ctxt.metadata.has_mac_address()) + { + outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DMAC; + memcpy(outbound_ca_to_pa_attr.value.mac, ctxt.metadata.mac_address().c_str(), sizeof(sai_mac_t)); + outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + } - if (ctxt.metadata.has_use_dst_vni()) - { - outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_USE_DST_VNET_VNI; - outbound_ca_to_pa_attr.value.booldata = ctxt.metadata.use_dst_vni(); - outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + if (ctxt.metadata.has_use_dst_vni()) + { + outbound_ca_to_pa_attr.id = SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_USE_DST_VNET_VNI; + outbound_ca_to_pa_attr.value.booldata = ctxt.metadata.use_dst_vni(); + outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr); + } } object_statuses.emplace_back(); diff --git a/tests/dash/test_dash_vnet.py b/tests/dash/test_dash_vnet.py index dcc8be8c..bea9aa88 100644 --- a/tests/dash/test_dash_vnet.py +++ b/tests/dash/test_dash_vnet.py @@ -182,7 +182,6 @@ def test_vnet_map(self, dash_db: DashDB): attrs = dash_db.get_asic_db_entry(ASIC_OUTBOUND_CA_TO_PA_TABLE, vnet_ca_to_pa_maps[0]) assert_sai_attribute_exists("SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_UNDERLAY_DIP", attrs, self.underlay_ip) assert_sai_attribute_exists("SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DMAC", attrs, self.mac_address) - assert_sai_attribute_exists("SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_DASH_ENCAPSULATION", attrs, "SAI_DASH_ENCAPSULATION_NVGRE") assert_sai_attribute_exists("SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_METER_CLASS_OR", attrs, self.vnet_map_metering_class_or) vnet_pa_validation_maps = dash_db.wait_for_asic_db_keys(ASIC_PA_VALIDATION_TABLE) diff --git a/tests/mock_tests/dashvnetorch_ut.cpp b/tests/mock_tests/dashvnetorch_ut.cpp index 473e79a6..d155eecc 100644 --- a/tests/mock_tests/dashvnetorch_ut.cpp +++ b/tests/mock_tests/dashvnetorch_ut.cpp @@ -71,10 +71,16 @@ namespace dashvnetorch_test { InSequence seq; EXPECT_CALL(*mock_sai_dash_vnet_api, create_vnets).Times(1); - EXPECT_CALL(*mock_sai_dash_outbound_ca_to_pa_api, create_outbound_ca_to_pa_entries).Times(1); + EXPECT_CALL(*mock_sai_dash_outbound_ca_to_pa_api, create_outbound_ca_to_pa_entries).WillOnce(DoAll( + Return(SAI_STATUS_SUCCESS) + )); EXPECT_CALL(*mock_sai_dash_pa_validation_api, create_pa_validation_entries).Times(1); - EXPECT_CALL(*mock_sai_dash_outbound_ca_to_pa_api, create_outbound_ca_to_pa_entries).Times(1); - EXPECT_CALL(*mock_sai_dash_outbound_ca_to_pa_api, remove_outbound_ca_to_pa_entries).Times(2); + EXPECT_CALL(*mock_sai_dash_outbound_ca_to_pa_api, create_outbound_ca_to_pa_entries).WillOnce(DoAll( + Return(SAI_STATUS_SUCCESS) + )); + EXPECT_CALL(*mock_sai_dash_outbound_ca_to_pa_api, remove_outbound_ca_to_pa_entries).Times(2).WillRepeatedly(DoAll( + Return(SAI_STATUS_SUCCESS) + )); EXPECT_CALL(*mock_sai_dash_pa_validation_api, remove_pa_validation_entries).Times(1); EXPECT_CALL(*mock_sai_dash_vnet_api, remove_vnets).Times(1); } diff --git a/tests/mock_tests/mock_dash_orch_test.cpp b/tests/mock_tests/mock_dash_orch_test.cpp index 077ad8c5..7f1b1572 100644 --- a/tests/mock_tests/mock_dash_orch_test.cpp +++ b/tests/mock_tests/mock_dash_orch_test.cpp @@ -16,7 +16,7 @@ namespace mock_orch_test auto consumer = make_unique( new swss::ConsumerStateTable(m_app_db.get(), table_name), target_orch, table_name); - auto op = set ? SET_COMMAND : DEL_COMMAND; + std::string op = set ? SET_COMMAND : DEL_COMMAND; consumer->addToSync( swss::KeyOpFieldsValuesTuple(key, op, { { "pb", message.SerializeAsString() } })); target_orch->doTask(*consumer.get()); @@ -25,13 +25,13 @@ namespace mock_orch_test if (expect_empty) { EXPECT_EQ(it2, consumer->m_toSync.end()) - << "Expected consumer to be empty after operation on table " << table_name + << "Expected consumer to be empty after " << op << " operation on table " << table_name << " with key " << key; } else { EXPECT_NE(it2, consumer->m_toSync.end()) - << "Expected consumer to not be empty after operation on table " << table_name + << "Expected consumer to not be empty after " << op << " operation on table " << table_name << " with key " << key; } }