Skip to content

Revert "[meta] do not fail bulk operations if MODE_IGNORE_ERROR (#89)" #96

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
prabhataravind merged 1 commit into from
Oct 30, 2025
Merged

Revert "[meta] do not fail bulk operations if MODE_IGNORE_ERROR (#89)" #96

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions meta/Meta.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@

#define CHECK_STATUS_SUCCESS(s) { if ((s) != SAI_STATUS_SUCCESS) return (s); }

#define CHECK_STATUS_SUCCESS_MODE(s,m) \
{ \
if ((s) != SAI_STATUS_SUCCESS && m != SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR) return (s); \
} \

#define VALIDATION_LIST(md,vlist) \
{ \
auto _status = meta_genetic_validation_list(md,vlist.count,vlist.list); \
Expand Down Expand Up @@ -628,14 +623,14 @@
for (uint32_t idx = 0; idx < object_count; idx++) \
{ \
sai_status_t status = meta_sai_validate_ ##ot (&ot[idx], true); \
CHECK_STATUS_SUCCESS_MODE(status, mode); \
CHECK_STATUS_SUCCESS(status); \
sai_object_meta_key_t meta_key = { \
.objecttype = (sai_object_type_t)SAI_OBJECT_TYPE_ ## OT, \
.objectkey = { .key = { .ot = ot[idx] } } \
}; \
vmk.push_back(meta_key); \
status = meta_generic_validation_create(meta_key, ot[idx].switch_id, attr_count[idx], attr_list[idx]); \
CHECK_STATUS_SUCCESS_MODE(status, mode); \
CHECK_STATUS_SUCCESS(status); \
} \
auto status = m_implementation->bulkCreate(object_count, ot, attr_count, attr_list, mode, object_statuses); \
for (uint32_t idx = 0; idx < object_count; idx++) \
Expand Down Expand Up @@ -672,14 +667,14 @@
for (uint32_t idx = 0; idx < object_count; idx++) \
{ \
sai_status_t status = meta_sai_validate_ ##ot (&ot[idx], false); \
CHECK_STATUS_SUCCESS_MODE(status, mode); \
CHECK_STATUS_SUCCESS(status); \
sai_object_meta_key_t meta_key = { \
.objecttype = (sai_object_type_t)SAI_OBJECT_TYPE_ ## OT, \
.objectkey = { .key = { .ot = ot[idx] } } \
}; \
vmk.push_back(meta_key); \
status = meta_generic_validation_remove(meta_key); \
CHECK_STATUS_SUCCESS_MODE(status, mode); \
CHECK_STATUS_SUCCESS(status); \
} \
auto status = m_implementation->bulkRemove(object_count, ot, mode, object_statuses); \
for (uint32_t idx = 0; idx < object_count; idx++) \
Expand Down Expand Up @@ -718,14 +713,14 @@
for (uint32_t idx = 0; idx < object_count; idx++) \
{ \
sai_status_t status = meta_sai_validate_ ##ot (&ot[idx], false); \
CHECK_STATUS_SUCCESS_MODE(status, mode); \
CHECK_STATUS_SUCCESS(status); \
sai_object_meta_key_t meta_key = { \
.objecttype = (sai_object_type_t)SAI_OBJECT_TYPE_ ## OT, \
.objectkey = { .key = { .ot = ot[idx] } } \
}; \
vmk.push_back(meta_key); \
status = meta_generic_validation_set(meta_key, &attr_list[idx]); \
CHECK_STATUS_SUCCESS_MODE(status, mode); \
CHECK_STATUS_SUCCESS(status); \
} \
auto status = m_implementation->bulkSet(object_count, ot, attr_list, mode, object_statuses); \
for (uint32_t idx = 0; idx < object_count; idx++) \
Expand Down Expand Up @@ -1211,15 +1206,15 @@
{
sai_status_t status = meta_sai_validate_oid(object_type, &object_id[idx], SAI_NULL_OBJECT_ID, false);

CHECK_STATUS_SUCCESS_MODE(status, mode);
CHECK_STATUS_SUCCESS(status);

sai_object_meta_key_t meta_key = { .objecttype = object_type, .objectkey = { .key = { .object_id = object_id[idx] } } };

vmk.push_back(meta_key);

status = meta_generic_validation_remove(meta_key);

CHECK_STATUS_SUCCESS_MODE(status, mode);
CHECK_STATUS_SUCCESS(status);
}

auto status = m_implementation->bulkRemove(object_type, object_count, object_id, mode, object_statuses);
Expand Down Expand Up @@ -1273,15 +1268,15 @@
{
sai_status_t status = meta_sai_validate_oid(object_type, &object_id[idx], SAI_NULL_OBJECT_ID, false);

CHECK_STATUS_SUCCESS_MODE(status, mode);
CHECK_STATUS_SUCCESS(status);

sai_object_meta_key_t meta_key = { .objecttype = object_type, .objectkey = { .key = { .object_id = object_id[idx] } } };

vmk.push_back(meta_key);

status = meta_generic_validation_set(meta_key, &attr_list[idx]);

CHECK_STATUS_SUCCESS_MODE(status, mode);
CHECK_STATUS_SUCCESS(status);
}

auto status = m_implementation->bulkSet(object_type, object_count, object_id, attr_list, mode, object_statuses);
Expand Down Expand Up @@ -1333,15 +1328,18 @@
{
sai_status_t status = meta_sai_validate_oid(object_type, &object_id[idx], SAI_NULL_OBJECT_ID, false);

CHECK_STATUS_SUCCESS_MODE(status, mode);
CHECK_STATUS_SUCCESS(status);

sai_object_meta_key_t meta_key = { .objecttype = object_type, .objectkey = { .key = { .object_id = object_id[idx] } } };

vmk.push_back(meta_key);

status = meta_generic_validation_get(meta_key, attr_count[idx], attr_list[idx]);

CHECK_STATUS_SUCCESS_MODE(status, mode);
// FIXME: This macro returns on failure.

Check notice

Code scanning / CodeQL

FIXME comment Note

FIXME comment: This macro returns on failure.

Copilot Autofix

AI 6 months ago

To fix the issue, we need to alter the handling of failed calls to meta_generic_validation_get (and similar places if relevant), so that when the bulk operation error mode is set to SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR, the code DOES NOT return immediately on validation failure. Instead, it should record the error in object_statuses[idx], set the appropriate status (e.g., SAI_STATUS_INVALID_PARAMETER), and continue processing the remaining objects, allowing the bulk operation to complete as expected in the ignore-error mode.

Specifically, replace the CHECK_STATUS_SUCCESS(status); macro (which returns immediately on failure) with explicit handling:

  • If mode is IGNORE_ERROR, record the error in object_statuses[idx] and continue.
  • Otherwise, return the status as before.

Remove the FIXME comment and update the code as appropriate.

Required changes:

  • Remove line 1339's FIXME.
  • Replace lines 1342 (CHECK_STATUS_SUCCESS...) with new logic checking mode.
  • Ensure the handling fits the local code style convention; do not alter lines outside the shown snippet.
Suggested changeset 1
meta/Meta.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/meta/Meta.cpp b/meta/Meta.cpp
--- a/meta/Meta.cpp
+++ b/meta/Meta.cpp
@@ -1336,11 +1336,22 @@
 
         status = meta_generic_validation_get(meta_key, attr_count[idx], attr_list[idx]);
 
-        // FIXME: This macro returns on failure.
-        // When mode is SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR we should continue instead of return.
-        // This issue exists for all bulk operations.
-        CHECK_STATUS_SUCCESS(status);
-    }
+        if (status != SAI_STATUS_SUCCESS)
+        {
+            if (mode == SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR)
+            {
+                // Record error for this object and continue
+                if (object_statuses != nullptr)
+                {
+                    object_statuses[idx] = status;
+                }
+                continue;
+            }
+            else
+            {
+                return status;
+            }
+        }
 
     auto status = m_implementation->bulkGet(object_type, object_count, object_id, attr_count, attr_list, mode, object_statuses);
 
EOF
@@ -1336,11 +1336,22 @@

status = meta_generic_validation_get(meta_key, attr_count[idx], attr_list[idx]);

// FIXME: This macro returns on failure.
// When mode is SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR we should continue instead of return.
// This issue exists for all bulk operations.
CHECK_STATUS_SUCCESS(status);
}
if (status != SAI_STATUS_SUCCESS)
{
if (mode == SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR)
{
// Record error for this object and continue
if (object_statuses != nullptr)
{
object_statuses[idx] = status;
}
continue;
}
else
{
return status;
}
}

auto status = m_implementation->bulkGet(object_type, object_count, object_id, attr_count, attr_list, mode, object_statuses);

Copilot is powered by AI and may make mistakes. Always verify output.
// When mode is SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR we should continue instead of return.
// This issue exists for all bulk operations.
CHECK_STATUS_SUCCESS(status);
}

auto status = m_implementation->bulkGet(object_type, object_count, object_id, attr_count, attr_list, mode, object_statuses);
Expand Down Expand Up @@ -1416,7 +1414,7 @@
{
sai_status_t status = meta_sai_validate_oid(object_type, &object_id[idx], switchId, true);

CHECK_STATUS_SUCCESS_MODE(status, mode);
CHECK_STATUS_SUCCESS(status);

// this is create, oid's don't exist yet

Expand All @@ -1426,7 +1424,7 @@

status = meta_generic_validation_create(meta_key, switchId, attr_count[idx], attr_list[idx]);

CHECK_STATUS_SUCCESS_MODE(status, mode);
CHECK_STATUS_SUCCESS(status);
}

auto status = m_implementation->bulkCreate(object_type, switchId, object_count, attr_count, attr_list, mode, object_id, object_statuses);
Expand Down
Loading