On Thu, Apr 21, 2016 at 11:36 AM, chen zhihui <chenzhihui...@msn.com> wrote: > From: chenzhihui <chenzhihui...@msn.com> > > 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. > Signed-off-by: chenzhihui <chenzhih...@huawei.com> > Tested-by: Jerome Forissier <jerome.foriss...@linaro.org> > --- > 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. ... > +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. ... > 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. > + 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). > 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. _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel