All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] Support DHCPv6 and UEFI IPv6 PXE
@ 2015-04-15  9:05 Michael Chang
  2015-04-15  9:05 ` [PATCH 1/3] Added net_bootp6 command Michael Chang
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Michael Chang @ 2015-04-15  9:05 UTC (permalink / raw)
  To: grub-devel

Hi,

This patch set tries to make support of configuring IPv6 network interface
through the DHCPv6 protocol. A new command, net_bootp6, is therefore introduced
to serve the purpose. You can think it as DHCPv6/IPv6 version of the existing
net_bootp command.

In addition to that, the UEFI IPv6 PXE support can be easily done by sharing
the same routine with net_bootp6 to parse DHCPv6 Reply packet cached in the
firmware.

I'd like to know any comment on this ?

Thanks,
Michael



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] Added net_bootp6 command
  2015-04-15  9:05 [RFC] Support DHCPv6 and UEFI IPv6 PXE Michael Chang
@ 2015-04-15  9:05 ` Michael Chang
  2015-04-16 14:40   ` Andrei Borzenkov
  2015-04-15  9:05 ` [PATCH 2/3] UEFI IPv6 PXE support Michael Chang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Michael Chang @ 2015-04-15  9:05 UTC (permalink / raw)
  To: grub-devel

The net_bootp6 is used to configure the ipv6 network interface through the
DHCPv6 protocol Solict/Advertise/Request/Reply.
---
 grub-core/net/bootp.c  |  885 +++++++++++++++++++++++++++++++++++++++++++++++-
 grub-core/net/ip.c     |   35 ++
 include/grub/efi/api.h |   56 +++-
 include/grub/net.h     |   19 +
 4 files changed, 993 insertions(+), 2 deletions(-)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 6136755..477f205 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -24,6 +24,7 @@
 #include <grub/net/netbuff.h>
 #include <grub/net/udp.h>
 #include <grub/datetime.h>
+#include <grub/time.h>
 
 static void
 parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask)
@@ -256,6 +257,653 @@ grub_net_configure_by_dhcp_ack (const char *name,
   return inter;
 }
 
+struct grub_dhcpv6_option {
+  grub_uint16_t code;
+  grub_uint16_t len;
+  grub_uint8_t data[0];
+} GRUB_PACKED;
+
+
+struct grub_dhcpv6_iana_option {
+  grub_uint32_t iaid;
+  grub_uint32_t t1;
+  grub_uint32_t t2;
+  grub_uint8_t data[0];
+} GRUB_PACKED;
+
+struct grub_dhcpv6_iaaddr_option {
+  grub_uint8_t addr[16];
+  grub_uint32_t preferred_lifetime;
+  grub_uint32_t valid_lifetime;
+  grub_uint8_t data[0];
+} GRUB_PACKED;
+
+struct grub_DUID_LL
+{
+  grub_uint16_t type;
+  grub_uint16_t hw_type;
+  grub_uint8_t hwaddr[6];
+} GRUB_PACKED;
+
+struct grub_dhcpv6_dns_servers {
+  grub_uint8_t addr[16];
+  grub_uint8_t next_addr[0];
+} GRUB_PACKED;
+
+#define DHCPv6_REPLY 7
+#define DHCPv6_ADVERTISE 2
+#define DHCPv6_REQUEST 3
+#define OPTION_BOOTFILE_URL 59
+#define OPTION_DNS_SERVERS 23
+#define OPTION_IA_NA 3
+#define OPTION_IAADDR 5
+#define OPTION_CLIENTID 1
+#define OPTION_SERVERID 2
+#define OPTION_ORO 6
+#define OPTION_ELAPSED_TIME 8
+
+struct grub_dhcpv6_session
+{
+  struct grub_dhcpv6_session *next;
+  struct grub_dhcpv6_session **prev;
+  grub_uint32_t iaid;
+  grub_uint32_t transaction_id:24;
+  grub_uint64_t start_time;
+  struct grub_net_network_level_interface *ifaces;
+};
+
+static struct grub_dhcpv6_session *grub_dhcpv6_sessions = NULL;
+#define FOR_DHCPV6_SESSIONS(var) \
+    for (var = grub_dhcpv6_sessions ; var; var = var->next)
+
+static void
+grub_dhcpv6_session_add (struct grub_dhcpv6_session *session)
+{
+  struct grub_datetime date;
+  grub_err_t err;
+  grub_int32_t t = 0;
+
+  err = grub_get_datetime (&date);
+  if (err || !grub_datetime2unixtime (&date, &t))
+    {
+      grub_errno = GRUB_ERR_NONE;
+      t = 0;
+    }
+
+  session->transaction_id = t;
+  session->start_time = grub_get_time_ms ();
+
+  session->prev = &grub_dhcpv6_sessions;
+  session->next = grub_dhcpv6_sessions;
+
+  if (session->next)
+    session->next->prev = &session->next;
+
+  grub_dhcpv6_sessions = session;
+  return;
+}
+
+static void
+grub_dhcpv6_session_remove (struct grub_dhcpv6_session *session)
+{
+  *session->prev = session->next;
+  if (session->next)
+    session->next->prev = session->prev;
+  session->next = NULL;
+  session->prev = NULL;
+  return;
+}
+
+static const struct grub_dhcpv6_option*
+find_dhcpv6_option (const struct grub_net_dhcpv6_packet *packet,
+		    grub_uint16_t option)
+{
+  grub_uint16_t code, len;
+  const struct grub_dhcpv6_option *popt;
+
+  popt = (const struct grub_dhcpv6_option *)packet->dhcp_options;
+  code = grub_be_to_cpu16 (popt->code);
+  len = grub_be_to_cpu16 (popt->len);
+
+  while (0 != code && option != code)
+    {
+      popt = (const struct grub_dhcpv6_option *)((grub_uint8_t *)popt +
+		len + sizeof(*popt));
+      code = grub_be_to_cpu16 (popt->code);
+      len = grub_be_to_cpu16 (popt->len);
+    }
+
+  if (option == code)
+      return popt;
+
+  return NULL;
+}
+
+static const grub_uint8_t*
+find_dhcpv6_address (const struct grub_net_dhcpv6_packet *packet)
+{
+  const struct grub_dhcpv6_option* popt = find_dhcpv6_option (packet, OPTION_IA_NA);
+  const struct grub_dhcpv6_iana_option *ia_na;
+  const struct grub_dhcpv6_option *iaaddr_hdr;
+  const struct grub_dhcpv6_iaaddr_option *iaaddr;
+  grub_uint16_t ia_na_data_offset, ia_na_data_len, len;
+
+  if (grub_be_to_cpu16 (popt->code) != OPTION_IA_NA)
+    {
+      grub_error (GRUB_ERR_IO, N_("not an IA_NA DHCPv6 option"));
+      return NULL;
+    }
+
+  ia_na = (const struct grub_dhcpv6_iana_option *)popt->data;
+
+  if (grub_be_to_cpu16(popt->len) <= sizeof (*ia_na))
+    {
+      grub_error (GRUB_ERR_IO, N_("invalid size for IAADDR"));
+      return NULL;
+    }
+
+  ia_na_data_len = grub_be_to_cpu16(popt->len) - sizeof (*ia_na);
+  ia_na_data_offset = 0;
+
+  iaaddr_hdr = (const struct grub_dhcpv6_option *) ia_na->data;
+  len = grub_be_to_cpu16 (iaaddr_hdr->len);
+
+  while (grub_be_to_cpu16(iaaddr_hdr->code) != OPTION_IAADDR)
+    {
+      ia_na_data_offset += (len + sizeof (*iaaddr_hdr));
+
+      if (ia_na_data_offset < ia_na_data_len)
+	{
+	  iaaddr_hdr =(const struct grub_dhcpv6_option *)(ia_na->data +
+	    ia_na_data_offset);
+	  len = grub_be_to_cpu16 (iaaddr_hdr->len);
+	}
+      else
+	{
+	  iaaddr_hdr = NULL;
+	  break;
+	}
+    }
+
+  if (!iaaddr_hdr)
+    {
+      grub_error (GRUB_ERR_IO, N_("IAADDR not found"));
+      return NULL;
+    }
+
+  if ((ia_na_data_offset + sizeof (*iaaddr_hdr) + len) > ia_na_data_len)
+    {
+      grub_error (GRUB_ERR_IO, N_("IAADDR size check failed"));
+      return NULL;
+    }
+
+  iaaddr = (const struct grub_dhcpv6_iaaddr_option *) iaaddr_hdr->data;
+
+  return iaaddr->addr;
+}
+
+static void
+get_dhcpv6_dns_address (const struct grub_net_dhcpv6_packet *packet,
+	grub_net_network_level_address_t **addr, grub_uint16_t *naddr)
+{
+  const struct grub_dhcpv6_option* popt;
+  const struct grub_dhcpv6_dns_servers *dns;
+  grub_uint16_t len;
+  const grub_uint8_t *pa;
+  int i, ln;
+  grub_net_network_level_address_t *la;
+
+  if (addr)
+    *addr = NULL;
+
+  if (naddr)
+    *naddr = 0;
+
+  popt = find_dhcpv6_option (packet, OPTION_DNS_SERVERS);
+  if (!popt)
+    return;
+
+  len = grub_be_to_cpu16 (popt->len);
+  if ((len % 16) != 0)
+    {
+      grub_error (GRUB_ERR_IO, N_("invalid dns address length"));
+      return;
+    }
+
+  dns = (const struct grub_dhcpv6_dns_servers *)popt->data;
+
+  ln = len / 16;
+  la = grub_zalloc (sizeof (grub_net_network_level_address_t) * ln);
+
+  for (i = 0, pa = dns->addr; i < ln; i++, pa = dns->next_addr)
+    {
+      (la + i)->type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+      (la + i)->ipv6[0] = grub_get_unaligned64 (pa);
+      (la + i)->ipv6[1] = grub_get_unaligned64 (pa + 8);
+      (la + i)->option = DNS_OPTION_PREFER_IPV6;
+    }
+
+  *addr = la;
+  *naddr = ln;
+
+  return;
+}
+
+static void
+find_dhcpv6_bootfile_url (const struct grub_net_dhcpv6_packet *packet,
+	char **proto, char **server_ip, char **boot_file)
+{
+  char *bootfile_url;
+  const struct grub_dhcpv6_option* opt_url;
+  char *ip_start, *ip_end;
+  char *path;
+  grub_size_t ip_len;
+  grub_uint16_t len;
+  const char *protos[] = {"tftp://", "http://", NULL};
+  const char *pr;
+  int i;
+
+  if (proto)
+    *proto = NULL;
+
+  if (server_ip)
+    *server_ip = NULL;
+
+  if (boot_file)
+    *boot_file = NULL;
+
+  opt_url = find_dhcpv6_option (packet, OPTION_BOOTFILE_URL);
+
+  if (!opt_url)
+    {
+      grub_error (GRUB_ERR_IO, N_("no bootfile-url in DHCPv6 option"));
+      return;
+    }
+
+  len = grub_be_to_cpu16 (opt_url->len);
+
+  bootfile_url = grub_malloc (len + 1);
+
+  if (!bootfile_url)
+    return;
+
+  grub_memcpy (bootfile_url, opt_url->data, len);
+  bootfile_url[len]   = '\0';
+
+  for (i = 0; (pr = *(protos + i)); ++i)
+      if (grub_strncmp (bootfile_url, pr, grub_strlen(pr)) == 0)
+	break;
+
+  if (!pr)
+    {
+      grub_error (GRUB_ERR_IO,
+	N_("unsupported protocol, only tftp and http are supported"));
+      goto cleanup;
+    }
+
+  ip_start = ip_end = NULL;
+  ip_start = bootfile_url + grub_strlen(pr);
+
+  if (*ip_start != '[')
+    ip_start = NULL;
+  else
+    ip_end = grub_strchr (++ip_start, ']');
+
+  if (!ip_start || !ip_end)
+    {
+      grub_error (GRUB_ERR_IO, N_("IPv6-address not in square brackets"));
+      goto cleanup;
+    }
+
+  ip_len = ip_end - ip_start;
+
+  if (proto)
+    {
+      grub_size_t proto_len  = grub_strlen (pr) - 3;
+
+      *proto = grub_malloc (proto_len + 1);
+      if (!*proto)
+	goto cleanup;
+
+      grub_memcpy (*proto, pr, proto_len);
+      *(*proto + proto_len)  = '\0';
+    }
+
+  if (server_ip)
+    {
+      *server_ip = grub_malloc (ip_len + 1);
+
+      if (!*server_ip)
+	goto cleanup;
+
+      grub_memcpy (*server_ip, ip_start, ip_len);
+      *(*server_ip + ip_len) = '\0';
+    }
+
+  path = ip_end + 1;
+
+  if (boot_file)
+    {
+      *boot_file = grub_strdup (path);
+
+      if (!*boot_file)
+	goto cleanup;
+    }
+
+cleanup:
+
+  if (bootfile_url)
+    grub_free (bootfile_url);
+
+  if (grub_errno)
+    {
+      if (proto && *proto)
+	{
+	  grub_free (proto);
+	  *proto = NULL;
+	}
+
+      if (server_ip && *server_ip)
+	{
+	  grub_free (server_ip);
+	  *server_ip = NULL;
+	}
+
+      if (boot_file && *boot_file)
+	{
+	  grub_free (boot_file);
+	  *boot_file = NULL;
+	}
+    }
+
+  return;
+}
+
+
+static grub_err_t
+grub_net_configure_by_dhcpv6_adv (const struct grub_net_dhcpv6_packet *v6_adv,
+	struct grub_dhcpv6_session *session)
+{
+  struct grub_net_buff *nb;
+  const struct grub_dhcpv6_option *opt_client, *opt_server, *opt_iana;
+  struct grub_dhcpv6_option *popt;
+  struct grub_net_dhcpv6_packet *v6;
+  struct udphdr *udph;
+  grub_net_network_level_address_t multicast;
+  grub_net_link_level_address_t ll_multicast;
+  struct grub_net_network_level_interface *inf;
+  grub_err_t err;
+  grub_uint16_t len;
+  grub_uint64_t elapsed;
+  char err_msg[64];
+
+  if (v6_adv->message_type != DHCPv6_ADVERTISE)
+    {
+      grub_error (GRUB_ERR_IO, N_("DHCPv6 info not found"));
+      return grub_errno;
+    }
+
+  opt_client = find_dhcpv6_option (v6_adv, OPTION_CLIENTID);
+  opt_server = find_dhcpv6_option (v6_adv, OPTION_SERVERID);
+  opt_iana = find_dhcpv6_option (v6_adv, OPTION_IA_NA);
+
+  err_msg[0] = '\0';
+  if (!opt_client)
+      grub_strcpy (err_msg, "client id");
+
+  if (!opt_server)
+    {
+      if (grub_strlen (err_msg))
+	grub_strcpy (err_msg + grub_strlen (err_msg), ", server id");
+      else
+	grub_strcpy (err_msg, "server id");
+    }
+
+  if (!opt_iana)
+    {
+      if (grub_strlen (err_msg))
+	grub_strcpy (err_msg + grub_strlen (err_msg), ", iana");
+      else
+	grub_strcpy (err_msg, "iana");
+    }
+
+  if (grub_strlen (err_msg))
+    {
+      grub_strcpy (err_msg + grub_strlen (err_msg), " missing");
+      grub_error (GRUB_ERR_IO, N_(err_msg));
+      return grub_errno;
+    }
+
+  inf = session->ifaces;
+
+  multicast.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+  multicast.ipv6[0] = grub_cpu_to_be64_compile_time (0xff02ULL << 48);
+  multicast.ipv6[1] = grub_cpu_to_be64_compile_time (0x10002ULL);
+
+  err = grub_net_link_layer_resolve (inf, &multicast, &ll_multicast);
+  if (err)
+    return err;
+
+  nb = grub_netbuff_alloc (512);
+
+  if (!nb)
+    {
+      grub_netbuff_free (nb);
+      return grub_errno;
+    }
+
+  err = grub_netbuff_reserve (nb, 512);
+  if (err)
+    {
+      grub_netbuff_free (nb);
+      return err;
+    }
+
+  len = grub_cpu_to_be16(opt_client->len);
+  err = grub_netbuff_push (nb, len + 4);
+  if (err)
+    {
+      grub_netbuff_free (nb);
+      return err;
+    }
+  grub_memcpy (nb->data, opt_client, len + 4);
+
+  len = grub_cpu_to_be16(opt_server->len);
+  err = grub_netbuff_push (nb, len + 4);
+  if (err)
+    {
+      grub_netbuff_free (nb);
+      return err;
+    }
+  grub_memcpy (nb->data, opt_server, len + 4);
+
+  len = grub_cpu_to_be16(opt_iana->len);
+  err = grub_netbuff_push (nb, len + 4);
+  if (err)
+    {
+      grub_netbuff_free (nb);
+      return err;
+    }
+  grub_memcpy (nb->data, opt_iana, len + 4);
+
+  err = grub_netbuff_push (nb, 8);
+  if (err)
+    {
+      grub_netbuff_free (nb);
+      return err;
+    }
+
+  popt = (struct grub_dhcpv6_option*) nb->data;
+  popt->code = grub_cpu_to_be16_compile_time (OPTION_ORO);
+  popt->len = grub_cpu_to_be16_compile_time (4);
+  grub_set_unaligned16 (popt->data, grub_cpu_to_be16_compile_time (OPTION_BOOTFILE_URL));
+  grub_set_unaligned16 (popt->data + 2, grub_cpu_to_be16_compile_time (OPTION_DNS_SERVERS));
+
+  err = grub_netbuff_push (nb, 6);
+  if (err)
+    {
+      grub_netbuff_free (nb);
+      return err;
+    }
+  popt = (struct grub_dhcpv6_option*) nb->data;
+  popt->code = grub_cpu_to_be16_compile_time (OPTION_ELAPSED_TIME);
+  popt->len = grub_cpu_to_be16_compile_time (2);
+
+  // the time is expressed in hundredths of a second
+  elapsed = grub_divmod64 (grub_get_time_ms () - session->start_time, 10, 0);
+
+  if (elapsed > 0xffff)
+    elapsed = 0xffff;
+
+  grub_set_unaligned16 (popt->data,  grub_cpu_to_be16 ((grub_uint16_t)elapsed));
+
+  err = grub_netbuff_push (nb, 4);
+  if (err)
+    {
+      grub_netbuff_free (nb);
+      return err;
+    }
+
+  v6 = (struct grub_net_dhcpv6_packet *) nb->data;
+  v6->message_type = DHCPv6_REQUEST;
+  v6->transaction_id = v6_adv->transaction_id;
+
+  err = grub_netbuff_push (nb, sizeof (*udph));
+  if (err)
+    {
+      grub_netbuff_free (nb);
+      return err;
+    }
+
+  udph = (struct udphdr *) nb->data;
+  udph->src = grub_cpu_to_be16_compile_time (546);
+  udph->dst = grub_cpu_to_be16_compile_time (547);
+  udph->chksum = 0;
+  udph->len = grub_cpu_to_be16 (nb->tail - nb->data);
+
+  udph->chksum = grub_net_ip_transport_checksum (nb, GRUB_NET_IP_UDP,
+						 &inf->address,
+						 &multicast);
+  err = grub_net_send_ip_packet (inf, &multicast, &ll_multicast, nb,
+				 GRUB_NET_IP_UDP);
+
+  grub_netbuff_free (nb);
+
+  if (err)
+    return err;
+
+  return GRUB_ERR_NONE;
+}
+
+
+struct grub_net_network_level_interface *
+grub_net_configure_by_dhcpv6_reply (const char *name,
+	struct grub_net_card *card,
+	grub_net_interface_flags_t flags,
+	const struct grub_net_dhcpv6_packet *v6,
+	grub_size_t size __attribute__ ((unused)),
+	int is_def,
+	char **device, char **path)
+{
+  grub_net_network_level_address_t addr;
+  grub_net_network_level_netaddress_t netaddr;
+  struct grub_net_network_level_interface *inf;
+  const grub_uint8_t *your_ip;
+  char *proto;
+  char *server_ip;
+  char *boot_file;
+  grub_net_network_level_address_t *dns;
+  grub_uint16_t num_dns;
+
+  if (device)
+    *device = NULL;
+
+  if (path)
+    *path = NULL;
+
+  if (v6->message_type != DHCPv6_REPLY)
+    {
+      grub_error (GRUB_ERR_IO, N_("DHCPv6 info not found"));
+      return NULL;
+    }
+
+  your_ip = find_dhcpv6_address(v6);
+
+  if (!your_ip)
+    {
+      grub_error (GRUB_ERR_IO, N_("DHCPv6 address not found"));
+      return NULL;
+    }
+
+  get_dhcpv6_dns_address (v6, &dns, &num_dns);
+
+  if (dns && num_dns)
+    {
+      int i;
+
+      for (i = 0; i < num_dns; ++i)
+	grub_net_add_dns_server (dns + i);
+
+      grub_free (dns);
+    }
+  else
+    {
+      if (grub_errno)
+	grub_print_error ();
+    }
+
+  find_dhcpv6_bootfile_url (v6, &proto, &server_ip, &boot_file);
+
+  if (grub_errno)
+    grub_print_error ();
+
+  addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+  addr.ipv6[0] = grub_get_unaligned64 (your_ip);
+  addr.ipv6[1] = grub_get_unaligned64 (your_ip + 8);
+  inf = grub_net_add_addr (name, card, &addr, &card->default_address, flags);
+
+  netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+  netaddr.ipv6.base[0] = grub_get_unaligned64 (your_ip);
+  netaddr.ipv6.base[1] = 0;
+  netaddr.ipv6.masksize = 64;
+  grub_net_add_route (name, netaddr, inf);
+
+  grub_env_set_net_property (name, "boot_file", boot_file,
+			  grub_strlen (boot_file));
+
+  if (is_def && server_ip)
+    {
+      grub_net_default_server = grub_strdup (server_ip);
+      grub_env_set ("net_default_interface", name);
+      grub_env_export ("net_default_interface");
+    }
+
+  if (device && server_ip && proto)
+    {
+      *device = grub_xasprintf ("%s,%s", proto, server_ip);
+      if (!*device)
+	return NULL;
+    }
+
+  if (path && boot_file)
+    {
+      *path = grub_strdup (boot_file);
+      if (*path)
+	{
+	  char *slash;
+	  slash = grub_strrchr (*path, '/');
+	  if (slash)
+	    *slash = 0;
+	  else
+	    **path = 0;
+	}
+      else
+	return NULL;
+    }
+
+  return inf;
+}
+
 void
 grub_net_process_dhcp (struct grub_net_buff *nb,
 		       struct grub_net_card *card)
@@ -288,6 +936,67 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
     }
 }
 
+void
+grub_net_process_dhcp6 (struct grub_net_buff *nb,
+	struct grub_net_card *card __attribute__ ((unused)))
+{
+  const struct grub_net_dhcpv6_packet *v6;
+  struct grub_dhcpv6_session *session;
+  const struct grub_dhcpv6_option *opt_iana;
+  const struct grub_dhcpv6_iana_option *ia_na;
+
+  v6 = (const struct grub_net_dhcpv6_packet *) nb->data;
+
+  opt_iana = find_dhcpv6_option (v6, OPTION_IA_NA);
+  if (!opt_iana)
+    return;
+
+  ia_na = (const struct grub_dhcpv6_iana_option *)opt_iana->data;
+  FOR_DHCPV6_SESSIONS (session)
+    {
+      if (session->transaction_id == v6->transaction_id
+	  && session->iaid == grub_cpu_to_be32 (ia_na->iaid))
+	break;
+    }
+
+  if (!session)
+    return;
+
+
+  if (v6->message_type == DHCPv6_ADVERTISE)
+    {
+      grub_net_configure_by_dhcpv6_adv (
+	  (const struct grub_net_dhcpv6_packet*) nb->data, session);
+    }
+  else if (v6->message_type == DHCPv6_REPLY)
+    {
+      char *name;
+      struct grub_net_network_level_interface *inf;
+
+      inf = session->ifaces;
+      name = grub_xasprintf ("%s:dhcp", inf->card->name);
+      if (!name)
+	return;
+
+      grub_net_configure_by_dhcpv6_reply (name, inf->card,
+	  0, (const struct grub_net_dhcpv6_packet *) nb->data,
+	  (nb->tail - nb->data), 0, 0, 0);
+
+      if (!grub_errno)
+	{
+	  grub_dhcpv6_session_remove (session);
+	  grub_free (session);
+	}
+
+      grub_free (name);
+    }
+
+  if (grub_errno)
+    grub_print_error ();
+
+  return;
+}
+
 static char
 hexdigit (grub_uint8_t val)
 {
@@ -564,7 +1273,177 @@ grub_cmd_bootp (struct grub_command *cmd __attribute__ ((unused)),
   return err;
 }
 
-static grub_command_t cmd_getdhcp, cmd_bootp;
+static grub_err_t
+grub_cmd_bootp6 (struct grub_command *cmd __attribute__ ((unused)),
+	int argc, char **args)
+{
+  struct grub_net_card *card;
+  grub_size_t ncards = 0;
+  unsigned j = 0;
+  int interval;
+  grub_err_t err;
+  struct grub_dhcpv6_session *session;
+
+  err = GRUB_ERR_NONE;
+
+  FOR_NET_CARDS (card)
+  {
+    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
+      continue;
+    ncards++;
+  }
+
+  FOR_NET_CARDS (card)
+  {
+    struct grub_net_network_level_interface *ifaces;
+
+    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
+      continue;
+
+    ifaces = grub_net_ipv6_get_link_local (card, &card->default_address);
+    if (!ifaces)
+      {
+	grub_free (ifaces);
+	return grub_errno;
+      }
+
+    session = grub_zalloc (sizeof (*session));
+    session->ifaces = ifaces;
+    session->iaid = j;
+    grub_dhcpv6_session_add (session);
+    j++;
+  }
+
+  for (interval = 200; interval < 10000; interval *= 2)
+    {
+      int done = 1;
+
+      FOR_DHCPV6_SESSIONS (session)
+	{
+	  struct grub_net_buff *nb;
+	  struct grub_dhcpv6_option *opt;
+	  struct grub_net_dhcpv6_packet *v6;
+	  struct grub_DUID_LL *duid;
+	  struct grub_dhcpv6_iana_option *ia_na;
+	  grub_net_network_level_address_t multicast;
+	  grub_net_link_level_address_t ll_multicast;
+	  struct udphdr *udph;
+
+	  multicast.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+	  multicast.ipv6[0] = grub_cpu_to_be64_compile_time (0xff02ULL << 48);
+	  multicast.ipv6[1] = grub_cpu_to_be64_compile_time (0x10002ULL);
+
+	  err = grub_net_link_layer_resolve (session->ifaces,
+		    &multicast, &ll_multicast);
+	  if (err)
+	    return grub_errno;
+	  nb = grub_netbuff_alloc (512);
+	  if (!nb)
+	    {
+	      grub_netbuff_free (nb);
+	      return grub_errno;
+	    }
+
+	  err = grub_netbuff_reserve (nb, 512);
+	  if (err)
+	    {
+	      grub_netbuff_free (nb);
+	      return err;
+	    }
+
+	  err = grub_netbuff_push (nb, 6);
+	  if (err)
+	    {
+	      grub_netbuff_free (nb);
+	      return err;
+	    }
+
+	  opt = (struct grub_dhcpv6_option *)nb->data;
+	  opt->code = grub_cpu_to_be16_compile_time (OPTION_ELAPSED_TIME);
+	  opt->len = grub_cpu_to_be16_compile_time (2);
+	  grub_set_unaligned16 (opt->data, 0);
+
+	  err = grub_netbuff_push (nb, sizeof(*duid) + 4);
+	  if (err)
+	    {
+	      grub_netbuff_free (nb);
+	      return err;
+	    }
+
+	  opt = (struct grub_dhcpv6_option *)nb->data;
+	  opt->code = grub_cpu_to_be16_compile_time (OPTION_CLIENTID); //option_client_id
+	  opt->len = grub_cpu_to_be16 (sizeof(*duid));
+
+	  duid = (struct grub_DUID_LL *) opt->data;
+
+	  duid->type = grub_cpu_to_be16_compile_time (3) ;
+	  duid->hw_type = grub_cpu_to_be16_compile_time (1);
+	  grub_memcpy (&duid->hwaddr, &session->ifaces->hwaddress.mac,
+	      sizeof (session->ifaces->hwaddress.mac));
+
+	  err = grub_netbuff_push (nb, sizeof (*ia_na) + 4);
+	  if (err)
+	    {
+	      grub_netbuff_free (nb);
+	      return err;
+	    }
+
+	  opt = (struct grub_dhcpv6_option *)nb->data;
+	  opt->code = grub_cpu_to_be16_compile_time (OPTION_IA_NA);
+	  opt->len = grub_cpu_to_be16 (sizeof (*ia_na));
+	  ia_na = (struct grub_dhcpv6_iana_option *)opt->data;
+	  ia_na->iaid = grub_cpu_to_be32 (session->iaid);
+	  ia_na->t1 = 0;
+	  ia_na->t2 = 0;
+
+	  err = grub_netbuff_push (nb, 4);
+	  if (err)
+	    {
+	      grub_netbuff_free (nb);
+	      return err;
+	    }
+
+	  v6 = (struct grub_net_dhcpv6_packet *)nb->data;
+	  v6->message_type = 1;
+	  v6->transaction_id = session->transaction_id;
+
+	  grub_netbuff_push (nb, sizeof (*udph));
+
+	  udph = (struct udphdr *) nb->data;
+	  udph->src = grub_cpu_to_be16_compile_time (546);
+	  udph->dst = grub_cpu_to_be16_compile_time (547);
+	  udph->chksum = 0;
+	  udph->len = grub_cpu_to_be16 (nb->tail - nb->data);
+
+	  udph->chksum = grub_net_ip_transport_checksum (nb, GRUB_NET_IP_UDP,
+			    &session->ifaces->address, &multicast);
+
+	  err = grub_net_send_ip_packet (session->ifaces, &multicast,
+		    &ll_multicast, nb, GRUB_NET_IP_UDP);
+	  done = 0;
+	  grub_netbuff_free (nb);
+
+	  if (err)
+	    return err;
+	}
+      if (!done)
+	grub_net_poll_cards (interval, 0);
+    }
+
+  FOR_DHCPV6_SESSIONS (session)
+    {
+      err = grub_error (GRUB_ERR_FILE_NOT_FOUND,
+			N_("couldn't autoconfigure %s"),
+			session->ifaces->card->name);
+      grub_dhcpv6_session_remove (session);
+      grub_free (session);
+    }
+
+
+  return err;
+}
+
+static grub_command_t cmd_getdhcp, cmd_bootp, cmd_bootp6;
 
 void
 grub_bootp_init (void)
@@ -575,6 +1454,9 @@ grub_bootp_init (void)
   cmd_getdhcp = grub_register_command ("net_get_dhcp_option", grub_cmd_dhcpopt,
 				       N_("VAR INTERFACE NUMBER DESCRIPTION"),
 				       N_("retrieve DHCP option and save it into VAR. If VAR is - then print the value."));
+  cmd_bootp6 = grub_register_command ("net_bootp6", grub_cmd_bootp6,
+				     N_("[CARD]"),
+				     N_("perform a dhcpv6 autoconfiguration"));
 }
 
 void
@@ -582,4 +1464,5 @@ grub_bootp_fini (void)
 {
   grub_unregister_command (cmd_getdhcp);
   grub_unregister_command (cmd_bootp);
+  grub_unregister_command (cmd_bootp6);
 }
diff --git a/grub-core/net/ip.c b/grub-core/net/ip.c
index 8c56baa..8bb56be 100644
--- a/grub-core/net/ip.c
+++ b/grub-core/net/ip.c
@@ -238,6 +238,41 @@ handle_dgram (struct grub_net_buff *nb,
   {
     struct udphdr *udph;
     udph = (struct udphdr *) nb->data;
+
+    if (proto == GRUB_NET_IP_UDP && grub_be_to_cpu16 (udph->dst) == 546)
+      {
+	if (udph->chksum)
+	  {
+	    grub_uint16_t chk, expected;
+	    chk = udph->chksum;
+	    udph->chksum = 0;
+	    expected = grub_net_ip_transport_checksum (nb,
+						       GRUB_NET_IP_UDP,
+						       source,
+						       dest);
+	    if (expected != chk)
+	      {
+		grub_dprintf ("net", "Invalid UDP checksum. "
+			      "Expected %x, got %x\n",
+			      grub_be_to_cpu16 (expected),
+			      grub_be_to_cpu16 (chk));
+		grub_netbuff_free (nb);
+		return GRUB_ERR_NONE;
+	      }
+	    udph->chksum = chk;
+	  }
+
+	err = grub_netbuff_pull (nb, sizeof (*udph));
+	if (err)
+	  {
+	    grub_netbuff_free (nb);
+	    return err;
+	  }
+	grub_net_process_dhcp6 (nb, card);
+	grub_netbuff_free (nb);
+	return GRUB_ERR_NONE;
+      }
+
     if (proto == GRUB_NET_IP_UDP && grub_be_to_cpu16 (udph->dst) == 68)
       {
 	const struct grub_net_bootp_packet *bootp;
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index e5dd543..ff77750 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -1340,14 +1340,68 @@ typedef struct grub_efi_simple_text_output_interface grub_efi_simple_text_output
 
 typedef grub_uint8_t grub_efi_pxe_packet_t[1472];
 
+typedef struct {
+  grub_uint8_t addr[4];
+} grub_efi_pxe_ipv4_address_t;
+
+typedef struct {
+  grub_uint8_t addr[16];
+} grub_efi_pxe_ipv6_address_t;
+
+typedef struct {
+  grub_uint8_t addr[32];
+} grub_efi_pxe_mac_address_t;
+
+typedef union {
+    grub_uint32_t addr[4];
+    grub_efi_pxe_ipv4_address_t v4;
+    grub_efi_pxe_ipv6_address_t v6;
+} grub_efi_pxe_ip_address_t;
+
+#define EFI_PXE_BASE_CODE_MAX_IPCNT             8
+typedef struct {
+    grub_uint8_t filters;
+    grub_uint8_t ip_cnt;
+    grub_uint16_t reserved;
+    grub_efi_pxe_ip_address_t ip_list[EFI_PXE_BASE_CODE_MAX_IPCNT];
+} grub_efi_pxe_ip_filter_t;
+
+typedef struct {
+    grub_efi_pxe_ip_address_t ip_addr;
+    grub_efi_pxe_mac_address_t mac_addr;
+} grub_efi_pxe_arp_entry_t;
+
+typedef struct {
+    grub_efi_pxe_ip_address_t ip_addr;
+    grub_efi_pxe_ip_address_t subnet_mask;
+    grub_efi_pxe_ip_address_t gw_addr;
+} grub_efi_pxe_route_entry_t;
+
+
+#define EFI_PXE_BASE_CODE_MAX_ARP_ENTRIES 8
+#define EFI_PXE_BASE_CODE_MAX_ROUTE_ENTRIES 8
+
 typedef struct grub_efi_pxe_mode
 {
-  grub_uint8_t unused[52];
+  grub_uint8_t started;
+  grub_uint8_t ipv6_available;
+  grub_uint8_t ipv6_supported;
+  grub_uint8_t using_ipv6;
+  //grub_uint8_t unused[48];
+  grub_uint8_t unused[16];
+  grub_efi_pxe_ip_address_t station_ip;
+  grub_efi_pxe_ip_address_t subnet_mask;
   grub_efi_pxe_packet_t dhcp_discover;
   grub_efi_pxe_packet_t dhcp_ack;
   grub_efi_pxe_packet_t proxy_offer;
   grub_efi_pxe_packet_t pxe_discover;
   grub_efi_pxe_packet_t pxe_reply;
+  grub_efi_pxe_packet_t pxe_bis_reply;
+  grub_efi_pxe_ip_filter_t ip_filter;
+  grub_uint32_t arp_cache_entries;
+  grub_efi_pxe_arp_entry_t arp_cache[EFI_PXE_BASE_CODE_MAX_ARP_ENTRIES];
+  grub_uint32_t route_table_entries;
+  grub_efi_pxe_route_entry_t route_table[EFI_PXE_BASE_CODE_MAX_ROUTE_ENTRIES];
 } grub_efi_pxe_mode_t;
 
 typedef struct grub_efi_pxe
diff --git a/include/grub/net.h b/include/grub/net.h
index 538baa3..71dc243 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -418,6 +418,13 @@ struct grub_net_bootp_packet
   grub_uint8_t vendor[0];
 } GRUB_PACKED;
 
+struct grub_net_dhcpv6_packet
+{
+  grub_uint32_t message_type:8;
+  grub_uint32_t transaction_id:24;
+  grub_uint8_t dhcp_options[0];
+} GRUB_PACKED;
+
 #define	GRUB_NET_BOOTP_RFC1048_MAGIC_0	0x63
 #define	GRUB_NET_BOOTP_RFC1048_MAGIC_1	0x82
 #define	GRUB_NET_BOOTP_RFC1048_MAGIC_2	0x53
@@ -444,6 +451,14 @@ grub_net_configure_by_dhcp_ack (const char *name,
 				grub_size_t size,
 				int is_def, char **device, char **path);
 
+struct grub_net_network_level_interface *
+grub_net_configure_by_dhcpv6_reply (const char *name,
+				    struct grub_net_card *card,
+				    grub_net_interface_flags_t flags,
+				    const struct grub_net_dhcpv6_packet *v6,
+				    grub_size_t size,
+				    int is_def, char **device, char **path);
+
 grub_err_t
 grub_net_add_ipv4_local (struct grub_net_network_level_interface *inf,
 			 int mask);
@@ -452,6 +467,10 @@ void
 grub_net_process_dhcp (struct grub_net_buff *nb,
 		       struct grub_net_card *card);
 
+void
+grub_net_process_dhcp6 (struct grub_net_buff *nb,
+			struct grub_net_card *card);
+
 int
 grub_net_hwaddr_cmp (const grub_net_link_level_address_t *a,
 		     const grub_net_link_level_address_t *b);
-- 
1.7.3.4



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] UEFI IPv6 PXE support
  2015-04-15  9:05 [RFC] Support DHCPv6 and UEFI IPv6 PXE Michael Chang
  2015-04-15  9:05 ` [PATCH 1/3] Added net_bootp6 command Michael Chang
@ 2015-04-15  9:05 ` Michael Chang
  2015-04-15  9:05 ` [PATCH 3/3] Use UEFI MAC device as default configured by net_bootp6 Michael Chang
  2015-04-17  9:48 ` [RFC] Support DHCPv6 and UEFI IPv6 PXE Andrei Borzenkov
  3 siblings, 0 replies; 13+ messages in thread
From: Michael Chang @ 2015-04-15  9:05 UTC (permalink / raw)
  To: grub-devel

When system is booted from UEFI IPv6 PXE, the network interface can be
configured directly by cached DHCPv6 packet in firmware without doing it all
over again by contacting DHCP server.
---
 grub-core/net/drivers/efi/efinet.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/grub-core/net/drivers/efi/efinet.c b/grub-core/net/drivers/efi/efinet.c
index f171f20..b1837e3 100644
--- a/grub-core/net/drivers/efi/efinet.c
+++ b/grub-core/net/drivers/efi/efinet.c
@@ -257,11 +257,25 @@ grub_efi_net_config_real (grub_efi_handle_t hnd, char **device,
     if (! pxe)
       continue;
     pxe_mode = pxe->mode;
-    grub_net_configure_by_dhcp_ack (card->name, card, 0,
-				    (struct grub_net_bootp_packet *)
-				    &pxe_mode->dhcp_ack,
-				    sizeof (pxe_mode->dhcp_ack),
-				    1, device, path);
+
+    if (pxe_mode->using_ipv6)
+      {
+	grub_net_configure_by_dhcpv6_reply (card->name, card, 0,
+					    (struct grub_net_dhcpv6_packet *)
+					    &pxe_mode->dhcp_ack,
+					    sizeof (pxe_mode->dhcp_ack),
+					    1, device, path);
+	if (grub_errno)
+	  grub_print_error ();
+      }
+    else
+      {
+	grub_net_configure_by_dhcp_ack (card->name, card, 0,
+					(struct grub_net_bootp_packet *)
+					&pxe_mode->dhcp_ack,
+					sizeof (pxe_mode->dhcp_ack),
+					1, device, path);
+      }
     return;
   }
 }
-- 
1.7.3.4



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] Use UEFI MAC device as default configured by net_bootp6
  2015-04-15  9:05 [RFC] Support DHCPv6 and UEFI IPv6 PXE Michael Chang
  2015-04-15  9:05 ` [PATCH 1/3] Added net_bootp6 command Michael Chang
  2015-04-15  9:05 ` [PATCH 2/3] UEFI IPv6 PXE support Michael Chang
@ 2015-04-15  9:05 ` Michael Chang
  2015-04-16 19:58   ` Andrei Borzenkov
  2015-04-17  9:48 ` [RFC] Support DHCPv6 and UEFI IPv6 PXE Andrei Borzenkov
  3 siblings, 1 reply; 13+ messages in thread
From: Michael Chang @ 2015-04-15  9:05 UTC (permalink / raw)
  To: grub-devel

The grub_efinet_findcards will register cards by checking if it can support EFI
Simple Netowork Protocol which create more than one device to a physical NIC
device.

If without specifying any device to be configured by net_bootp6, it should pick
up one from them but not all. In my case three firmware device are listed.
IPv4, IPv6 and MAC device. Both IPv4 and IPv6 are derived from MAC device for
providing PXE Base Code Protocol. I think we should use MAC device instead of
those two to avoid collision, because net_bootp6 command does not depend on PXE
Base Code but only Simple Network Protocol to work
---
 grub-core/net/bootp.c              |    8 +++++++
 grub-core/net/drivers/efi/efinet.c |   40 ++++++++++++++++++++++++++++++++++++
 include/grub/net.h                 |    1 +
 3 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 477f205..c4963ca 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -1290,6 +1290,10 @@ grub_cmd_bootp6 (struct grub_command *cmd __attribute__ ((unused)),
   {
     if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
       continue;
+#ifdef GRUB_MACHINE_EFI
+    else if (!card->is_efi_mac_device (card))
+      continue;
+#endif
     ncards++;
   }
 
@@ -1299,6 +1303,10 @@ grub_cmd_bootp6 (struct grub_command *cmd __attribute__ ((unused)),
 
     if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
       continue;
+#ifdef GRUB_MACHINE_EFI
+    else if (!card->is_efi_mac_device (card))
+      continue;
+#endif
 
     ifaces = grub_net_ipv6_get_link_local (card, &card->default_address);
     if (!ifaces)
diff --git a/grub-core/net/drivers/efi/efinet.c b/grub-core/net/drivers/efi/efinet.c
index b1837e3..900384f 100644
--- a/grub-core/net/drivers/efi/efinet.c
+++ b/grub-core/net/drivers/efi/efinet.c
@@ -157,6 +157,45 @@ grub_efinet_get_device_handle (struct grub_net_card *card)
   return card->efi_handle;
 }
 
+static int
+grub_efinet_is_mac_device (struct grub_net_card *card)
+{
+  grub_efi_handle_t efi_handle;
+  grub_efi_device_path_t *dp;
+  grub_efi_device_path_t *next, *p;
+  grub_efi_uint8_t type;
+  grub_efi_uint8_t subtype;
+
+  efi_handle = grub_efinet_get_device_handle (card);
+
+  if (!efi_handle)
+    return 0;
+
+  dp = grub_efi_get_device_path (efi_handle);
+
+  if (GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp))
+    return 0;
+
+  for (p = (grub_efi_device_path_t *) dp, next = GRUB_EFI_NEXT_DEVICE_PATH (p);
+       ! GRUB_EFI_END_ENTIRE_DEVICE_PATH (next);
+       p = next, next = GRUB_EFI_NEXT_DEVICE_PATH (next))
+    ;
+
+  if (p)
+    {
+      type = GRUB_EFI_DEVICE_PATH_TYPE (p);
+      subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (p);
+
+      if (type == GRUB_EFI_MESSAGING_DEVICE_PATH_TYPE
+	  && subtype == GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
+	{
+	  return 1;
+	}
+    }
+
+  return 0;
+}
+
 static void
 grub_efinet_findcards (void)
 {
@@ -223,6 +262,7 @@ grub_efinet_findcards (void)
 		   sizeof (card->default_address.mac));
       card->efi_net = net;
       card->efi_handle = *handle;
+      card->is_efi_mac_device = grub_efinet_is_mac_device;
 
       grub_net_card_register (card);
     }
diff --git a/include/grub/net.h b/include/grub/net.h
index 71dc243..4571b72 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -140,6 +140,7 @@ struct grub_net_card
       struct grub_efi_simple_network *efi_net;
       grub_efi_handle_t efi_handle;
       grub_size_t last_pkt_size;
+      int (*is_efi_mac_device) (struct grub_net_card* card);
     };
 #endif
     void *data;
-- 
1.7.3.4



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Added net_bootp6 command
  2015-04-15  9:05 ` [PATCH 1/3] Added net_bootp6 command Michael Chang
@ 2015-04-16 14:40   ` Andrei Borzenkov
  2015-04-17  5:04     ` Michael Chang
  0 siblings, 1 reply; 13+ messages in thread
From: Andrei Borzenkov @ 2015-04-16 14:40 UTC (permalink / raw)
  To: Michael Chang; +Cc: grub-devel

В Wed, 15 Apr 2015 17:05:07 +0800
Michael Chang <mchang@suse.com> пишет:

> The net_bootp6 is used to configure the ipv6 network interface through the
> DHCPv6 protocol Solict/Advertise/Request/Reply.
> ---
>  grub-core/net/bootp.c  |  885 +++++++++++++++++++++++++++++++++++++++++++++++-
>  grub-core/net/ip.c     |   35 ++
>  include/grub/efi/api.h |   56 +++-
>  include/grub/net.h     |   19 +
>  4 files changed, 993 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> index 6136755..477f205 100644
> --- a/grub-core/net/bootp.c
> +++ b/grub-core/net/bootp.c
> @@ -24,6 +24,7 @@
>  #include <grub/net/netbuff.h>
>  #include <grub/net/udp.h>
>  #include <grub/datetime.h>
> +#include <grub/time.h>
>  
>  static void
>  parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask)
> @@ -256,6 +257,653 @@ grub_net_configure_by_dhcp_ack (const char *name,
>    return inter;
>  }
>  
> +struct grub_dhcpv6_option {
> +  grub_uint16_t code;
> +  grub_uint16_t len;
> +  grub_uint8_t data[0];
> +} GRUB_PACKED;
> +
> +
> +struct grub_dhcpv6_iana_option {
> +  grub_uint32_t iaid;
> +  grub_uint32_t t1;
> +  grub_uint32_t t2;
> +  grub_uint8_t data[0];
> +} GRUB_PACKED;
> +
> +struct grub_dhcpv6_iaaddr_option {
> +  grub_uint8_t addr[16];
> +  grub_uint32_t preferred_lifetime;
> +  grub_uint32_t valid_lifetime;
> +  grub_uint8_t data[0];
> +} GRUB_PACKED;
> +
> +struct grub_DUID_LL
> +{
> +  grub_uint16_t type;
> +  grub_uint16_t hw_type;
> +  grub_uint8_t hwaddr[6];
> +} GRUB_PACKED;
> +
> +struct grub_dhcpv6_dns_servers {
> +  grub_uint8_t addr[16];
> +  grub_uint8_t next_addr[0];
> +} GRUB_PACKED;
> +

Do we really need it? Code just fetches 16 bytes blocks, it does not
really need any structure definition?

> +#define DHCPv6_REPLY 7
> +#define DHCPv6_ADVERTISE 2
> +#define DHCPv6_REQUEST 3
> +#define OPTION_BOOTFILE_URL 59
> +#define OPTION_DNS_SERVERS 23
> +#define OPTION_IA_NA 3
> +#define OPTION_IAADDR 5
> +#define OPTION_CLIENTID 1
> +#define OPTION_SERVERID 2
> +#define OPTION_ORO 6
> +#define OPTION_ELAPSED_TIME 8
> +

This is better as enum and GRUB_ prefix. Also options need GRUB_DHCPv6_
prefix.

> +struct grub_dhcpv6_session
> +{
> +  struct grub_dhcpv6_session *next;
> +  struct grub_dhcpv6_session **prev;
> +  grub_uint32_t iaid;
> +  grub_uint32_t transaction_id:24;
> +  grub_uint64_t start_time;
> +  struct grub_net_network_level_interface *ifaces;

Can there be multiple interfaces as implied by plural?

> +};
> +
> +static struct grub_dhcpv6_session *grub_dhcpv6_sessions = NULL;
> +#define FOR_DHCPV6_SESSIONS(var) \
> +    for (var = grub_dhcpv6_sessions ; var; var = var->next)
> +

FOR_LIST_ELEMENTS

> +static void
> +grub_dhcpv6_session_add (struct grub_dhcpv6_session *session)
> +{
> +  struct grub_datetime date;
> +  grub_err_t err;
> +  grub_int32_t t = 0;
> +
> +  err = grub_get_datetime (&date);
> +  if (err || !grub_datetime2unixtime (&date, &t))
> +    {
> +      grub_errno = GRUB_ERR_NONE;
> +      t = 0;
> +    }
> +
> +  session->transaction_id = t;
> +  session->start_time = grub_get_time_ms ();
> +
> +  session->prev = &grub_dhcpv6_sessions;
> +  session->next = grub_dhcpv6_sessions;
> +
> +  if (session->next)
> +    session->next->prev = &session->next;
> +

grub_list_push

> +  grub_dhcpv6_sessions = session;
> +  return;
> +}
> +
> +static void
> +grub_dhcpv6_session_remove (struct grub_dhcpv6_session *session)
> +{
> +  *session->prev = session->next;
> +  if (session->next)
> +    session->next->prev = session->prev;
> +  session->next = NULL;
> +  session->prev = NULL;
> +  return;
> +}
> +

grub_list_remove

> +static const struct grub_dhcpv6_option*
> +find_dhcpv6_option (const struct grub_net_dhcpv6_packet *packet,
> +		    grub_uint16_t option)
> +{
> +  grub_uint16_t code, len;
> +  const struct grub_dhcpv6_option *popt;
> +
> +  popt = (const struct grub_dhcpv6_option *)packet->dhcp_options;
> +  code = grub_be_to_cpu16 (popt->code);
> +  len = grub_be_to_cpu16 (popt->len);
> +
> +  while (0 != code && option != code)

This probably needs upper boundary check in case we are dealing with
corrupted packets.

> +    {
> +      popt = (const struct grub_dhcpv6_option *)((grub_uint8_t *)popt +
> +		len + sizeof(*popt));
> +      code = grub_be_to_cpu16 (popt->code);
> +      len = grub_be_to_cpu16 (popt->len);
> +    }
> +
> +  if (option == code)
> +      return popt;
> +

This can just be moved inside a loop, right?

> +  return NULL;
> +}
> +
> +static const grub_uint8_t*
> +find_dhcpv6_address (const struct grub_net_dhcpv6_packet *packet)
> +{
> +  const struct grub_dhcpv6_option* popt = find_dhcpv6_option (packet, OPTION_IA_NA);
> +  const struct grub_dhcpv6_iana_option *ia_na;
> +  const struct grub_dhcpv6_option *iaaddr_hdr;
> +  const struct grub_dhcpv6_iaaddr_option *iaaddr;
> +  grub_uint16_t ia_na_data_offset, ia_na_data_len, len;
> +
> +  if (grub_be_to_cpu16 (popt->code) != OPTION_IA_NA)

if (popt == NULL) 

> +    {
> +      grub_error (GRUB_ERR_IO, N_("not an IA_NA DHCPv6 option"));
> +      return NULL;
> +    }
> +
> +  ia_na = (const struct grub_dhcpv6_iana_option *)popt->data;
> +
> +  if (grub_be_to_cpu16(popt->len) <= sizeof (*ia_na))
> +    {
> +      grub_error (GRUB_ERR_IO, N_("invalid size for IAADDR"));
> +      return NULL;
> +    }
> +

Does it need upper boundary check?

> +  ia_na_data_len = grub_be_to_cpu16(popt->len) - sizeof (*ia_na);
> +  ia_na_data_offset = 0;
> +
> +  iaaddr_hdr = (const struct grub_dhcpv6_option *) ia_na->data;
> +  len = grub_be_to_cpu16 (iaaddr_hdr->len);
> +
> +  while (grub_be_to_cpu16(iaaddr_hdr->code) != OPTION_IAADDR)

grub_cpu_to_be16_compile_time(OPTION_IAADDR)

> +    {
> +      ia_na_data_offset += (len + sizeof (*iaaddr_hdr));
> +
> +      if (ia_na_data_offset < ia_na_data_len)
> +	{
> +	  iaaddr_hdr =(const struct grub_dhcpv6_option *)(ia_na->data +
> +	    ia_na_data_offset);
> +	  len = grub_be_to_cpu16 (iaaddr_hdr->len);
> +	}
> +      else
> +	{
> +	  iaaddr_hdr = NULL;
> +	  break;
> +	}
> +    }
> +
> +  if (!iaaddr_hdr)
> +    {
> +      grub_error (GRUB_ERR_IO, N_("IAADDR not found"));
> +      return NULL;
> +    }
> +
> +  if ((ia_na_data_offset + sizeof (*iaaddr_hdr) + len) > ia_na_data_len)
> +    {
> +      grub_error (GRUB_ERR_IO, N_("IAADDR size check failed"));
> +      return NULL;
> +    }
> +
> +  iaaddr = (const struct grub_dhcpv6_iaaddr_option *) iaaddr_hdr->data;
> +
> +  return iaaddr->addr;

It sounds again like most code could be folded inside a loop

while (remaining_length > 0)
  if (option_length > remaining_length)
    error
  if (option_code == IAADDR)
    return hdr->addr
  hdr = (char *)hdr + option_length
  remaining_length -= option_length

return not found
> +}
> +
> +static void
> +get_dhcpv6_dns_address (const struct grub_net_dhcpv6_packet *packet,
> +	grub_net_network_level_address_t **addr, grub_uint16_t *naddr)
> +{
> +  const struct grub_dhcpv6_option* popt;
> +  const struct grub_dhcpv6_dns_servers *dns;
> +  grub_uint16_t len;
> +  const grub_uint8_t *pa;
> +  int i, ln;
> +  grub_net_network_level_address_t *la;
> +
> +  if (addr)
> +    *addr = NULL;
> +
> +  if (naddr)
> +    *naddr = 0;
> +
> +  popt = find_dhcpv6_option (packet, OPTION_DNS_SERVERS);
> +  if (!popt)
> +    return;
> +
> +  len = grub_be_to_cpu16 (popt->len);
> +  if ((len % 16) != 0)

len & 0xf; 0 comparison probably redundant as well.

Again, what about upper boundary check?

> +    {
> +      grub_error (GRUB_ERR_IO, N_("invalid dns address length"));
> +      return;
> +    }
> +
> +  dns = (const struct grub_dhcpv6_dns_servers *)popt->data;
> +
> +  ln = len / 16;

len >> 4

> +  la = grub_zalloc (sizeof (grub_net_network_level_address_t) * ln);
> +

     *addr = la = grub_zalloc (...)

NULL check

> +  for (i = 0, pa = dns->addr; i < ln; i++, pa = dns->next_addr)
                 ^^^ not needed               pa += 16, la++
> +    {
> +      (la + i)->type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> +      (la + i)->ipv6[0] = grub_get_unaligned64 (pa);
> +      (la + i)->ipv6[1] = grub_get_unaligned64 (pa + 8);
> +      (la + i)->option = DNS_OPTION_PREFER_IPV6;

          la->...

> +    }
> +
> +  *addr = la;
     not needed

> +  *naddr = ln;
> +
> +  return;
> +}
> +
> +static void
> +find_dhcpv6_bootfile_url (const struct grub_net_dhcpv6_packet *packet,
> +	char **proto, char **server_ip, char **boot_file)
> +{
> +  char *bootfile_url;
> +  const struct grub_dhcpv6_option* opt_url;
> +  char *ip_start, *ip_end;
> +  char *path;
> +  grub_size_t ip_len;
> +  grub_uint16_t len;
> +  const char *protos[] = {"tftp://", "http://", NULL};
> +  const char *pr;
> +  int i;
> +
> +  if (proto)
> +    *proto = NULL;
> +
> +  if (server_ip)
> +    *server_ip = NULL;
> +
> +  if (boot_file)
> +    *boot_file = NULL;
> +
> +  opt_url = find_dhcpv6_option (packet, OPTION_BOOTFILE_URL);
> +
> +  if (!opt_url)
> +    {
> +      grub_error (GRUB_ERR_IO, N_("no bootfile-url in DHCPv6 option"));
> +      return;
> +    }
> +
> +  len = grub_be_to_cpu16 (opt_url->len);
> +

Obligatory question about upper boundary check :)

> +  bootfile_url = grub_malloc (len + 1);
> +
> +  if (!bootfile_url)
> +    return;
> +
> +  grub_memcpy (bootfile_url, opt_url->data, len);
> +  bootfile_url[len]   = '\0';
> +
> +  for (i = 0; (pr = *(protos + i)); ++i)
> +      if (grub_strncmp (bootfile_url, pr, grub_strlen(pr)) == 0)
> +	break;
> +
> +  if (!pr)
> +    {
> +      grub_error (GRUB_ERR_IO,
> +	N_("unsupported protocol, only tftp and http are supported"));
> +      goto cleanup;
> +    }
> +
> +  ip_start = ip_end = NULL;
> +  ip_start = bootfile_url + grub_strlen(pr);
> +
> +  if (*ip_start != '[')
> +    ip_start = NULL;
> +  else
> +    ip_end = grub_strchr (++ip_start, ']');
> +
> +  if (!ip_start || !ip_end)
> +    {
> +      grub_error (GRUB_ERR_IO, N_("IPv6-address not in square brackets"));
> +      goto cleanup;
> +    }
> +

Can bootfile_url contain a name and not IPv6 address? Or is address
mandatory?

> +  ip_len = ip_end - ip_start;
> +
> +  if (proto)
> +    {
> +      grub_size_t proto_len  = grub_strlen (pr) - 3;
> +
> +      *proto = grub_malloc (proto_len + 1);
> +      if (!*proto)
> +	goto cleanup;
> +
> +      grub_memcpy (*proto, pr, proto_len);
> +      *(*proto + proto_len)  = '\0';

Umm ... we really need something like grub_mem2str. Actually
grub_xasprintf("%.*s") would be just fine if grub supported it.

> +    }
> +
> +  if (server_ip)
> +    {
> +      *server_ip = grub_malloc (ip_len + 1);
> +
> +      if (!*server_ip)
> +	goto cleanup;
> +
> +      grub_memcpy (*server_ip, ip_start, ip_len);
> +      *(*server_ip + ip_len) = '\0';
> +    }
> +
> +  path = ip_end + 1;
> +
> +  if (boot_file)
> +    {
> +      *boot_file = grub_strdup (path);
> +
> +      if (!*boot_file)
> +	goto cleanup;
> +    }
> +
> +cleanup:
> +
> +  if (bootfile_url)
> +    grub_free (bootfile_url);
> +

grub_free checks for NULL

> +  if (grub_errno)
> +    {
> +      if (proto && *proto)
> +	{
> +	  grub_free (proto);

grub_free (*proto)

> +	  *proto = NULL;
> +	}
> +
> +      if (server_ip && *server_ip)
> +	{
> +	  grub_free (server_ip);
grub_free (*server_ip)

> +	  *server_ip = NULL;
> +	}
> +
> +      if (boot_file && *boot_file)
> +	{
> +	  grub_free (boot_file);
grub_free (*boot_file)

> +	  *boot_file = NULL;
> +	}
> +    }
> +
> +  return;
> +}
> +
> +
> +static grub_err_t
> +grub_net_configure_by_dhcpv6_adv (const struct grub_net_dhcpv6_packet *v6_adv,
> +	struct grub_dhcpv6_session *session)
> +{
> +  struct grub_net_buff *nb;
> +  const struct grub_dhcpv6_option *opt_client, *opt_server, *opt_iana;
> +  struct grub_dhcpv6_option *popt;
> +  struct grub_net_dhcpv6_packet *v6;
> +  struct udphdr *udph;
> +  grub_net_network_level_address_t multicast;
> +  grub_net_link_level_address_t ll_multicast;
> +  struct grub_net_network_level_interface *inf;
> +  grub_err_t err;
> +  grub_uint16_t len;
> +  grub_uint64_t elapsed;
> +  char err_msg[64];
> +
> +  if (v6_adv->message_type != DHCPv6_ADVERTISE)

We come here after checking message type, or am I wrong?

> +    {
> +      grub_error (GRUB_ERR_IO, N_("DHCPv6 info not found"));
> +      return grub_errno;
> +    }
> +
> +  opt_client = find_dhcpv6_option (v6_adv, OPTION_CLIENTID);
> +  opt_server = find_dhcpv6_option (v6_adv, OPTION_SERVERID);
> +  opt_iana = find_dhcpv6_option (v6_adv, OPTION_IA_NA);
> +
> +  err_msg[0] = '\0';
> +  if (!opt_client)
> +      grub_strcpy (err_msg, "client id");
> +
> +  if (!opt_server)
> +    {
> +      if (grub_strlen (err_msg))
> +	grub_strcpy (err_msg + grub_strlen (err_msg), ", server id");
> +      else
> +	grub_strcpy (err_msg, "server id");
> +    }
> +
> +  if (!opt_iana)
> +    {
> +      if (grub_strlen (err_msg))
> +	grub_strcpy (err_msg + grub_strlen (err_msg), ", iana");
> +      else
> +	grub_strcpy (err_msg, "iana");
> +    }
> +
> +  if (grub_strlen (err_msg))
> +    {
> +      grub_strcpy (err_msg + grub_strlen (err_msg), " missing");
> +      grub_error (GRUB_ERR_IO, N_(err_msg));

This means it will not be extracted by xgettext and we need 7 different
strings added manually. This needs some change if you really want it to
be translated. May be use grub_error_push with "Mandatory DHCPv6 option
%s missing" or similar?

> +      return grub_errno;
> +    }
> +
> +  inf = session->ifaces;
> +
> +  multicast.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> +  multicast.ipv6[0] = grub_cpu_to_be64_compile_time (0xff02ULL << 48);
> +  multicast.ipv6[1] = grub_cpu_to_be64_compile_time (0x10002ULL);
> +
> +  err = grub_net_link_layer_resolve (inf, &multicast, &ll_multicast);
> +  if (err)
> +    return err;
> +
> +  nb = grub_netbuff_alloc (512);

Symbolic constant would be nice. Where this size comes from?

> +
> +  if (!nb)
> +    {
> +      grub_netbuff_free (nb);

nb is NULL at this point, not?

> +      return grub_errno;
> +    }
> +
> +  err = grub_netbuff_reserve (nb, 512);
> +  if (err)
> +    {
> +      grub_netbuff_free (nb);
> +      return err;
> +    }
> +
> +  len = grub_cpu_to_be16(opt_client->len);
> +  err = grub_netbuff_push (nb, len + 4);
                                  len + sizeof (*opt_client)
> +  if (err)
> +    {
> +      grub_netbuff_free (nb);
> +      return err;
> +    }
> +  grub_memcpy (nb->data, opt_client, len + 4);
  ditto

> +
> +  len = grub_cpu_to_be16(opt_server->len);
> +  err = grub_netbuff_push (nb, len + 4);
  ditto

> +  if (err)
> +    {
> +      grub_netbuff_free (nb);
> +      return err;
> +    }
> +  grub_memcpy (nb->data, opt_server, len + 4);
  ditto

> +
> +  len = grub_cpu_to_be16(opt_iana->len);
> +  err = grub_netbuff_push (nb, len + 4);
  ditto

> +  if (err)
> +    {
> +      grub_netbuff_free (nb);
> +      return err;
> +    }
> +  grub_memcpy (nb->data, opt_iana, len + 4);
  ditto

> +
> +  err = grub_netbuff_push (nb, 8);
sizeof (struct grub_dhcpv6_option) + 2 * sizeof (grub_uint16_t)
probably makes it more clear.

> +  if (err)
> +    {
> +      grub_netbuff_free (nb);
> +      return err;
> +    }
> +
> +  popt = (struct grub_dhcpv6_option*) nb->data;
> +  popt->code = grub_cpu_to_be16_compile_time (OPTION_ORO);
> +  popt->len = grub_cpu_to_be16_compile_time (4);

and here 2 * sizeof (grub_uint16_t) as well.

> +  grub_set_unaligned16 (popt->data, grub_cpu_to_be16_compile_time (OPTION_BOOTFILE_URL));
> +  grub_set_unaligned16 (popt->data + 2, grub_cpu_to_be16_compile_time (OPTION_DNS_SERVERS));
> +
> +  err = grub_netbuff_push (nb, 6);
> +  if (err)
> +    {
> +      grub_netbuff_free (nb);
> +      return err;
> +    }
> +  popt = (struct grub_dhcpv6_option*) nb->data;
> +  popt->code = grub_cpu_to_be16_compile_time (OPTION_ELAPSED_TIME);
> +  popt->len = grub_cpu_to_be16_compile_time (2);
> +

similar

> +  // the time is expressed in hundredths of a second
Please, let's stick to C comments

> +  elapsed = grub_divmod64 (grub_get_time_ms () - session->start_time, 10, 0);
> +
> +  if (elapsed > 0xffff)
> +    elapsed = 0xffff;
> +
> +  grub_set_unaligned16 (popt->data,  grub_cpu_to_be16 ((grub_uint16_t)elapsed));
> +
> +  err = grub_netbuff_push (nb, 4);
also better sizeof to avoid magic constants

> +  if (err)
> +    {
> +      grub_netbuff_free (nb);
> +      return err;
> +    }
> +
> +  v6 = (struct grub_net_dhcpv6_packet *) nb->data;
> +  v6->message_type = DHCPv6_REQUEST;
> +  v6->transaction_id = v6_adv->transaction_id;
> +
> +  err = grub_netbuff_push (nb, sizeof (*udph));
> +  if (err)
> +    {
> +      grub_netbuff_free (nb);
> +      return err;
> +    }
> +
> +  udph = (struct udphdr *) nb->data;
> +  udph->src = grub_cpu_to_be16_compile_time (546);
> +  udph->dst = grub_cpu_to_be16_compile_time (547);
> +  udph->chksum = 0;
> +  udph->len = grub_cpu_to_be16 (nb->tail - nb->data);
> +
> +  udph->chksum = grub_net_ip_transport_checksum (nb, GRUB_NET_IP_UDP,
> +						 &inf->address,
> +						 &multicast);
> +  err = grub_net_send_ip_packet (inf, &multicast, &ll_multicast, nb,
> +				 GRUB_NET_IP_UDP);
> +
> +  grub_netbuff_free (nb);
> +
> +  if (err)

Just initialize it from start to GRUB_ERR_NONE?

> +    return err;
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +
> +struct grub_net_network_level_interface *
> +grub_net_configure_by_dhcpv6_reply (const char *name,
> +	struct grub_net_card *card,
> +	grub_net_interface_flags_t flags,
> +	const struct grub_net_dhcpv6_packet *v6,
> +	grub_size_t size __attribute__ ((unused)),
> +	int is_def,
> +	char **device, char **path)
> +{
> +  grub_net_network_level_address_t addr;
> +  grub_net_network_level_netaddress_t netaddr;
> +  struct grub_net_network_level_interface *inf;
> +  const grub_uint8_t *your_ip;
> +  char *proto;
> +  char *server_ip;
> +  char *boot_file;
> +  grub_net_network_level_address_t *dns;
> +  grub_uint16_t num_dns;
> +
> +  if (device)
> +    *device = NULL;
> +
> +  if (path)
> +    *path = NULL;
> +
> +  if (v6->message_type != DHCPv6_REPLY)

Is it really possible? We come here if packet is DHCPv6_REPLY only.

> +    {
> +      grub_error (GRUB_ERR_IO, N_("DHCPv6 info not found"));
> +      return NULL;
> +    }
> +
> +  your_ip = find_dhcpv6_address(v6);
> +
> +  if (!your_ip)
> +    {
> +      grub_error (GRUB_ERR_IO, N_("DHCPv6 address not found"));
> +      return NULL;
> +    }
> +
> +  get_dhcpv6_dns_address (v6, &dns, &num_dns);
> +
> +  if (dns && num_dns)
> +    {
> +      int i;
> +
> +      for (i = 0; i < num_dns; ++i)
> +	grub_net_add_dns_server (dns + i);
> +
> +      grub_free (dns);
> +    }
> +  else
> +    {
> +      if (grub_errno)
> +	grub_print_error ();
> +    }
> +

braces not needed.

> +  find_dhcpv6_bootfile_url (v6, &proto, &server_ip, &boot_file);
> +
> +  if (grub_errno)
> +    grub_print_error ();
> +
> +  addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> +  addr.ipv6[0] = grub_get_unaligned64 (your_ip);
> +  addr.ipv6[1] = grub_get_unaligned64 (your_ip + 8);
> +  inf = grub_net_add_addr (name, card, &addr, &card->default_address, flags);
> +
> +  netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> +  netaddr.ipv6.base[0] = grub_get_unaligned64 (your_ip);
> +  netaddr.ipv6.base[1] = 0;
> +  netaddr.ipv6.masksize = 64;
> +  grub_net_add_route (name, netaddr, inf);
> +
> +  grub_env_set_net_property (name, "boot_file", boot_file,
> +			  grub_strlen (boot_file));
> +

boot_file can be NULL, right?

> +  if (is_def && server_ip)
> +    {
> +      grub_net_default_server = grub_strdup (server_ip);
> +      grub_env_set ("net_default_interface", name);
> +      grub_env_export ("net_default_interface");
> +    }
> +
> +  if (device && server_ip && proto)
> +    {
> +      *device = grub_xasprintf ("%s,%s", proto, server_ip);
> +      if (!*device)
> +	return NULL;
> +    }
> +
> +  if (path && boot_file)
> +    {
> +      *path = grub_strdup (boot_file);
> +      if (*path)
> +	{
> +	  char *slash;
> +	  slash = grub_strrchr (*path, '/');
> +	  if (slash)
> +	    *slash = 0;
> +	  else
> +	    **path = 0;
> +	}
> +      else
> +	return NULL;
> +    }
> +
> +  return inf;

This smells like memory leak in multiple places above. proto,
server_ip, etc

> +}
> +
>  void
>  grub_net_process_dhcp (struct grub_net_buff *nb,
>  		       struct grub_net_card *card)
> @@ -288,6 +936,67 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
>      }
>  }
>  
> +void
> +grub_net_process_dhcp6 (struct grub_net_buff *nb,
> +	struct grub_net_card *card __attribute__ ((unused)))
> +{
> +  const struct grub_net_dhcpv6_packet *v6;
> +  struct grub_dhcpv6_session *session;
> +  const struct grub_dhcpv6_option *opt_iana;
> +  const struct grub_dhcpv6_iana_option *ia_na;
> +
> +  v6 = (const struct grub_net_dhcpv6_packet *) nb->data;
> +
> +  opt_iana = find_dhcpv6_option (v6, OPTION_IA_NA);
> +  if (!opt_iana)
> +    return;
> +
> +  ia_na = (const struct grub_dhcpv6_iana_option *)opt_iana->data;
> +  FOR_DHCPV6_SESSIONS (session)
> +    {
> +      if (session->transaction_id == v6->transaction_id
> +	  && session->iaid == grub_cpu_to_be32 (ia_na->iaid))
> +	break;
> +    }
> +
> +  if (!session)
> +    return;
> +
> +
> +  if (v6->message_type == DHCPv6_ADVERTISE)
> +    {
> +      grub_net_configure_by_dhcpv6_adv (
> +	  (const struct grub_net_dhcpv6_packet*) nb->data, session);
> +    }
> +  else if (v6->message_type == DHCPv6_REPLY)
> +    {
> +      char *name;
> +      struct grub_net_network_level_interface *inf;
> +
> +      inf = session->ifaces;
> +      name = grub_xasprintf ("%s:dhcp", inf->card->name);
> +      if (!name)
> +	return;
> +
> +      grub_net_configure_by_dhcpv6_reply (name, inf->card,
> +	  0, (const struct grub_net_dhcpv6_packet *) nb->data,
> +	  (nb->tail - nb->data), 0, 0, 0);
> +
> +      if (!grub_errno)
> +	{
> +	  grub_dhcpv6_session_remove (session);
> +	  grub_free (session);
> +	}
> +
> +      grub_free (name);
> +    }
> +
> +  if (grub_errno)
> +    grub_print_error ();
> +
> +  return;
> +}
> +
>  static char
>  hexdigit (grub_uint8_t val)
>  {
> @@ -564,7 +1273,177 @@ grub_cmd_bootp (struct grub_command *cmd __attribute__ ((unused)),
>    return err;
>  }
>  
> -static grub_command_t cmd_getdhcp, cmd_bootp;
> +static grub_err_t
> +grub_cmd_bootp6 (struct grub_command *cmd __attribute__ ((unused)),
> +	int argc, char **args)
> +{
> +  struct grub_net_card *card;
> +  grub_size_t ncards = 0;
> +  unsigned j = 0;
> +  int interval;
> +  grub_err_t err;
> +  struct grub_dhcpv6_session *session;
> +
> +  err = GRUB_ERR_NONE;
> +
> +  FOR_NET_CARDS (card)
> +  {
> +    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
> +      continue;
> +    ncards++;
> +  }
> +

where ncards is used?

> +  FOR_NET_CARDS (card)
> +  {
> +    struct grub_net_network_level_interface *ifaces;
> +

Why plural? It was array in original code, but not here, right?

> +    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
> +      continue;
> +
> +    ifaces = grub_net_ipv6_get_link_local (card, &card->default_address);
> +    if (!ifaces)
> +      {
> +	grub_free (ifaces);

You still insist on freeing NULL pointers :)

> +	return grub_errno;
> +      }
> +

Allocated sessions are leaked.

> +    session = grub_zalloc (sizeof (*session));
> +    session->ifaces = ifaces;
> +    session->iaid = j;
> +    grub_dhcpv6_session_add (session);
> +    j++;
> +  }
> +
> +  for (interval = 200; interval < 10000; interval *= 2)
> +    {
> +      int done = 1;
> +
> +      FOR_DHCPV6_SESSIONS (session)
> +	{
Something strange with indentation.

> +	  struct grub_net_buff *nb;
> +	  struct grub_dhcpv6_option *opt;
> +	  struct grub_net_dhcpv6_packet *v6;
> +	  struct grub_DUID_LL *duid;
> +	  struct grub_dhcpv6_iana_option *ia_na;
> +	  grub_net_network_level_address_t multicast;
> +	  grub_net_link_level_address_t ll_multicast;
> +	  struct udphdr *udph;
> +
> +	  multicast.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> +	  multicast.ipv6[0] = grub_cpu_to_be64_compile_time (0xff02ULL << 48);
> +	  multicast.ipv6[1] = grub_cpu_to_be64_compile_time (0x10002ULL);
> +
> +	  err = grub_net_link_layer_resolve (session->ifaces,
> +		    &multicast, &ll_multicast);
> +	  if (err)
> +	    return grub_errno;

Allocated sessions?

> +	  nb = grub_netbuff_alloc (512);
> +	  if (!nb)
> +	    {
> +	      grub_netbuff_free (nb);

free(NULL)

> +	      return grub_errno;
> +	    }
> +

memory leak

> +	  err = grub_netbuff_reserve (nb, 512);
> +	  if (err)
> +	    {
> +	      grub_netbuff_free (nb);
> +	      return err;
> +	    }
> +

memory leak

> +	  err = grub_netbuff_push (nb, 6);

I wonder if some good macro around sizeof could be used here to avoid
hardcoding magic numbers.

> +	  if (err)
> +	    {
> +	      grub_netbuff_free (nb);
> +	      return err;
> +	    }
> +

memory leak

> +	  opt = (struct grub_dhcpv6_option *)nb->data;
> +	  opt->code = grub_cpu_to_be16_compile_time (OPTION_ELAPSED_TIME);
> +	  opt->len = grub_cpu_to_be16_compile_time (2);
> +	  grub_set_unaligned16 (opt->data, 0);
> +
> +	  err = grub_netbuff_push (nb, sizeof(*duid) + 4);
> +	  if (err)
> +	    {
> +	      grub_netbuff_free (nb);
> +	      return err;
> +	    }
> +

memory leak

> +	  opt = (struct grub_dhcpv6_option *)nb->data;
> +	  opt->code = grub_cpu_to_be16_compile_time (OPTION_CLIENTID); //option_client_id

C++ comments

> +	  opt->len = grub_cpu_to_be16 (sizeof(*duid));
> +
> +	  duid = (struct grub_DUID_LL *) opt->data;
> +
> +	  duid->type = grub_cpu_to_be16_compile_time (3) ;
> +	  duid->hw_type = grub_cpu_to_be16_compile_time (1);
> +	  grub_memcpy (&duid->hwaddr, &session->ifaces->hwaddress.mac,
> +	      sizeof (session->ifaces->hwaddress.mac));
> +
> +	  err = grub_netbuff_push (nb, sizeof (*ia_na) + 4);
> +	  if (err)
> +	    {
> +	      grub_netbuff_free (nb);
> +	      return err;
> +	    }
> +

memory leak

> +	  opt = (struct grub_dhcpv6_option *)nb->data;
> +	  opt->code = grub_cpu_to_be16_compile_time (OPTION_IA_NA);
> +	  opt->len = grub_cpu_to_be16 (sizeof (*ia_na));
> +	  ia_na = (struct grub_dhcpv6_iana_option *)opt->data;
> +	  ia_na->iaid = grub_cpu_to_be32 (session->iaid);
> +	  ia_na->t1 = 0;
> +	  ia_na->t2 = 0;
> +
> +	  err = grub_netbuff_push (nb, 4);
> +	  if (err)
> +	    {
> +	      grub_netbuff_free (nb);
> +	      return err;
> +	    }
> +

memory leak

> +	  v6 = (struct grub_net_dhcpv6_packet *)nb->data;
> +	  v6->message_type = 1;
> +	  v6->transaction_id = session->transaction_id;
> +
> +	  grub_netbuff_push (nb, sizeof (*udph));
> +
> +	  udph = (struct udphdr *) nb->data;
> +	  udph->src = grub_cpu_to_be16_compile_time (546);
> +	  udph->dst = grub_cpu_to_be16_compile_time (547);
> +	  udph->chksum = 0;
> +	  udph->len = grub_cpu_to_be16 (nb->tail - nb->data);
> +
> +	  udph->chksum = grub_net_ip_transport_checksum (nb, GRUB_NET_IP_UDP,
> +			    &session->ifaces->address, &multicast);
> +
> +	  err = grub_net_send_ip_packet (session->ifaces, &multicast,
> +		    &ll_multicast, nb, GRUB_NET_IP_UDP);
> +	  done = 0;
> +	  grub_netbuff_free (nb);
> +
> +	  if (err)
> +	    return err;

memory leak

> +	}
> +      if (!done)
> +	grub_net_poll_cards (interval, 0);
> +    }
> +
> +  FOR_DHCPV6_SESSIONS (session)
> +    {
> +      err = grub_error (GRUB_ERR_FILE_NOT_FOUND,
> +			N_("couldn't autoconfigure %s"),
> +			session->ifaces->card->name);

Unless I misunderstand something only last error is printed anyway. May
be it should push and then print them outside of loop?

> +      grub_dhcpv6_session_remove (session);
> +      grub_free (session);
> +    }
> +
> +
> +  return err;
> +}
> +
> +static grub_command_t cmd_getdhcp, cmd_bootp, cmd_bootp6;
>  
>  void
>  grub_bootp_init (void)
> @@ -575,6 +1454,9 @@ grub_bootp_init (void)
>    cmd_getdhcp = grub_register_command ("net_get_dhcp_option", grub_cmd_dhcpopt,
>  				       N_("VAR INTERFACE NUMBER DESCRIPTION"),
>  				       N_("retrieve DHCP option and save it into VAR. If VAR is - then print the value."));
> +  cmd_bootp6 = grub_register_command ("net_bootp6", grub_cmd_bootp6,
> +				     N_("[CARD]"),
> +				     N_("perform a dhcpv6 autoconfiguration"));
>  }
>  
>  void
> @@ -582,4 +1464,5 @@ grub_bootp_fini (void)
>  {
>    grub_unregister_command (cmd_getdhcp);
>    grub_unregister_command (cmd_bootp);
> +  grub_unregister_command (cmd_bootp6);
>  }
> diff --git a/grub-core/net/ip.c b/grub-core/net/ip.c
> index 8c56baa..8bb56be 100644
> --- a/grub-core/net/ip.c
> +++ b/grub-core/net/ip.c
> @@ -238,6 +238,41 @@ handle_dgram (struct grub_net_buff *nb,
>    {
>      struct udphdr *udph;
>      udph = (struct udphdr *) nb->data;
> +
> +    if (proto == GRUB_NET_IP_UDP && grub_be_to_cpu16 (udph->dst) == 546)

udph->dst == grub_cpu_to_be16_compile_time

> +      {
> +	if (udph->chksum)
> +	  {
> +	    grub_uint16_t chk, expected;
> +	    chk = udph->chksum;
> +	    udph->chksum = 0;
> +	    expected = grub_net_ip_transport_checksum (nb,
> +						       GRUB_NET_IP_UDP,
> +						       source,
> +						       dest);
> +	    if (expected != chk)
> +	      {
> +		grub_dprintf ("net", "Invalid UDP checksum. "
> +			      "Expected %x, got %x\n",
> +			      grub_be_to_cpu16 (expected),
> +			      grub_be_to_cpu16 (chk));
> +		grub_netbuff_free (nb);
> +		return GRUB_ERR_NONE;
> +	      }
> +	    udph->chksum = chk;
> +	  }
> +
> +	err = grub_netbuff_pull (nb, sizeof (*udph));
> +	if (err)
> +	  {
> +	    grub_netbuff_free (nb);
> +	    return err;
> +	  }
> +	grub_net_process_dhcp6 (nb, card);
> +	grub_netbuff_free (nb);
> +	return GRUB_ERR_NONE;
> +      }
> +
>      if (proto == GRUB_NET_IP_UDP && grub_be_to_cpu16 (udph->dst) == 68)
>        {
>  	const struct grub_net_bootp_packet *bootp;
> diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> index e5dd543..ff77750 100644
> --- a/include/grub/efi/api.h
> +++ b/include/grub/efi/api.h

This does not really belong to this patch.

> @@ -1340,14 +1340,68 @@ typedef struct grub_efi_simple_text_output_interface grub_efi_simple_text_output
>  
>  typedef grub_uint8_t grub_efi_pxe_packet_t[1472];
>  
> +typedef struct {
> +  grub_uint8_t addr[4];
> +} grub_efi_pxe_ipv4_address_t;
> +
> +typedef struct {
> +  grub_uint8_t addr[16];
> +} grub_efi_pxe_ipv6_address_t;
> +
> +typedef struct {
> +  grub_uint8_t addr[32];
> +} grub_efi_pxe_mac_address_t;
> +
> +typedef union {
> +    grub_uint32_t addr[4];
> +    grub_efi_pxe_ipv4_address_t v4;
> +    grub_efi_pxe_ipv6_address_t v6;
> +} grub_efi_pxe_ip_address_t;
> +
> +#define EFI_PXE_BASE_CODE_MAX_IPCNT             8
GRUB_EFI_* please

> +typedef struct {
> +    grub_uint8_t filters;
> +    grub_uint8_t ip_cnt;
> +    grub_uint16_t reserved;
> +    grub_efi_pxe_ip_address_t ip_list[EFI_PXE_BASE_CODE_MAX_IPCNT];
> +} grub_efi_pxe_ip_filter_t;
> +
> +typedef struct {
> +    grub_efi_pxe_ip_address_t ip_addr;
> +    grub_efi_pxe_mac_address_t mac_addr;
> +} grub_efi_pxe_arp_entry_t;
> +
> +typedef struct {
> +    grub_efi_pxe_ip_address_t ip_addr;
> +    grub_efi_pxe_ip_address_t subnet_mask;
> +    grub_efi_pxe_ip_address_t gw_addr;
> +} grub_efi_pxe_route_entry_t;
> +
> +
> +#define EFI_PXE_BASE_CODE_MAX_ARP_ENTRIES 8
> +#define EFI_PXE_BASE_CODE_MAX_ROUTE_ENTRIES 8
> +

Same

>  typedef struct grub_efi_pxe_mode
>  {
> -  grub_uint8_t unused[52];
> +  grub_uint8_t started;
> +  grub_uint8_t ipv6_available;
> +  grub_uint8_t ipv6_supported;
> +  grub_uint8_t using_ipv6;
> +  //grub_uint8_t unused[48];
> +  grub_uint8_t unused[16];
> +  grub_efi_pxe_ip_address_t station_ip;
> +  grub_efi_pxe_ip_address_t subnet_mask;
>    grub_efi_pxe_packet_t dhcp_discover;
>    grub_efi_pxe_packet_t dhcp_ack;
>    grub_efi_pxe_packet_t proxy_offer;
>    grub_efi_pxe_packet_t pxe_discover;
>    grub_efi_pxe_packet_t pxe_reply;
> +  grub_efi_pxe_packet_t pxe_bis_reply;
> +  grub_efi_pxe_ip_filter_t ip_filter;
> +  grub_uint32_t arp_cache_entries;
> +  grub_efi_pxe_arp_entry_t arp_cache[EFI_PXE_BASE_CODE_MAX_ARP_ENTRIES];
> +  grub_uint32_t route_table_entries;
> +  grub_efi_pxe_route_entry_t route_table[EFI_PXE_BASE_CODE_MAX_ROUTE_ENTRIES];
>  } grub_efi_pxe_mode_t;
>  
>  typedef struct grub_efi_pxe
> diff --git a/include/grub/net.h b/include/grub/net.h
> index 538baa3..71dc243 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -418,6 +418,13 @@ struct grub_net_bootp_packet
>    grub_uint8_t vendor[0];
>  } GRUB_PACKED;
>  
> +struct grub_net_dhcpv6_packet
> +{
> +  grub_uint32_t message_type:8;
> +  grub_uint32_t transaction_id:24;
> +  grub_uint8_t dhcp_options[0];
> +} GRUB_PACKED;
> +
>  #define	GRUB_NET_BOOTP_RFC1048_MAGIC_0	0x63
>  #define	GRUB_NET_BOOTP_RFC1048_MAGIC_1	0x82
>  #define	GRUB_NET_BOOTP_RFC1048_MAGIC_2	0x53
> @@ -444,6 +451,14 @@ grub_net_configure_by_dhcp_ack (const char *name,
>  				grub_size_t size,
>  				int is_def, char **device, char **path);
>  
> +struct grub_net_network_level_interface *
> +grub_net_configure_by_dhcpv6_reply (const char *name,
> +				    struct grub_net_card *card,
> +				    grub_net_interface_flags_t flags,
> +				    const struct grub_net_dhcpv6_packet *v6,
> +				    grub_size_t size,
> +				    int is_def, char **device, char **path);
> +
>  grub_err_t
>  grub_net_add_ipv4_local (struct grub_net_network_level_interface *inf,
>  			 int mask);
> @@ -452,6 +467,10 @@ void
>  grub_net_process_dhcp (struct grub_net_buff *nb,
>  		       struct grub_net_card *card);
>  
> +void
> +grub_net_process_dhcp6 (struct grub_net_buff *nb,
> +			struct grub_net_card *card);
> +
>  int
>  grub_net_hwaddr_cmp (const grub_net_link_level_address_t *a,
>  		     const grub_net_link_level_address_t *b);



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] Use UEFI MAC device as default configured by net_bootp6
  2015-04-15  9:05 ` [PATCH 3/3] Use UEFI MAC device as default configured by net_bootp6 Michael Chang
@ 2015-04-16 19:58   ` Andrei Borzenkov
  2015-04-17  6:41     ` Michael Chang
  0 siblings, 1 reply; 13+ messages in thread
From: Andrei Borzenkov @ 2015-04-16 19:58 UTC (permalink / raw)
  To: Michael Chang; +Cc: grub-devel

В Wed, 15 Apr 2015 17:05:09 +0800
Michael Chang <mchang@suse.com> пишет:

> The grub_efinet_findcards will register cards by checking if it can support EFI
> Simple Netowork Protocol which create more than one device to a physical NIC
> device.
> 

Yes, indeed.

  /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/EndEntire
  28be27e5-66cc-4a31-a315-db14c3744d85
  fa3cde4c-87c2-427d-aede-7dd096c88c58
  b95e9fda-26de-48d2-8807-1f9107ac5e3a
  d9760ff3-3cca-4267-80f9-7527fafa4223
  9fb9a8a1-2f4a-43a6-889c-d0f7b6c47ad5
  66ed4721-3c98-4d3e-81e3-d03dd39a7254
  ec20eb79-6c1a-4664-9a0d-d2e4cc16d664
  00720665-67eb-4a99-baf7-d3c33a1c7cc9
  937fe521-95ae-4d1a-8929-48bcd90ad31a
  ec835dd3-fe0f-617b-a621-b350c3e13388
  2fe800be-8f01-4aa6-946b-d71388e1833f
  9d9a39d8-bd42-4a73-a4d5-8ee94be11380
  83f01464-99bd-45e5-b383-af6305d8e9e6
  c51711e7-b4bf-404a-bfb8-0a048ef1ffe4
  3b95aa31-3793-434b-8667-c8070892e05e
  f44c00ee-1f2c-4a00-aa09-1c9f3e0800a3
  e4f61863-fe2c-4b56-a8f4-08519bc439df
  f36ff770-a7e1-42cf-9ed2-56f0f271f44c (Managed Network Protocol)
  9e23d768-d2f3-4366-9fc3-3a7aba864374 (VLAN Configuration Protocol)
  device path
  network

  /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/HardwareVendor(d79df6b0-ef44-43bd-9797-43e93bcf5fa8)[0:
  ]/EndEntire HII configuration access
  device path

  /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/HardwareVendor(d8944553-c4dd-41f4-9b30-e1397cfb267b)[0:
  ]/EndEntire HII configuration access
  device path

  /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/HardwareVendor(5bedb5cc-d830-4eb2-8742-2d4cc9b54f2c)[0:
  ]/EndEntire HII configuration access
  device path

  /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/IPv4(0.0.0.0,0.0.0.0,0,0,0,0)/EndEntire
  network
  pxe
  load file
  device path

  /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/IPv6(0:0:0:0:0:0:0:0,0:0:0:0:0:0:0:0,0,0,0,0)/EndEntire
  network
  pxe
  load file
  device path


> If without specifying any device to be configured by net_bootp6, it should pick
> up one from them but not all.

Right now I am not even able to netboot in QEMU. Booting from CD and
attempting to read anything over net results in random failure.

There are multiple reports of similar problem in EFI. One of possible
explanations is that EFI drivers are active and steal some packets from
GRUB.

There is patch suggested in bug report
https://savannah.gnu.org/bugs/index.php?41731 but it currently crashes
OVMF so I cannot test it. Also it effectively makes multiple efinet on
single HW card useless anyway (it will open exclusively IPv4 instance
thus blocking any access to MAC device using SNP).

So it looks like efinet needs redesign anyway.

>                              In my case three firmware device are listed.
> IPv4, IPv6 and MAC device. Both IPv4 and IPv6 are derived from MAC device for
> providing PXE Base Code Protocol. I think we should use MAC device instead of
> those two to avoid collision, because net_bootp6 command does not depend on PXE
> Base Code but only Simple Network Protocol to work
> ---
>  grub-core/net/bootp.c              |    8 +++++++
>  grub-core/net/drivers/efi/efinet.c |   40 ++++++++++++++++++++++++++++++++++++
>  include/grub/net.h                 |    1 +
>  3 files changed, 49 insertions(+), 0 deletions(-)
> 
> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> index 477f205..c4963ca 100644
> --- a/grub-core/net/bootp.c
> +++ b/grub-core/net/bootp.c
> @@ -1290,6 +1290,10 @@ grub_cmd_bootp6 (struct grub_command *cmd __attribute__ ((unused)),
>    {
>      if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
>        continue;
> +#ifdef GRUB_MACHINE_EFI
> +    else if (!card->is_efi_mac_device (card))
> +      continue;
> +#endif
>      ncards++;
>    }
>  
> @@ -1299,6 +1303,10 @@ grub_cmd_bootp6 (struct grub_command *cmd __attribute__ ((unused)),
>  
>      if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
>        continue;
> +#ifdef GRUB_MACHINE_EFI
> +    else if (!card->is_efi_mac_device (card))
> +      continue;
> +#endif
>  
>      ifaces = grub_net_ipv6_get_link_local (card, &card->default_address);
>      if (!ifaces)
> diff --git a/grub-core/net/drivers/efi/efinet.c b/grub-core/net/drivers/efi/efinet.c
> index b1837e3..900384f 100644
> --- a/grub-core/net/drivers/efi/efinet.c
> +++ b/grub-core/net/drivers/efi/efinet.c
> @@ -157,6 +157,45 @@ grub_efinet_get_device_handle (struct grub_net_card *card)
>    return card->efi_handle;
>  }
>  
> +static int
> +grub_efinet_is_mac_device (struct grub_net_card *card)
> +{
> +  grub_efi_handle_t efi_handle;
> +  grub_efi_device_path_t *dp;
> +  grub_efi_device_path_t *next, *p;
> +  grub_efi_uint8_t type;
> +  grub_efi_uint8_t subtype;
> +
> +  efi_handle = grub_efinet_get_device_handle (card);
> +
> +  if (!efi_handle)
> +    return 0;
> +
> +  dp = grub_efi_get_device_path (efi_handle);
> +
> +  if (GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp))
> +    return 0;
> +
> +  for (p = (grub_efi_device_path_t *) dp, next = GRUB_EFI_NEXT_DEVICE_PATH (p);
> +       ! GRUB_EFI_END_ENTIRE_DEVICE_PATH (next);
> +       p = next, next = GRUB_EFI_NEXT_DEVICE_PATH (next))
> +    ;
> +
> +  if (p)
> +    {
> +      type = GRUB_EFI_DEVICE_PATH_TYPE (p);
> +      subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (p);
> +
> +      if (type == GRUB_EFI_MESSAGING_DEVICE_PATH_TYPE
> +	  && subtype == GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
> +	{
> +	  return 1;
> +	}
> +    }
> +
> +  return 0;
> +}
> +
>  static void
>  grub_efinet_findcards (void)
>  {
> @@ -223,6 +262,7 @@ grub_efinet_findcards (void)
>  		   sizeof (card->default_address.mac));
>        card->efi_net = net;
>        card->efi_handle = *handle;
> +      card->is_efi_mac_device = grub_efinet_is_mac_device;
>  
>        grub_net_card_register (card);
>      }
> diff --git a/include/grub/net.h b/include/grub/net.h
> index 71dc243..4571b72 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -140,6 +140,7 @@ struct grub_net_card
>        struct grub_efi_simple_network *efi_net;
>        grub_efi_handle_t efi_handle;
>        grub_size_t last_pkt_size;
> +      int (*is_efi_mac_device) (struct grub_net_card* card);
>      };
>  #endif
>      void *data;



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Added net_bootp6 command
  2015-04-16 14:40   ` Andrei Borzenkov
@ 2015-04-17  5:04     ` Michael Chang
  2015-04-19  8:15       ` Andrei Borzenkov
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Chang @ 2015-04-17  5:04 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: grub-devel

Hi Andrei,

Please see my comments and please let me know any problems remains.

I'll submit next version soon.

Thanks a lot for your detailed review.

Regards,
Michael

On Thu, Apr 16, 2015 at 05:40:56PM +0300, Andrei Borzenkov wrote:
> В Wed, 15 Apr 2015 17:05:07 +0800
> Michael Chang <mchang@suse.com> пишет:
> 
> > The net_bootp6 is used to configure the ipv6 network interface through the
> > DHCPv6 protocol Solict/Advertise/Request/Reply.
> > ---
> >  grub-core/net/bootp.c  |  885 +++++++++++++++++++++++++++++++++++++++++++++++-
> >  grub-core/net/ip.c     |   35 ++
> >  include/grub/efi/api.h |   56 +++-
> >  include/grub/net.h     |   19 +
> >  4 files changed, 993 insertions(+), 2 deletions(-)
> > 
> > diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> > index 6136755..477f205 100644
> > --- a/grub-core/net/bootp.c
> > +++ b/grub-core/net/bootp.c
> > @@ -24,6 +24,7 @@
> >  #include <grub/net/netbuff.h>
> >  #include <grub/net/udp.h>
> >  #include <grub/datetime.h>
> > +#include <grub/time.h>
> >  
> >  static void
> >  parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask)
> > @@ -256,6 +257,653 @@ grub_net_configure_by_dhcp_ack (const char *name,
> >    return inter;
> >  }
> >  
> > +struct grub_dhcpv6_option {
> > +  grub_uint16_t code;
> > +  grub_uint16_t len;
> > +  grub_uint8_t data[0];
> > +} GRUB_PACKED;
> > +
> > +
> > +struct grub_dhcpv6_iana_option {
> > +  grub_uint32_t iaid;
> > +  grub_uint32_t t1;
> > +  grub_uint32_t t2;
> > +  grub_uint8_t data[0];
> > +} GRUB_PACKED;
> > +
> > +struct grub_dhcpv6_iaaddr_option {
> > +  grub_uint8_t addr[16];
> > +  grub_uint32_t preferred_lifetime;
> > +  grub_uint32_t valid_lifetime;
> > +  grub_uint8_t data[0];
> > +} GRUB_PACKED;
> > +
> > +struct grub_DUID_LL
> > +{
> > +  grub_uint16_t type;
> > +  grub_uint16_t hw_type;
> > +  grub_uint8_t hwaddr[6];
> > +} GRUB_PACKED;
> > +
> > +struct grub_dhcpv6_dns_servers {
> > +  grub_uint8_t addr[16];
> > +  grub_uint8_t next_addr[0];
> > +} GRUB_PACKED;
> > +
> 
> Do we really need it? Code just fetches 16 bytes blocks, it does not
> really need any structure definition?

No, It doesn't really matter that much. I was just sticking to use
structure to represent any option data and hopefully that helps in
understanding what it is. But in this case it seems to be the oppsite by
treating it as 16 bytes chunk of data generates more tidy code and
easier to understand.

I can change to your suggested code as commented in get_dns function and
remove this structure.

> 
> > +#define DHCPv6_REPLY 7
> > +#define DHCPv6_ADVERTISE 2
> > +#define DHCPv6_REQUEST 3
> > +#define OPTION_BOOTFILE_URL 59
> > +#define OPTION_DNS_SERVERS 23
> > +#define OPTION_IA_NA 3
> > +#define OPTION_IAADDR 5
> > +#define OPTION_CLIENTID 1
> > +#define OPTION_SERVERID 2
> > +#define OPTION_ORO 6
> > +#define OPTION_ELAPSED_TIME 8
> > +
> 
> This is better as enum and GRUB_ prefix. Also options need GRUB_DHCPv6_
> prefix.

OK.

> 
> > +struct grub_dhcpv6_session
> > +{
> > +  struct grub_dhcpv6_session *next;
> > +  struct grub_dhcpv6_session **prev;
> > +  grub_uint32_t iaid;
> > +  grub_uint32_t transaction_id:24;
> > +  grub_uint64_t start_time;
> > +  struct grub_net_network_level_interface *ifaces;
> 
> Can there be multiple interfaces as implied by plural?

No, One session for configuring one interface, the plural just created
confusing. I'll fix it.

> 
> > +};
> > +
> > +static struct grub_dhcpv6_session *grub_dhcpv6_sessions = NULL;
> > +#define FOR_DHCPV6_SESSIONS(var) \
> > +    for (var = grub_dhcpv6_sessions ; var; var = var->next)
> > +
> 
> FOR_LIST_ELEMENTS

OK.
> 
> > +static void
> > +grub_dhcpv6_session_add (struct grub_dhcpv6_session *session)
> > +{
> > +  struct grub_datetime date;
> > +  grub_err_t err;
> > +  grub_int32_t t = 0;
> > +
> > +  err = grub_get_datetime (&date);
> > +  if (err || !grub_datetime2unixtime (&date, &t))
> > +    {
> > +      grub_errno = GRUB_ERR_NONE;
> > +      t = 0;
> > +    }
> > +
> > +  session->transaction_id = t;
> > +  session->start_time = grub_get_time_ms ();
> > +
> > +  session->prev = &grub_dhcpv6_sessions;
> > +  session->next = grub_dhcpv6_sessions;
> > +
> > +  if (session->next)
> > +    session->next->prev = &session->next;
> > +
> 
> grub_list_push

OK.
> 
> > +  grub_dhcpv6_sessions = session;
> > +  return;
> > +}
> > +
> > +static void
> > +grub_dhcpv6_session_remove (struct grub_dhcpv6_session *session)
> > +{
> > +  *session->prev = session->next;
> > +  if (session->next)
> > +    session->next->prev = session->prev;
> > +  session->next = NULL;
> > +  session->prev = NULL;
> > +  return;
> > +}
> > +
> 
> grub_list_remove

OK.
> 
> > +static const struct grub_dhcpv6_option*
> > +find_dhcpv6_option (const struct grub_net_dhcpv6_packet *packet,
> > +		    grub_uint16_t option)
> > +{
> > +  grub_uint16_t code, len;
> > +  const struct grub_dhcpv6_option *popt;
> > +
> > +  popt = (const struct grub_dhcpv6_option *)packet->dhcp_options;
> > +  code = grub_be_to_cpu16 (popt->code);
> > +  len = grub_be_to_cpu16 (popt->len);
> > +
> > +  while (0 != code && option != code)
> 
> This probably needs upper boundary check in case we are dealing with
> corrupted packets.


Do you mean checking the option length can't exceed netbuff length?

The ( 0!= code ) did certain kind of upper boundary check for me, but
I know you may disagree that. :)

Btw, in grub-core/net/ip.c::handle_dgram() has checksum for the receiving
packets, so the corrputed packets wouldn't sneak in easily.

> 
> > +    {
> > +      popt = (const struct grub_dhcpv6_option *)((grub_uint8_t *)popt +
> > +		len + sizeof(*popt));
> > +      code = grub_be_to_cpu16 (popt->code);
> > +      len = grub_be_to_cpu16 (popt->len);
> > +    }
> > +
> > +  if (option == code)
> > +      return popt;
> > +
> 
> This can just be moved inside a loop, right?

OK.

> 
> > +  return NULL;
> > +}
> > +
> > +static const grub_uint8_t*
> > +find_dhcpv6_address (const struct grub_net_dhcpv6_packet *packet)
> > +{
> > +  const struct grub_dhcpv6_option* popt = find_dhcpv6_option (packet, OPTION_IA_NA);
> > +  const struct grub_dhcpv6_iana_option *ia_na;
> > +  const struct grub_dhcpv6_option *iaaddr_hdr;
> > +  const struct grub_dhcpv6_iaaddr_option *iaaddr;
> > +  grub_uint16_t ia_na_data_offset, ia_na_data_len, len;
> > +
> > +  if (grub_be_to_cpu16 (popt->code) != OPTION_IA_NA)
> 
> if (popt == NULL) 

OK.

> 
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("not an IA_NA DHCPv6 option"));
> > +      return NULL;
> > +    }
> > +
> > +  ia_na = (const struct grub_dhcpv6_iana_option *)popt->data;
> > +
> > +  if (grub_be_to_cpu16(popt->len) <= sizeof (*ia_na))
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("invalid size for IAADDR"));
> > +      return NULL;
> > +    }
> > +
> 
> Does it need upper boundary check?

If the check has been done in find_dhcpv6_option I don't think we need
to check again here, The find_dhcpv6_option should ensure that all
length are within the netbuff range before returning it.

> 
> > +  ia_na_data_len = grub_be_to_cpu16(popt->len) - sizeof (*ia_na);
> > +  ia_na_data_offset = 0;
> > +
> > +  iaaddr_hdr = (const struct grub_dhcpv6_option *) ia_na->data;
> > +  len = grub_be_to_cpu16 (iaaddr_hdr->len);
> > +
> > +  while (grub_be_to_cpu16(iaaddr_hdr->code) != OPTION_IAADDR)
> 
> grub_cpu_to_be16_compile_time(OPTION_IAADDR)

OK.

> 
> > +    {
> > +      ia_na_data_offset += (len + sizeof (*iaaddr_hdr));
> > +
> > +      if (ia_na_data_offset < ia_na_data_len)
> > +	{
> > +	  iaaddr_hdr =(const struct grub_dhcpv6_option *)(ia_na->data +
> > +	    ia_na_data_offset);
> > +	  len = grub_be_to_cpu16 (iaaddr_hdr->len);
> > +	}
> > +      else
> > +	{
> > +	  iaaddr_hdr = NULL;
> > +	  break;
> > +	}
> > +    }
> > +
> > +  if (!iaaddr_hdr)
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("IAADDR not found"));
> > +      return NULL;
> > +    }
> > +
> > +  if ((ia_na_data_offset + sizeof (*iaaddr_hdr) + len) > ia_na_data_len)
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("IAADDR size check failed"));
> > +      return NULL;
> > +    }
> > +
> > +  iaaddr = (const struct grub_dhcpv6_iaaddr_option *) iaaddr_hdr->data;
> > +
> > +  return iaaddr->addr;
> 
> It sounds again like most code could be folded inside a loop
> 
> while (remaining_length > 0)
>   if (option_length > remaining_length)
>     error
>   if (option_code == IAADDR)
>     return hdr->addr
>   hdr = (char *)hdr + option_length
>   remaining_length -= option_length
> 
> return not found

OK. I'll clean it for more readable code like what you suggested.

> > +}
> > +
> > +static void
> > +get_dhcpv6_dns_address (const struct grub_net_dhcpv6_packet *packet,
> > +	grub_net_network_level_address_t **addr, grub_uint16_t *naddr)
> > +{
> > +  const struct grub_dhcpv6_option* popt;
> > +  const struct grub_dhcpv6_dns_servers *dns;
> > +  grub_uint16_t len;
> > +  const grub_uint8_t *pa;
> > +  int i, ln;
> > +  grub_net_network_level_address_t *la;
> > +
> > +  if (addr)
> > +    *addr = NULL;
> > +
> > +  if (naddr)
> > +    *naddr = 0;
> > +
> > +  popt = find_dhcpv6_option (packet, OPTION_DNS_SERVERS);
> > +  if (!popt)
> > +    return;
> > +
> > +  len = grub_be_to_cpu16 (popt->len);
> > +  if ((len % 16) != 0)
> 
> len & 0xf; 0 comparison probably redundant as well.

OK.

> 
> Again, what about upper boundary check?


See above.

> 
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("invalid dns address length"));
> > +      return;
> > +    }
> > +
> > +  dns = (const struct grub_dhcpv6_dns_servers *)popt->data;
> > +
> > +  ln = len / 16;
> 
> len >> 4

OK.

> 
> > +  la = grub_zalloc (sizeof (grub_net_network_level_address_t) * ln);
> > +
> 
>      *addr = la = grub_zalloc (...)
> 
> NULL check

OK.

> 
> > +  for (i = 0, pa = dns->addr; i < ln; i++, pa = dns->next_addr)
>                  ^^^ not needed               pa += 16, la++
> > +    {
> > +      (la + i)->type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> > +      (la + i)->ipv6[0] = grub_get_unaligned64 (pa);
> > +      (la + i)->ipv6[1] = grub_get_unaligned64 (pa + 8);
> > +      (la + i)->option = DNS_OPTION_PREFER_IPV6;
> 
>           la->...
> 
> > +    }
> > +
> > +  *addr = la;
>      not needed


See above (agree to use 16 byte chuncks).

> 
> > +  *naddr = ln;
> > +
> > +  return;
> > +}
> > +
> > +static void
> > +find_dhcpv6_bootfile_url (const struct grub_net_dhcpv6_packet *packet,
> > +	char **proto, char **server_ip, char **boot_file)
> > +{
> > +  char *bootfile_url;
> > +  const struct grub_dhcpv6_option* opt_url;
> > +  char *ip_start, *ip_end;
> > +  char *path;
> > +  grub_size_t ip_len;
> > +  grub_uint16_t len;
> > +  const char *protos[] = {"tftp://", "http://", NULL};
> > +  const char *pr;
> > +  int i;
> > +
> > +  if (proto)
> > +    *proto = NULL;
> > +
> > +  if (server_ip)
> > +    *server_ip = NULL;
> > +
> > +  if (boot_file)
> > +    *boot_file = NULL;
> > +
> > +  opt_url = find_dhcpv6_option (packet, OPTION_BOOTFILE_URL);
> > +
> > +  if (!opt_url)
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("no bootfile-url in DHCPv6 option"));
> > +      return;
> > +    }
> > +
> > +  len = grub_be_to_cpu16 (opt_url->len);
> > +
> 
> Obligatory question about upper boundary check :)

See above. Do it in find_dhcpv6_option once and for all? :)

> 
> > +  bootfile_url = grub_malloc (len + 1);
> > +
> > +  if (!bootfile_url)
> > +    return;
> > +
> > +  grub_memcpy (bootfile_url, opt_url->data, len);
> > +  bootfile_url[len]   = '\0';
> > +
> > +  for (i = 0; (pr = *(protos + i)); ++i)
> > +      if (grub_strncmp (bootfile_url, pr, grub_strlen(pr)) == 0)
> > +	break;
> > +
> > +  if (!pr)
> > +    {
> > +      grub_error (GRUB_ERR_IO,
> > +	N_("unsupported protocol, only tftp and http are supported"));
> > +      goto cleanup;
> > +    }
> > +
> > +  ip_start = ip_end = NULL;
> > +  ip_start = bootfile_url + grub_strlen(pr);
> > +
> > +  if (*ip_start != '[')
> > +    ip_start = NULL;
> > +  else
> > +    ip_end = grub_strchr (++ip_start, ']');
> > +
> > +  if (!ip_start || !ip_end)
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("IPv6-address not in square brackets"));
> > +      goto cleanup;
> > +    }
> > +
> 
> Can bootfile_url contain a name and not IPv6 address? Or is address
> mandatory?

Yes. The string in URL form is mandatory.

> 
> > +  ip_len = ip_end - ip_start;
> > +
> > +  if (proto)
> > +    {
> > +      grub_size_t proto_len  = grub_strlen (pr) - 3;
> > +
> > +      *proto = grub_malloc (proto_len + 1);
> > +      if (!*proto)
> > +	goto cleanup;
> > +
> > +      grub_memcpy (*proto, pr, proto_len);
> > +      *(*proto + proto_len)  = '\0';
> 
> Umm ... we really need something like grub_mem2str. Actually
> grub_xasprintf("%.*s") would be just fine if grub supported it.

OK.

> 
> > +    }
> > +
> > +  if (server_ip)
> > +    {
> > +      *server_ip = grub_malloc (ip_len + 1);
> > +
> > +      if (!*server_ip)
> > +	goto cleanup;
> > +
> > +      grub_memcpy (*server_ip, ip_start, ip_len);
> > +      *(*server_ip + ip_len) = '\0';
> > +    }
> > +
> > +  path = ip_end + 1;
> > +
> > +  if (boot_file)
> > +    {
> > +      *boot_file = grub_strdup (path);
> > +
> > +      if (!*boot_file)
> > +	goto cleanup;
> > +    }
> > +
> > +cleanup:
> > +
> > +  if (bootfile_url)
> > +    grub_free (bootfile_url);
> > +
> 
> grub_free checks for NULL
OK.
> 
> > +  if (grub_errno)
> > +    {
> > +      if (proto && *proto)
> > +	{
> > +	  grub_free (proto);
> 
> grub_free (*proto)
OK.
> 
> > +	  *proto = NULL;
> > +	}
> > +
> > +      if (server_ip && *server_ip)
> > +	{
> > +	  grub_free (server_ip);
> grub_free (*server_ip)
OK.
> 
> > +	  *server_ip = NULL;
> > +	}
> > +
> > +      if (boot_file && *boot_file)
> > +	{
> > +	  grub_free (boot_file);
> grub_free (*boot_file)
OK.
> 
> > +	  *boot_file = NULL;
> > +	}
> > +    }
> > +
> > +  return;
> > +}
> > +
> > +
> > +static grub_err_t
> > +grub_net_configure_by_dhcpv6_adv (const struct grub_net_dhcpv6_packet *v6_adv,
> > +	struct grub_dhcpv6_session *session)
> > +{
> > +  struct grub_net_buff *nb;
> > +  const struct grub_dhcpv6_option *opt_client, *opt_server, *opt_iana;
> > +  struct grub_dhcpv6_option *popt;
> > +  struct grub_net_dhcpv6_packet *v6;
> > +  struct udphdr *udph;
> > +  grub_net_network_level_address_t multicast;
> > +  grub_net_link_level_address_t ll_multicast;
> > +  struct grub_net_network_level_interface *inf;
> > +  grub_err_t err;
> > +  grub_uint16_t len;
> > +  grub_uint64_t elapsed;
> > +  char err_msg[64];
> > +
> > +  if (v6_adv->message_type != DHCPv6_ADVERTISE)
> 
> We come here after checking message type, or am I wrong?

OK. We either have to remove the outer check or this,

> 
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("DHCPv6 info not found"));
> > +      return grub_errno;
> > +    }
> > +
> > +  opt_client = find_dhcpv6_option (v6_adv, OPTION_CLIENTID);
> > +  opt_server = find_dhcpv6_option (v6_adv, OPTION_SERVERID);
> > +  opt_iana = find_dhcpv6_option (v6_adv, OPTION_IA_NA);
> > +
> > +  err_msg[0] = '\0';
> > +  if (!opt_client)
> > +      grub_strcpy (err_msg, "client id");
> > +
> > +  if (!opt_server)
> > +    {
> > +      if (grub_strlen (err_msg))
> > +	grub_strcpy (err_msg + grub_strlen (err_msg), ", server id");
> > +      else
> > +	grub_strcpy (err_msg, "server id");
> > +    }
> > +
> > +  if (!opt_iana)
> > +    {
> > +      if (grub_strlen (err_msg))
> > +	grub_strcpy (err_msg + grub_strlen (err_msg), ", iana");
> > +      else
> > +	grub_strcpy (err_msg, "iana");
> > +    }
> > +
> > +  if (grub_strlen (err_msg))
> > +    {
> > +      grub_strcpy (err_msg + grub_strlen (err_msg), " missing");
> > +      grub_error (GRUB_ERR_IO, N_(err_msg));
> 
> This means it will not be extracted by xgettext and we need 7 different
> strings added manually. This needs some change if you really want it to
> be translated. May be use grub_error_push with "Mandatory DHCPv6 option
> %s missing" or similar?

OK.

> 
> > +      return grub_errno;
> > +    }
> > +
> > +  inf = session->ifaces;
> > +
> > +  multicast.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> > +  multicast.ipv6[0] = grub_cpu_to_be64_compile_time (0xff02ULL << 48);
> > +  multicast.ipv6[1] = grub_cpu_to_be64_compile_time (0x10002ULL);
> > +
> > +  err = grub_net_link_layer_resolve (inf, &multicast, &ll_multicast);
> > +  if (err)
> > +    return err;
> > +
> > +  nb = grub_netbuff_alloc (512);
> 
> Symbolic constant would be nice. Where this size comes from?

It's quite liberal : any size that's sufficiently large to hold the
incoming populated netbuff data. :)

> 
> > +
> > +  if (!nb)
> > +    {
> > +      grub_netbuff_free (nb);
> 
> nb is NULL at this point, not?

Duh, really stupid code. Will fix it. 

> 
> > +      return grub_errno;
> > +    }
> > +
> > +  err = grub_netbuff_reserve (nb, 512);
> > +  if (err)
> > +    {
> > +      grub_netbuff_free (nb);
> > +      return err;
> > +    }
> > +
> > +  len = grub_cpu_to_be16(opt_client->len);
> > +  err = grub_netbuff_push (nb, len + 4);
>                                   len + sizeof (*opt_client)
OK.
> > +  if (err)
> > +    {
> > +      grub_netbuff_free (nb);
> > +      return err;
> > +    }
> > +  grub_memcpy (nb->data, opt_client, len + 4);
>   ditto
OK.
> 
> > +
> > +  len = grub_cpu_to_be16(opt_server->len);
> > +  err = grub_netbuff_push (nb, len + 4);
>   ditto
OK.
> 
> > +  if (err)
> > +    {
> > +      grub_netbuff_free (nb);
> > +      return err;
> > +    }
> > +  grub_memcpy (nb->data, opt_server, len + 4);
>   ditto
OK.
> 
> > +
> > +  len = grub_cpu_to_be16(opt_iana->len);
> > +  err = grub_netbuff_push (nb, len + 4);
>   ditto
OK.
> 
> > +  if (err)
> > +    {
> > +      grub_netbuff_free (nb);
> > +      return err;
> > +    }
> > +  grub_memcpy (nb->data, opt_iana, len + 4);
>   ditto
OK.
> 
> > +
> > +  err = grub_netbuff_push (nb, 8);
> sizeof (struct grub_dhcpv6_option) + 2 * sizeof (grub_uint16_t)
> probably makes it more clear.
OK.
> 
> > +  if (err)
> > +    {
> > +      grub_netbuff_free (nb);
> > +      return err;
> > +    }
> > +
> > +  popt = (struct grub_dhcpv6_option*) nb->data;
> > +  popt->code = grub_cpu_to_be16_compile_time (OPTION_ORO);
> > +  popt->len = grub_cpu_to_be16_compile_time (4);
> 
> and here 2 * sizeof (grub_uint16_t) as well.
OK.
> 
> > +  grub_set_unaligned16 (popt->data, grub_cpu_to_be16_compile_time (OPTION_BOOTFILE_URL));
> > +  grub_set_unaligned16 (popt->data + 2, grub_cpu_to_be16_compile_time (OPTION_DNS_SERVERS));
> > +
> > +  err = grub_netbuff_push (nb, 6);
> > +  if (err)
> > +    {
> > +      grub_netbuff_free (nb);
> > +      return err;
> > +    }
> > +  popt = (struct grub_dhcpv6_option*) nb->data;
> > +  popt->code = grub_cpu_to_be16_compile_time (OPTION_ELAPSED_TIME);
> > +  popt->len = grub_cpu_to_be16_compile_time (2);
> > +
> 
> similar
> 
> > +  // the time is expressed in hundredths of a second
> Please, let's stick to C comments
OK.
> 
> > +  elapsed = grub_divmod64 (grub_get_time_ms () - session->start_time, 10, 0);
> > +
> > +  if (elapsed > 0xffff)
> > +    elapsed = 0xffff;
> > +
> > +  grub_set_unaligned16 (popt->data,  grub_cpu_to_be16 ((grub_uint16_t)elapsed));
> > +
> > +  err = grub_netbuff_push (nb, 4);
> also better sizeof to avoid magic constants
OK.
> 
> > +  if (err)
> > +    {
> > +      grub_netbuff_free (nb);
> > +      return err;
> > +    }
> > +
> > +  v6 = (struct grub_net_dhcpv6_packet *) nb->data;
> > +  v6->message_type = DHCPv6_REQUEST;
> > +  v6->transaction_id = v6_adv->transaction_id;
> > +
> > +  err = grub_netbuff_push (nb, sizeof (*udph));
> > +  if (err)
> > +    {
> > +      grub_netbuff_free (nb);
> > +      return err;
> > +    }
> > +
> > +  udph = (struct udphdr *) nb->data;
> > +  udph->src = grub_cpu_to_be16_compile_time (546);
> > +  udph->dst = grub_cpu_to_be16_compile_time (547);
> > +  udph->chksum = 0;
> > +  udph->len = grub_cpu_to_be16 (nb->tail - nb->data);
> > +
> > +  udph->chksum = grub_net_ip_transport_checksum (nb, GRUB_NET_IP_UDP,
> > +						 &inf->address,
> > +						 &multicast);
> > +  err = grub_net_send_ip_packet (inf, &multicast, &ll_multicast, nb,
> > +				 GRUB_NET_IP_UDP);
> > +
> > +  grub_netbuff_free (nb);
> > +
> > +  if (err)
> 
> Just initialize it from start to GRUB_ERR_NONE?
OK.
> 
> > +    return err;
> > +
> > +  return GRUB_ERR_NONE;
> > +}
> > +
> > +
> > +struct grub_net_network_level_interface *
> > +grub_net_configure_by_dhcpv6_reply (const char *name,
> > +	struct grub_net_card *card,
> > +	grub_net_interface_flags_t flags,
> > +	const struct grub_net_dhcpv6_packet *v6,
> > +	grub_size_t size __attribute__ ((unused)),
> > +	int is_def,
> > +	char **device, char **path)
> > +{
> > +  grub_net_network_level_address_t addr;
> > +  grub_net_network_level_netaddress_t netaddr;
> > +  struct grub_net_network_level_interface *inf;
> > +  const grub_uint8_t *your_ip;
> > +  char *proto;
> > +  char *server_ip;
> > +  char *boot_file;
> > +  grub_net_network_level_address_t *dns;
> > +  grub_uint16_t num_dns;
> > +
> > +  if (device)
> > +    *device = NULL;
> > +
> > +  if (path)
> > +    *path = NULL;
> > +
> > +  if (v6->message_type != DHCPv6_REPLY)
> 
> Is it really possible? We come here if packet is DHCPv6_REPLY only.

I'm in case some evil firmware to cache some other packets type rather
than reply (or even junks in it ..).

> 
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("DHCPv6 info not found"));
> > +      return NULL;
> > +    }
> > +
> > +  your_ip = find_dhcpv6_address(v6);
> > +
> > +  if (!your_ip)
> > +    {
> > +      grub_error (GRUB_ERR_IO, N_("DHCPv6 address not found"));
> > +      return NULL;
> > +    }
> > +
> > +  get_dhcpv6_dns_address (v6, &dns, &num_dns);
> > +
> > +  if (dns && num_dns)
> > +    {
> > +      int i;
> > +
> > +      for (i = 0; i < num_dns; ++i)
> > +	grub_net_add_dns_server (dns + i);
> > +
> > +      grub_free (dns);
> > +    }
> > +  else
> > +    {
> > +      if (grub_errno)
> > +	grub_print_error ();
> > +    }
> > +
> 
> braces not needed.

OK.

> 
> > +  find_dhcpv6_bootfile_url (v6, &proto, &server_ip, &boot_file);
> > +
> > +  if (grub_errno)
> > +    grub_print_error ();
> > +
> > +  addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> > +  addr.ipv6[0] = grub_get_unaligned64 (your_ip);
> > +  addr.ipv6[1] = grub_get_unaligned64 (your_ip + 8);
> > +  inf = grub_net_add_addr (name, card, &addr, &card->default_address, flags);
> > +
> > +  netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> > +  netaddr.ipv6.base[0] = grub_get_unaligned64 (your_ip);
> > +  netaddr.ipv6.base[1] = 0;
> > +  netaddr.ipv6.masksize = 64;
> > +  grub_net_add_route (name, netaddr, inf);
> > +
> > +  grub_env_set_net_property (name, "boot_file", boot_file,
> > +			  grub_strlen (boot_file));
> > +
> 
> boot_file can be NULL, right?

Yes. I will fix it.

> 
> > +  if (is_def && server_ip)
> > +    {
> > +      grub_net_default_server = grub_strdup (server_ip);
> > +      grub_env_set ("net_default_interface", name);
> > +      grub_env_export ("net_default_interface");
> > +    }
> > +
> > +  if (device && server_ip && proto)
> > +    {
> > +      *device = grub_xasprintf ("%s,%s", proto, server_ip);
> > +      if (!*device)
> > +	return NULL;
> > +    }
> > +
> > +  if (path && boot_file)
> > +    {
> > +      *path = grub_strdup (boot_file);
> > +      if (*path)
> > +	{
> > +	  char *slash;
> > +	  slash = grub_strrchr (*path, '/');
> > +	  if (slash)
> > +	    *slash = 0;
> > +	  else
> > +	    **path = 0;
> > +	}
> > +      else
> > +	return NULL;
> > +    }
> > +
> > +  return inf;
> 
> This smells like memory leak in multiple places above. proto,
> server_ip, etc

Yes. I will fix them.

> 
> > +}
> > +
> >  void
> >  grub_net_process_dhcp (struct grub_net_buff *nb,
> >  		       struct grub_net_card *card)
> > @@ -288,6 +936,67 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
> >      }
> >  }
> >  
> > +void
> > +grub_net_process_dhcp6 (struct grub_net_buff *nb,
> > +	struct grub_net_card *card __attribute__ ((unused)))
> > +{
> > +  const struct grub_net_dhcpv6_packet *v6;
> > +  struct grub_dhcpv6_session *session;
> > +  const struct grub_dhcpv6_option *opt_iana;
> > +  const struct grub_dhcpv6_iana_option *ia_na;
> > +
> > +  v6 = (const struct grub_net_dhcpv6_packet *) nb->data;
> > +
> > +  opt_iana = find_dhcpv6_option (v6, OPTION_IA_NA);
> > +  if (!opt_iana)
> > +    return;
> > +
> > +  ia_na = (const struct grub_dhcpv6_iana_option *)opt_iana->data;
> > +  FOR_DHCPV6_SESSIONS (session)
> > +    {
> > +      if (session->transaction_id == v6->transaction_id
> > +	  && session->iaid == grub_cpu_to_be32 (ia_na->iaid))
> > +	break;
> > +    }
> > +
> > +  if (!session)
> > +    return;
> > +
> > +
> > +  if (v6->message_type == DHCPv6_ADVERTISE)
> > +    {
> > +      grub_net_configure_by_dhcpv6_adv (
> > +	  (const struct grub_net_dhcpv6_packet*) nb->data, session);
> > +    }
> > +  else if (v6->message_type == DHCPv6_REPLY)
> > +    {
> > +      char *name;
> > +      struct grub_net_network_level_interface *inf;
> > +
> > +      inf = session->ifaces;
> > +      name = grub_xasprintf ("%s:dhcp", inf->card->name);
> > +      if (!name)
> > +	return;
> > +
> > +      grub_net_configure_by_dhcpv6_reply (name, inf->card,
> > +	  0, (const struct grub_net_dhcpv6_packet *) nb->data,
> > +	  (nb->tail - nb->data), 0, 0, 0);
> > +
> > +      if (!grub_errno)
> > +	{
> > +	  grub_dhcpv6_session_remove (session);
> > +	  grub_free (session);
> > +	}
> > +
> > +      grub_free (name);
> > +    }
> > +
> > +  if (grub_errno)
> > +    grub_print_error ();
> > +
> > +  return;
> > +}
> > +
> >  static char
> >  hexdigit (grub_uint8_t val)
> >  {
> > @@ -564,7 +1273,177 @@ grub_cmd_bootp (struct grub_command *cmd __attribute__ ((unused)),
> >    return err;
> >  }
> >  
> > -static grub_command_t cmd_getdhcp, cmd_bootp;
> > +static grub_err_t
> > +grub_cmd_bootp6 (struct grub_command *cmd __attribute__ ((unused)),
> > +	int argc, char **args)
> > +{
> > +  struct grub_net_card *card;
> > +  grub_size_t ncards = 0;
> > +  unsigned j = 0;
> > +  int interval;
> > +  grub_err_t err;
> > +  struct grub_dhcpv6_session *session;
> > +
> > +  err = GRUB_ERR_NONE;
> > +
> > +  FOR_NET_CARDS (card)
> > +  {
> > +    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
> > +      continue;
> > +    ncards++;
> > +  }
> > +
> 
> where ncards is used?

It's not used, simply uncleaned code copy-paste from origianl net_bootp
command.

> 
> > +  FOR_NET_CARDS (card)
> > +  {
> > +    struct grub_net_network_level_interface *ifaces;
> > +
> 
> Why plural? It was array in original code, but not here, right?

Yes. You bet it. I will fix it.

> 
> > +    if (argc > 0 && grub_strcmp (card->name, args[0]) != 0)
> > +      continue;
> > +
> > +    ifaces = grub_net_ipv6_get_link_local (card, &card->default_address);
> > +    if (!ifaces)
> > +      {
> > +	grub_free (ifaces);
> 
> You still insist on freeing NULL pointers :)

(Sigh). I'm speech-less to my own stupid code. :(
It will be fixed next version. 

> 
> > +	return grub_errno;
> > +      }
> > +
> 
> Allocated sessions are leaked.

OK.
> 
> > +    session = grub_zalloc (sizeof (*session));
> > +    session->ifaces = ifaces;
> > +    session->iaid = j;
> > +    grub_dhcpv6_session_add (session);
> > +    j++;
> > +  }
> > +
> > +  for (interval = 200; interval < 10000; interval *= 2)
> > +    {
> > +      int done = 1;
> > +
> > +      FOR_DHCPV6_SESSIONS (session)
> > +	{
> Something strange with indentation.

OK.

> 
> > +	  struct grub_net_buff *nb;
> > +	  struct grub_dhcpv6_option *opt;
> > +	  struct grub_net_dhcpv6_packet *v6;
> > +	  struct grub_DUID_LL *duid;
> > +	  struct grub_dhcpv6_iana_option *ia_na;
> > +	  grub_net_network_level_address_t multicast;
> > +	  grub_net_link_level_address_t ll_multicast;
> > +	  struct udphdr *udph;
> > +
> > +	  multicast.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> > +	  multicast.ipv6[0] = grub_cpu_to_be64_compile_time (0xff02ULL << 48);
> > +	  multicast.ipv6[1] = grub_cpu_to_be64_compile_time (0x10002ULL);
> > +
> > +	  err = grub_net_link_layer_resolve (session->ifaces,
> > +		    &multicast, &ll_multicast);
> > +	  if (err)
> > +	    return grub_errno;
> 
> Allocated sessions?
> 
> > +	  nb = grub_netbuff_alloc (512);
> > +	  if (!nb)
> > +	    {
> > +	      grub_netbuff_free (nb);
> 
> free(NULL)
> 
> > +	      return grub_errno;
> > +	    }
> > +
> 
> memory leak

OK.
> 
> > +	  err = grub_netbuff_reserve (nb, 512);
> > +	  if (err)
> > +	    {
> > +	      grub_netbuff_free (nb);
> > +	      return err;
> > +	    }
> > +
> 
> memory leak

OK.
> 
> > +	  err = grub_netbuff_push (nb, 6);
> 
> I wonder if some good macro around sizeof could be used here to avoid
> hardcoding magic numbers.
> 
> > +	  if (err)
> > +	    {
> > +	      grub_netbuff_free (nb);
> > +	      return err;
> > +	    }
> > +
> 
> memory leak
OK.
> 
> > +	  opt = (struct grub_dhcpv6_option *)nb->data;
> > +	  opt->code = grub_cpu_to_be16_compile_time (OPTION_ELAPSED_TIME);
> > +	  opt->len = grub_cpu_to_be16_compile_time (2);
> > +	  grub_set_unaligned16 (opt->data, 0);
> > +
> > +	  err = grub_netbuff_push (nb, sizeof(*duid) + 4);
> > +	  if (err)
> > +	    {
> > +	      grub_netbuff_free (nb);
> > +	      return err;
> > +	    }
> > +
> 
> memory leak
OK.
> 
> > +	  opt = (struct grub_dhcpv6_option *)nb->data;
> > +	  opt->code = grub_cpu_to_be16_compile_time (OPTION_CLIENTID); //option_client_id
> 
> C++ comments
OK.
> 
> > +	  opt->len = grub_cpu_to_be16 (sizeof(*duid));
> > +
> > +	  duid = (struct grub_DUID_LL *) opt->data;
> > +
> > +	  duid->type = grub_cpu_to_be16_compile_time (3) ;
> > +	  duid->hw_type = grub_cpu_to_be16_compile_time (1);
> > +	  grub_memcpy (&duid->hwaddr, &session->ifaces->hwaddress.mac,
> > +	      sizeof (session->ifaces->hwaddress.mac));
> > +
> > +	  err = grub_netbuff_push (nb, sizeof (*ia_na) + 4);
> > +	  if (err)
> > +	    {
> > +	      grub_netbuff_free (nb);
> > +	      return err;
> > +	    }
> > +
> 
> memory leak
OK.
> 
> > +	  opt = (struct grub_dhcpv6_option *)nb->data;
> > +	  opt->code = grub_cpu_to_be16_compile_time (OPTION_IA_NA);
> > +	  opt->len = grub_cpu_to_be16 (sizeof (*ia_na));
> > +	  ia_na = (struct grub_dhcpv6_iana_option *)opt->data;
> > +	  ia_na->iaid = grub_cpu_to_be32 (session->iaid);
> > +	  ia_na->t1 = 0;
> > +	  ia_na->t2 = 0;
> > +
> > +	  err = grub_netbuff_push (nb, 4);
> > +	  if (err)
> > +	    {
> > +	      grub_netbuff_free (nb);
> > +	      return err;
> > +	    }
> > +
> 
> memory leak
OK.
> 
> > +	  v6 = (struct grub_net_dhcpv6_packet *)nb->data;
> > +	  v6->message_type = 1;
> > +	  v6->transaction_id = session->transaction_id;
> > +
> > +	  grub_netbuff_push (nb, sizeof (*udph));
> > +
> > +	  udph = (struct udphdr *) nb->data;
> > +	  udph->src = grub_cpu_to_be16_compile_time (546);
> > +	  udph->dst = grub_cpu_to_be16_compile_time (547);
> > +	  udph->chksum = 0;
> > +	  udph->len = grub_cpu_to_be16 (nb->tail - nb->data);
> > +
> > +	  udph->chksum = grub_net_ip_transport_checksum (nb, GRUB_NET_IP_UDP,
> > +			    &session->ifaces->address, &multicast);
> > +
> > +	  err = grub_net_send_ip_packet (session->ifaces, &multicast,
> > +		    &ll_multicast, nb, GRUB_NET_IP_UDP);
> > +	  done = 0;
> > +	  grub_netbuff_free (nb);
> > +
> > +	  if (err)
> > +	    return err;
> 
> memory leak
OK.
> 
> > +	}
> > +      if (!done)
> > +	grub_net_poll_cards (interval, 0);
> > +    }
> > +
> > +  FOR_DHCPV6_SESSIONS (session)
> > +    {
> > +      err = grub_error (GRUB_ERR_FILE_NOT_FOUND,
> > +			N_("couldn't autoconfigure %s"),
> > +			session->ifaces->card->name);
> 
> Unless I misunderstand something only last error is printed anyway. May
> be it should push and then print them outside of loop?

OK.
> 
> > +      grub_dhcpv6_session_remove (session);
> > +      grub_free (session);
> > +    }
> > +
> > +
> > +  return err;
> > +}
> > +
> > +static grub_command_t cmd_getdhcp, cmd_bootp, cmd_bootp6;
> >  
> >  void
> >  grub_bootp_init (void)
> > @@ -575,6 +1454,9 @@ grub_bootp_init (void)
> >    cmd_getdhcp = grub_register_command ("net_get_dhcp_option", grub_cmd_dhcpopt,
> >  				       N_("VAR INTERFACE NUMBER DESCRIPTION"),
> >  				       N_("retrieve DHCP option and save it into VAR. If VAR is - then print the value."));
> > +  cmd_bootp6 = grub_register_command ("net_bootp6", grub_cmd_bootp6,
> > +				     N_("[CARD]"),
> > +				     N_("perform a dhcpv6 autoconfiguration"));
> >  }
> >  
> >  void
> > @@ -582,4 +1464,5 @@ grub_bootp_fini (void)
> >  {
> >    grub_unregister_command (cmd_getdhcp);
> >    grub_unregister_command (cmd_bootp);
> > +  grub_unregister_command (cmd_bootp6);
> >  }
> > diff --git a/grub-core/net/ip.c b/grub-core/net/ip.c
> > index 8c56baa..8bb56be 100644
> > --- a/grub-core/net/ip.c
> > +++ b/grub-core/net/ip.c
> > @@ -238,6 +238,41 @@ handle_dgram (struct grub_net_buff *nb,
> >    {
> >      struct udphdr *udph;
> >      udph = (struct udphdr *) nb->data;
> > +
> > +    if (proto == GRUB_NET_IP_UDP && grub_be_to_cpu16 (udph->dst) == 546)
> 
> udph->dst == grub_cpu_to_be16_compile_time

OK.

> 
> > +      {
> > +	if (udph->chksum)
> > +	  {
> > +	    grub_uint16_t chk, expected;
> > +	    chk = udph->chksum;
> > +	    udph->chksum = 0;
> > +	    expected = grub_net_ip_transport_checksum (nb,
> > +						       GRUB_NET_IP_UDP,
> > +						       source,
> > +						       dest);
> > +	    if (expected != chk)
> > +	      {
> > +		grub_dprintf ("net", "Invalid UDP checksum. "
> > +			      "Expected %x, got %x\n",
> > +			      grub_be_to_cpu16 (expected),
> > +			      grub_be_to_cpu16 (chk));
> > +		grub_netbuff_free (nb);
> > +		return GRUB_ERR_NONE;
> > +	      }
> > +	    udph->chksum = chk;
> > +	  }
> > +
> > +	err = grub_netbuff_pull (nb, sizeof (*udph));
> > +	if (err)
> > +	  {
> > +	    grub_netbuff_free (nb);
> > +	    return err;
> > +	  }
> > +	grub_net_process_dhcp6 (nb, card);
> > +	grub_netbuff_free (nb);
> > +	return GRUB_ERR_NONE;
> > +      }
> > +
> >      if (proto == GRUB_NET_IP_UDP && grub_be_to_cpu16 (udph->dst) == 68)
> >        {
> >  	const struct grub_net_bootp_packet *bootp;
> > diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> > index e5dd543..ff77750 100644
> > --- a/include/grub/efi/api.h
> > +++ b/include/grub/efi/api.h
> 
> This does not really belong to this patch.
> 
> > @@ -1340,14 +1340,68 @@ typedef struct grub_efi_simple_text_output_interface grub_efi_simple_text_output
> >  
> >  typedef grub_uint8_t grub_efi_pxe_packet_t[1472];
> >  
> > +typedef struct {
> > +  grub_uint8_t addr[4];
> > +} grub_efi_pxe_ipv4_address_t;
> > +
> > +typedef struct {
> > +  grub_uint8_t addr[16];
> > +} grub_efi_pxe_ipv6_address_t;
> > +
> > +typedef struct {
> > +  grub_uint8_t addr[32];
> > +} grub_efi_pxe_mac_address_t;
> > +
> > +typedef union {
> > +    grub_uint32_t addr[4];
> > +    grub_efi_pxe_ipv4_address_t v4;
> > +    grub_efi_pxe_ipv6_address_t v6;
> > +} grub_efi_pxe_ip_address_t;
> > +
> > +#define EFI_PXE_BASE_CODE_MAX_IPCNT             8
> GRUB_EFI_* please
OK.
> 
> > +typedef struct {
> > +    grub_uint8_t filters;
> > +    grub_uint8_t ip_cnt;
> > +    grub_uint16_t reserved;
> > +    grub_efi_pxe_ip_address_t ip_list[EFI_PXE_BASE_CODE_MAX_IPCNT];
> > +} grub_efi_pxe_ip_filter_t;
> > +
> > +typedef struct {
> > +    grub_efi_pxe_ip_address_t ip_addr;
> > +    grub_efi_pxe_mac_address_t mac_addr;
> > +} grub_efi_pxe_arp_entry_t;
> > +
> > +typedef struct {
> > +    grub_efi_pxe_ip_address_t ip_addr;
> > +    grub_efi_pxe_ip_address_t subnet_mask;
> > +    grub_efi_pxe_ip_address_t gw_addr;
> > +} grub_efi_pxe_route_entry_t;
> > +
> > +
> > +#define EFI_PXE_BASE_CODE_MAX_ARP_ENTRIES 8
> > +#define EFI_PXE_BASE_CODE_MAX_ROUTE_ENTRIES 8
> > +
> 
> Same
OK.
> 
> >  typedef struct grub_efi_pxe_mode
> >  {
> > -  grub_uint8_t unused[52];
> > +  grub_uint8_t started;
> > +  grub_uint8_t ipv6_available;
> > +  grub_uint8_t ipv6_supported;
> > +  grub_uint8_t using_ipv6;
> > +  //grub_uint8_t unused[48];
> > +  grub_uint8_t unused[16];
> > +  grub_efi_pxe_ip_address_t station_ip;
> > +  grub_efi_pxe_ip_address_t subnet_mask;
> >    grub_efi_pxe_packet_t dhcp_discover;
> >    grub_efi_pxe_packet_t dhcp_ack;
> >    grub_efi_pxe_packet_t proxy_offer;
> >    grub_efi_pxe_packet_t pxe_discover;
> >    grub_efi_pxe_packet_t pxe_reply;
> > +  grub_efi_pxe_packet_t pxe_bis_reply;
> > +  grub_efi_pxe_ip_filter_t ip_filter;
> > +  grub_uint32_t arp_cache_entries;
> > +  grub_efi_pxe_arp_entry_t arp_cache[EFI_PXE_BASE_CODE_MAX_ARP_ENTRIES];
> > +  grub_uint32_t route_table_entries;
> > +  grub_efi_pxe_route_entry_t route_table[EFI_PXE_BASE_CODE_MAX_ROUTE_ENTRIES];
> >  } grub_efi_pxe_mode_t;
> >  
> >  typedef struct grub_efi_pxe
> > diff --git a/include/grub/net.h b/include/grub/net.h
> > index 538baa3..71dc243 100644
> > --- a/include/grub/net.h
> > +++ b/include/grub/net.h
> > @@ -418,6 +418,13 @@ struct grub_net_bootp_packet
> >    grub_uint8_t vendor[0];
> >  } GRUB_PACKED;
> >  
> > +struct grub_net_dhcpv6_packet
> > +{
> > +  grub_uint32_t message_type:8;
> > +  grub_uint32_t transaction_id:24;
> > +  grub_uint8_t dhcp_options[0];
> > +} GRUB_PACKED;
> > +
> >  #define	GRUB_NET_BOOTP_RFC1048_MAGIC_0	0x63
> >  #define	GRUB_NET_BOOTP_RFC1048_MAGIC_1	0x82
> >  #define	GRUB_NET_BOOTP_RFC1048_MAGIC_2	0x53
> > @@ -444,6 +451,14 @@ grub_net_configure_by_dhcp_ack (const char *name,
> >  				grub_size_t size,
> >  				int is_def, char **device, char **path);
> >  
> > +struct grub_net_network_level_interface *
> > +grub_net_configure_by_dhcpv6_reply (const char *name,
> > +				    struct grub_net_card *card,
> > +				    grub_net_interface_flags_t flags,
> > +				    const struct grub_net_dhcpv6_packet *v6,
> > +				    grub_size_t size,
> > +				    int is_def, char **device, char **path);
> > +
> >  grub_err_t
> >  grub_net_add_ipv4_local (struct grub_net_network_level_interface *inf,
> >  			 int mask);
> > @@ -452,6 +467,10 @@ void
> >  grub_net_process_dhcp (struct grub_net_buff *nb,
> >  		       struct grub_net_card *card);
> >  
> > +void
> > +grub_net_process_dhcp6 (struct grub_net_buff *nb,
> > +			struct grub_net_card *card);
> > +
> >  int
> >  grub_net_hwaddr_cmp (const grub_net_link_level_address_t *a,
> >  		     const grub_net_link_level_address_t *b);
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] Use UEFI MAC device as default configured by net_bootp6
  2015-04-16 19:58   ` Andrei Borzenkov
@ 2015-04-17  6:41     ` Michael Chang
  2015-04-17  9:01       ` Andrei Borzenkov
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Chang @ 2015-04-17  6:41 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: grub-devel

On Thu, Apr 16, 2015 at 10:58:19PM +0300, Andrei Borzenkov wrote:
> В Wed, 15 Apr 2015 17:05:09 +0800
> Michael Chang <mchang@suse.com> пишет:
> 
> > The grub_efinet_findcards will register cards by checking if it can support EFI
> > Simple Netowork Protocol which create more than one device to a physical NIC
> > device.
> > 
> 
> Yes, indeed.
> 
>   /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/EndEntire
>   28be27e5-66cc-4a31-a315-db14c3744d85
>   fa3cde4c-87c2-427d-aede-7dd096c88c58
>   b95e9fda-26de-48d2-8807-1f9107ac5e3a
>   d9760ff3-3cca-4267-80f9-7527fafa4223
>   9fb9a8a1-2f4a-43a6-889c-d0f7b6c47ad5
>   66ed4721-3c98-4d3e-81e3-d03dd39a7254
>   ec20eb79-6c1a-4664-9a0d-d2e4cc16d664
>   00720665-67eb-4a99-baf7-d3c33a1c7cc9
>   937fe521-95ae-4d1a-8929-48bcd90ad31a
>   ec835dd3-fe0f-617b-a621-b350c3e13388
>   2fe800be-8f01-4aa6-946b-d71388e1833f
>   9d9a39d8-bd42-4a73-a4d5-8ee94be11380
>   83f01464-99bd-45e5-b383-af6305d8e9e6
>   c51711e7-b4bf-404a-bfb8-0a048ef1ffe4
>   3b95aa31-3793-434b-8667-c8070892e05e
>   f44c00ee-1f2c-4a00-aa09-1c9f3e0800a3
>   e4f61863-fe2c-4b56-a8f4-08519bc439df
>   f36ff770-a7e1-42cf-9ed2-56f0f271f44c (Managed Network Protocol)
>   9e23d768-d2f3-4366-9fc3-3a7aba864374 (VLAN Configuration Protocol)
>   device path
>   network
> 
>   /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/HardwareVendor(d79df6b0-ef44-43bd-9797-43e93bcf5fa8)[0:
>   ]/EndEntire HII configuration access
>   device path
> 
>   /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/HardwareVendor(d8944553-c4dd-41f4-9b30-e1397cfb267b)[0:
>   ]/EndEntire HII configuration access
>   device path
> 
>   /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/HardwareVendor(5bedb5cc-d830-4eb2-8742-2d4cc9b54f2c)[0:
>   ]/EndEntire HII configuration access
>   device path
> 
>   /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/IPv4(0.0.0.0,0.0.0.0,0,0,0,0)/EndEntire
>   network
>   pxe
>   load file
>   device path
> 
>   /ACPI(a0341d0,0)/PCI(0,3)/MacAddr(52:54:00:12:34:56,1)/IPv6(0:0:0:0:0:0:0:0,0:0:0:0:0:0:0:0,0,0,0,0)/EndEntire
>   network
>   pxe
>   load file
>   device path
> 
> 
> > If without specifying any device to be configured by net_bootp6, it should pick
> > up one from them but not all.
> 
> Right now I am not even able to netboot in QEMU. Booting from CD and
> attempting to read anything over net results in random failure.

Are you using OVMF from openSUSE Factory ? I was actually using it to
test and verify the UEFI netboot, it works for me without problem for
IPv4 and IPv6 network.

I didn't test by booting from CD but instead selecting UEFI PXE boot
from firmware and see if the menu comes up. To test net_boop6 I
intetionally made the boot failure and dropped in grub resce mode to run
the command to see if it can bring the interface up and enter normal
mode afterwards.

Could that make any difference here?

> 
> There are multiple reports of similar problem in EFI. One of possible
> explanations is that EFI drivers are active and steal some packets from
> GRUB.
> 
> There is patch suggested in bug report
> https://savannah.gnu.org/bugs/index.php?41731 but it currently crashes
> OVMF so I cannot test it. Also it effectively makes multiple efinet on
> single HW card useless anyway (it will open exclusively IPv4 instance
> thus blocking any access to MAC device using SNP).

Yes, I have to skip that patch to test IPv6 netboot, somehow it doesn't
work with it ...

> 
> So it looks like efinet needs redesign anyway.

Agreed, but I'm not dare to do that so that's why I created this
workaround patch.

Let me know should I continue carrying it in the patch set or not? In my
tests, without it somehow the net transfer stalls a lot due to multiple
efinet card instance is configured ...

Thanks,
Michael


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] Use UEFI MAC device as default configured by net_bootp6
  2015-04-17  6:41     ` Michael Chang
@ 2015-04-17  9:01       ` Andrei Borzenkov
  0 siblings, 0 replies; 13+ messages in thread
From: Andrei Borzenkov @ 2015-04-17  9:01 UTC (permalink / raw)
  To: Michael Chang; +Cc: The development of GNU GRUB

On Fri, Apr 17, 2015 at 9:41 AM, Michael Chang <mchang@suse.com> wrote:
>>
>> Right now I am not even able to netboot in QEMU. Booting from CD and
>> attempting to read anything over net results in random failure.
>
> Are you using OVMF from openSUSE Factory

Virtualization repo.

 >
I was actually using it to
> test and verify the UEFI netboot, it works for me without problem for
> IPv4 and IPv6 network.
>

This is highly timing dependent so may differ for host, qemu, number
and speed of CPU versions etc.

>>
>> There is patch suggested in bug report
>> https://savannah.gnu.org/bugs/index.php?41731 but it currently crashes
>> OVMF so I cannot test it. Also it effectively makes multiple efinet on
>> single HW card useless anyway (it will open exclusively IPv4 instance
>> thus blocking any access to MAC device using SNP).
>
> Yes, I have to skip that patch to test IPv6 netboot, somehow it doesn't
> work with it ...
>

Try to enable serial console (-serial stdio) and see if OVMF crashed.

>>
>> So it looks like efinet needs redesign anyway.
>
> Agreed, but I'm not dare to do that so that's why I created this
> workaround patch.
>
> Let me know should I continue carrying it in the patch set or not? In my
> tests, without it somehow the net transfer stalls a lot due to multiple
> efinet card instance is configured ...
>

I have an idea what happens, I'll try to work on it at weekend. It
should make your patch unnecessary.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Support DHCPv6 and UEFI IPv6 PXE
  2015-04-15  9:05 [RFC] Support DHCPv6 and UEFI IPv6 PXE Michael Chang
                   ` (2 preceding siblings ...)
  2015-04-15  9:05 ` [PATCH 3/3] Use UEFI MAC device as default configured by net_bootp6 Michael Chang
@ 2015-04-17  9:48 ` Andrei Borzenkov
  3 siblings, 0 replies; 13+ messages in thread
From: Andrei Borzenkov @ 2015-04-17  9:48 UTC (permalink / raw)
  To: The development of GNU GRUB

could you also include documentation update for net_bootp6 as separate
patch? Thank you.

On Wed, Apr 15, 2015 at 12:05 PM, Michael Chang <mchang@suse.com> wrote:
> Hi,
>
> This patch set tries to make support of configuring IPv6 network interface
> through the DHCPv6 protocol. A new command, net_bootp6, is therefore introduced
> to serve the purpose. You can think it as DHCPv6/IPv6 version of the existing
> net_bootp command.
>
> In addition to that, the UEFI IPv6 PXE support can be easily done by sharing
> the same routine with net_bootp6 to parse DHCPv6 Reply packet cached in the
> firmware.
>
> I'd like to know any comment on this ?
>
> Thanks,
> Michael
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Added net_bootp6 command
  2015-04-17  5:04     ` Michael Chang
@ 2015-04-19  8:15       ` Andrei Borzenkov
  2015-04-20  3:09         ` Michael Chang
  0 siblings, 1 reply; 13+ messages in thread
From: Andrei Borzenkov @ 2015-04-19  8:15 UTC (permalink / raw)
  To: Michael Chang; +Cc: grub-devel

В Fri, 17 Apr 2015 13:04:27 +0800
Michael Chang <mchang@suse.com> пишет:

> > 
> > > +static const struct grub_dhcpv6_option*
> > > +find_dhcpv6_option (const struct grub_net_dhcpv6_packet *packet,
> > > +		    grub_uint16_t option)
> > > +{
> > > +  grub_uint16_t code, len;
> > > +  const struct grub_dhcpv6_option *popt;
> > > +
> > > +  popt = (const struct grub_dhcpv6_option *)packet->dhcp_options;
> > > +  code = grub_be_to_cpu16 (popt->code);
> > > +  len = grub_be_to_cpu16 (popt->len);
> > > +
> > > +  while (0 != code && option != code)
> > 
> > This probably needs upper boundary check in case we are dealing with
> > corrupted packets.
> 
> 
> Do you mean checking the option length can't exceed netbuff length?
> 

Yes (and in all other cases below as well).

> The ( 0!= code ) did certain kind of upper boundary check for me, but
> I know you may disagree that. :)
> 
> Btw, in grub-core/net/ip.c::handle_dgram() has checksum for the receiving
> packets, so the corrputed packets wouldn't sneak in easily.
> 

Correct checksum just means content was not altered; it does not mean
content was correct to start with.

> > > +
> > > +  ip_start = ip_end = NULL;
> > > +  ip_start = bootfile_url + grub_strlen(pr);
> > > +
> > > +  if (*ip_start != '[')
> > > +    ip_start = NULL;
> > > +  else
> > > +    ip_end = grub_strchr (++ip_start, ']');
> > > +
> > > +  if (!ip_start || !ip_end)
> > > +    {
> > > +      grub_error (GRUB_ERR_IO, N_("IPv6-address not in square brackets"));
> > > +      goto cleanup;
> > > +    }
> > > +
> > 
> > Can bootfile_url contain a name and not IPv6 address? Or is address
> > mandatory?
> 
> Yes. The string in URL form is mandatory.
>

I probably was not clear. Must it always be IPv6 address literal as
implied by code or can it be e.g. DNS name?

> > > +struct grub_net_network_level_interface *
> > > +grub_net_configure_by_dhcpv6_reply (const char *name,
> > > +	struct grub_net_card *card,
> > > +	grub_net_interface_flags_t flags,
> > > +	const struct grub_net_dhcpv6_packet *v6,
> > > +	grub_size_t size __attribute__ ((unused)),
> > > +	int is_def,
> > > +	char **device, char **path)
> > > +{
> > > +  grub_net_network_level_address_t addr;
> > > +  grub_net_network_level_netaddress_t netaddr;
> > > +  struct grub_net_network_level_interface *inf;
> > > +  const grub_uint8_t *your_ip;
> > > +  char *proto;
> > > +  char *server_ip;
> > > +  char *boot_file;
> > > +  grub_net_network_level_address_t *dns;
> > > +  grub_uint16_t num_dns;
> > > +
> > > +  if (device)
> > > +    *device = NULL;
> > > +
> > > +  if (path)
> > > +    *path = NULL;
> > > +
> > > +  if (v6->message_type != DHCPv6_REPLY)
> > 
> > Is it really possible? We come here if packet is DHCPv6_REPLY only.
> 
> I'm in case some evil firmware to cache some other packets type rather
> than reply (or even junks in it ..).
>

Yes, but what I mean - this function is only called when we already
checked for message_type == DHCPv6_REPLY. So the only reason it can be
different here is physical memory corruption.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Added net_bootp6 command
  2015-04-19  8:15       ` Andrei Borzenkov
@ 2015-04-20  3:09         ` Michael Chang
  2015-04-20  3:38           ` Andrei Borzenkov
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Chang @ 2015-04-20  3:09 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: grub-devel

On Sun, Apr 19, 2015 at 11:15:21AM +0300, Andrei Borzenkov wrote:
> В Fri, 17 Apr 2015 13:04:27 +0800
> Michael Chang <mchang@suse.com> пишет:
> 
> > > 
> > > > +static const struct grub_dhcpv6_option*
> > > > +find_dhcpv6_option (const struct grub_net_dhcpv6_packet *packet,
> > > > +		    grub_uint16_t option)
> > > > +{
> > > > +  grub_uint16_t code, len;
> > > > +  const struct grub_dhcpv6_option *popt;
> > > > +
> > > > +  popt = (const struct grub_dhcpv6_option *)packet->dhcp_options;
> > > > +  code = grub_be_to_cpu16 (popt->code);
> > > > +  len = grub_be_to_cpu16 (popt->len);
> > > > +
> > > > +  while (0 != code && option != code)
> > > 
> > > This probably needs upper boundary check in case we are dealing with
> > > corrupted packets.
> > 
> > 
> > Do you mean checking the option length can't exceed netbuff length?
> > 
> 
> Yes (and in all other cases below as well).

OK.

> 
> > The ( 0!= code ) did certain kind of upper boundary check for me, but
> > I know you may disagree that. :)
> > 
> > Btw, in grub-core/net/ip.c::handle_dgram() has checksum for the receiving
> > packets, so the corrputed packets wouldn't sneak in easily.
> > 
> 
> Correct checksum just means content was not altered; it does not mean
> content was correct to start with.

I agree with you that it doesn't mean the content was OK to start with,
the sender might intentionally or unintentionally fill the payload
length wrong that triggers buffer overflow on the client side.

FWIW, the answer is subject to "corrupted packet" that usually means
corruption during the transit and checksum is generally used for
detecting such errors . 

> 
> > > > +
> > > > +  ip_start = ip_end = NULL;
> > > > +  ip_start = bootfile_url + grub_strlen(pr);
> > > > +
> > > > +  if (*ip_start != '[')
> > > > +    ip_start = NULL;
> > > > +  else
> > > > +    ip_end = grub_strchr (++ip_start, ']');
> > > > +
> > > > +  if (!ip_start || !ip_end)
> > > > +    {
> > > > +      grub_error (GRUB_ERR_IO, N_("IPv6-address not in square brackets"));
> > > > +      goto cleanup;
> > > > +    }
> > > > +
> > > 
> > > Can bootfile_url contain a name and not IPv6 address? Or is address
> > > mandatory?
> > 
> > Yes. The string in URL form is mandatory.
> >
> 
> I probably was not clear. Must it always be IPv6 address literal as
> implied by code or can it be e.g. DNS name?

The DNS name could be legit for URL, but current elilo and edk2
explicitly checks for the starting and ending delimiter '[..]' for using
the server address and any name is treated as invalid parameters. The
implementation just followed them.

> 
> > > > +struct grub_net_network_level_interface *
> > > > +grub_net_configure_by_dhcpv6_reply (const char *name,
> > > > +	struct grub_net_card *card,
> > > > +	grub_net_interface_flags_t flags,
> > > > +	const struct grub_net_dhcpv6_packet *v6,
> > > > +	grub_size_t size __attribute__ ((unused)),
> > > > +	int is_def,
> > > > +	char **device, char **path)
> > > > +{
> > > > +  grub_net_network_level_address_t addr;
> > > > +  grub_net_network_level_netaddress_t netaddr;
> > > > +  struct grub_net_network_level_interface *inf;
> > > > +  const grub_uint8_t *your_ip;
> > > > +  char *proto;
> > > > +  char *server_ip;
> > > > +  char *boot_file;
> > > > +  grub_net_network_level_address_t *dns;
> > > > +  grub_uint16_t num_dns;
> > > > +
> > > > +  if (device)
> > > > +    *device = NULL;
> > > > +
> > > > +  if (path)
> > > > +    *path = NULL;
> > > > +
> > > > +  if (v6->message_type != DHCPv6_REPLY)
> > > 
> > > Is it really possible? We come here if packet is DHCPv6_REPLY only.
> > 
> > I'm in case some evil firmware to cache some other packets type rather
> > than reply (or even junks in it ..).
> >
> 
> Yes, but what I mean - this function is only called when we already
> checked for message_type == DHCPv6_REPLY. So the only reason it can be
> different here is physical memory corruption.

It also gets called from grub_efi_net_config_real () which doesn't have
any check for message type.

Thanks,
Michael


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Added net_bootp6 command
  2015-04-20  3:09         ` Michael Chang
@ 2015-04-20  3:38           ` Andrei Borzenkov
  0 siblings, 0 replies; 13+ messages in thread
From: Andrei Borzenkov @ 2015-04-20  3:38 UTC (permalink / raw)
  To: Michael Chang; +Cc: grub-devel

В Mon, 20 Apr 2015 11:09:10 +0800
Michael Chang <mchang@suse.com> пишет:

> 
> > 
> > > > > +
> > > > > +  ip_start = ip_end = NULL;
> > > > > +  ip_start = bootfile_url + grub_strlen(pr);
> > > > > +
> > > > > +  if (*ip_start != '[')
> > > > > +    ip_start = NULL;
> > > > > +  else
> > > > > +    ip_end = grub_strchr (++ip_start, ']');
> > > > > +
> > > > > +  if (!ip_start || !ip_end)
> > > > > +    {
> > > > > +      grub_error (GRUB_ERR_IO, N_("IPv6-address not in square brackets"));
> > > > > +      goto cleanup;
> > > > > +    }
> > > > > +
> > > > 
> > > > Can bootfile_url contain a name and not IPv6 address? Or is address
> > > > mandatory?
> > > 
> > > Yes. The string in URL form is mandatory.
> > >
> > 
> > I probably was not clear. Must it always be IPv6 address literal as
> > implied by code or can it be e.g. DNS name?
> 
> The DNS name could be legit for URL, but current elilo and edk2
> explicitly checks for the starting and ending delimiter '[..]' for using
> the server address and any name is treated as invalid parameters. The
> implementation just followed them.
> 

OK, could you put in comments here which explain it, probably marked as
TODO.

> > 
> > Yes, but what I mean - this function is only called when we already
> > checked for message_type == DHCPv6_REPLY. So the only reason it can be
> > different here is physical memory corruption.
> 
> It also gets called from grub_efi_net_config_real () which doesn't have
> any check for message type.
> 

So you do not trust firmware, do you? :) But OK, let's leave check in
place.

P.S. grub_efi_net_config_real should actually have checked
DhcpAckReceived to start with, but I'm afraid to add it now, at least
until we have real report of invalid packages.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-04-20  3:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-15  9:05 [RFC] Support DHCPv6 and UEFI IPv6 PXE Michael Chang
2015-04-15  9:05 ` [PATCH 1/3] Added net_bootp6 command Michael Chang
2015-04-16 14:40   ` Andrei Borzenkov
2015-04-17  5:04     ` Michael Chang
2015-04-19  8:15       ` Andrei Borzenkov
2015-04-20  3:09         ` Michael Chang
2015-04-20  3:38           ` Andrei Borzenkov
2015-04-15  9:05 ` [PATCH 2/3] UEFI IPv6 PXE support Michael Chang
2015-04-15  9:05 ` [PATCH 3/3] Use UEFI MAC device as default configured by net_bootp6 Michael Chang
2015-04-16 19:58   ` Andrei Borzenkov
2015-04-17  6:41     ` Michael Chang
2015-04-17  9:01       ` Andrei Borzenkov
2015-04-17  9:48 ` [RFC] Support DHCPv6 and UEFI IPv6 PXE Andrei Borzenkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.