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.

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

[PATCH v1] validation: cls: add capability check for hash result test #2183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[PATCH v1] validation: cls: add capability check for hash result test #2183

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
6 changes: 4 additions & 2 deletions test/validation/api/classification/odp_classification_basic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,6 @@ static void cls_create_cos_with_hash_queues(void)

ret = odp_cls_capability(&capa);
CU_ASSERT_FATAL(ret == 0);
CU_ASSERT_FATAL(capa.hash_protocols.all_bits != 0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message lacks any mention of this change. And this change looks wrong anyway, since this test function is run only when multiple hash queues are supported and that in turn implies that there must be at least one protocol field included in the hash.


odp_queue_param_init(&q_param);
q_param.type = ODP_QUEUE_TYPE_SCHED;
Expand All @@ -733,6 +732,9 @@ static int check_capa_cos_hashing(void)
if (odp_cls_capability(&capa) != 0)
return ODP_TEST_INACTIVE;

if (capa.hash_protocols.all_bits == 0)
return ODP_TEST_INACTIVE;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking the value of max_hash_queues is sufficient for testing if hash is supported. There should not be a case where max_hash_queues is greater than one but hash_protocols.all_bits is zero.


return capa.max_hash_queues > 1 ? ODP_TEST_ACTIVE : ODP_TEST_INACTIVE;
}

Expand Down Expand Up @@ -835,7 +837,7 @@ odp_testinfo_t classification_suite_basic[] = {
ODP_TEST_INFO(cls_cos_set_pool),
ODP_TEST_INFO(cls_pmr_composite_create),
ODP_TEST_INFO_CONDITIONAL(cls_create_cos_with_hash_queues, check_capa_cos_hashing),
ODP_TEST_INFO(cls_hash_result_single_queue),
ODP_TEST_INFO_CONDITIONAL(cls_hash_result_single_queue, check_capa_cos_hashing),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cls_hash_result_single_queue() is intended to be run even if hashing is not supported. In that odp_cls_hash_result() is supposed to return the single queue configured for the CoS. The spec for odp_cls_hash_result() does not say that it depends on hash capability and it appears that the function was meant to be usable for any CoS without the caller having to know the capabilities or the configured queues in the CoS.

ODP_TEST_INFO_CONDITIONAL(cls_hash_result_many_queues, check_capa_cos_hashing),
ODP_TEST_INFO_NULL,
};