From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from list by lists.gnu.org with archive (Exim 4.90_1)
	id 1iMYrU-0001mg-G8
	for mharc-grub-devel@gnu.org; Mon, 21 Oct 2019 10:40:45 -0400
Received: from eggs.gnu.org ([2001:470:142:3::10]:49809)
 by lists.gnu.org with esmtp (Exim 4.90_1)
 (envelope-from <daniel.kiper@oracle.com>) id 1iMYrO-0001kw-1d
 for grub-devel@gnu.org; Mon, 21 Oct 2019 10:40:39 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
 (envelope-from <daniel.kiper@oracle.com>) id 1iMYrM-0005wW-DV
 for grub-devel@gnu.org; Mon, 21 Oct 2019 10:40:37 -0400
Received: from aserp2120.oracle.com ([141.146.126.78]:50640)
 by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)
 (Exim 4.71) (envelope-from <daniel.kiper@oracle.com>)
 id 1iMYrM-0005wO-4F
 for grub-devel@gnu.org; Mon, 21 Oct 2019 10:40:36 -0400
Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1])
 by aserp2120.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x9LEYNtk046067;
 Mon, 21 Oct 2019 14:40:33 GMT
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com;
 h=date : from : to : cc
 : subject : message-id : references : mime-version : content-type :
 in-reply-to; s=corp-2019-08-05;
 bh=i28zGdxo3vHnIPcEsJFAVsLbe0ETOmlNZBxSEJnEKmQ=;
 b=NelUIerdDcVKopgtOxrpsnNa8NaQD0IX7Ur5HXKfVXqJr2Jbva/JRXO1nmjcREwnWxvo
 +Tvsa1TYUDcuWlv8QPnT615c6L1j6cBftHC0R7KuIywjtEr+duqAWT4x2Rx2Za0Gm/EH
 qt21U37D6hVR/WhqVlrrCY6okhPwvVnUmxqeoQ5Lgpao9pspeMldShiCMVYDUKbpyskv
 Y5Uu6Pk9Mz9Y3LnDs/3wBr2LmOYeO3DpatwD3odMSdgEZPrS0K+JCx3CfSLEToxtYH59
 ircWXfwYfHr2YpG1VEAHRmgq5WktHA3JLkRIJ65cBBrpFa9FjTttMAPf7l04BAZlE/+x ZQ== 
Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79])
 by aserp2120.oracle.com with ESMTP id 2vqtepg4p2-1
 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK);
 Mon, 21 Oct 2019 14:40:33 +0000
Received: from pps.filterd (userp3020.oracle.com [127.0.0.1])
 by userp3020.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x9LEd0im001344;
 Mon, 21 Oct 2019 14:40:32 GMT
Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236])
 by userp3020.oracle.com with ESMTP id 2vrcmn7e9q-1
 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK);
 Mon, 21 Oct 2019 14:40:32 +0000
Received: from abhmp0007.oracle.com (abhmp0007.oracle.com [141.146.116.13])
 by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id x9LEeUKn008431;
 Mon, 21 Oct 2019 14:40:31 GMT
Received: from tomti.i.net-space.pl (/10.175.219.206)
 by default (Oracle Beehive Gateway v4.0)
 with ESMTP ; Mon, 21 Oct 2019 07:40:30 -0700
Date: Mon, 21 Oct 2019 16:40:20 +0200
From: Daniel Kiper <daniel.kiper@oracle.com>
To: Javier Martinez Canillas <javierm@redhat.com>
Cc: grub-devel@gnu.org, Vladimir Serbinenko <phcoder@gmail.com>
Subject: Re: [PATCH v2 2/3] Set net_<interface>_client{id,uuid} variables
 from DHCP options
Message-ID: <20191021144020.hmfuj6aecbrb3h43@tomti.i.net-space.pl>
References: <20191018075910.31263-1-javierm@redhat.com>
 <20191018075910.31263-3-javierm@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191018075910.31263-3-javierm@redhat.com>
User-Agent: NeoMutt/20170113 (1.7.2)
X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9416
 signatures=668684
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0
 malwarescore=0
 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.0.1-1908290000 definitions=main-1910210139
X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9416
 signatures=668684
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0
 priorityscore=1501 malwarescore=0
 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015
 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1908290000
 definitions=main-1910210139
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]
X-Received-From: 141.146.126.78
X-BeenThere: grub-devel@gnu.org
X-Mailman-Version: 2.1.23
Precedence: list
List-Id: The development of GNU GRUB <grub-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/grub-devel>,
 <mailto:grub-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <https://lists.gnu.org/archive/html/grub-devel>
List-Post: <mailto:grub-devel@gnu.org>
List-Help: <mailto:grub-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/grub-devel>,
 <mailto:grub-devel-request@gnu.org?subject=subscribe>
X-List-Received-Date: Mon, 21 Oct 2019 14:40:39 -0000

On Fri, Oct 18, 2019 at 09:59:09AM +0200, Javier Martinez Canillas wrote:
> From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
>
> This patch sets a net_<interface>_clientid and net_<interface>_clientuuid
> GRUB environment variables, using the DHCP client ID and UUID options if
> these are found.
>
> In the same way than net_<interface>_<option> variables are set for other
> options such domain name, boot file, next server, etc.
>
> Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

It seems to me that this RB should land in patch #3. Does not it?
Anyway, Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> :-)
except one thing...

> ---
>
> Changes in v2:
> - Use the existing grub_env_set_net_property() and remove duplicated code.
>
>  grub-core/net/bootp.c | 48 +++++++++++++++++++++++++++++++++++--------
>  include/grub/net.h    |  2 ++
>  2 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> index 04cfbb04504..377006da938 100644
> --- a/grub-core/net/bootp.c
> +++ b/grub-core/net/bootp.c
> @@ -95,6 +95,14 @@ enum
>  /* Max timeout when waiting for BOOTP/DHCP reply */
>  #define GRUB_DHCP_MAX_PACKET_TIMEOUT 32
>
> +static char
> +hexdigit (grub_uint8_t val)
> +{
> +  if (val < 10)
> +    return val + '0';
> +  return val + 'a' - 10;
> +}
> +
>  static const void *
>  find_dhcp_option (const struct grub_net_bootp_packet *bp, grub_size_t size,
>  		  grub_uint8_t opt_code, grub_uint8_t *opt_len)
> @@ -152,6 +160,9 @@ again:
>        if (i + taglength >= size)
>  	return NULL;
>
> +      grub_dprintf("net", "DHCP option %u (0x%02x) found with length %u.\n",
> +                   tagtype, tagtype, taglength);
> +
>        /* FIXME RFC 3396 options concatentation */
>        if (tagtype == opt_code)
>  	{
> @@ -406,6 +417,35 @@ grub_net_configure_by_dhcp_ack (const char *name,
>    if (opt && opt_len)
>      grub_env_set_net_property (name, "extensionspath", (const char *) opt, opt_len);
>
> +  opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_ID, &opt_len);
> +  if (opt && opt_len)
> +    grub_env_set_net_property (name, "clientid", (const char *) opt, opt_len);
> +
> +  opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_UUID, &opt_len);
> +  if (opt && opt_len == 17)
> +    {
> +      /* The format is 9cfe245e-d0c8-bd45-a79f-54ea5fbd3d97 */
> +
> +      opt += 1;
> +      opt_len -= 1;
> +
> +      char *val = grub_malloc (2 * opt_len + 4 + 1);
> +      int i = 0;
> +      int j = 0;
> +      for (i = 0; i < opt_len; i++)
> +        {
> +          val[2 * i + j] = hexdigit (opt[i] >> 4);
> +          val[2 * i + 1 + j] = hexdigit (opt[i] & 0xf);
> +
> +          if ((i == 3) || (i == 5) || (i == 7) || (i == 9))
> +            {
> +              j++;
> +              val[2 * i + 1+ j] = '-';
> +            }
> +        }
> +      grub_env_set_net_property (name, "clientuuid", (char *) val, 2 * opt_len + 4);
> +    }
> +
>    inter->dhcp_ack = grub_malloc (size);
>    if (inter->dhcp_ack)
>      {
> @@ -631,14 +671,6 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
>      }
>  }
>
> -static char
> -hexdigit (grub_uint8_t val)
> -{
> -  if (val < 10)
> -    return val + '0';
> -  return val + 'a' - 10;
> -}
> -
>  static grub_err_t
>  grub_cmd_dhcpopt (struct grub_command *cmd __attribute__ ((unused)),
>  		  int argc, char **args)
> diff --git a/include/grub/net.h b/include/grub/net.h
> index 4a9069a1474..556c54e579f 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -462,6 +462,8 @@ enum
>      GRUB_NET_BOOTP_DOMAIN = 0x0f,
>      GRUB_NET_BOOTP_ROOT_PATH = 0x11,
>      GRUB_NET_BOOTP_EXTENSIONS_PATH = 0x12,
> +    GRUB_NET_BOOTP_CLIENT_ID = 0x3d,
> +    GRUB_NET_BOOTP_CLIENT_UUID = 0x61,
>      GRUB_NET_DHCP_REQUESTED_IP_ADDRESS = 50,
>      GRUB_NET_DHCP_OVERLOAD = 52,
>      GRUB_NET_DHCP_MESSAGE_TYPE = 53,

I have just realized that this enum is a mixture of decimals and hexes.
This is a mess. Could you add a patch before that patch changing all
decimals to hexes and sort them properly?

Daniel