[syslinux] [PATCH 0/1] Network UEFI PXE DHCP/proxyDHCP fix

Gene Cumm gene.cumm at gmail.com
Thu Jun 11 19:17:23 PDT 2015


On Thu, Jun 11, 2015 at 4:02 PM,  <jeff_sloan at selinc.com> wrote:
> from: Jeff Sloan <jeff_sloan at selinc.com>
>
> Update UEFI PXE proxyDHCP handling.
>
> This patch is based on commit ID 8a00e49
>
> Modify two files to specify valid ip addresses. These files are efi/pxe.c
> and efi/udp.c.

1) In commit 2e266c35, I proposed using UseDefaultAddress.  As I
recall, there were clients that didn't utilize the default routing
otherwise.  This may require more code to distinguish subnet-local
versus remote.  Looks like the thread starting at
http://www.syslinux.org/archives/2013-November/021039.html

2) Why not touch efi/tcp.c also which also uses UseDefaultAddress?

> In efi/pxe.c: In net_parse_dhcp function.
>
> If ProxyOffer has been received, start with DhcpAck packet since it is the
> most complete. This requires a minimum of changes to the packet except that
> it contains NULL server ip so populate server from ProxyOffer packet. The
> information is loaded into pkt_v4 and then parsed.
>
> In efi/udp.c: Both updates are in core_udp_connect and core_udp_sendto
> functions.
>
> 1. Disable UseDefaultAddress, which is not complete.

Were you able to do a packet capture?  Why is it incomplete on your systems?

> 2. Set StationAddress and SubnetMask to correct values from IPInfo struct
> because both fields are not necessarily populated.
>
> Signed-off-by: Jeff Sloan <jeff_sloan at selinc.com>
> ---
> diffstat results:
>  pxe.c |   16 +++++++++++++++-
>  udp.c |   12 ++++++++++--
>  2 files changed, 25 insertions(+), 3 deletions(-)
>
> Common subdirectories: orig_repo/syslinux-master/efi/i386 and
> syslinux-master/efi/i386
> diff -up orig_repo/syslinux-master/efi/pxe.c syslinux-master/efi/pxe.c
> --- orig_repo/syslinux-master/efi/pxe.c        2015-06-09 02:50:15.000000000
> -0700
> +++ syslinux-master/efi/pxe.c        2015-06-10 13:45:29.000000000 -0700
> @@ -150,7 +150,21 @@ void net_parse_dhcp(void)
>      if (mode->PxeReplyReceived)
>          pkt_v4 = &mode->PxeReply.Dhcpv4;
>      else if (mode->ProxyOfferReceived)
> -        pkt_v4 = &mode->ProxyOffer.Dhcpv4;
> +        {
> +       /*
> +        * Starting with DhcpAck packet to get all ip addresses except
> server ip
> +        * which can be pulled from the ProxyOffer packet.
> +        */
> +       pkt_v4 = &mode->DhcpAck.Dhcpv4;
> +
> +       /*
> +        * Grab the proxyserver ip from ProxyOffer packet
> +        */
> +       pkt_v4->BootpSiAddr[0] = mode->ProxyOffer.Dhcpv4.BootpSiAddr[0];
> +       pkt_v4->BootpSiAddr[1] = mode->ProxyOffer.Dhcpv4.BootpSiAddr[1];
> +       pkt_v4->BootpSiAddr[2] = mode->ProxyOffer.Dhcpv4.BootpSiAddr[2];
> +       pkt_v4->BootpSiAddr[3] = mode->ProxyOffer.Dhcpv4.BootpSiAddr[3];
> +    }
>
>      if (pkt_v4)
>          parse_dhcp(pkt_v4, pkt_len);

1) Why modify the packet instead of parsing that data later?

2) We still need file/BootpBootFile in addition to siaddr/BootpSiAddr.

3) Perhaps we need to be more selective about parsing, perhaps adding
a new parameter to parse_dhcp() specifying what packet it is,
indicating what pieces are important.

> diff -up orig_repo/syslinux-master/efi/udp.c syslinux-master/efi/udp.c
> --- orig_repo/syslinux-master/efi/udp.c        2015-06-09 02:50:15.000000000
> -0700
> +++ syslinux-master/efi/udp.c        2015-06-02 05:54:11.000000000 -0700
> @@ -150,11 +150,15 @@ void core_udp_connect(struct pxe_pvt_ino
>      /* Re-use the existing local port number */
>      udata.StationPort = socket->net.efi.localport;
>
> -    udata.UseDefaultAddress = TRUE;
> +    udata.UseDefaultAddress = FALSE;
>      memcpy(&udata.RemoteAddress, &ip, sizeof(ip));
>      udata.RemotePort = port;
>      udata.AcceptPromiscuous = TRUE;
>      udata.TimeToLive = 64;
> +    ip = IPInfo.myip;
> +    memcpy(&udata.StationAddress, &ip, sizeof(ip));
> +    ip = IPInfo.netmask;
> +    memcpy(&udata.SubnetMask, &ip, sizeof(ip));
>
>      status = core_udp_configure(udp, &udata, L"core_udp_connect");
>      if (status != EFI_SUCCESS) {
> @@ -372,11 +376,15 @@ void core_udp_sendto(struct pxe_pvt_inod
>      /* Re-use the existing local port number */
>      udata.StationPort = socket->net.efi.localport;
>
> -    udata.UseDefaultAddress = TRUE;
> +    udata.UseDefaultAddress = FALSE;
>      memcpy(&udata.RemoteAddress, &ip, sizeof(ip));
>      udata.RemotePort = port;
>      udata.AcceptPromiscuous = TRUE;
>      udata.TimeToLive = 64;
> +    ip = IPInfo.myip;
> +    memcpy(&udata.StationAddress, &ip, sizeof(ip));
> +    ip = IPInfo.netmask;
> +    memcpy(&udata.SubnetMask, &ip, sizeof(ip));
>
>      status = core_udp_configure(udp, &udata, L"core_udp_sendto");
>      if (status != EFI_SUCCESS)
> Common subdirectories: orig_repo/syslinux-master/efi/x86_64 and
> syslinux-master/efi/x86_64

1) This will need some more testing/review.  Users that had issues
prior to my commit should be involved.  There will likely be more code
needed.

2) Why not just reverse the previous commit?

-- 
-Gene


More information about the Syslinux mailing list