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

[Alexander Graf]

On 01/31/2019 03:25 PM, Patrick Wildt wrote:
> On Sun, Dec 02, 2018 at 10:21:12PM +0100, Alexander Graf wrote:
>> On 27.03.18 18:05, Heinrich Schuchardt wrote:
>>> On 03/27/2018 02:24 PM, Patrick Wildt wrote:
>>>> The PXE object contains a flag that specifies whether or not a DHCP
>>>> ACK has been received.  This can be used by EFI Applications to find
>>>> out whether or not it is worth to read the DHCP information from our
>>>> object.
>>>>
>>>> Signed-off-by: Patrick Wildt <patrick at blueri.se>
>>>> ---
>>>>   lib/efi_loader/efi_net.c | 4 +++-
>>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
>>>> index 8c5d5b492c..0b9c7b9345 100644
>>>> --- a/lib/efi_loader/efi_net.c
>>>> +++ b/lib/efi_loader/efi_net.c
>>>> @@ -332,8 +332,10 @@ int efi_net_register(void)
>>>>   	netobj->net_mode.max_packet_size = PKTSIZE;
>>>>   
>>>>   	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 = 1;
>>>> +	}
>>> We have received a DHCPOFFER and we now send a DHCPREQUEST to the
>>> selected server. This is when efi_net_set_dhcp_ack() is called which
>>> sets the variable dhcp_ack.
>>>
>>> If the server sustains its offer it responds with a DHCPACK or with a
>>> DHCPNACK. Shouldn't we ensure a DHCPACK was received (and not a DHCNACK)
>>> before setting dhcp_ack_received?
>> Patrick, ping? :)
>>
>> Alex
> Sorry, I had a bit of other stuff to take care of and didn't get to it.
> Turns out the change is rather easy to do, since we can just add another
> function in the EFI subsystem to call once we receive the actual ACK.
> This version works for me.
>
> Patrick
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 53f08161ab..3dcfc4b16f 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -285,6 +285,8 @@ 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 we got an ack */
> +void efi_net_set_dhcp_ack_received(void);
>   /* Called by efi_set_watchdog_timer to reset the timer */
>   efi_status_t efi_set_watchdog(unsigned long timeout);
>   
> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
> index c7d9da8521..96610c768e 100644
> --- a/lib/efi_loader/efi_net.c
> +++ b/lib/efi_loader/efi_net.c
> @@ -15,6 +15,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
> @@ -536,6 +537,16 @@ void efi_net_set_dhcp_ack(void *pkt, int len)
>   	memcpy(dhcp_ack, pkt, min(len, maxsize));
>   }
>   
> +/**
> + * efi_net_set_dhcp_ack_received() - take note of a received ACK
> + *
> + * This function is called by dhcp_handler().
> + */
> +void efi_net_set_dhcp_ack_received(void)
> +{
> +	dhcp_ack_received = 1;

Is there any way to pass an object reference in as parameter that allows 
us to find the exact efi net object back again? IIRC the network code 
was pretty much single target, but maybe we don't have to encode that in 
every single place ...

> +}
> +
>   /**
>    * efi_net_push() - callback for received network packet
>    *
> @@ -645,8 +656,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..b0c26418ca 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -1087,6 +1087,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>   			dhcp_packet_process_options(bp);
>   			/* Store net params from reply */
>   			store_net_params(bp);
> +			efi_net_set_dhcp_ack_received();

Won't this fail to compile with !CONFIG_EFI_LOADER?

Alex

>   			dhcp_state = BOUND;
>   			printf("DHCP client bound to address %pI4 (%lu ms)\n",
>   			       &net_ip, get_timer(bootp_start));