-
Notifications
You must be signed in to change notification settings - Fork 830
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
add HW error codes to tfm.h, sp_int.h, integer.h #6565
Conversation
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.
Why are we adding return codes that are not used anywhere? Why not use the error codes / values from error-crypt.h?
This is in prep for my upcoming Espressif hardware acceleration changes, PR soon. The new error codes will allow a graceful fallback to software math.
I had considered that, but I thought other platforms that might use hardware acceleration would also use these same error codes, instead of having something specific to each platform. This would allow propagation of math errors to higher levels for improved visibility as to root causes of problems. For example: I ended up working on the math acceleration only because some corner cases in the math acceleration did not work properly. The original symptom was an RSA padding error. It would have been tremendously helpful if the RSA error actually indicated there was an underlying math problem. |
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.
Please include this change with the PR that will actually use these. My preference is to use error-crypt values, not these math values.
ok. that's why this was in a separate PR: so that we could discuss issues in advance and without cluttering the big PR. I'll renumber the exit code values to include in
What do you think of the other consistency changes? (i.e. all the lines without "HW") |
Why can't you just extend and use the existing wolfCrypt error codes in error-crypt.h and error.c? |
Yes, I will (already did), good suggestion about What I am referring to is the other changes: Missing from Missing from But I suppose since they are not being used... perhaps not important. I just thought it would be good to be consistent. |
On the topic of error codes: although I was able to successfully add new values to Specifically the wolfssl/openssl/ssl.h does this:
As and we end up with
I'd rather not fudge the new hardware codes with misleading and unintuitive approximate matches to existing codes. But it looks like there's no practical room for new error codes in What do you prefer that I do here? *edit: perhaps this change in
|
/* Unused error. Defined for backward compatibility. */ | ||
#define MP_RANGE MP_NOT_INF | ||
/* hardware error, consider falling back to SW */ | ||
#define MP_HW_ERROR (-6) |
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.
Lets use the existing WC_HW_E
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.
Debug logs with WOLFSSL_MSG could be used to narrow down location of hardware error and distinguish between math hardware acceleration and others if wanted.
/* hardware error, consider falling back to SW */ | ||
#define MP_HW_ERROR (-6) | ||
/* hardware busy; wait or fall back to SW */ | ||
#define MP_HW_BUSY (-7) |
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.
Lets use the existing WC_HW_WAIT_E
/* hardware busy; wait or fall back to SW */ | ||
#define MP_HW_BUSY (-7) | ||
/* signal to caller to fall back to SW (e.g unsupported, etc) */ | ||
#define MP_HW_FALLBACK (-8) |
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.
Can NOT_COMPILED_IN or BAD_STATE_E be used instead?
Closing this lingering draft PR, as I believe this issue has been resolved in other PRs. |
Description
Adds new hardware-acceleration-specific return error code definitions to math library header files.
Also makes return codes consistent between
tfm.h
,sp_int.h
, andinteger.h
.The intention is to allow better visibility to math errors with hardware acceleration. This change will also allow the ability to fall back to SW if hardware fails, is busy, etc.
These changes are needed for my current WIP for Espressif HW acceleration.
Fixes zd# n/a
Testing
How did you test?
ran wolfcrypt benchmark and test apps.
Checklist