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

[Nikunj A Dadhania]

Andrei Borzenkov <address@hidden> writes:

> On Wed, Mar 16, 2016 at 1:16 PM, Nikunj A Dadhania
> <address@hidden> wrote:
>> Hi Andrei,
>>
>> Thanks for a quick review.
>>
>> Andrei Borzenkov <address@hidden> writes:
>>> On Wed, Mar 16, 2016 at 11:35 AM, Nikunj A Dadhania
>>> <address@hidden> 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 <address@hidden>
>>>> ---
>>>>  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