From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick Wildt Date: Thu, 31 Jan 2019 15:54:23 +0100 Subject: [U-Boot] [PATCH 2/2] efi_loader: set the dhcp ack received flag In-Reply-To: <22d7dfc2-6013-00e4-b477-0c7d4151dceb@suse.de> 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> Message-ID: <20190131145423.GA37343@nyx.local> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > > > > --- > > > > > 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 ... I agree, but I have to say that I don't know the u-boot code enough to know that. > > +} > > + > > /** > > * 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));