[PATCH] bootp: set is_def while processing dhcp ack

[PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

While testing netboot on ppc64, found that the grub_net_default_server
is not set even when the dhcp response has sent the server IP. This would
result in a failure during netbooting with following error:

error: no server is specified.

This patch sets the is_def variable which would result in setting the
grub_net_default_server and then netboot from the server.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 grub-core/net/bootp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index a088244..74b8b79 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -278,7 +278,7 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
     }
   grub_net_configure_by_dhcp_ack (name, card,
 				  0, (const struct grub_net_bootp_packet *) nb->data,
-				  (nb->tail - nb->data), 0, 0, 0);
+				  (nb->tail - nb->data), 1, 0, 0);
   grub_free (name);
   if (grub_errno)
     grub_print_error ();
-- 
2.5.0

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Andrei Borzenkov]

On Wed, Mar 16, 2016 at 11:35 AM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
> While testing netboot on ppc64, found that the grub_net_default_server
> is not set even when the dhcp response has sent the server IP. This would
> result in a failure during netbooting with following error:
>
> error: no server is specified.
>
> This patch sets the is_def variable which would result in setting the
> grub_net_default_server and then netboot from the server.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  grub-core/net/bootp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> index a088244..74b8b79 100644
> --- a/grub-core/net/bootp.c
> +++ b/grub-core/net/bootp.c
> @@ -278,7 +278,7 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
>      }
>    grub_net_configure_by_dhcp_ack (name, card,
>                                   0, (const struct grub_net_bootp_packet *) nb->data,
> -                                 (nb->tail - nb->data), 0, 0, 0);
> +                                 (nb->tail - nb->data), 1, 0, 0);

That's wrong, sorry. "is default" is intended to be used from initial
configuration based on information from firmware (i.e. - firmware does
PXE boot and we configure interface based on PXE packet firmware made
available for us). If you call net_bootp manually, server is stored in
environment variable net_<if>_dhcp_server_name; you are free to use it
as needed, e.g. set $root and $prefix.

Does your platform use PXE or some other protocol for netboot? We may
need to provide platform-specific network configuration function that
understands how to fetch information from firmware.

>    grub_free (name);
>    if (grub_errno)
>      grub_print_error ();
> --
> 2.5.0
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

Hi Andrei,

Thanks for a quick review.

Andrei Borzenkov <arvidjaar@gmail.com> writes:
> On Wed, Mar 16, 2016 at 11:35 AM, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>> While testing netboot on ppc64, found that the grub_net_default_server
>> is not set even when the dhcp response has sent the server IP. This would
>> result in a failure during netbooting with following error:
>>
>> error: no server is specified.
>>
>> This patch sets the is_def variable which would result in setting the
>> grub_net_default_server and then netboot from the server.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  grub-core/net/bootp.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>> index a088244..74b8b79 100644
>> --- a/grub-core/net/bootp.c
>> +++ b/grub-core/net/bootp.c
>> @@ -278,7 +278,7 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
>>      }
>>    grub_net_configure_by_dhcp_ack (name, card,
>>                                   0, (const struct grub_net_bootp_packet *) nb->data,
>> -                                 (nb->tail - nb->data), 0, 0, 0);
>> +                                 (nb->tail - nb->data), 1, 0, 0);
>
> That's wrong, sorry. "is default" is intended to be used from initial
> configuration based on information from firmware (i.e. - firmware does
> PXE boot and we configure interface based on PXE packet firmware made
> available for us).

> If you call net_bootp manually,

Thats what we are using in this current case, booting from the disk,
and the grub.cfg has a netboot entry.

menuentry 'Linux dev netboot' --class os {
     insmod net
     insmod ofnet
     insmod tftp

     net_bootp

     echo 'Network status: '
     net_ls_cards
     net_ls_addr
     net_ls_routes

     echo 'Loading Linux ...'
     linux  (tftp)/ubuntu-installer/ppc64el/vmlinux tasks=standard pkgsel/language-pack-patterns= pkgsel/install-language-support=false --- quiet
     initrd	(tftp)/ubuntu-installer/ppc64el/initrd.gz
}

> server is stored in environment variable net_<if>_dhcp_server_name;
> you are free to use it as needed, e.g. set $root and $prefix.
>
> Does your platform use PXE or some other protocol for netboot?

Its not exactly same as PXE, below link explains that in detail:

https://www.ibm.com/developerworks/community/wikis/home?lang=en#!/wiki/W51a7ffcf4dfd_4b40_9d82_446ebc23c550/page/Netbooting%20on%20POWER%20-%20An%20Introduction

Basically, a DHCP request is sent and in the DHCP ACK, we get the
details of client-ip, bootfile (core.elf of grub), server-ip, netmask,
etc. Later tftp is used to get the kernel/initrd images.

During this case everything else gets parsed properly except the
server-ip. Which was under the is_def case.

> We may need to provide platform-specific network configuration
> function that understands how to fetch information from firmware.

This information is not there in the firmware. Its there in the DHCP-ACK
packet. When the firmware does the netboot, we have most of the
information encoded in the device tree and is parsed fine in grub.

In this case we have a disk and one of the entry is to netboot, thats
when it fails.

Regards,
Nikunj

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Andrei Borzenkov]

On Wed, Mar 16, 2016 at 1:16 PM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
> Hi Andrei,
>
> Thanks for a quick review.
>
> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>> On Wed, Mar 16, 2016 at 11:35 AM, Nikunj A Dadhania
>> <nikunj@linux.vnet.ibm.com> wrote:
>>> While testing netboot on ppc64, found that the grub_net_default_server
>>> is not set even when the dhcp response has sent the server IP. This would
>>> result in a failure during netbooting with following error:
>>>
>>> error: no server is specified.
>>>
>>> This patch sets the is_def variable which would result in setting the
>>> grub_net_default_server and then netboot from the server.
>>>
>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>> ---
>>>  grub-core/net/bootp.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>>> index a088244..74b8b79 100644
>>> --- a/grub-core/net/bootp.c
>>> +++ b/grub-core/net/bootp.c
>>> @@ -278,7 +278,7 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
>>>      }
>>>    grub_net_configure_by_dhcp_ack (name, card,
>>>                                   0, (const struct grub_net_bootp_packet *) nb->data,
>>> -                                 (nb->tail - nb->data), 0, 0, 0);
>>> +                                 (nb->tail - nb->data), 1, 0, 0);
>>
>> That's wrong, sorry. "is default" is intended to be used from initial
>> configuration based on information from firmware (i.e. - firmware does
>> PXE boot and we configure interface based on PXE packet firmware made
>> available for us).
>
>> If you call net_bootp manually,
>
> Thats what we are using in this current case, booting from the disk,
> and the grub.cfg has a netboot entry.
>
> menuentry 'Linux dev netboot' --class os {
>      insmod net
>      insmod ofnet
>      insmod tftp
>
>      net_bootp
>
>      echo 'Network status: '
>      net_ls_cards
>      net_ls_addr
>      net_ls_routes
>
>      echo 'Loading Linux ...'
>      linux  (tftp)/ubuntu-installer/ppc64el/vmlinux tasks=standard pkgsel/language-pack-patterns= pkgsel/install-language-support=false --- quiet
>      initrd     (tftp)/ubuntu-installer/ppc64el/initrd.gz
> }
>

OK, so you attempt to configure everything. The first obvious question
is - *which* interface should now become default? Your patch would
make them compete, whoever is the last would win.

We would need to define some mechanism to designate interface as "default" here.

>> server is stored in environment variable net_<if>_dhcp_server_name;
>> you are free to use it as needed, e.g. set $root and $prefix.
>>

You can already fetch this information from environment. Something like

for i in 0 1 2 3 4 5; do
  eval "set ip=\$net_ofnet${i}_dhcp_server"
  if [ -n "$ip" ]; then
    set boot_from=tftp,$ip
    break
  fi
done

And later use ($boot_from) everywhere.

>> Does your platform use PXE or some other protocol for netboot?
>
> Its not exactly same as PXE, below link explains that in detail:
>

Thank you for the link but it is irrelevant here.

> https://www.ibm.com/developerworks/community/wikis/home?lang=en#!/wiki/W51a7ffcf4dfd_4b40_9d82_446ebc23c550/page/Netbooting%20on%20POWER%20-%20An%20Introduction
>
> Basically, a DHCP request is sent and in the DHCP ACK, we get the
> details of client-ip, bootfile (core.elf of grub), server-ip, netmask,
> etc. Later tftp is used to get the kernel/initrd images.
>
> During this case everything else gets parsed properly except the
> server-ip. Which was under the is_def case.
>
>> We may need to provide platform-specific network configuration
>> function that understands how to fetch information from firmware.
>
> This information is not there in the firmware. Its there in the DHCP-ACK
> packet. When the firmware does the netboot, we have most of the
> information encoded in the device tree and is parsed fine in grub.
>
> In this case we have a disk and one of the entry is to netboot, thats
> when it fails.
>
> Regards,
> Nikunj
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

Andrei Borzenkov <arvidjaar@gmail.com> writes:

> On Wed, Mar 16, 2016 at 1:16 PM, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>> Hi Andrei,
>>
>> Thanks for a quick review.
>>
>> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>>> On Wed, Mar 16, 2016 at 11:35 AM, Nikunj A Dadhania
>>> <nikunj@linux.vnet.ibm.com> wrote:
>>>> While testing netboot on ppc64, found that the grub_net_default_server
>>>> is not set even when the dhcp response has sent the server IP. This would
>>>> result in a failure during netbooting with following error:
>>>>
>>>> error: no server is specified.
>>>>
>>>> This patch sets the is_def variable which would result in setting the
>>>> grub_net_default_server and then netboot from the server.
>>>>
>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>> ---
>>>>  grub-core/net/bootp.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>>>> index a088244..74b8b79 100644
>>>> --- a/grub-core/net/bootp.c
>>>> +++ b/grub-core/net/bootp.c
>>>> @@ -278,7 +278,7 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
>>>>      }
>>>>    grub_net_configure_by_dhcp_ack (name, card,
>>>>                                   0, (const struct grub_net_bootp_packet *) nb->data,
>>>> -                                 (nb->tail - nb->data), 0, 0, 0);
>>>> +                                 (nb->tail - nb->data), 1, 0, 0);
>>>
>>> That's wrong, sorry. "is default" is intended to be used from initial
>>> configuration based on information from firmware (i.e. - firmware does
>>> PXE boot and we configure interface based on PXE packet firmware made
>>> available for us).
>>
>>> If you call net_bootp manually,
>>
>> Thats what we are using in this current case, booting from the disk,
>> and the grub.cfg has a netboot entry.
>>
>> menuentry 'Linux dev netboot' --class os {
>>      insmod net
>>      insmod ofnet
>>      insmod tftp
>>
>>      net_bootp
>>
>>      echo 'Network status: '
>>      net_ls_cards
>>      net_ls_addr
>>      net_ls_routes
>>
>>      echo 'Loading Linux ...'
>>      linux  (tftp)/ubuntu-installer/ppc64el/vmlinux tasks=standard pkgsel/language-pack-patterns= pkgsel/install-language-support=false --- quiet
>>      initrd     (tftp)/ubuntu-installer/ppc64el/initrd.gz
>> }
>>
>
> OK, so you attempt to configure everything. The first obvious question
> is - *which* interface should now become default?

In this case I only had one interface. So thats the default.

> Your patch would make them compete, whoever is the last would win.

Oh ok. Will "net_bootp" try all the interfaces ?

> We would need to define some mechanism to designate interface as "default" here.
>
>>> server is stored in environment variable net_<if>_dhcp_server_name;
>>> you are free to use it as needed, e.g. set $root and $prefix.
>>>
>
> You can already fetch this information from environment. Something like
>
> for i in 0 1 2 3 4 5; do
>   eval "set ip=\$net_ofnet${i}_dhcp_server"
>   if [ -n "$ip" ]; then
>     set boot_from=tftp,$ip
>     break
>   fi
> done
>

Thanks, will try this out.

> And later use ($boot_from) everywhere.

Regards
Nikunj

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Andrei Borzenkov]

On Wed, Mar 16, 2016 at 2:00 PM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
>>>
>>> Thats what we are using in this current case, booting from the disk,
>>> and the grub.cfg has a netboot entry.
>>>
>>> menuentry 'Linux dev netboot' --class os {
>>>      insmod net
>>>      insmod ofnet
>>>      insmod tftp
>>>
>>>      net_bootp
>>>
>>>      echo 'Network status: '
>>>      net_ls_cards
>>>      net_ls_addr
>>>      net_ls_routes
>>>
>>>      echo 'Loading Linux ...'
>>>      linux  (tftp)/ubuntu-installer/ppc64el/vmlinux tasks=standard pkgsel/language-pack-patterns= pkgsel/install-language-support=false --- quiet
>>>      initrd     (tftp)/ubuntu-installer/ppc64el/initrd.gz
>>> }
>>>
>>
>> OK, so you attempt to configure everything. The first obvious question
>> is - *which* interface should now become default?
>
> In this case I only had one interface. So thats the default.
>
>> Your patch would make them compete, whoever is the last would win.
>
> Oh ok. Will "net_bootp" try all the interfaces ?
>

Yes. It is even documented :)

>> We would need to define some mechanism to designate interface as "default" here.
>>
>>>> server is stored in environment variable net_<if>_dhcp_server_name;
>>>> you are free to use it as needed, e.g. set $root and $prefix.
>>>>
>>
>> You can already fetch this information from environment. Something like
>>
>> for i in 0 1 2 3 4 5; do
>>   eval "set ip=\$net_ofnet${i}_dhcp_server"
>>   if [ -n "$ip" ]; then
>>     set boot_from=tftp,$ip
>>     break
>>   fi
>> done
>>
>
> Thanks, will try this out.
>
>> And later use ($boot_from) everywhere.
>
> Regards
> Nikunj
>

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

Andrei Borzenkov <arvidjaar@gmail.com> writes:

> On Wed, Mar 16, 2016 at 1:16 PM, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>> Hi Andrei,
>>
>>> If you call net_bootp manually,
>>
>> Thats what we are using in this current case, booting from the disk,
>> and the grub.cfg has a netboot entry.
>>
>> menuentry 'Linux dev netboot' --class os {
>>      insmod net
>>      insmod ofnet
>>      insmod tftp
>>
>>      net_bootp
>>
>>      echo 'Network status: '
>>      net_ls_cards
>>      net_ls_addr
>>      net_ls_routes
>>
>>      echo 'Loading Linux ...'
>>      linux  (tftp)/ubuntu-installer/ppc64el/vmlinux tasks=standard pkgsel/language-pack-patterns= pkgsel/install-language-support=false --- quiet
>>      initrd     (tftp)/ubuntu-installer/ppc64el/initrd.gz
>> }
>>
>
> OK, so you attempt to configure everything. The first obvious question
> is - *which* interface should now become default? Your patch would
> make them compete, whoever is the last would win.
>
> We would need to define some mechanism to designate interface as "default" here.
>
>>> server is stored in environment variable net_<if>_dhcp_server_name;
>>> you are free to use it as needed, e.g. set $root and $prefix.
>>>
>
> You can already fetch this information from environment. Something like
>
> for i in 0 1 2 3 4 5; do
>   eval "set ip=\$net_ofnet${i}_dhcp_server"

I dont see net_ofnet0_dhcp_server or net_ofnet_dhcp_server set

>   if [ -n "$ip" ]; then
>     set boot_from=tftp,$ip
>     break
>   fi
> done
>
> And later use ($boot_from) everywhere.

Regards
Nikunj

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Andrei Borzenkov]

On Thu, Mar 17, 2016 at 9:16 AM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>
>> On Wed, Mar 16, 2016 at 1:16 PM, Nikunj A Dadhania
>> <nikunj@linux.vnet.ibm.com> wrote:
>>> Hi Andrei,
>>>
>>>> If you call net_bootp manually,
>>>
>>> Thats what we are using in this current case, booting from the disk,
>>> and the grub.cfg has a netboot entry.
>>>
>>> menuentry 'Linux dev netboot' --class os {
>>>      insmod net
>>>      insmod ofnet
>>>      insmod tftp
>>>
>>>      net_bootp
>>>
>>>      echo 'Network status: '
>>>      net_ls_cards
>>>      net_ls_addr
>>>      net_ls_routes
>>>
>>>      echo 'Loading Linux ...'
>>>      linux  (tftp)/ubuntu-installer/ppc64el/vmlinux tasks=standard pkgsel/language-pack-patterns= pkgsel/install-language-support=false --- quiet
>>>      initrd     (tftp)/ubuntu-installer/ppc64el/initrd.gz
>>> }
>>>
>>
>> OK, so you attempt to configure everything. The first obvious question
>> is - *which* interface should now become default? Your patch would
>> make them compete, whoever is the last would win.
>>
>> We would need to define some mechanism to designate interface as "default" here.
>>
>>>> server is stored in environment variable net_<if>_dhcp_server_name;
>>>> you are free to use it as needed, e.g. set $root and $prefix.
>>>>
>>
>> You can already fetch this information from environment. Something like
>>
>> for i in 0 1 2 3 4 5; do
>>   eval "set ip=\$net_ofnet${i}_dhcp_server"
>
> I dont see net_ofnet0_dhcp_server or net_ofnet_dhcp_server set
>

This should be net_ofnet0_dhcp_server_name, sorry. What variables are
set (could you show full list)?

>>   if [ -n "$ip" ]; then
>>     set boot_from=tftp,$ip
>>     break
>>   fi
>> done
>>
>> And later use ($boot_from) everywhere.
>
> Regards
> Nikunj
>

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

Andrei Borzenkov <arvidjaar@gmail.com> writes:

> On Thu, Mar 17, 2016 at 9:16 AM, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>>
>>> On Wed, Mar 16, 2016 at 1:16 PM, Nikunj A Dadhania
>>> <nikunj@linux.vnet.ibm.com> wrote:
>>>> Hi Andrei,
>>>>
>>>>> If you call net_bootp manually,
>>>>
>>>> Thats what we are using in this current case, booting from the disk,
>>>> and the grub.cfg has a netboot entry.
>>>>
>>>> menuentry 'Linux dev netboot' --class os {
>>>>      insmod net
>>>>      insmod ofnet
>>>>      insmod tftp
>>>>
>>>>      net_bootp
>>>>
>>>>      echo 'Network status: '
>>>>      net_ls_cards
>>>>      net_ls_addr
>>>>      net_ls_routes
>>>>
>>>>      echo 'Loading Linux ...'
>>>>      linux  (tftp)/ubuntu-installer/ppc64el/vmlinux tasks=standard pkgsel/language-pack-patterns= pkgsel/install-language-support=false --- quiet
>>>>      initrd     (tftp)/ubuntu-installer/ppc64el/initrd.gz
>>>> }
>>>>
>>>
>>> OK, so you attempt to configure everything. The first obvious question
>>> is - *which* interface should now become default? Your patch would
>>> make them compete, whoever is the last would win.
>>>
>>> We would need to define some mechanism to designate interface as "default" here.
>>>
>>>>> server is stored in environment variable net_<if>_dhcp_server_name;
>>>>> you are free to use it as needed, e.g. set $root and $prefix.
>>>>>
>>>
>>> You can already fetch this information from environment. Something like
>>>
>>> for i in 0 1 2 3 4 5; do
>>>   eval "set ip=\$net_ofnet${i}_dhcp_server"
>>
>> I dont see net_ofnet0_dhcp_server or net_ofnet_dhcp_server set
>>
>
> This should be net_ofnet0_dhcp_server_name, sorry. What variables are
> set (could you show full list)?

?=0
cmdpath=(ieee1275/disk)
color_highlight=black/light-gray
color_normal=light-gray/black
default=
feature_200_final=y
feature_all_video_module=y
feature_chainloader_bpb=y
feature_default_font_path=y
feature_menuentry_id=y
feature_menuentry_options=y
feature_nativedisk_cmd=y
feature_ntldr=y
feature_platform_search_hint=y
feature_timeout_style=y
grub_cpu=powerpc
grub_platform=ieee1275
lang=
locale_dir=
menuentry_id_option=--id
net_default_interface=ofnet_net:dhcp
net_default_ip=(null)
net_default_mac=(null)
net_default_server=10.0.2.2
net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
net_ofnet_net_dhcp_ip=10.0.2.15
net_ofnet_net_dhcp_mac=52:54:00:12:34:56
pager=1
prefix=(ieee1275/disk,msdos2)/grub
pxe_default_server=10.0.2.2
root=ieee1275/disk,msdos2
secondary_locale_dir=
timeout_style=menu

Regards
Nikunj

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:

> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>
>> On Thu, Mar 17, 2016 at 9:16 AM, Nikunj A Dadhania
>> <nikunj@linux.vnet.ibm.com> wrote:
>>> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>>>
>>>> You can already fetch this information from environment. Something like
>>>>
>>>> for i in 0 1 2 3 4 5; do
>>>>   eval "set ip=\$net_ofnet${i}_dhcp_server"
>>>
>>> I dont see net_ofnet0_dhcp_server or net_ofnet_dhcp_server set
>>>
>>
>> This should be net_ofnet0_dhcp_server_name, sorry. What variables are
>> set (could you show full list)?
>
> ?=0
> cmdpath=(ieee1275/disk)
> color_highlight=black/light-gray
> color_normal=light-gray/black
> default=
> feature_200_final=y
> feature_all_video_module=y
> feature_chainloader_bpb=y
> feature_default_font_path=y
> feature_menuentry_id=y
> feature_menuentry_options=y
> feature_nativedisk_cmd=y
> feature_ntldr=y
> feature_platform_search_hint=y
> feature_timeout_style=y
> grub_cpu=powerpc
> grub_platform=ieee1275
> lang=
> locale_dir=
> menuentry_id_option=--id
> net_default_interface=ofnet_net:dhcp
> net_default_ip=(null)
> net_default_mac=(null)
> net_default_server=10.0.2.2

This is with above patch, that is why net_default_server is set.

> net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
> net_ofnet_net_dhcp_ip=10.0.2.15
> net_ofnet_net_dhcp_mac=52:54:00:12:34:56
> pager=1
> prefix=(ieee1275/disk,msdos2)/grub
> pxe_default_server=10.0.2.2
> root=ieee1275/disk,msdos2
> secondary_locale_dir=
> timeout_style=menu

Regards
Nikunj

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Andrei Borzenkov]

On Thu, Mar 17, 2016 at 11:04 AM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
> net_default_interface=ofnet_net:dhcp
> net_default_ip=(null)
> net_default_mac=(null)
> net_default_server=10.0.2.2
> net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
> net_ofnet_net_dhcp_ip=10.0.2.15
> net_ofnet_net_dhcp_mac=52:54:00:12:34:56


OK, we probably need to export not only name, but also IP to match
default case. Could you attach bootp reply packet?

I wonder if anyone ever seen or used net_if_dhcp_server_name variable;
because net_boot sets interface name to card:dhcp, this actually
becomes net_card_dhcp_dhcp_server_name. Someone would sure have
noticed it.

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

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

Andrei Borzenkov <arvidjaar@gmail.com> writes:

> On Thu, Mar 17, 2016 at 11:04 AM, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>> net_default_interface=ofnet_net:dhcp
>> net_default_ip=(null)
>> net_default_mac=(null)
>> net_default_server=10.0.2.2
>> net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
>> net_ofnet_net_dhcp_ip=10.0.2.15
>> net_ofnet_net_dhcp_mac=52:54:00:12:34:56
>
>
> OK, we probably need to export not only name, but also IP to match
> default case. Could you attach bootp reply packet?

Attached the pcap file, it has the request as well as the ack.

> I wonder if anyone ever seen or used net_if_dhcp_server_name variable;
> because net_boot sets interface name to card:dhcp, this actually
> becomes net_card_dhcp_dhcp_server_name. Someone would sure have
> noticed it.

Regards
Nikunj


[-- Attachment #2: qemu_guest.pcap --]
[-- Type: application/vnd.tcpdump.pcap, Size: 988 bytes --]

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Andrei Borzenkov]

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

17.03.2016 11:45, Nikunj A Dadhania пишет:
> Andrei Borzenkov <arvidjaar@gmail.com> writes:
> 
>> On Thu, Mar 17, 2016 at 11:04 AM, Nikunj A Dadhania
>> <nikunj@linux.vnet.ibm.com> wrote:
>>> net_default_interface=ofnet_net:dhcp
>>> net_default_ip=(null)
>>> net_default_mac=(null)
>>> net_default_server=10.0.2.2
>>> net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
>>> net_ofnet_net_dhcp_ip=10.0.2.15
>>> net_ofnet_net_dhcp_mac=52:54:00:12:34:56
>>
>>
>> OK, we probably need to export not only name, but also IP to match
>> default case. Could you attach bootp reply packet?
> 
> Attached the pcap file, it has the request as well as the ack.
> 
>> I wonder if anyone ever seen or used net_if_dhcp_server_name variable;
>> because net_boot sets interface name to card:dhcp, this actually
>> becomes net_card_dhcp_dhcp_server_name. Someone would sure have
>> noticed it.
> 

Please test attached patch. It sets net_<card>_dhcp_server_ip variable.

We probably need to offer programmatic way to iterate through all cards
in CLI. But as long as you have single card with known name it should be
OK.

[-- Attachment #2: bootp_server_ip.patch --]
[-- Type: text/x-patch, Size: 2661 bytes --]

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



---
 grub-core/net/bootp.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index a088244..c70cc09 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 = 0;
 
   addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4;
   addr.ipv4 = bp->your_ip;
@@ -189,15 +190,27 @@ 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)
+    {
+      server_ip = 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_print_error ();
+    }
+
+  if (server_ip)
+    {
+      grub_env_set_net_property (name, "server_ip", server_ip, grub_strlen (server_ip));
+      grub_print_error ();
+    }
+
   if (is_def)
     grub_net_default_server = 0;
-  if (is_def && !grub_net_default_server && bp->server_ip)
+  if (is_def && !grub_net_default_server && 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 ();
     }
 
@@ -207,13 +220,9 @@ grub_net_configure_by_dhcp_ack (const char *name,
       grub_env_export ("net_default_interface");
     }
 
-  if (device && !*device && bp->server_ip)
+  if (device && !*device && 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)
@@ -260,6 +269,7 @@ grub_net_configure_by_dhcp_ack (const char *name,
   else
     grub_errno = GRUB_ERR_NONE;
 
+  grub_free (server_ip);
   return inter;
 }
 
-- 
tg: (76eac44..) u/bootp-server-ip (depends on: master)

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

Andrei Borzenkov <arvidjaar@gmail.com> writes:

> 17.03.2016 11:45, Nikunj A Dadhania пишет:
>> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>> 
>>> On Thu, Mar 17, 2016 at 11:04 AM, Nikunj A Dadhania
>>> <nikunj@linux.vnet.ibm.com> wrote:
>>>> net_default_interface=ofnet_net:dhcp
>>>> net_default_ip=(null)
>>>> net_default_mac=(null)
>>>> net_default_server=10.0.2.2
>>>> net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
>>>> net_ofnet_net_dhcp_ip=10.0.2.15
>>>> net_ofnet_net_dhcp_mac=52:54:00:12:34:56
>>>
>>>
>>> OK, we probably need to export not only name, but also IP to match
>>> default case. Could you attach bootp reply packet?
>> 
>> Attached the pcap file, it has the request as well as the ack.
>> 
>>> I wonder if anyone ever seen or used net_if_dhcp_server_name variable;
>>> because net_boot sets interface name to card:dhcp, this actually
>>> becomes net_card_dhcp_dhcp_server_name. Someone would sure have
>>> noticed it.
>> 
>
> Please test attached patch. It sets net_<card>_dhcp_server_ip variable.
>
> We probably need to offer programmatic way to iterate through all cards
> in CLI. But as long as you have single card with known name it should be
> OK.

So the variable net_ofnet_net_dhcp_server_ip is set now, although the
boot does not proceed and bails out.


    net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
    net_ofnet_net_dhcp_ip=10.0.2.15
    net_ofnet_net_dhcp_mac=52:54:00:12:34:56
    net_ofnet_net_dhcp_server_ip=10.0.2.2
    pager=1
    prefix=(ieee1275/disk,msdos2)/grub
    pxe_default_server=
    root=ieee1275/disk,msdos2
    secondary_locale_dir=
    Loading Linux ...
    error: no server is specified.

Regards
Nikunj

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Andrei Borzenkov]

On Fri, Mar 18, 2016 at 8:06 AM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>
>> 17.03.2016 11:45, Nikunj A Dadhania пишет:
>>> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>>>
>>>> On Thu, Mar 17, 2016 at 11:04 AM, Nikunj A Dadhania
>>>> <nikunj@linux.vnet.ibm.com> wrote:
>>>>> net_default_interface=ofnet_net:dhcp
>>>>> net_default_ip=(null)
>>>>> net_default_mac=(null)
>>>>> net_default_server=10.0.2.2
>>>>> net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
>>>>> net_ofnet_net_dhcp_ip=10.0.2.15
>>>>> net_ofnet_net_dhcp_mac=52:54:00:12:34:56
>>>>
>>>>
>>>> OK, we probably need to export not only name, but also IP to match
>>>> default case. Could you attach bootp reply packet?
>>>
>>> Attached the pcap file, it has the request as well as the ack.
>>>
>>>> I wonder if anyone ever seen or used net_if_dhcp_server_name variable;
>>>> because net_boot sets interface name to card:dhcp, this actually
>>>> becomes net_card_dhcp_dhcp_server_name. Someone would sure have
>>>> noticed it.
>>>
>>
>> Please test attached patch. It sets net_<card>_dhcp_server_ip variable.
>>
>> We probably need to offer programmatic way to iterate through all cards
>> in CLI. But as long as you have single card with known name it should be
>> OK.
>
> So the variable net_ofnet_net_dhcp_server_ip is set now, although the
> boot does not proceed and bails out.
>

There is misunderstanding. Of course you need to change your script to
actually use this variable. It does not magically set default server.

>
>     net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
>     net_ofnet_net_dhcp_ip=10.0.2.15
>     net_ofnet_net_dhcp_mac=52:54:00:12:34:56
>     net_ofnet_net_dhcp_server_ip=10.0.2.2
>     pager=1
>     prefix=(ieee1275/disk,msdos2)/grub
>     pxe_default_server=
>     root=ieee1275/disk,msdos2
>     secondary_locale_dir=
>     Loading Linux ...
>     error: no server is specified.
>
> Regards
> Nikunj
>

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

Andrei Borzenkov <arvidjaar@gmail.com> writes:

> On Fri, Mar 18, 2016 at 8:06 AM, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>>
>>> 17.03.2016 11:45, Nikunj A Dadhania пишет:
>>>> Andrei Borzenkov <arvidjaar@gmail.com> writes:
>>>>
>>>>> On Thu, Mar 17, 2016 at 11:04 AM, Nikunj A Dadhania
>>>>> <nikunj@linux.vnet.ibm.com> wrote:
>>>>>> net_default_interface=ofnet_net:dhcp
>>>>>> net_default_ip=(null)
>>>>>> net_default_mac=(null)
>>>>>> net_default_server=10.0.2.2
>>>>>> net_ofnet_net_dhcp_boot_file=ubuntu-installer/ppc64el/powerpc-ieee1275/core.elf
>>>>>> net_ofnet_net_dhcp_ip=10.0.2.15
>>>>>> net_ofnet_net_dhcp_mac=52:54:00:12:34:56
>>>>>
>>>>>
>>>>> OK, we probably need to export not only name, but also IP to match
>>>>> default case. Could you attach bootp reply packet?
>>>>
>>>> Attached the pcap file, it has the request as well as the ack.
>>>>
>>>>> I wonder if anyone ever seen or used net_if_dhcp_server_name variable;
>>>>> because net_boot sets interface name to card:dhcp, this actually
>>>>> becomes net_card_dhcp_dhcp_server_name. Someone would sure have
>>>>> noticed it.
>>>>
>>>
>>> Please test attached patch. It sets net_<card>_dhcp_server_ip variable.
>>>
>>> We probably need to offer programmatic way to iterate through all cards
>>> in CLI. But as long as you have single card with known name it should be
>>> OK.
>>
>> So the variable net_ofnet_net_dhcp_server_ip is set now, although the
>> boot does not proceed and bails out.
>>
>
> There is misunderstanding. Of course you need to change your script to
> actually use this variable. It does not magically set default server.

Yes, i was expecting that setting *device=tftp,server-ip might do the
trick.

So I did following for picking up the server IP, and this works:

    set net_default_server=$net_ofnet_net_dhcp_server_ip

As you had mentioned, can we have a programmatic way ?

Regards
Nikunj

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Andrei Borzenkov]

On Fri, Mar 18, 2016 at 9:08 AM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
>>>>>
>>>>>> I wonder if anyone ever seen or used net_if_dhcp_server_name variable;
>>>>>> because net_boot sets interface name to card:dhcp, this actually
>>>>>> becomes net_card_dhcp_dhcp_server_name. Someone would sure have
>>>>>> noticed it.
>>>>>
>>>>
>>>> Please test attached patch. It sets net_<card>_dhcp_server_ip variable.
>>>>
>>>> We probably need to offer programmatic way to iterate through all cards
>>>> in CLI. But as long as you have single card with known name it should be
>>>> OK.
>>>
>>> So the variable net_ofnet_net_dhcp_server_ip is set now, although the
>>> boot does not proceed and bails out.
>>>
>>
>> There is misunderstanding. Of course you need to change your script to
>> actually use this variable. It does not magically set default server.
>
> Yes, i was expecting that setting *device=tftp,server-ip might do the
> trick.
>
> So I did following for picking up the server IP, and this works:
>
>     set net_default_server=$net_ofnet_net_dhcp_server_ip
>
> As you had mentioned, can we have a programmatic way ?
>

The above looks pretty much programmatic to me :)

That's more a policy question - how do you determine which
card/interface to use (remember, there could be any number of them).
As you come here from disk boot, you could define card name in
environment block as example.

Re: [PATCH] bootp: set is_def while processing dhcp ack

[Nikunj A Dadhania]

Andrei Borzenkov <arvidjaar@gmail.com> writes:

> On Fri, Mar 18, 2016 at 9:08 AM, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>>>>>>
>>>>>>> I wonder if anyone ever seen or used net_if_dhcp_server_name variable;
>>>>>>> because net_boot sets interface name to card:dhcp, this actually
>>>>>>> becomes net_card_dhcp_dhcp_server_name. Someone would sure have
>>>>>>> noticed it.
>>>>>>
>>>>>
>>>>> Please test attached patch. It sets net_<card>_dhcp_server_ip variable.
>>>>>
>>>>> We probably need to offer programmatic way to iterate through all cards
>>>>> in CLI. But as long as you have single card with known name it should be
>>>>> OK.
>>>>
>>>> So the variable net_ofnet_net_dhcp_server_ip is set now, although the
>>>> boot does not proceed and bails out.
>>>>
>>>
>>> There is misunderstanding. Of course you need to change your script to
>>> actually use this variable. It does not magically set default server.
>>
>> Yes, i was expecting that setting *device=tftp,server-ip might do the
>> trick.
>>
>> So I did following for picking up the server IP, and this works:
>>
>>     set net_default_server=$net_ofnet_net_dhcp_server_ip
>>
>> As you had mentioned, can we have a programmatic way ?
>>
>
> The above looks pretty much programmatic to me :)

Sure, this will do :)

> That's more a policy question - how do you determine which
> card/interface to use (remember, there could be any number of them).
> As you come here from disk boot, you could define card name in
> environment block as example.

Thanks for the support, you are free to add my

Tested-by:  Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>