On 8/2/19 9:26 PM, Patrick Wildt wrote:
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 <common.h>
  #include <efi_loader.h>
  #include <malloc.h>
+#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?


The network address can be set in U-Boot by the 'dhcp' or via 'setaddr
ipaddr'.

U-Boot supports multiple network interfaces. The UEFI sub-system uses
the one that is active when one of the following commands is invoked:

efidebug, env -e, printenv -e, bootefi.

The tftp or dhcp can be invoked before or after any of these.

UEFI payloads may implement a DHCP command themselves.

So you could have:

u-boot> setenv ethact eth0
u-boot> printenv -e
u-boot> setenv ethact eth1
u-boot> dhcp
u-boot> setenv ipaddr 192.168.0.4
u-boot> load mmc 0:1 $fdt_addr_r dtb
u-boot> load mmc 0:1 $kernel_addr_r snp.efi
u-boot> bootefi $kernel_addr_r $fdt_addr
ipxe> dhcp

What do you expect to happen in each of these commands?

With your patch the UEFI sub-system would use eth0 but update the UEFI
network device with the ipaddress assigned by U-Boot's dhcp command for
eth1. That cannot be right.

Best regards

Heinrich
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to