Page MenuHomeFreeBSD

loader: add HTTP support using UEFI
ClosedPublic

Authored by bcran on Jun 14 2019, 8:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 13, 7:03 AM
Unknown Object (File)
Wed, Nov 13, 4:10 AM
Unknown Object (File)
Sun, Nov 3, 10:01 PM
Unknown Object (File)
Thu, Oct 31, 8:10 AM
Unknown Object (File)
Tue, Oct 29, 9:41 PM
Unknown Object (File)
Sun, Oct 27, 7:50 AM
Unknown Object (File)
Wed, Oct 23, 10:33 AM
Unknown Object (File)
Oct 3 2024, 9:43 AM
Subscribers

Details

Summary

Add support for an HTTP "network filesystem" using the UEFI's HTTP
stack.

This also supports HTTPS, but TianoCore EDK2 implementations currently crash while fetching loader files.
Only IPv4 is supported at the moment. IPv6 support is planned for a follow-up changeset.

Note that we include some headers from the TianoCore EDK II project in stand/efi/include/Protocol verbatim, including links to the license instead of including the full text because that's their preferred way of communicating it, despite not being normal FreeBSD project practice.

Test Plan
  • Do UEFI PXE boot, see that it doesn't break - DONE.
  • Do UEFI HTTP boot, see that loader.conf &c can be fetched over http - DONE.
  • Do UEFI boot on system that doesn't support HTTP booting, ensure it doesn't hang trying to open the http protocol - DONE
  • Send out a Call For Testing to freebsd-current ML - DONE.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I thought I had a computer capable of doing uefi http but I was wrong. UEFI PXE works fine though.

I realized one issue: we should perhaps add a configuration option such as LOADER_EFIHTTP_SUPPORT to let people disable this.

This looks pretty close, but there are still a couple of issues that need to be resolved before the commit.

stand/efi/include/Protocol/Http.h
13 ↗(On Diff #58638)

I'd note in your commit message that several files were pulled in verbatim from EDK2 and they contain pointers to a URL for the license. And that we're committing them verbatim because it's the vendor's preferred method of communicating the license, even though it's a bit at odds with normal project practice.

stand/efi/include/efidevp.h
299 ↗(On Diff #58638)

I'd use C style comments here, and below. This file is a FreeBSD file.

stand/efi/include/efilib.h
47 ↗(On Diff #58638)

this seems misplaced.... why is it here? Others are in stand.h

stand/efi/libefi/efihttp.c
279 ↗(On Diff #58638)

checked too late

339 ↗(On Diff #58638)

error handling above seems thin.

443 ↗(On Diff #58638)

host not checked.
Also, consider using strdup instead.

447 ↗(On Diff #58638)

assumes c is non-null.

481 ↗(On Diff #58638)

Is there some way to time-bound this wait? And the one below?

500–501 ↗(On Diff #58638)

wouldn't it be safer to set this BEFORE you call CreateEvent? This pattern is used several times and I'll just comment once.

stand/efi/loader/conf.c
58 ↗(On Diff #58638)

All these other fsops are defined in stand.h

stand/efi/loader/conf.c
58 ↗(On Diff #58638)

stand.h looks to be the right place for extern struct fs_ops efihttp_fsops, but this seems to be the right place for the &efihttp_fsops?

stand/efi/libefi/efihttp.c
481 ↗(On Diff #58638)

Yes, I'll add a call to BS->Stall() and wait 10 seconds.

500–501 ↗(On Diff #58638)

The done? Or calling __compiler_membar()?
I'll move the done up before the CreateEvent.

stand/efi/libefi/efihttp.c
481 ↗(On Diff #58638)

I'd cap the the upper bound to be some variable that can be set.

500–501 ↗(On Diff #58638)

shouldn't we set done to false, forcing it to be false before we create a task/thread that might change it?

stand/efi/loader/conf.c
58 ↗(On Diff #58638)

Correct on both accounts.

stand/efi/libefi/efihttp.c
481 ↗(On Diff #58638)

Given that the other network code doesn't do that, and was instead taking over an hour to timeout, I think simply capping it at 10 seconds is sufficient for now.

500–501 ↗(On Diff #58638)

Yes, I agree - I'll fix it.

stand/efi/libefi/efihttp.c
481 ↗(On Diff #58638)

Some reasonable upper limit is needed for how long the network waits. Where else do we wait an unbounded amount of time?

stand/efi/libefi/efihttp.c
481 ↗(On Diff #58638)

We used to take a really long time for send/recv in stand/libsa/net.c to timeout. From https://svnweb.freebsd.org/base/head/stand/libsa/net.c?revision=342282&view=markup :P

Wait a maximum of 300 seconds for network send/recv in libsa

The reason for this change is that currently, a send/recv takes many hours to time out.
This is suboptimal in the bootloader because it means for example that NFS will take hours to fail before allowing subsequent access methods such as gzip to be tried.

Setting MAXWAIT to 300 seconds (5 minutes) still allows slow connections of 1Mb to be used to download a 30MB kernel file.

bcran removed a reviewer: bcran.
bcran edited the summary of this revision. (Show Details)
  • loader: add HTTP support using UEFI
  • Updates per review feedback.
bcran marked 12 inline comments as done.Jun 24 2019, 3:44 AM
  • Fixed a couple more issues
bcran edited the summary of this revision. (Show Details)

Looks a lot better. Found one minor thing...

stand/efi/libefi/efihttp.c
736 ↗(On Diff #58960)

no error handling here...

  • Added error handling around realloc.

I'm tapped out. Can't find anything else.

This revision is now accepted and ready to land.Jun 24 2019, 7:45 PM
This revision was automatically updated to reflect the committed changes.