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@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));