NetworkPkg: UefiPxeBcDxe: Fix error packet detection

[dbnicholson]

Description

Per RFC 1350, TFTP error packets include 2 byte OpCode and ErrorCode fields in network byte order. Those need to be swapped to host order to be interpreted correctly. Without this change, the TftpErrorReceived and TftpError Mode fields are never set and EFI applications can't inspect the error received from the TFTP server.

• Breaking change?
  • Breaking change - Does this PR cause a break in build or boot behavior?
  • Examples: Does it add a new library class or move a module to a different repo.
• Impacts security?
  • Security - Does this PR have a direct security impact?
  • Examples: Crypto algorithm change or buffer overflow fix.
• Includes tests?
  • Tests - Does this PR include any explicit test code?
  • Examples: Unit tests or integration tests.

How This Was Tested

I was testing shim to see how it handles TFTP file not found errors in rhboot/shim#695. The OVMF firmware was used to boot a VM using QEMU's TFTP server support. Without this change shim would not report the specific error since TftpErrorReceived is FALSE. With this change it was able to see TftpErrorReceived is TRUE and TftpError.ErrorCode set to 1, which is file not found as expected.

Integration Instructions

N/A, will be included in the UefiPxeBcDxe package.

[dbnicholson]

Is there anything I can do to help get this reviewed? Would it help to open an issue? Should I try to create a test?

[mdkinney]

@SaloniKasbekar @Zclarkwilliams Please review

[dbnicholson]

Taking a look in bugzilla, I suspect this fixes https://bugzilla.tianocore.org/show_bug.cgi?id=2396.

[mdkinney]

May fix Issue #9842

[mdkinney]

I see the specific with rhboot has been resolved in that project. Is this change still required? Would there be any compatibility impacts?

[dbnicholson]

I see the specific with rhboot has been resolved in that project. Is this change still required? Would there be any compatibility impacts?

I think it should be. I wrote the rhboot/shim change and all it does is treat the error as "file not found" if it can't get the error details from UEFI. See rhboot/shim@6410312#diff-874c9b6e2bf4c63b0f0a443f612426773d76e4f09dd313fbe0ad3fd6d42c22d4R415. If it does have access to the error details, it will bubble up a more accurate error.

So, the issue is resolved because it will do something reasonable without this fix, but it would be better if it was.

[mdkinney]

Understood. I will merge it.