On 04/21/2016 09:02 PM, Andrei Borzenkov wrote: > On Thu, Apr 21, 2016 at 11:36 AM, chen zhihui <[email protected]> wrote: >> From: chenzhihui <[email protected]> >> >> Bootstrap server ip address and boot file name maybe come from >> a separate proxy DHCP server, check the proxy_offer packet if >> failed with dhcp_ack packet. >> > Yes, this came up before. Thank you for a patch. This is likely > post-2.02 material though. Thanks for your comments! > >> Signed-off-by: chenzhihui <[email protected]> >> Tested-by: Jerome Forissier <[email protected]> >> --- >> grub-core/net/bootp.c | 170 >> ++++++++++++++++++++++++++++++++++++- >> grub-core/net/drivers/efi/efinet.c | 23 ++++- >> include/grub/net.h | 10 +++ >> 3 files changed, 200 insertions(+), 3 deletions(-) >> >> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c >> index 4fdeac3..52f4051 100644 >> --- a/grub-core/net/bootp.c >> +++ b/grub-core/net/bootp.c >> @@ -186,7 +186,7 @@ grub_net_configure_by_dhcp_ack (const char *name, >> } >> #endif >> >> - if (size > OFFSET_OF (boot_file, bp)) >> + if (size > OFFSET_OF (boot_file, bp) && bp->boot_file[0]) >> grub_env_set_net_property (name, "boot_file", bp->boot_file, >> sizeof (bp->boot_file)); >> if (is_def) >> @@ -233,7 +233,7 @@ grub_net_configure_by_dhcp_ack (const char *name, >> } >> } >> >> - if (size > OFFSET_OF (boot_file, bp) && path) >> + if (size > OFFSET_OF (boot_file, bp) && bp->boot_file[0] && path) >> { >> *path = grub_strndup (bp->boot_file, sizeof (bp->boot_file)); >> grub_print_error (); >> @@ -263,6 +263,172 @@ grub_net_configure_by_dhcp_ack (const char *name, >> return inter; >> } >> >> +struct dhcp4_packet_option { >> + grub_uint8_t code; >> + grub_uint8_t length; >> + grub_uint8_t data[0]; >> +}; >> + >> +/* >> + * Get specified option from DHCP extension data >> + * >> + * from PxeBcDhcp.c of UEFI >> + * >> + */ >> +static struct dhcp4_packet_option * >> +dhcp_proxy_extension_option (const grub_uint8_t *buf, >> + grub_size_t size, >> + grub_uint8_t code) >> +{ >> + struct dhcp4_packet_option *option = (struct dhcp4_packet_option >> *)buf; >> + grub_size_t offset = 0; >> + >> + while (offset < size && option->code != GRUB_NET_BOOTP_END) { > OK, could you please rebase it on top of > http://lists.gnu.org/archive/html/grub-devel/2016-03/msg00280.html. Of > course comments to this patch are welcome. Without comments it will > land here at some point and it implements DHCP options processing > already. If you find it easier, I can create branch with this patch. No thanks, maybe I'll send v2 patch when your patch is accepted by upstream. > ... >> +void >> +grub_net_configure_by_proxy_offer (const struct grub_net_bootp_packet *bp, >> + grub_size_t size, >> + char **device, >> + char **path) >> +{ >> + const grub_uint8_t *buf = bp->vendor + sizeof (grub_uint32_t); >> + grub_uint32_t option_size = >> + size - OFFSET_OF(vendor, bp) - sizeof (grub_uint32_t); >> + struct dhcp4_packet_option *option; >> + >> + if (device == NULL) >> + return; >> + >> + if (!proxy_offer_is_valid(bp, size)) >> + return; >> + >> + if (!*device && bp->server_ip) >> + { >> + *device = grub_xasprintf ("tftp,%d.%d.%d.%d", >> + ((grub_uint8_t *) &bp->server_ip)[0], >> + ((grub_uint8_t *) &bp->server_ip)[1], >> + ((grub_uint8_t *) &bp->server_ip)[2], >> + ((grub_uint8_t *) &bp->server_ip)[3]); >> + grub_print_error (); >> + } >> + >> + option = dhcp_proxy_extension_option(buf, >> + option_size, GRUB_NET_DHCP_OVERLOAD); >> + >> + if ((option == NULL || option->data[0] == 1) && !*device && >> bp->server_name[0]) >> + { >> + *device = grub_xasprintf ("tftp,%s", bp->server_name); >> + grub_print_error (); >> + } >> + >> + if (!*device) >> + { >> + option = dhcp_proxy_extension_option(buf, >> + option_size, GRUB_NET_DHCP_SERVER_ID); >> + >> + if (option) { >> + *device = grub_xasprintf("tftp,%d.%d.%d.%d", >> + option->data[0], >> + option->data[1], >> + option->data[2], >> + option->data[3]); > DHCP_SERVER_ID (option 54) is defined for DHCP client/server > communication only. There is no implied usage of this option as next > server (i.e. boot server) so I do not think this is correct. Yes, I'll remove this in v2. > > ... >> diff --git a/grub-core/net/drivers/efi/efinet.c >> b/grub-core/net/drivers/efi/efinet.c >> index 5388f95..ef0ccd9 100644 >> --- a/grub-core/net/drivers/efi/efinet.c >> +++ b/grub-core/net/drivers/efi/efinet.c >> @@ -338,6 +338,7 @@ grub_efi_net_config_real (grub_efi_handle_t hnd, char >> **device, >> FOR_NET_CARDS (card) >> { >> grub_efi_device_path_t *cdp; >> + struct grub_net_network_level_interface *inter; >> struct grub_efi_pxe *pxe; >> struct grub_efi_pxe_mode *pxe_mode; >> if (card->driver != &efidriver) >> @@ -378,11 +379,31 @@ grub_efi_net_config_real (grub_efi_handle_t hnd, char >> **device, >> if (! pxe) >> continue; >> pxe_mode = pxe->mode; >> - grub_net_configure_by_dhcp_ack (card->name, card, 0, >> + inter = grub_net_configure_by_dhcp_ack (card->name, card, 0, >> (struct grub_net_bootp_packet *) >> &pxe_mode->dhcp_ack, >> sizeof (pxe_mode->dhcp_ack), >> 1, device, path); >> + >> + /* >> + * Bootstrap server ip address and file name maybe >> + * come from a separate proxy DHCP server, >> + * so check the proxy_offer DHCP packet >> + * >> + */ >> + if (inter && *path == NULL) { >> + if (*device) { >> + grub_free(*device); >> + *device = NULL; >> + } >> + >> + grub_net_configure_by_proxy_offer( >> + (struct grub_net_bootp_packet >> *)&pxe_mode->proxy_offer, > Please check ProxyOfferReceived as required by UEFI. I know we do not > do it currently but let's do it right at least in new code. You're right, I'll change in v2 > >> + sizeof (pxe_mode->proxy_offer), >> + device, >> + path); >> + } >> + > Well, this opens up the question about precedence of data from > different packets. With my patch (that abstracts away options > processing) we can actually simply pass both packets at once and fetch > IP information from (original) DHCPACK and other information from > proxy DHCPOFFER as required (falling back to DHCPACK if proxy is > NULL). Great! If you can do that in your v3 patch, I'll has nothing to do. >> return; >> } >> } >> diff --git a/include/grub/net.h b/include/grub/net.h >> index b62643a..f107a23 100644 >> --- a/include/grub/net.h >> +++ b/include/grub/net.h >> @@ -433,6 +433,10 @@ enum >> GRUB_NET_BOOTP_DOMAIN = 0x0f, >> GRUB_NET_BOOTP_ROOT_PATH = 0x11, >> GRUB_NET_BOOTP_EXTENSIONS_PATH = 0x12, >> + GRUB_NET_DHCP_OVERLOAD = 0x34, >> + GRUB_NET_DHCP_SERVER_ID = 0x36, >> + GRUB_NET_DHCP_CLASS_ID = 0x3c, >> + GRUB_NET_DHCP_BOOTFILE = 0x43, > Please use decimal option numbers for any new option you add. They are > defined as decimal in RFC and it makes it easier to cross-reference > with RFC later.
OK. -- ========================================== Best Regards 陈志辉(Chenzhihui) 华为海思图灵软件架构组 Huawei Hisilicon Turing Software Architecture Team http://www.hisilicon.com/ _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
