Skip to content

Commit

Permalink
[dash]: Wait for routing type config when adding VNET mapping (#3312)
Browse files Browse the repository at this point in the history
What I did
Block adding VNET mapping until the routing type for that VNET mapping has been configured
Perform setup/teardown of DASH route group tests on a per-testcase basis instead of per-module.
  • Loading branch information
theasianpianist authored Oct 11, 2024
1 parent 766e755 commit 55fd3f1
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 16 deletions.
22 changes: 11 additions & 11 deletions orchagent/dash/dashvnetorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ void DashVnetOrch::doTaskVnetTable(ConsumerBase& consumer)
}
}

void DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt)
bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt)
{
SWSS_LOG_ENTER();

Expand All @@ -291,6 +291,7 @@ void DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt
if (!dash_orch->getRouteTypeActions(ctxt.metadata.routing_type(), route_type_actions))
{
SWSS_LOG_INFO("Failed to get route type actions for %s", key.c_str());
return false;
}

for (auto action: route_type_actions.items())
Expand All @@ -309,7 +310,7 @@ void DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt
else
{
SWSS_LOG_ERROR("Invalid encap type %d for %s", action.encap_type(), key.c_str());
return;
return false;
}
outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr);

Expand Down Expand Up @@ -359,9 +360,10 @@ void DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt
object_statuses.emplace_back();
outbound_ca_to_pa_bulker_.create_entry(&object_statuses.back(), &outbound_ca_to_pa_entry,
(uint32_t)outbound_ca_to_pa_attrs.size(), outbound_ca_to_pa_attrs.data());
return true;
}

void DashVnetOrch::addPaValidation(const string& key, VnetMapBulkContext& ctxt)
bool DashVnetOrch::addPaValidation(const string& key, VnetMapBulkContext& ctxt)
{
SWSS_LOG_ENTER();

Expand All @@ -380,7 +382,7 @@ void DashVnetOrch::addPaValidation(const string& key, VnetMapBulkContext& ctxt)
SWSS_LOG_INFO("Increment PA refcount to %u for PA IP %s",
pa_refcount_table_[pa_ref_key],
underlay_ip_str.c_str());
return;
return true;
}

uint32_t attr_count = 1;
Expand All @@ -399,6 +401,7 @@ void DashVnetOrch::addPaValidation(const string& key, VnetMapBulkContext& ctxt)
pa_refcount_table_[pa_ref_key] = 1;
SWSS_LOG_INFO("Initialize PA refcount to 1 for PA IP %s",
underlay_ip_str.c_str());
return true;
}

bool DashVnetOrch::addVnetMap(const string& key, VnetMapBulkContext& ctxt)
Expand All @@ -408,17 +411,14 @@ bool DashVnetOrch::addVnetMap(const string& key, VnetMapBulkContext& ctxt)
bool exists = (vnet_map_table_.find(key) != vnet_map_table_.end());
if (!exists)
{

bool vnet_exists = (gVnetNameToId.find(ctxt.vnet_name) != gVnetNameToId.end());
if (vnet_exists)
{
addOutboundCaToPa(key, ctxt);
addPaValidation(key, ctxt);
}
else
if (!vnet_exists)
{
SWSS_LOG_INFO("Not creating VNET map for %s since VNET %s doesn't exist", key.c_str(), ctxt.vnet_name.c_str());
return false;
}
return false;
return addOutboundCaToPa(key, ctxt) && addPaValidation(key, ctxt);
}
/*
* If the VNET map is already added, don't add it to the bulker and
Expand Down
4 changes: 2 additions & 2 deletions orchagent/dash/dashvnetorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ class DashVnetOrch : public ZmqOrch
bool addVnetPost(const std::string& key, const DashVnetBulkContext& ctxt);
bool removeVnet(const std::string& key, DashVnetBulkContext& ctxt);
bool removeVnetPost(const std::string& key, const DashVnetBulkContext& ctxt);
void addOutboundCaToPa(const std::string& key, VnetMapBulkContext& ctxt);
bool addOutboundCaToPa(const std::string& key, VnetMapBulkContext& ctxt);
bool addOutboundCaToPaPost(const std::string& key, const VnetMapBulkContext& ctxt);
void removeOutboundCaToPa(const std::string& key, VnetMapBulkContext& ctxt);
bool removeOutboundCaToPaPost(const std::string& key, const VnetMapBulkContext& ctxt);
void addPaValidation(const std::string& key, VnetMapBulkContext& ctxt);
bool addPaValidation(const std::string& key, VnetMapBulkContext& ctxt);
bool addPaValidationPost(const std::string& key, const VnetMapBulkContext& ctxt);
void removePaValidation(const std::string& key, VnetMapBulkContext& ctxt);
bool removePaValidationPost(const std::string& key, const VnetMapBulkContext& ctxt);
Expand Down
3 changes: 2 additions & 1 deletion tests/dash/test_dash_pl.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@

@pytest.fixture(autouse=True)
def common_setup_teardown(dash_db: DashDB):
dash_db.set_app_db_entry(APP_DASH_ROUTING_TYPE_TABLE_NAME, PRIVATELINK, ROUTING_TYPE_PL_CONFIG)
dash_db.set_app_db_entry(APP_DASH_APPLIANCE_TABLE_NAME, APPLIANCE_ID, APPLIANCE_CONFIG)
dash_db.set_app_db_entry(APP_DASH_VNET_TABLE_NAME, VNET1, VNET_CONFIG)
dash_db.set_app_db_entry(APP_DASH_ENI_TABLE_NAME, ENI_ID, ENI_CONFIG)
dash_db.set_app_db_entry(APP_DASH_VNET_MAPPING_TABLE_NAME, VNET1, VNET_MAP_IP1, VNET_MAPPING_CONFIG_PRIVATELINK)
dash_db.set_app_db_entry(APP_DASH_ROUTE_GROUP_TABLE_NAME, ROUTE_GROUP1, ROUTE_GROUP1_CONFIG)
dash_db.set_app_db_entry(APP_DASH_ROUTING_TYPE_TABLE_NAME, PRIVATELINK, ROUTING_TYPE_PL_CONFIG)
# Don't set DASH_ROUTE_TABLE and DASH_ENI_ROUTE_TABLE entries here for flexibility, test cases will set them as needed

yield
Expand Down Expand Up @@ -79,3 +79,4 @@ def test_pl_outbound_ca_to_pa_attrs(dash_db: DashDB):
assert_sai_attribute_exists(SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DIP_MASK, outbound_attrs, PL_OVERLAY_DIP_MASK)
assert_sai_attribute_exists(SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_TUNNEL_KEY, outbound_attrs, ENCAP_VNI)
assert_sai_attribute_exists(SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_DASH_ENCAPSULATION, outbound_attrs, SAI_DASH_ENCAPSULATION_NVGRE)
assert_sai_attribute_exists(SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_UNDERLAY_DIP, outbound_attrs, UNDERLAY_IP)
2 changes: 1 addition & 1 deletion tests/dash/test_dash_route_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
APP_DASH_ROUTE_GROUP_TABLE_NAME,
)

@pytest.fixture(scope='module', autouse=True)
@pytest.fixture(autouse=True)
def common_setup_teardown(dash_db: DashDB):
dash_db.set_app_db_entry(APP_DASH_APPLIANCE_TABLE_NAME, APPLIANCE_ID, APPLIANCE_CONFIG)
dash_db.set_app_db_entry(APP_DASH_ROUTING_TYPE_TABLE_NAME, PRIVATELINK, ROUTING_TYPE_PL_CONFIG)
Expand Down
8 changes: 7 additions & 1 deletion tests/sai_attrs.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
SAI_DIRECTION_LOOKUP_ENTRY_ATTR_ACTION = "SAI_DIRECTION_LOOKUP_ENTRY_ATTR_ACTION"
SAI_DIRECTION_LOOKUP_ENTRY_ACTION_SET_OUTBOUND_DIRECTION = "SAI_DIRECTION_LOOKUP_ENTRY_ACTION_SET_OUTBOUND_DIRECTION"

SAI_ENI_ATTR_PL_SIP = 'SAI_ENI_ATTR_PL_SIP'
SAI_ENI_ATTR_PL_SIP_MASK = 'SAI_ENI_ATTR_PL_SIP_MASK'
SAI_ENI_ATTR_PL_UNDERLAY_SIP = 'SAI_ENI_ATTR_PL_UNDERLAY_SIP'
SAI_ENI_ATTR_OUTBOUND_ROUTING_GROUP_ID = 'SAI_ENI_ATTR_OUTBOUND_ROUTING_GROUP_ID'

SAI_OUTBOUND_CA_TO_PA_ENTRY_ACTION_SET_TUNNEL_MAPPING = 'SAI_OUTBOUND_CA_TO_PA_ENTRY_ACTION_SET_TUNNEL_MAPPING'
SAI_OUTBOUND_CA_TO_PA_ENTRY_ACTION_SET_PRIVATE_LINK_MAPPING = 'SAI_OUTBOUND_CA_TO_PA_ENTRY_ACTION_SET_PRIVATE_LINK_MAPPING'
SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DIP = 'SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DIP'
Expand All @@ -13,5 +17,7 @@
SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DIP_MASK = 'SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_OVERLAY_DIP_MASK'
SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_DASH_ENCAPSULATION = 'SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_DASH_ENCAPSULATION'
SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_TUNNEL_KEY = 'SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_TUNNEL_KEY'
SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_UNDERLAY_DIP = 'SAI_OUTBOUND_CA_TO_PA_ENTRY_ATTR_UNDERLAY_DIP'
SAI_DASH_ENCAPSULATION_NVGRE = 'SAI_DASH_ENCAPSULATION_NVGRE'
SAI_OUTBOUND_ROUTING_ENTRY_ATTR_UNDERLAY_SIP = 'SAI_OUTBOUND_ROUTING_ENTRY_ATTR_UNDERLAY_SIP'

SAI_OUTBOUND_ROUTING_ENTRY_ATTR_UNDERLAY_SIP = 'SAI_OUTBOUND_ROUTING_ENTRY_ATTR_UNDERLAY_SIP'

0 comments on commit 55fd3f1

Please sign in to comment.