Skip to content

Commit 0ea3eb2

Browse files
shi-supreetham-singh
authored andcommitted
[bulk mode] Fix bulk conflict when in case there are both remove and set operations (sonic-net#2071)
What I did Check if there are items pending removal in bulk before calling bulk set API. Fixes sonic-net/sonic-buildimage#9434 Why I did it When there are items pending removal in bulk before calling set API, it means the item will be removed before the set and it should do create instead.
1 parent 5711b7e commit 0ea3eb2

File tree

4 files changed

+51
-2
lines changed

4 files changed

+51
-2
lines changed

orchagent/bulker.h

+5
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,11 @@ class EntityBulker
414414
return creating_entries.count(entry);
415415
}
416416

417+
bool bulk_entry_pending_removal(const Te& entry) const
418+
{
419+
return removing_entries.find(entry) != removing_entries.end();
420+
}
421+
417422
private:
418423
std::unordered_map< // A map of
419424
Te, // entry ->

orchagent/mplsrouteorch.cpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -598,8 +598,12 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey
598598
* in m_syncdLabelRoutes, then we need to update the route with a new next hop
599599
* (group) id. The old next hop (group) is then not used and the reference
600600
* count will decrease by 1.
601+
*
602+
* In case the entry is already pending removal in the bulk, it would be removed
603+
* from m_syncdLabelRoutes during the bulk call. Therefore, such entries need to be
604+
* re-created rather than set attribute.
601605
*/
602-
if (it_route == m_syncdLabelRoutes.at(vrf_id).end())
606+
if (it_route == m_syncdLabelRoutes.at(vrf_id).end() || gLabelRouteBulker.bulk_entry_pending_removal(inseg_entry))
603607
{
604608
vector<sai_attribute_t> inseg_attrs;
605609
if (blackhole)

orchagent/routeorch.cpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -1826,8 +1826,12 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops)
18261826
* in m_syncdRoutes, then we need to update the route with a new next hop
18271827
* (group) id. The old next hop (group) is then not used and the reference
18281828
* count will decrease by 1.
1829+
*
1830+
* In case the entry is already pending removal in the bulk, it would be removed
1831+
* from m_syncdRoutes during the bulk call. Therefore, such entries need to be
1832+
* re-created rather than set attribute.
18291833
*/
1830-
if (it_route == m_syncdRoutes.at(vrf_id).end())
1834+
if (it_route == m_syncdRoutes.at(vrf_id).end() || gRouteBulker.bulk_entry_pending_removal(route_entry))
18311835
{
18321836
if (blackhole)
18331837
{

tests/mock_tests/bulker_ut.cpp

+36
Original file line numberDiff line numberDiff line change
@@ -106,4 +106,40 @@ namespace bulker_test
106106
ASSERT_EQ(ia->first.id, SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION);
107107
ASSERT_EQ(ia->first.value.s32, SAI_PACKET_ACTION_FORWARD);
108108
}
109+
110+
TEST_F(BulkerTest, BulkerPendindRemoval)
111+
{
112+
// Create bulker
113+
EntityBulker<sai_route_api_t> gRouteBulker(sai_route_api, 1000);
114+
deque<sai_status_t> object_statuses;
115+
116+
// Check max bulk size
117+
ASSERT_EQ(gRouteBulker.max_bulk_size, 1000);
118+
119+
// Create a dummy route entry
120+
sai_route_entry_t route_entry_remove;
121+
route_entry_remove.destination.addr_family = SAI_IP_ADDR_FAMILY_IPV4;
122+
route_entry_remove.destination.addr.ip4 = htonl(0x0a00000f);
123+
route_entry_remove.destination.mask.ip4 = htonl(0xffffff00);
124+
route_entry_remove.vr_id = 0x0;
125+
route_entry_remove.switch_id = 0x0;
126+
127+
// Put route entry into remove
128+
object_statuses.emplace_back();
129+
gRouteBulker.remove_entry(&object_statuses.back(), &route_entry_remove);
130+
131+
// Confirm route entry is pending removal
132+
ASSERT_TRUE(gRouteBulker.bulk_entry_pending_removal(route_entry_remove));
133+
134+
// Create another dummy route entry that will not be removed
135+
sai_route_entry_t route_entry_non_remove;
136+
route_entry_non_remove.destination.addr_family = SAI_IP_ADDR_FAMILY_IPV4;
137+
route_entry_non_remove.destination.addr.ip4 = htonl(0x0a00010f);
138+
route_entry_non_remove.destination.mask.ip4 = htonl(0xffffff00);
139+
route_entry_non_remove.vr_id = 0x0;
140+
route_entry_non_remove.switch_id = 0x0;
141+
142+
// Confirm route entry is not pending removal
143+
ASSERT_FALSE(gRouteBulker.bulk_entry_pending_removal(route_entry_non_remove));
144+
}
109145
}

0 commit comments

Comments
 (0)