[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