@Laszlo
Thanks for regression test!
I need to correct the typo in commit msg, (bit 3 -> bit 2).
Code logic is not affected.
@Michael
I am now wondering whether bit 3 is actually relevant to server choice.
Bit 3:
== 0 -> prompt user to choose a boot file. Which means to me: show
minimal menu with prompt (tag 10 - PXE_MENU_PROMPT) and options (tag 9 -
PXE_BOOT_MENU).
== 1 -> do not prompt user. If boot file name is present (option 67),
download that boot file.
Bit 3 does not seem to specify/regulate which server to use.
Choice of server IP might look like:
if (option 43 is present, tag 6 is present, tag_6.bit_2 is set and tag 8
is present and valid)
take server IP from tag 8 (PXE_BOOT_SERVERS)
else if (option 66 is present)
take server IP from option 66 (TFTP server name)
else if (option 54 is present)
take server IP from option 54 (Server Identifier)
else
failure
Looking forward to discussion.
Thanks,
Maciej
On 19-Aug-20 21:20, Laszlo Ersek wrote:
On 08/19/20 20:13, Laszlo Ersek wrote:
On 08/19/20 18:53, Maciej Rabeda wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2876
According to PXE 2.1 spec, DHCP option 43, tag 6 (PXE_DISCOVERY_CONTROL),
bit 3 specifies whether PXE client should use/accept TFTP servers defined
within PXE_BOOT_SERVERS list (tag 8). This bit was not being taken into
account when choosing boot server IP in PxeBcDhcp4BootInfo().
Cc: Jiaxin Wu <jiaxin...@intel.com>
Cc: Siyuan Fu <siyuan...@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rab...@linux.intel.com>
---
NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
index d062a526077b..ed9bca0f7de3 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
@@ -499,12 +499,16 @@ PxeBcDhcp4BootInfo (
//
// Parse the boot server address.
- // If prompt/discover is disabled, get the first boot server from the boot
servers list.
- // Otherwise, parse the boot server Ipv4 address from next server address
field in DHCP header.
+ // If prompt/discover is disabled, server list should be used and is present
(DHCP option 43),
+ // get the first boot server from the boot servers list.
+ // Otherwise, parse the boot server IPv4 address from next server address
field in DHCP header.
// If all these fields are not available, use option 54 instead.
//
VendorOpt = &Cache4->VendorOpt;
- if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) && IS_VALID_BOOT_SERVERS
(VendorOpt->BitMap)) {
+ if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) &&
+ IS_VALID_BOOT_SERVERS (VendorOpt->BitMap) &&
+ IS_ENABLE_USE_SERVER_LIST (VendorOpt->DiscoverCtrl))
+ {
Entry = VendorOpt->BootSvr;
if (VendorOpt->BootSvrLen >= sizeof (PXEBC_BOOT_SVR_ENTRY) && Entry->IpCnt
> 0) {
CopyMem (
I'm still undecided whether option#43 / tag#6 / bit#3 being clear means
we should *ignore* PXE_BOOT_SERVERS (tag#8), but I'm willing to defer to
you on that. So, I can give a cautious
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
for this patch.
Regarding the feature freeze -- in theory, this is a bugfix. However,
before we merge it, it would be really nice to get feedback from the
original reporter (CC'd).
I also intend to regression-test this patch, I'll report back.
I've repeated my usual PXEv4 boot tests (using OVMF IA32X64 and
ArmVirtQemu AARCH64 guests, covering shim + grub + vmlinuz + initrd);
nothing seems to have regressed.
Regression-tested-by: Laszlo Ersek <ler...@redhat.com>
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#64491): https://edk2.groups.io/g/devel/message/64491
Mute This Topic: https://groups.io/mt/76290910/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
