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

[Patrick Wildt <patrick@blueri.se>]

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)
> > +{
> > +	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));