From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick Wildt Date: Fri, 2 Aug 2019 21:26:39 +0200 Subject: [U-Boot] [PATCH 2/2] efi_loader: set the dhcp ack received flag In-Reply-To: <20190410092453.GA96737@thor.local> References: <20180327122405.GB94872@nyx.local> <51ad49ad-05ec-c502-5b16-5abc61418413@gmx.de> <141d2897-07ae-f1ef-bb01-83cd49d792fd@suse.de> <20190131142525.GA37117@nyx.local> <22d7dfc2-6013-00e4-b477-0c7d4151dceb@suse.de> <20190131145423.GA37343@nyx.local> <12c99515-030e-3cd6-1eca-cb839d47b00b@gmx.de> <20190410092035.GA96616@thor.local> <20190410092453.GA96737@thor.local> Message-ID: <20190802192639.GA4631@nyx.fritz.box> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > #include > #include > +#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?