microsoft / net-offloads

Specs for new networking hardware offloads.
MIT License
27 stars 3 forks source link

[QEO] connection status types #63

Open mtfriesen opened 1 year ago

mtfriesen commented 1 year ago

Should we continue to use the NDIS_STATUS data type to report the result of each connection offload attempt? There are two major downsides:

  1. If this project is undocked, it means we need to integrate any new failure statuses/reasons into both NDIS and NT header files.
  2. NDIS status types, in particular, require an egregious number of transformations to be reported to the standard user mode error type: NDIS_STATUS -> NTSTATUS -> WIN32 error -> HRESULT. There is bound to be loss of fidelity.

If we define our own failure reasons, it will simplify the end-to-end control path flow and perhaps simplify error-handling, too.

anrossi commented 1 year ago

That's a good point. If we define our own errors, I think we should try to line them up so that the lower two bytes are easily convertible to HRESULT or WINERROR, maybe they're just the same as the standard WIN32 errors and the higher bytes can be used to define whether it's a standard error, or a hardware-specific error?

mtfriesen commented 1 year ago

If we define our own errors for per-connection status, I'd argue we shouldn't use HRESULTs in those APIs, and treat them as a "failure reason" rather than an "error". We should make an effort to avoid using reserved bits that would make it difficult for them to reuse with HRESULTs, for sure.

anrossi commented 1 year ago

ok, I am willing to hear further arguments, but if nothing comes in by tomorrow, let's go with defining our own errors. This sounds like a better design.