-
Notifications
You must be signed in to change notification settings - Fork 447
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
Dpdk backend: Fix dpdk test infrastructure to enable comparison of error output #3680
Conversation
Does -f work for dpdk successful and failed tests with this PR? |
Should I merge this? |
Yes. I verified by manually running few tests. |
@hesingh Any other comments/questions? |
@@ -0,0 +1,167 @@ | |||
{ | |||
"schema_version" : "1.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to require checking in expected .bfrt.json files for programs that you expect an error message for? Do you expect to get much test coverage out of such expected output files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can avoid this check. I will update
[--Werror=unexpected] error: execute method for different DirectMeter instances (meter0_0 and meter1_0) called within same action | ||
pna-dpdk-direct-meter-err-1.p4(116): [--Werror=target-error] error: Direct counters and direct meters are unsupported for wildcard match table ipv4_da_lpm | ||
table ipv4_da_lpm { | ||
^^^^^^^^^^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this error message accurate? That is, does DPDK restrict direct counters and meters to ONLY be used on tables where all key fields are match_kind exact? If that is a restriction for the DPDK implementation, why? It seems like a strange restriction to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes Andy. Currently, dpdk target supports direct meter and direct counter only for exact match tables (all keys with match kind exact). We will remove this check as and when the target support is added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to merge to me. My comments are minor, and can be addressed later after merge, if it is determined they are significant comments (which they might not be).
No, thanks. Good to go. |
Fixes #3640
The changes in this PR includes: