[U-Boot] [PATCH 2/2] efi_loader: set the dhcp ack received flag

[Patrick Wildt]

On Wed, Apr 10, 2019 at 11:24:53AM +0200, Patrick Wildt wrote:
> On Wed, Apr 10, 2019 at 11:20:35AM +0200, Patrick Wildt wrote:
> > On Thu, Jan 31, 2019 at 07:29:04PM +0100, Heinrich Schuchardt wrote:
> > > Do we really need multiple functions to update the DHCP status?
> > > 
> > > I would prefer a single function with a parameter telling which DHCP
> > > status has been reached.
> > > 
> > 
> > I think this diff below might be better.  There we have an update
> > function that is called after it switch the state, and on REQUESTING
> > we save the packet information and on BOUND we received the ack.
> > 
> > > 
> > > The current network device can be determined via eth_get_dev().
> > > In function eth_current_changed() this eth_get_dev() function is used to
> > > update the ethact environment variable.
> > > 
> > > If we have a single update function we can determine the active network
> > > device there.
> > 
> > The efi network object is only created on bootefi, so there is no DHCP
> > going on at that point.  It all happens some time before bootefi is
> > called.  The only thing that we could do is try to memorize for which
> > ethernet we received the DHCP packets, and when we create the EFI
> > network object we can try to see if the DHCP packets match to the
> > current ethernet we create the object for.
> > 
> > Patrick
> 
> Updated diff.  We should probably reset the DHCP Ack flag when we
> switch to the REQUEST state.  Thus on a second dhcp call, where we
> might get a different IP (on a different device) the ACK is properly
> reset.
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 00b81c6010..7e8f3b04b5 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -290,8 +290,8 @@ efi_status_t efi_smbios_register(void);
>  struct efi_simple_file_system_protocol *
>  efi_fs_from_path(struct efi_device_path *fp);
>  
> -/* Called by networking code to memorize the dhcp ack package */
> -void efi_net_set_dhcp_ack(void *pkt, int len);
> +/* Called by networking code to memorize dhcp information */
> +void efi_net_update_dhcp(int state, void *pkt, int len);
>  /* Called by efi_set_watchdog_timer to reset the timer */
>  efi_status_t efi_set_watchdog(unsigned long timeout);
>  
> @@ -578,7 +578,7 @@ static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len)
>  static inline void efi_restore_gd(void) { }
>  static inline void efi_set_bootdev(const char *dev, const char *devnr,
>  				   const char *path) { }
> -static inline void efi_net_set_dhcp_ack(void *pkt, int len) { }
> +static inline void efi_net_update_dhcp(int state, void *pkt, int len) {}
>  static inline void efi_print_image_infos(void *pc) { }
>  
>  #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
> index c7d9da8521..192e7f0bb7 100644
> --- a/lib/efi_loader/efi_net.c
> +++ b/lib/efi_loader/efi_net.c
> @@ -8,6 +8,7 @@
>  #include <common.h>
>  #include <efi_loader.h>
>  #include <malloc.h>
> +#include "../net/bootp.h"
>  
>  static const efi_guid_t efi_net_guid = EFI_SIMPLE_NETWORK_GUID;
>  static const efi_guid_t efi_pxe_guid = EFI_PXE_GUID;
> @@ -15,6 +16,7 @@ static struct efi_pxe_packet *dhcp_ack;
>  static bool new_rx_packet;
>  static void *new_tx_packet;
>  static void *transmit_buffer;
> +static int dhcp_ack_received;
>  
>  /*
>   * The notification function of this event is called in every timer cycle
> @@ -522,18 +524,24 @@ out:
>  }
>  
>  /**
> - * efi_net_set_dhcp_ack() - take note of a selected DHCP IP address
> + * efi_net_update_dhcp() - take note of DHCP information
>   *
>   * This function is called by dhcp_handler().
>   */
> -void efi_net_set_dhcp_ack(void *pkt, int len)
> +void efi_net_update_dhcp(int state, void *pkt, int len)
>  {
>  	int maxsize = sizeof(*dhcp_ack);
>  
>  	if (!dhcp_ack)
>  		dhcp_ack = malloc(maxsize);
>  
> -	memcpy(dhcp_ack, pkt, min(len, maxsize));
> +	if (state == REQUESTING) {
> +		memcpy(dhcp_ack, pkt, min(len, maxsize));
> +		dhcp_ack_received = 0;
> +	}
> +
> +	if (state == BOUND)
> +		dhcp_ack_received = 1;
>  }
>  
>  /**
> @@ -645,8 +653,10 @@ efi_status_t efi_net_register(void)
>  	netobj->net_mode.if_type = ARP_ETHER;
>  
>  	netobj->pxe.mode = &netobj->pxe_mode;
> -	if (dhcp_ack)
> +	if (dhcp_ack) {
>  		netobj->pxe_mode.dhcp_ack = *dhcp_ack;
> +		netobj->pxe_mode.dhcp_ack_received = dhcp_ack_received;
> +	}
>  
>  	/*
>  	 * Create WaitForPacket event.
> diff --git a/net/bootp.c b/net/bootp.c
> index 9a2b512e4a..987fc47d06 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -1067,11 +1067,11 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>  			    strlen(CONFIG_SYS_BOOTFILE_PREFIX)) == 0) {
>  #endif	/* CONFIG_SYS_BOOTFILE_PREFIX */
>  			dhcp_packet_process_options(bp);
> -			efi_net_set_dhcp_ack(pkt, len);
>  
>  			debug("TRANSITIONING TO REQUESTING STATE\n");
>  			dhcp_state = REQUESTING;
>  
> +			efi_net_update_dhcp(dhcp_state, pkt, len);
>  			net_set_timeout_handler(5000, bootp_timeout_handler);
>  			dhcp_send_request_packet(bp);
>  #ifdef CONFIG_SYS_BOOTFILE_PREFIX
> @@ -1090,6 +1090,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>  			dhcp_state = BOUND;
>  			printf("DHCP client bound to address %pI4 (%lu ms)\n",
>  			       &net_ip, get_timer(bootp_start));
> +			efi_net_update_dhcp(dhcp_state, pkt, len);
>  			net_set_timeout_handler(0, (thand_f *)0);
>  			bootstage_mark_name(BOOTSTAGE_ID_BOOTP_STOP,
>  					    "bootp_stop");

Ping?