[2.02][PATCH] bootp: export server IP as environment variable

[2.02][PATCH] bootp: export server IP as environment variable

[Andrei Borzenkov]

Network boot autoconfiguration sets default server to next server IP
(siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
exports only server name. Unfortunately semantic of server name is not
clearly defined. BOOTP RFC 951 defines it only for client request, and
DHCP RFC 1541 only mentions it, without any implied usage. It looks like
this field is mostly empty in server replies.

Export next server IP as net_<interface>_server_ip variable. This allows
grub configuration script to set $root/$prefix based on information obtained
by net_bootp.

Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Cc: nikunj@linux.vnet.ibm.com

---

@Nikunj: cosmetic changes comparing with version you tested - no need to
allocate small buffer dynamically.

@Vladimir: I think this is useful; I will update networking documentation
in followup patch.

 grub-core/net/bootp.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index a088244..c6fb807 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -142,6 +142,7 @@ grub_net_configure_by_dhcp_ack (const char *name,
   grub_net_link_level_address_t hwaddr;
   struct grub_net_network_level_interface *inter;
   int mask = -1;
+  char server_ip[sizeof ("xxx.xxx.xxx.xxx")];
 
   addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4;
   addr.ipv4 = bp->your_ip;
@@ -189,15 +190,22 @@ grub_net_configure_by_dhcp_ack (const char *name,
   if (size > OFFSET_OF (boot_file, bp))
     grub_env_set_net_property (name, "boot_file", bp->boot_file,
                                sizeof (bp->boot_file));
+  if (bp->server_ip)
+    {
+      grub_snprintf (server_ip, sizeof (server_ip), "%d.%d.%d.%d",
+		     ((grub_uint8_t *) &bp->server_ip)[0],
+		     ((grub_uint8_t *) &bp->server_ip)[1],
+		     ((grub_uint8_t *) &bp->server_ip)[2],
+		     ((grub_uint8_t *) &bp->server_ip)[3]);
+      grub_env_set_net_property (name, "server_ip", server_ip, sizeof (server_ip));
+      grub_print_error ();
+    }
+
   if (is_def)
     grub_net_default_server = 0;
   if (is_def && !grub_net_default_server && bp->server_ip)
     {
-      grub_net_default_server = grub_xasprintf ("%d.%d.%d.%d",
-						((grub_uint8_t *) &bp->server_ip)[0],
-						((grub_uint8_t *) &bp->server_ip)[1],
-						((grub_uint8_t *) &bp->server_ip)[2],
-						((grub_uint8_t *) &bp->server_ip)[3]);
+      grub_net_default_server = grub_strdup (server_ip);
       grub_print_error ();
     }
 
@@ -209,11 +217,7 @@ grub_net_configure_by_dhcp_ack (const char *name,
 
   if (device && !*device && bp->server_ip)
     {
-      *device = grub_xasprintf ("tftp,%d.%d.%d.%d",
-				((grub_uint8_t *) &bp->server_ip)[0],
-				((grub_uint8_t *) &bp->server_ip)[1],
-				((grub_uint8_t *) &bp->server_ip)[2],
-				((grub_uint8_t *) &bp->server_ip)[3]);
+      *device = grub_xasprintf ("tftp,%s", server_ip);
       grub_print_error ();
     }
   if (size > OFFSET_OF (server_name, bp)
-- 
tg: (76eac44..) u/bootp-server-ip (depends on: master)

Re: [2.02][PATCH] bootp: export server IP as environment variable

[Daniel Kiper]

On Fri, Mar 18, 2016 at 08:42:00PM +0300, Andrei Borzenkov wrote:
> Network boot autoconfiguration sets default server to next server IP
> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
> exports only server name. Unfortunately semantic of server name is not
> clearly defined. BOOTP RFC 951 defines it only for client request, and
> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
> this field is mostly empty in server replies.
>
> Export next server IP as net_<interface>_server_ip variable. This allows
> grub configuration script to set $root/$prefix based on information obtained
> by net_bootp.
>
> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> Cc: nikunj@linux.vnet.ibm.com
>
> ---
>
> @Nikunj: cosmetic changes comparing with version you tested - no need to
> allocate small buffer dynamically.
>
> @Vladimir: I think this is useful; I will update networking documentation
> in followup patch.

Vladimir, Andrei, could you take care of it?

Daniel

Re: [2.02][PATCH] bootp: export server IP as environment variable

[Andrei Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1201 bytes --]

17.11.2016 22:02, Daniel Kiper пишет:
> On Fri, Mar 18, 2016 at 08:42:00PM +0300, Andrei Borzenkov wrote:
>> Network boot autoconfiguration sets default server to next server IP
>> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
>> exports only server name. Unfortunately semantic of server name is not
>> clearly defined. BOOTP RFC 951 defines it only for client request, and
>> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
>> this field is mostly empty in server replies.
>>
>> Export next server IP as net_<interface>_server_ip variable. This allows
>> grub configuration script to set $root/$prefix based on information obtained
>> by net_bootp.
>>
>> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> Cc: nikunj@linux.vnet.ibm.com
>>
>> ---
>>
>> @Nikunj: cosmetic changes comparing with version you tested - no need to
>> allocate small buffer dynamically.
>>
>> @Vladimir: I think this is useful; I will update networking documentation
>> in followup patch.
> 
> Vladimir, Andrei, could you take care of it?
> 
> Daniel
> 

I added documentation (not worth separate patch); if there are no
objections I commit.



[-- Attachment #2: bootp-server-ip.patch --]
[-- Type: text/x-patch, Size: 4162 bytes --]

From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [2.02][PATCH] bootp: export server IP as environment variable

Network boot autoconfiguration sets default server to next server IP
(siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
exports only server name. Unfortunately semantic of server name is not
clearly defined. BOOTP RFC 951 defines it only for client request, and
DHCP RFC 1541 only mentions it, without any implied usage. It looks like
this field is mostly empty in server replies.

Export next server IP as net_<interface>_server_ip variable. This allows
grub configuration script to set $root/$prefix based on information obtained
by net_bootp.

Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Cc: nikunj@linux.vnet.ibm.com

---
 docs/grub.texi        | 11 +++++++++++
 grub-core/net/bootp.c | 24 ++++++++++++++----------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 82f6fa4..bb03875 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -2446,6 +2446,10 @@ The boot file name provided by DHCP.  Read-only.
 The name of the DHCP server responsible for these boot parameters.
 Read-only.
 
+@item net_@var{<interface>}_server_ip
+The IP address of the next (usually, TFTP) server provided by DHCP.
+Read-only.
+
 @item net_default_interface
 Initially set to name of network interface that was used to load grub.
 Read-write, although setting it affects only interpretation of
@@ -3062,6 +3066,7 @@ These variables have special meaning to GRUB.
 * net_@var{<interface>}_hostname::
 * net_@var{<interface>}_ip::
 * net_@var{<interface>}_mac::
+* net_@var{<interface>}_server_ip::
 * net_@var{<interface>}_rootpath::
 * net_default_interface::
 * net_default_ip::
@@ -3428,6 +3433,12 @@ The default is the value of @samp{color_normal} (@pxref{color_normal}).
 @xref{Network}.
 
 
+@node net_@var{<interface>}_server_ip
+@subsection net_@var{<interface>}_server_ip
+
+@xref{Network}.
+
+
 @node net_default_interface
 @subsection net_default_interface
 
diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 189551a..6503840 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -142,6 +142,7 @@ grub_net_configure_by_dhcp_ack (const char *name,
   grub_net_link_level_address_t hwaddr;
   struct grub_net_network_level_interface *inter;
   int mask = -1;
+  char server_ip[sizeof ("xxx.xxx.xxx.xxx")];
 
   addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4;
   addr.ipv4 = bp->your_ip;
@@ -192,15 +193,22 @@ grub_net_configure_by_dhcp_ack (const char *name,
   if (size > OFFSET_OF (boot_file, bp))
     grub_env_set_net_property (name, "boot_file", bp->boot_file,
                                sizeof (bp->boot_file));
+  if (bp->server_ip)
+    {
+      grub_snprintf (server_ip, sizeof (server_ip), "%d.%d.%d.%d",
+		     ((grub_uint8_t *) &bp->server_ip)[0],
+		     ((grub_uint8_t *) &bp->server_ip)[1],
+		     ((grub_uint8_t *) &bp->server_ip)[2],
+		     ((grub_uint8_t *) &bp->server_ip)[3]);
+      grub_env_set_net_property (name, "server_ip", server_ip, sizeof (server_ip));
+      grub_print_error ();
+    }
+
   if (is_def)
     grub_net_default_server = 0;
   if (is_def && !grub_net_default_server && bp->server_ip)
     {
-      grub_net_default_server = grub_xasprintf ("%d.%d.%d.%d",
-						((grub_uint8_t *) &bp->server_ip)[0],
-						((grub_uint8_t *) &bp->server_ip)[1],
-						((grub_uint8_t *) &bp->server_ip)[2],
-						((grub_uint8_t *) &bp->server_ip)[3]);
+      grub_net_default_server = grub_strdup (server_ip);
       grub_print_error ();
     }
 
@@ -212,11 +220,7 @@ grub_net_configure_by_dhcp_ack (const char *name,
 
   if (device && !*device && bp->server_ip)
     {
-      *device = grub_xasprintf ("tftp,%d.%d.%d.%d",
-				((grub_uint8_t *) &bp->server_ip)[0],
-				((grub_uint8_t *) &bp->server_ip)[1],
-				((grub_uint8_t *) &bp->server_ip)[2],
-				((grub_uint8_t *) &bp->server_ip)[3]);
+      *device = grub_xasprintf ("tftp,%s", server_ip);
       grub_print_error ();
     }
   if (size > OFFSET_OF (server_name, bp)
-- 
tg: (0d663b5..) u/bootp-server-ip (depends on: master)

Re: [2.02][PATCH] bootp: export server IP as environment variable

[Daniel Kiper]

On Fri, Nov 18, 2016 at 09:11:47PM +0300, Andrei Borzenkov wrote:
> 17.11.2016 22:02, Daniel Kiper ??????????:
> > On Fri, Mar 18, 2016 at 08:42:00PM +0300, Andrei Borzenkov wrote:
> >> Network boot autoconfiguration sets default server to next server IP
> >> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
> >> exports only server name. Unfortunately semantic of server name is not
> >> clearly defined. BOOTP RFC 951 defines it only for client request, and
> >> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
> >> this field is mostly empty in server replies.
> >>
> >> Export next server IP as net_<interface>_server_ip variable. This allows
> >> grub configuration script to set $root/$prefix based on information obtained
> >> by net_bootp.
> >>
> >> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >> Cc: nikunj@linux.vnet.ibm.com
> >>
> >> ---
> >>
> >> @Nikunj: cosmetic changes comparing with version you tested - no need to
> >> allocate small buffer dynamically.
> >>
> >> @Vladimir: I think this is useful; I will update networking documentation
> >> in followup patch.
> >
> > Vladimir, Andrei, could you take care of it?
> >
> > Daniel
> >
>
> I added documentation (not worth separate patch); if there are no
> objections I commit.
>
>
> From: Andrei Borzenkov <arvidjaar@gmail.com>
> Subject: [2.02][PATCH] bootp: export server IP as environment variable
>
> Network boot autoconfiguration sets default server to next server IP
> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
> exports only server name. Unfortunately semantic of server name is not
> clearly defined. BOOTP RFC 951 defines it only for client request, and
> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
> this field is mostly empty in server replies.
>
> Export next server IP as net_<interface>_server_ip variable. This allows
> grub configuration script to set $root/$prefix based on information obtained
> by net_bootp.
>
> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> Cc: nikunj@linux.vnet.ibm.com
>
> ---
>  docs/grub.texi        | 11 +++++++++++
>  grub-core/net/bootp.c | 24 ++++++++++++++----------
>  2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 82f6fa4..bb03875 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -2446,6 +2446,10 @@ The boot file name provided by DHCP.  Read-only.
>  The name of the DHCP server responsible for these boot parameters.
>  Read-only.
>
> +@item net_@var{<interface>}_server_ip
> +The IP address of the next (usually, TFTP) server provided by DHCP.

BOOTP instead of DHCP? However, then we should change DHCP everywhere too...

> +Read-only.
> +
>  @item net_default_interface
>  Initially set to name of network interface that was used to load grub.
>  Read-write, although setting it affects only interpretation of
> @@ -3062,6 +3066,7 @@ These variables have special meaning to GRUB.
>  * net_@var{<interface>}_hostname::
>  * net_@var{<interface>}_ip::
>  * net_@var{<interface>}_mac::
> +* net_@var{<interface>}_server_ip::

There is something known as net_default_server which contains next IP address.
Should not we be more consistent and use "net_@var{<interface>}_server" instead
of "net_@var{<interface>}_server_ip" here and there.

Otherwise LGTM.

Daniel

Re: [2.02][PATCH] bootp: export server IP as environment variable

[Andrei Borzenkov]

21.11.2016 16:09, Daniel Kiper пишет:
> On Fri, Nov 18, 2016 at 09:11:47PM +0300, Andrei Borzenkov wrote:
>> 17.11.2016 22:02, Daniel Kiper ??????????:
>>> On Fri, Mar 18, 2016 at 08:42:00PM +0300, Andrei Borzenkov wrote:
>>>> Network boot autoconfiguration sets default server to next server IP
>>>> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
>>>> exports only server name. Unfortunately semantic of server name is not
>>>> clearly defined. BOOTP RFC 951 defines it only for client request, and
>>>> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
>>>> this field is mostly empty in server replies.
>>>>
>>>> Export next server IP as net_<interface>_server_ip variable. This allows
>>>> grub configuration script to set $root/$prefix based on information obtained
>>>> by net_bootp.
>>>>
>>>> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>> Cc: nikunj@linux.vnet.ibm.com
>>>>
>>>> ---
>>>>
>>>> @Nikunj: cosmetic changes comparing with version you tested - no need to
>>>> allocate small buffer dynamically.
>>>>
>>>> @Vladimir: I think this is useful; I will update networking documentation
>>>> in followup patch.
>>>
>>> Vladimir, Andrei, could you take care of it?
>>>
>>> Daniel
>>>
>>
>> I added documentation (not worth separate patch); if there are no
>> objections I commit.
>>
>>
>> From: Andrei Borzenkov <arvidjaar@gmail.com>
>> Subject: [2.02][PATCH] bootp: export server IP as environment variable
>>
>> Network boot autoconfiguration sets default server to next server IP
>> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
>> exports only server name. Unfortunately semantic of server name is not
>> clearly defined. BOOTP RFC 951 defines it only for client request, and
>> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
>> this field is mostly empty in server replies.
>>
>> Export next server IP as net_<interface>_server_ip variable. This allows
>> grub configuration script to set $root/$prefix based on information obtained
>> by net_bootp.
>>
>> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> Cc: nikunj@linux.vnet.ibm.com
>>
>> ---
>>  docs/grub.texi        | 11 +++++++++++
>>  grub-core/net/bootp.c | 24 ++++++++++++++----------
>>  2 files changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/docs/grub.texi b/docs/grub.texi
>> index 82f6fa4..bb03875 100644
>> --- a/docs/grub.texi
>> +++ b/docs/grub.texi
>> @@ -2446,6 +2446,10 @@ The boot file name provided by DHCP.  Read-only.
>>  The name of the DHCP server responsible for these boot parameters.
>>  Read-only.
>>
>> +@item net_@var{<interface>}_server_ip
>> +The IP address of the next (usually, TFTP) server provided by DHCP.
> 
> BOOTP instead of DHCP? However, then we should change DHCP everywhere too...
> 

Yes. Sorry, it was probably my fault to refer to DHCP in the first place.

>> +Read-only.
>> +
>>  @item net_default_interface
>>  Initially set to name of network interface that was used to load grub.
>>  Read-write, although setting it affects only interpretation of
>> @@ -3062,6 +3066,7 @@ These variables have special meaning to GRUB.
>>  * net_@var{<interface>}_hostname::
>>  * net_@var{<interface>}_ip::
>>  * net_@var{<interface>}_mac::
>> +* net_@var{<interface>}_server_ip::
> 
> There is something known as net_default_server which contains next IP address.
> Should not we be more consistent and use "net_@var{<interface>}_server" instead
> of "net_@var{<interface>}_server_ip" here and there.
> 

Well, net_default_server is not really tied to any BOOTP/DHCP at all,
although it may be initially set based on content of ACK packet. I was
confused by dhcp_server_name, but they it is unrelated to next server.

I'd rather used net_<interface>_next_server for clarity, both "server"
and "server_ip" are indeed too vague in this case.

> Otherwise LGTM.
> 
> Daniel
> 

Re: [2.02][PATCH] bootp: export server IP as environment variable

[Daniel Kiper]

On Mon, Nov 21, 2016 at 10:52:24PM +0300, Andrei Borzenkov wrote:
> 21.11.2016 16:09, Daniel Kiper ??????????:
> > On Fri, Nov 18, 2016 at 09:11:47PM +0300, Andrei Borzenkov wrote:
> >> 17.11.2016 22:02, Daniel Kiper ??????????:
> >>> On Fri, Mar 18, 2016 at 08:42:00PM +0300, Andrei Borzenkov wrote:
> >>>> Network boot autoconfiguration sets default server to next server IP
> >>>> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
> >>>> exports only server name. Unfortunately semantic of server name is not
> >>>> clearly defined. BOOTP RFC 951 defines it only for client request, and
> >>>> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
> >>>> this field is mostly empty in server replies.
> >>>>
> >>>> Export next server IP as net_<interface>_server_ip variable. This allows
> >>>> grub configuration script to set $root/$prefix based on information obtained
> >>>> by net_bootp.
> >>>>
> >>>> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >>>> Cc: nikunj@linux.vnet.ibm.com
> >>>>
> >>>> ---
> >>>>
> >>>> @Nikunj: cosmetic changes comparing with version you tested - no need to
> >>>> allocate small buffer dynamically.
> >>>>
> >>>> @Vladimir: I think this is useful; I will update networking documentation
> >>>> in followup patch.
> >>>
> >>> Vladimir, Andrei, could you take care of it?
> >>>
> >>> Daniel
> >>>
> >>
> >> I added documentation (not worth separate patch); if there are no
> >> objections I commit.
> >>
> >>
> >> From: Andrei Borzenkov <arvidjaar@gmail.com>
> >> Subject: [2.02][PATCH] bootp: export server IP as environment variable
> >>
> >> Network boot autoconfiguration sets default server to next server IP
> >> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
> >> exports only server name. Unfortunately semantic of server name is not
> >> clearly defined. BOOTP RFC 951 defines it only for client request, and
> >> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
> >> this field is mostly empty in server replies.
> >>
> >> Export next server IP as net_<interface>_server_ip variable. This allows
> >> grub configuration script to set $root/$prefix based on information obtained
> >> by net_bootp.
> >>
> >> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >> Cc: nikunj@linux.vnet.ibm.com
> >>
> >> ---
> >>  docs/grub.texi        | 11 +++++++++++
> >>  grub-core/net/bootp.c | 24 ++++++++++++++----------
> >>  2 files changed, 25 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/docs/grub.texi b/docs/grub.texi
> >> index 82f6fa4..bb03875 100644
> >> --- a/docs/grub.texi
> >> +++ b/docs/grub.texi
> >> @@ -2446,6 +2446,10 @@ The boot file name provided by DHCP.  Read-only.
> >>  The name of the DHCP server responsible for these boot parameters.
> >>  Read-only.
> >>
> >> +@item net_@var{<interface>}_server_ip
> >> +The IP address of the next (usually, TFTP) server provided by DHCP.
> >
> > BOOTP instead of DHCP? However, then we should change DHCP everywhere too...
> >
>
> Yes. Sorry, it was probably my fault to refer to DHCP in the first place.
>
> >> +Read-only.
> >> +
> >>  @item net_default_interface
> >>  Initially set to name of network interface that was used to load grub.
> >>  Read-write, although setting it affects only interpretation of
> >> @@ -3062,6 +3066,7 @@ These variables have special meaning to GRUB.
> >>  * net_@var{<interface>}_hostname::
> >>  * net_@var{<interface>}_ip::
> >>  * net_@var{<interface>}_mac::
> >> +* net_@var{<interface>}_server_ip::
> >
> > There is something known as net_default_server which contains next IP address.
> > Should not we be more consistent and use "net_@var{<interface>}_server" instead
> > of "net_@var{<interface>}_server_ip" here and there.
> >
>
> Well, net_default_server is not really tied to any BOOTP/DHCP at all,
> although it may be initially set based on content of ACK packet. I was
> confused by dhcp_server_name, but they it is unrelated to next server.
>
> I'd rather used net_<interface>_next_server for clarity, both "server"
> and "server_ip" are indeed too vague in this case.

net_<interface>_next_server is OK for me. Should not we introduce
net_default_next_server as RO then too? At first its value will be
equal net_default_server but only net_default_server could be
changed by user. Does it make sense?

Daniel

Re: [2.02][PATCH] bootp: export server IP as environment variable

[Andrei Borzenkov]

22.11.2016 01:02, Daniel Kiper пишет:
> On Mon, Nov 21, 2016 at 10:52:24PM +0300, Andrei Borzenkov wrote:
>> 21.11.2016 16:09, Daniel Kiper ??????????:
>>> On Fri, Nov 18, 2016 at 09:11:47PM +0300, Andrei Borzenkov wrote:
>>>> 17.11.2016 22:02, Daniel Kiper ??????????:
>>>>> On Fri, Mar 18, 2016 at 08:42:00PM +0300, Andrei Borzenkov wrote:
>>>>>> Network boot autoconfiguration sets default server to next server IP
>>>>>> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
>>>>>> exports only server name. Unfortunately semantic of server name is not
>>>>>> clearly defined. BOOTP RFC 951 defines it only for client request, and
>>>>>> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
>>>>>> this field is mostly empty in server replies.
>>>>>>
>>>>>> Export next server IP as net_<interface>_server_ip variable. This allows
>>>>>> grub configuration script to set $root/$prefix based on information obtained
>>>>>> by net_bootp.
>>>>>>
>>>>>> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>>>> Cc: nikunj@linux.vnet.ibm.com
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> @Nikunj: cosmetic changes comparing with version you tested - no need to
>>>>>> allocate small buffer dynamically.
>>>>>>
>>>>>> @Vladimir: I think this is useful; I will update networking documentation
>>>>>> in followup patch.
>>>>>
>>>>> Vladimir, Andrei, could you take care of it?
>>>>>
>>>>> Daniel
>>>>>
>>>>
>>>> I added documentation (not worth separate patch); if there are no
>>>> objections I commit.
>>>>
>>>>
>>>> From: Andrei Borzenkov <arvidjaar@gmail.com>
>>>> Subject: [2.02][PATCH] bootp: export server IP as environment variable
>>>>
>>>> Network boot autoconfiguration sets default server to next server IP
>>>> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
>>>> exports only server name. Unfortunately semantic of server name is not
>>>> clearly defined. BOOTP RFC 951 defines it only for client request, and
>>>> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
>>>> this field is mostly empty in server replies.
>>>>
>>>> Export next server IP as net_<interface>_server_ip variable. This allows
>>>> grub configuration script to set $root/$prefix based on information obtained
>>>> by net_bootp.
>>>>
>>>> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>> Cc: nikunj@linux.vnet.ibm.com
>>>>
>>>> ---
>>>>  docs/grub.texi        | 11 +++++++++++
>>>>  grub-core/net/bootp.c | 24 ++++++++++++++----------
>>>>  2 files changed, 25 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/docs/grub.texi b/docs/grub.texi
>>>> index 82f6fa4..bb03875 100644
>>>> --- a/docs/grub.texi
>>>> +++ b/docs/grub.texi
>>>> @@ -2446,6 +2446,10 @@ The boot file name provided by DHCP.  Read-only.
>>>>  The name of the DHCP server responsible for these boot parameters.
>>>>  Read-only.
>>>>
>>>> +@item net_@var{<interface>}_server_ip
>>>> +The IP address of the next (usually, TFTP) server provided by DHCP.
>>>
>>> BOOTP instead of DHCP? However, then we should change DHCP everywhere too...
>>>
>>
>> Yes. Sorry, it was probably my fault to refer to DHCP in the first place.
>>
>>>> +Read-only.
>>>> +
>>>>  @item net_default_interface
>>>>  Initially set to name of network interface that was used to load grub.
>>>>  Read-write, although setting it affects only interpretation of
>>>> @@ -3062,6 +3066,7 @@ These variables have special meaning to GRUB.
>>>>  * net_@var{<interface>}_hostname::
>>>>  * net_@var{<interface>}_ip::
>>>>  * net_@var{<interface>}_mac::
>>>> +* net_@var{<interface>}_server_ip::
>>>
>>> There is something known as net_default_server which contains next IP address.
>>> Should not we be more consistent and use "net_@var{<interface>}_server" instead
>>> of "net_@var{<interface>}_server_ip" here and there.
>>>
>>
>> Well, net_default_server is not really tied to any BOOTP/DHCP at all,
>> although it may be initially set based on content of ACK packet. I was
>> confused by dhcp_server_name, but they it is unrelated to next server.
>>
>> I'd rather used net_<interface>_next_server for clarity, both "server"
>> and "server_ip" are indeed too vague in this case.
> 
> net_<interface>_next_server is OK for me. Should not we introduce
> net_default_next_server as RO then too? At first its value will be
> equal net_default_server but only net_default_server could be
> changed by user. Does it make sense?
> 

I would wait until someone has use case for it. I do not see it now.

Re: [2.02][PATCH] bootp: export server IP as environment variable

[Daniel Kiper]

On Tue, Nov 22, 2016 at 08:21:12PM +0300, Andrei Borzenkov wrote:
> 22.11.2016 01:02, Daniel Kiper ??????????:
> > On Mon, Nov 21, 2016 at 10:52:24PM +0300, Andrei Borzenkov wrote:
> >> 21.11.2016 16:09, Daniel Kiper ??????????:
> >>> On Fri, Nov 18, 2016 at 09:11:47PM +0300, Andrei Borzenkov wrote:
> >>>> 17.11.2016 22:02, Daniel Kiper ??????????:
> >>>>> On Fri, Mar 18, 2016 at 08:42:00PM +0300, Andrei Borzenkov wrote:
> >>>>>> Network boot autoconfiguration sets default server to next server IP
> >>>>>> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
> >>>>>> exports only server name. Unfortunately semantic of server name is not
> >>>>>> clearly defined. BOOTP RFC 951 defines it only for client request, and
> >>>>>> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
> >>>>>> this field is mostly empty in server replies.
> >>>>>>
> >>>>>> Export next server IP as net_<interface>_server_ip variable. This allows
> >>>>>> grub configuration script to set $root/$prefix based on information obtained
> >>>>>> by net_bootp.
> >>>>>>
> >>>>>> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >>>>>> Cc: nikunj@linux.vnet.ibm.com
> >>>>>>
> >>>>>> ---
> >>>>>>
> >>>>>> @Nikunj: cosmetic changes comparing with version you tested - no need to
> >>>>>> allocate small buffer dynamically.
> >>>>>>
> >>>>>> @Vladimir: I think this is useful; I will update networking documentation
> >>>>>> in followup patch.
> >>>>>
> >>>>> Vladimir, Andrei, could you take care of it?
> >>>>>
> >>>>> Daniel
> >>>>>
> >>>>
> >>>> I added documentation (not worth separate patch); if there are no
> >>>> objections I commit.
> >>>>
> >>>>
> >>>> From: Andrei Borzenkov <arvidjaar@gmail.com>
> >>>> Subject: [2.02][PATCH] bootp: export server IP as environment variable
> >>>>
> >>>> Network boot autoconfiguration sets default server to next server IP
> >>>> (siaddr) from BOOTP/DHCP reply, but manual configuration using net_bootp
> >>>> exports only server name. Unfortunately semantic of server name is not
> >>>> clearly defined. BOOTP RFC 951 defines it only for client request, and
> >>>> DHCP RFC 1541 only mentions it, without any implied usage. It looks like
> >>>> this field is mostly empty in server replies.
> >>>>
> >>>> Export next server IP as net_<interface>_server_ip variable. This allows
> >>>> grub configuration script to set $root/$prefix based on information obtained
> >>>> by net_bootp.
> >>>>
> >>>> Reported and tested by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >>>> Cc: nikunj@linux.vnet.ibm.com
> >>>>
> >>>> ---
> >>>>  docs/grub.texi        | 11 +++++++++++
> >>>>  grub-core/net/bootp.c | 24 ++++++++++++++----------
> >>>>  2 files changed, 25 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/docs/grub.texi b/docs/grub.texi
> >>>> index 82f6fa4..bb03875 100644
> >>>> --- a/docs/grub.texi
> >>>> +++ b/docs/grub.texi
> >>>> @@ -2446,6 +2446,10 @@ The boot file name provided by DHCP.  Read-only.
> >>>>  The name of the DHCP server responsible for these boot parameters.
> >>>>  Read-only.
> >>>>
> >>>> +@item net_@var{<interface>}_server_ip
> >>>> +The IP address of the next (usually, TFTP) server provided by DHCP.
> >>>
> >>> BOOTP instead of DHCP? However, then we should change DHCP everywhere too...
> >>>
> >>
> >> Yes. Sorry, it was probably my fault to refer to DHCP in the first place.
> >>
> >>>> +Read-only.
> >>>> +
> >>>>  @item net_default_interface
> >>>>  Initially set to name of network interface that was used to load grub.
> >>>>  Read-write, although setting it affects only interpretation of
> >>>> @@ -3062,6 +3066,7 @@ These variables have special meaning to GRUB.
> >>>>  * net_@var{<interface>}_hostname::
> >>>>  * net_@var{<interface>}_ip::
> >>>>  * net_@var{<interface>}_mac::
> >>>> +* net_@var{<interface>}_server_ip::
> >>>
> >>> There is something known as net_default_server which contains next IP address.
> >>> Should not we be more consistent and use "net_@var{<interface>}_server" instead
> >>> of "net_@var{<interface>}_server_ip" here and there.
> >>>
> >>
> >> Well, net_default_server is not really tied to any BOOTP/DHCP at all,
> >> although it may be initially set based on content of ACK packet. I was
> >> confused by dhcp_server_name, but they it is unrelated to next server.
> >>
> >> I'd rather used net_<interface>_next_server for clarity, both "server"
> >> and "server_ip" are indeed too vague in this case.
> >
> > net_<interface>_next_server is OK for me. Should not we introduce
> > net_default_next_server as RO then too? At first its value will be
> > equal net_default_server but only net_default_server could be
> > changed by user. Does it make sense?
>
> I would wait until someone has use case for it. I do not see it now.

OK, so, let's leave net_<interface>_next_server only right now.
Thanks for committing this stuff.

Daniel