Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-25 Thread Matthew Garrett
On Wed, Jan 25, 2017 at 9:30 AM, Andrei Borzenkov  wrote:
> 24.01.2017 23:50, Matthew Garrett пишет:
>> The DHCP server will return a string in the boot_file field. If you
>> want to indicate that this file should be obtained over http, the
>> easiest way to handle this is to provide a boot file in the form
>> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
>> configuration parameters and there appears to be no way to override
>> that, which makes it impossible to provide a boot file in this form.
>
> Really?
>
> dnsmasq -d -z --dhcp-range=192.168.11.20,192.168.11.30 --dhcp-option
> option:bootfile-name,'(http,1.2.3.4)/foo/bar' -i eth1
>
>
> Bootstrap Protocol (ACK)
> ...
> Option: (67) Bootfile name
> Length: 23
> Bootfile name: (http,1.2.3.4)/foo/bar
> Option: (255) End
> Option End: 255

Huh. Apparently I was very confused. Sorry about the waste of time!

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-25 Thread Andrei Borzenkov
24.01.2017 23:50, Matthew Garrett пишет:
> On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov  wrote:
>> 24.01.2017 03:36, Matthew Garrett пишет:
>>> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
>>> it impossible to pass boot files with commas in them. Allow using a
>>
>> grub_net_open() operates on devices, not files. Please give more details
>> about your problem.
> 
> The DHCP server will return a string in the boot_file field. If you
> want to indicate that this file should be obtained over http, the
> easiest way to handle this is to provide a boot file in the form
> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
> configuration parameters and there appears to be no way to override
> that, which makes it impossible to provide a boot file in this form.

Really?

dnsmasq -d -z --dhcp-range=192.168.11.20,192.168.11.30 --dhcp-option
option:bootfile-name,'(http,1.2.3.4)/foo/bar' -i eth1


Bootstrap Protocol (ACK)
...
Option: (67) Bootfile name
Length: 23
Bootfile name: (http,1.2.3.4)/foo/bar
Option: (255) End
Option End: 255


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-25 Thread Matthew Garrett
On Wed, Jan 25, 2017 at 12:35 AM, Michael Chang  wrote:
> On Tue, Jan 24, 2017 at 10:21:22PM -0800, Matthew Garrett wrote:
>> We're passing the bootfile to grub in order to obtain further
>> configuration, so the firmware isn't relevant here.
>
> I mean that firmware may not be involved for now but it may be helpful someday
> to carry that very same config booted by firmware as is.

The information is there. If local configuration doesn't choose to do
something that overrides it, it will stay there.

> And firmware did have implications to $prefix, if it's not been set, the value
> would be the booted device and path pass by firmware. To keep from ABI
> conflicts in modules loaded from elsewhere, you should not change it (in that
> further configration) unless you know the modules will always be compatible in
> new $prefix location.

The DHCP server provided the initial copy of grub, so it should ensure
that it only provides links to configuration that will provide
compatible modules. This is no worse than supporting reading config
fragments off disk that might redefine prefix.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-25 Thread Matthew Garrett
On Tue, Jan 24, 2017 at 11:37 PM, Andrei Borzenkov  wrote:
> On Wed, Jan 25, 2017 at 10:16 AM, Matthew Garrett  wrote:
>> My experience is that configfile (http,example.com)grub/config works
>> as you'd expect it to, and that set
>> endpoint=$net_efinet0_dhcp_boot_file; configfile $endpoint does the
>> same. Am I hitting some corner case where things are being incorrectly
>> parsed and so giving me unintended functionality?
>>
>
> When grub starts it tries to determine device and path it was booted
> from. For network boot it currently means examining DHCP ACK packet
> that is normally provided by firmware and setting device to
> tftp,$next_ip and path to $bootfile. There is no provision to set
> protocol to anything else nor to parse $bootfile to extract
> protocol/server.
>
> If you speak about "configfile something" you are past this point so
> DHCP $bootfile option is not relevant at all.

The variable is accessible to any configuration file that was built
into the grub binary, so it's still very relevant. Our flow is to have
the firmware DHCP and obtain grub, then have grub send a new DHCP
request with a different user agent (see previous patch) and obtain a
new bootfile.

>> How are custom DHCP options handled? I can't find anything in the
>> documentation about them being interpreted, and a quick look at the
>> code only shows it setting known variables.
>
> You have net_get_dhcp_option to fetch arbitrary option value from DHCP
> ACK packet and put it in variable for further processing. I'm
> definitely open to improving this command to make it more useful if it
> turns out lacking something.

Hm. In theory sticking everything in option 43 and parsing it out in
the config is possible - in practice the only reason that would be
necessary right now is that grub uses the same separator as dnsmasq,
and a trivial patch to grub would make it much easier to provide
configuration on the dnsmasq command line.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-25 Thread Michael Chang
On Tue, Jan 24, 2017 at 10:21:22PM -0800, Matthew Garrett wrote:
> On Tue, Jan 24, 2017 at 10:18 PM, Michael Chang  wrote:
> > On Tue, Jan 24, 2017 at 12:50:37PM -0800, Matthew Garrett wrote:
> >> The DHCP server will return a string in the boot_file field. If you
> >> want to indicate that this file should be obtained over http, the
> >> easiest way to handle this is to provide a boot file in the form
> >> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
> >> configuration parameters and there appears to be no way to override
> >> that, which makes it impossible to provide a boot file in this form.
> >> Allowing the use of an alternative character avoids this problem.
> >
> > To my understanding, you can (and have to) use the url form in boot-file and
> > specify vendor-class-identifier to "HTTPClient" to let the UEFI firmware 
> > knows
> > this as a response to HTTP boot request and handle that boot-file 
> > accordingly,
> > then continue with that url loaded and run as NBP.
> 
> We're passing the bootfile to grub in order to obtain further
> configuration, so the firmware isn't relevant here.

I mean that firmware may not be involved for now but it may be helpful someday
to carry that very same config booted by firmware as is.

And firmware did have implications to $prefix, if it's not been set, the value
would be the booted device and path pass by firmware. To keep from ABI
conflicts in modules loaded from elsewhere, you should not change it (in that
further configration) unless you know the modules will always be compatible in
new $prefix location.

Thanks,
Michael
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Andrei Borzenkov
On Wed, Jan 25, 2017 at 10:16 AM, Matthew Garrett  wrote:
> On Tue, Jan 24, 2017 at 10:56 PM, Andrei Borzenkov  
> wrote:
>> On Wed, Jan 25, 2017 at 7:25 AM, Matthew Garrett  wrote:
>>> If prefix isn't set then won't bootfile be interpreted as the device plus 
>>> file?
>>>
>>
>> No. That would mean "parsing URI" that I mentioned.
>
> My experience is that configfile (http,example.com)grub/config works
> as you'd expect it to, and that set
> endpoint=$net_efinet0_dhcp_boot_file; configfile $endpoint does the
> same. Am I hitting some corner case where things are being incorrectly
> parsed and so giving me unintended functionality?
>

When grub starts it tries to determine device and path it was booted
from. For network boot it currently means examining DHCP ACK packet
that is normally provided by firmware and setting device to
tftp,$next_ip and path to $bootfile. There is no provision to set
protocol to anything else nor to parse $bootfile to extract
protocol/server.

If you speak about "configfile something" you are past this point so
DHCP $bootfile option is not relevant at all.

>>> We need the ability to pass port as well, so that would still be 
>>> insufficient.
>>
>> Good. So start with design proposal that is extensible enough.
>>
>> But TBH - all of this can already be implemented using grub scripting.
>> Use custom DHCP options or parse bootfile using regex. No code changes
>> is needed - we support generic scripting for a reason.
>
> How are custom DHCP options handled? I can't find anything in the
> documentation about them being interpreted, and a quick look at the
> code only shows it setting known variables.

You have net_get_dhcp_option to fetch arbitrary option value from DHCP
ACK packet and put it in variable for further processing. I'm
definitely open to improving this command to make it more useful if it
turns out lacking something.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Matthew Garrett
On Tue, Jan 24, 2017 at 10:56 PM, Andrei Borzenkov  wrote:
> On Wed, Jan 25, 2017 at 7:25 AM, Matthew Garrett  wrote:
>> If prefix isn't set then won't bootfile be interpreted as the device plus 
>> file?
>>
>
> No. That would mean "parsing URI" that I mentioned.

My experience is that configfile (http,example.com)grub/config works
as you'd expect it to, and that set
endpoint=$net_efinet0_dhcp_boot_file; configfile $endpoint does the
same. Am I hitting some corner case where things are being incorrectly
parsed and so giving me unintended functionality?

>> We need the ability to pass port as well, so that would still be 
>> insufficient.
>
> Good. So start with design proposal that is extensible enough.
>
> But TBH - all of this can already be implemented using grub scripting.
> Use custom DHCP options or parse bootfile using regex. No code changes
> is needed - we support generic scripting for a reason.

How are custom DHCP options handled? I can't find anything in the
documentation about them being interpreted, and a quick look at the
code only shows it setting known variables.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Andrei Borzenkov
On Wed, Jan 25, 2017 at 7:25 AM, Matthew Garrett  wrote:
> On Tue, Jan 24, 2017 at 8:15 PM, Andrei Borzenkov  wrote:
>> 25.01.2017 07:06, Matthew Garrett пишет:
>>> I don't understand - grub_net_open_real() already handles this case:
>>
>> Because bootfile from DHCP packet is not used to set device part of
>> $prefix. Setting bootfile to (http,host)filename will end up with full
>> prefix "(tftp,$next_ip)(http,host)base-name-of-filename". If you mean
>> something different, please explain.
>
> If prefix isn't set then won't bootfile be interpreted as the device plus 
> file?
>

No. That would mean "parsing URI" that I mentioned.

>>> Because we ship prebuilt images but don't know what IP addresses users
>>> will be using for their deployment servers.
>>>
>>
>> I can think about implementing URI parsing (somewhat in line with UEFI
>> HTTPboot spec) and/or supporting partial $prefix that sets only
>> protocol, something like "grub-mknetdir --prefix=(http,)", where server
>> part is filled from DHCP ACK.
>
> We need the ability to pass port as well, so that would still be insufficient.

Good. So start with design proposal that is extensible enough.

But TBH - all of this can already be implemented using grub scripting.
Use custom DHCP options or parse bootfile using regex. No code changes
is needed - we support generic scripting for a reason.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Matthew Garrett
On Tue, Jan 24, 2017 at 10:18 PM, Michael Chang  wrote:
> On Tue, Jan 24, 2017 at 12:50:37PM -0800, Matthew Garrett wrote:
>> The DHCP server will return a string in the boot_file field. If you
>> want to indicate that this file should be obtained over http, the
>> easiest way to handle this is to provide a boot file in the form
>> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
>> configuration parameters and there appears to be no way to override
>> that, which makes it impossible to provide a boot file in this form.
>> Allowing the use of an alternative character avoids this problem.
>
> To my understanding, you can (and have to) use the url form in boot-file and
> specify vendor-class-identifier to "HTTPClient" to let the UEFI firmware knows
> this as a response to HTTP boot request and handle that boot-file accordingly,
> then continue with that url loaded and run as NBP.

We're passing the bootfile to grub in order to obtain further
configuration, so the firmware isn't relevant here.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Michael Chang
On Tue, Jan 24, 2017 at 12:50:37PM -0800, Matthew Garrett wrote:
> On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov  wrote:
> > 24.01.2017 03:36, Matthew Garrett пишет:
> >> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
> >> it impossible to pass boot files with commas in them. Allow using a
> >
> > grub_net_open() operates on devices, not files. Please give more details
> > about your problem.
> 
> The DHCP server will return a string in the boot_file field. If you
> want to indicate that this file should be obtained over http, the
> easiest way to handle this is to provide a boot file in the form
> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
> configuration parameters and there appears to be no way to override
> that, which makes it impossible to provide a boot file in this form.
> Allowing the use of an alternative character avoids this problem.

To my understanding, you can (and have to) use the url form in boot-file and
specify vendor-class-identifier to "HTTPClient" to let the UEFI firmware knows
this as a response to HTTP boot request and handle that boot-file accordingly,
then continue with that url loaded and run as NBP.

Here the article provide some examples.

https://en.opensuse.org/UEFI_HTTPBoot_with_OVMF

I suppose you may be running the bootp for setting up the client network
interface and wanted it to have work with http hosting the os images in some
way ? Anyway the config seems not to work with what UEFI prosposed for HTTP
Boot and I'd suggest to check that first.

Thanks,
Michael

> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Matthew Garrett
On Tue, Jan 24, 2017 at 8:15 PM, Andrei Borzenkov  wrote:
> 25.01.2017 07:06, Matthew Garrett пишет:
>> I don't understand - grub_net_open_real() already handles this case:
>
> Because bootfile from DHCP packet is not used to set device part of
> $prefix. Setting bootfile to (http,host)filename will end up with full
> prefix "(tftp,$next_ip)(http,host)base-name-of-filename". If you mean
> something different, please explain.

If prefix isn't set then won't bootfile be interpreted as the device plus file?

>> Because we ship prebuilt images but don't know what IP addresses users
>> will be using for their deployment servers.
>>
>
> I can think about implementing URI parsing (somewhat in line with UEFI
> HTTPboot spec) and/or supporting partial $prefix that sets only
> protocol, something like "grub-mknetdir --prefix=(http,)", where server
> part is filled from DHCP ACK.

We need the ability to pass port as well, so that would still be insufficient.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Andrei Borzenkov
25.01.2017 07:06, Matthew Garrett пишет:
> On Tue, Jan 24, 2017 at 7:48 PM, Andrei Borzenkov  wrote:
>> 24.01.2017 23:50, Matthew Garrett пишет:
>>> On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov  
>>> wrote:
 24.01.2017 03:36, Matthew Garrett пишет:
> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, 
> making
> it impossible to pass boot files with commas in them. Allow using a

 grub_net_open() operates on devices, not files. Please give more details
 about your problem.
>>>
>>> The DHCP server will return a string in the boot_file field. If you
>>> want to indicate that this file should be obtained over http, the
>>> easiest way to handle this is to provide a boot file in the form
>>> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
>>> configuration parameters and there appears to be no way to override
>>> that, which makes it impossible to provide a boot file in this form.
>>> Allowing the use of an alternative character avoids this problem.
>>>
>>
>> This won't work because (http,host) will never be interpreted as device
>> in current code so you need to support this first before this patch can
>> even be considered. Also I am not convinced that arbitrary changing
>> syntax is good idea.
> 
> I don't understand - grub_net_open_real() already handles this case:

Because bootfile from DHCP packet is not used to set device part of
$prefix. Setting bootfile to (http,host)filename will end up with full
prefix "(tftp,$next_ip)(http,host)base-name-of-filename". If you mean
something different, please explain.

> 
>   else
> {
>   const char *comma;
>   comma = grub_strchr (name, ',');
>   if (comma)
> {
>   protnamelen = comma - name;
>   server = comma + 1;
>   protname = name;
> }
>   else
> {
>   protnamelen = grub_strlen (name);
>   server = grub_net_default_server;
>   protname = name;
> }
> 
> 
>> You already can set $prefix when generating image. Why is it not enough?
> 
> Because we ship prebuilt images but don't know what IP addresses users
> will be using for their deployment servers.
> 

I can think about implementing URI parsing (somewhat in line with UEFI
HTTPboot spec) and/or supporting partial $prefix that sets only
protocol, something like "grub-mknetdir --prefix=(http,)", where server
part is filled from DHCP ACK.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Matthew Garrett
On Tue, Jan 24, 2017 at 7:48 PM, Andrei Borzenkov  wrote:
> 24.01.2017 23:50, Matthew Garrett пишет:
>> On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov  
>> wrote:
>>> 24.01.2017 03:36, Matthew Garrett пишет:
 Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
 it impossible to pass boot files with commas in them. Allow using a
>>>
>>> grub_net_open() operates on devices, not files. Please give more details
>>> about your problem.
>>
>> The DHCP server will return a string in the boot_file field. If you
>> want to indicate that this file should be obtained over http, the
>> easiest way to handle this is to provide a boot file in the form
>> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
>> configuration parameters and there appears to be no way to override
>> that, which makes it impossible to provide a boot file in this form.
>> Allowing the use of an alternative character avoids this problem.
>>
>
> This won't work because (http,host) will never be interpreted as device
> in current code so you need to support this first before this patch can
> even be considered. Also I am not convinced that arbitrary changing
> syntax is good idea.

I don't understand - grub_net_open_real() already handles this case:

  else
{
  const char *comma;
  comma = grub_strchr (name, ',');
  if (comma)
{
  protnamelen = comma - name;
  server = comma + 1;
  protname = name;
}
  else
{
  protnamelen = grub_strlen (name);
  server = grub_net_default_server;
  protname = name;
}


> You already can set $prefix when generating image. Why is it not enough?

Because we ship prebuilt images but don't know what IP addresses users
will be using for their deployment servers.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Andrei Borzenkov
24.01.2017 23:50, Matthew Garrett пишет:
> On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov  wrote:
>> 24.01.2017 03:36, Matthew Garrett пишет:
>>> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
>>> it impossible to pass boot files with commas in them. Allow using a
>>
>> grub_net_open() operates on devices, not files. Please give more details
>> about your problem.
> 
> The DHCP server will return a string in the boot_file field. If you
> want to indicate that this file should be obtained over http, the
> easiest way to handle this is to provide a boot file in the form
> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
> configuration parameters and there appears to be no way to override
> that, which makes it impossible to provide a boot file in this form.
> Allowing the use of an alternative character avoids this problem.
> 

This won't work because (http,host) will never be interpreted as device
in current code so you need to support this first before this patch can
even be considered. Also I am not convinced that arbitrary changing
syntax is good idea.

You already can set $prefix when generating image. Why is it not enough?

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-24 Thread Matthew Garrett
On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov  wrote:
> 24.01.2017 03:36, Matthew Garrett пишет:
>> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
>> it impossible to pass boot files with commas in them. Allow using a
>
> grub_net_open() operates on devices, not files. Please give more details
> about your problem.

The DHCP server will return a string in the boot_file field. If you
want to indicate that this file should be obtained over http, the
easiest way to handle this is to provide a boot file in the form
(http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
configuration parameters and there appears to be no way to override
that, which makes it impossible to provide a boot file in this form.
Allowing the use of an alternative character avoids this problem.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

2017-01-23 Thread Andrei Borzenkov
24.01.2017 03:36, Matthew Garrett пишет:
> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
> it impossible to pass boot files with commas in them. Allow using a

grub_net_open() operates on devices, not files. Please give more details
about your problem.

> semicolon to separate the protocol from host if a comma wasn't found.
> ---
>  grub-core/net/net.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index 585f4f7..efacb30 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -1280,6 +1280,10 @@ grub_net_open_real (const char *name)
>const char *comma;
>char *colon;
>comma = grub_strchr (name, ',');
> +  if (!comma)
> + {
> +   comma = grub_strchr (name, ';');
> + }
>colon = grub_strchr (name, ':');
>if (colon)
>   {
> 


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel