[syslinux] [PATCH 0/1] EFI PXE DHCP/proxyDHCP issues fix

[Gene Cumm]

On Mon, May 25, 2015 at 8:37 AM, Patrick Masotta <masottaus at yahoo.com> wrote:
>>>>
>  >       */
>  >      Print(L"Getting cached packet ");
>  > -    parse_dhcp(&mode->DhcpDiscover.Dhcpv4, pkt_len);
>  > +   parse_dhcp(&mode->DhcpAck.Dhcpv4, pkt_len);
>
>  When do you find client identifiers like the Vendor Class Identifier
>  (option 60)?  DHCP servers generally don't respond with this data.
>
>  Could the client identifiers provide any useful data?  Likely not but
>  not sure entirely why the original author did this.  I'd probably lean
>  towards keeping it as-is for now and perhaps separating this from the
>  main reason for the patch.
> <<<
>
> From:
> C:\UDK2014.SP1.P1.Complete\MyWorkSpace\NetworkPkg\UefiPxeBcDxe\PxeBcDhcp4.c/PxeBcDhcp4CallBack()
>
> DhcpDiscover.Dhcpv4 gets populated at the "Dhcp4SendDiscover" event

Syslinux on EFI won't be sending these packets.  The underlying EFI
framework will.

> ...
>   case Dhcp4SendDiscover:
>     //
>     // Cache the DHCPv4 discover packet to mode data directly.
>     // It need to check SendGuid as well as Dhcp4SendRequest.
>     //
>     CopyMem (&Mode->DhcpDiscover.Dhcpv4, &Packet->Dhcp4, Packet->Length);
> ...
>
> Then I think DhcpDiscover.Dhcpv4 is no the right packet for parsing here..
> DhcpAck.Dhcpv4 instead will hold the required info.

DhcpAck and DhcpOffer shouldn't contain the details.  DhcpDiscover and
DhcpRequest should.

> I agree with you the author has a convoluted way for parsing the required info.
> Please remember parse_dhcp() is invoked "twice"
> The second time it gets the PXE related info probably
> not available at the first call when parsing DhcpAck.Dhcpv4

In the current code, there's three calls to parse_dhcp().

1) With DhcpDiscover.  This is intended to get the client identifiers
that the servers never bother returning like DHCP option 60 (Vendor
Class Identifier).
2) With DhcpAck.  This is intended to get the server-provided
parameters for basic network interactions like IPv4 address, subnet
mask, gateway and DNS.
3) With PxeReply.  The intention here was to grab the advanced
server-provided parameters for network booting like the file and
server to grab it from.

The code of #3 is currently wrong as it ignores ProxyOffer.  I'm not
arguing this needs to be patched.

Your patch ignores the client identifiers (call #1).

-- 
-Gene