Re: [PATCH v2] bootp: add native DHCPv4 support

[Punit Agrawal <punit.agrawal@arm.com>]

Hi Andrei,

Thanks for refreshing the patch. I was using a locally fixed up version
of your wip post from earlier in the year.

On 22/03/16 18:26, Andrei Borzenkov wrote:
> This patch adds support for native DHCPv4 and removes requirement for
> BOOTP compatibility support in DHCP server.
>
> There is no provision for selecting preferred server. We take the first
> OFFER and try to REQUEST configuration from it. If NAK was received,
> transaction is restarted, but if we hit timeout, configuration fails.
>
> It also handles pure BOOTP reply (detected by lack of DHCP message type
> option) and proceeds with configuration immedately. I could not test it
> because I do not have pure BOOTP server.
>
> Because we need access to DHCP options in multiple places now, it also
> factors out DHCP option processing, adding support for option overload.
>
> Timeout handling is now per-interface, with independent timeouts for
> both DISCOVER and REQUEST. It should make it more responsive if answer
> was delayed initially. Total timeout remains the same as originally, as
> well as backoff algorithm, but we poll in fixed 200ms ticks so notice
> (successful) reply earlier.
>
> Failure to send packet to interface now does not terminate command (and
> leaks memory) but rather simply ignores this interface and continues with
> remaining ones if present.
>
> Finally it adds net_dhcp alias with intention to deprecate net_bootp
> completely (it would be possible to make net_bootp to actually send BOOTP
> packet if someone thinks it is required).
>

The patch works for me on an arm64 board. With the patch, the board is
able to get an IP via DHCP and subsequently is able to load a kernel
image via tftp.

I haven't tested the bootp functionality as I don't have a DHCP server I
can control.

> Features not implements:
>
> - DHCP server selection. We take first DHCPOFFER or BOOTPREPLY. No plans
> to implement.
>
> - DHCP option concatenation (RFC3396). I do not expect to hit it in real
> life and it rather complicates implementation, so let's wait for actual
> use case.
>
> - client identifier (RFC6842). So far we expect valid MAC address, which
> should be enough to uniquely identify client. It is also not clear how to
> generate unique client identifier if we ever need one.


The unimplemented features don't seem to be an issue in my environment.

It would be great to see this functionality merged and working out of
the box.

Thanks for your efforts.

Punit

>
> v2: change find_dhcp_option to use subscripts to avoid signed/unsigned
>      comparison warning.
>
>      Should fix "may be used uninitialized" warning (although I could not
>      reproduce it).
>
> ---
>   grub-core/net/bootp.c | 759 ++++++++++++++++++++++++++++++++++----------------
>   grub-core/net/ip.c    |   2 +-
>   include/grub/net.h    |  14 +-
>   3 files changed, 528 insertions(+), 247 deletions(-)
>

[...]
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.