Skip to content
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.

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

TEST DO NOT MERGE 1509 #1539

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

TEST DO NOT MERGE 1509 #1539

wants to merge 26 commits into from

Conversation

kcudnik
Copy link
Collaborator

@kcudnik kcudnik commented Feb 20, 2025

No description provided.

stepanblyschak and others added 16 commits January 15, 2025 09:43
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
…into bulk-get

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).


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

for (uint32_t idx = 0; idx < object_count; idx++)

Check notice

Code scanning / CodeQL

FIXME comment Note

FIXME comment: This macro returns on failure.
Comment on lines 666 to +675
SWSS_LOG_ENTER();
VENDOR_CHECK_API_INITIALIZED();

sai_bulk_object_get_attribute_fn ptr;

switch (object_type)
{
case SAI_OBJECT_TYPE_PORT:
ptr = m_apis.port_api->get_ports_attribute;
break;

Check notice

Code scanning / CodeQL

No trivial switch statements Note

This switch statement should either handle more cases, or be rewritten as an if statement.
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kcudnik
Copy link
Collaborator Author

kcudnik commented Feb 21, 2025

/zp run

Comment on lines 2266 to 2359
switch (api)
{
case SAI_COMMON_API_BULK_CREATE:

{
sai_object_id_t switch_id = m_sai->switchIdQuery(local_ids[0]);
std::vector<sai_object_id_t> ids(object_count);

for (uint32_t it = 0; it < object_count; it++)
{
if (m_sai->switchIdQuery(local_ids[it]) != switch_id ||
switch_id == SAI_NULL_OBJECT_ID)
{
SWSS_LOG_THROW("invalid switch_id translated from VID %s",
sai_serialize_object_id(local_ids[it]).c_str());
}
}

std::vector<uint32_t> attr_counts(object_count);

std::vector<const sai_attribute_t*> attr_lists(object_count);

for (uint32_t idx = 0; idx < object_count; idx++)
{
attr_counts[idx] = attributes[idx]->get_attr_count();
attr_lists[idx] = attributes[idx]->get_attr_list();
}

switch_id = translate_local_to_redis(switch_id);

status = m_sai->bulkCreate(object_type,
switch_id,
object_count,
attr_counts.data(),
attr_lists.data(),
mode,
ids.data(),
statuses.data());

if (status == SAI_STATUS_SUCCESS)
{
for (uint32_t it = 0; it < object_count; it++)
{
match_redis_with_rec(ids[it], local_ids[it]);

SWSS_LOG_INFO("saved VID %s to RID %s",
sai_serialize_object_id(local_ids[it]).c_str(),
sai_serialize_object_id(ids[it]).c_str());
}
}

return status;
}
break;

case SAI_COMMON_API_BULK_REMOVE:

{
std::vector<sai_object_id_t> ids(object_count);

for (uint32_t it = 0; it < object_count; it++)
{
ids[it] = translate_local_to_redis(local_ids[it]);
}

status = m_sai->bulkRemove(object_type, object_count, ids.data(), mode, statuses.data());
}
break;

case SAI_COMMON_API_BULK_SET:

{
std::vector<sai_object_id_t> ids(object_count);

for (uint32_t it = 0; it < object_count; it++)
{
ids[it] = translate_local_to_redis(local_ids[it]);
}

std::vector<sai_attribute_t> attr_list;

// route can have multiple attributes, so we need to handle them all
for (const auto &alist: attributes)
{
attr_list.push_back(alist->get_attr_list()[0]);
}

status = m_sai->bulkSet(object_type, object_count, ids.data(), attr_list.data(), mode, statuses.data());
}
break;

case SAI_COMMON_API_BULK_GET:

{

Check notice

Code scanning / CodeQL

Long switch case Note

Switch has at least one case that is too long:
SAI_COMMON_API_BULK_CREATE (52 lines)
.
Switch has at least one case that is too long:
SAI_COMMON_API_BULK_GET (32 lines)
.
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants