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

Heinrich Schuchardt xypron.glpk at gmx.de
Thu Jan 31 18:29:04 UTC 2019


On 1/31/19 3:54 PM, Patrick Wildt wrote:
> On Thu, Jan 31, 2019 at 03:31:34PM +0100, Alexander Graf wrote:
>> 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)

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.

>>> +{
>>> +	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 ...
> 
> I agree, but I have to say that I don't know the u-boot code enough to
> know that.

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.

Best regards

Heinrich

> 
>>> +}
>>> +
>>>   /**
>>>    * 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
> 
> Turns out I sent an older diff.  I had made that change, but I forgot
> to append the right one.  Sorry for the mishap.
> 
> Patrick
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 53f08161ab..4f131e52c2 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);
>  
> @@ -559,6 +561,7 @@ 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_set_dhcp_ack_received(void) { }
>  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..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;
> +}
> +
>  /**
>   * 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();
>  			dhcp_state = BOUND;
>  			printf("DHCP client bound to address %pI4 (%lu ms)\n",
>  			       &net_ip, get_timer(bootp_start));
> 



More information about the U-Boot mailing list