Bug report: PXE blocksize

Bug report: PXE blocksize

[Turner, Ian]

Hello list,

There seem to be a couple of problems with this variable:

1. The variable name in the code is pxe_blksize, whereas the info file references net_pxe_blksize.
2. Changing the blocksize from within a config file which is itself sourced from TFTP breaks the transfer of the remainder of that file, because grub_pxe_read() references the grub_pxe_blksize variable at each packet.

Issue #2 can be resolved just by changing the grub_pxe_read() reference to grub_pxe_blksize to reference data->block_size instead.

This is in the newreloc branch.

Cheers,

--Ian

Re: Bug report: PXE blocksize

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 08/27/2010 05:57 PM, Turner, Ian wrote:
> Hello list,
>
> There seem to be a couple of problems with this variable:
>
> 1. The variable name in the code is pxe_blksize, whereas the info file references net_pxe_blksize.
>   
Fixed. Thanks
> 2. Changing the blocksize from within a config file which is itself sourced from TFTP breaks the transfer of the remainder of that file, because grub_pxe_read() references the grub_pxe_blksize variable at each packet.
>   
> Issue #2 can be resolved just by changing the grub_pxe_read() reference to grub_pxe_blksize to reference data->block_size instead.
>   
I don't see where you mean. The only reference to pxe_blksize I see in
pxefs_read is following:
      o.packet_size = grub_pxe_blksize;
      grub_pxe_call (GRUB_PXENV_TFTP_OPEN, &o);
Which is called only when reopening file. Could you retry pulling? Now
newreloc is merged into mainline so you can just pull mainline.
> This is in the newreloc branch.
>
> Cheers,
>
> --Ian
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

RE: Bug report: PXE blocksize

[Turner, Ian]

> I don't see where you mean. The only reference to pxe_blksize I see in
> pxefs_read is following:
>       o.packet_size = grub_pxe_blksize;
>       grub_pxe_call (GRUB_PXENV_TFTP_OPEN, &o);
> Which is called only when reopening file. Could you retry pulling? Now
> newreloc is merged into mainline so you can just pull mainline.

You're right. The problem is more complex that I thought originally. I think it's something more like this:

1. Start loading a file 'grub.cfg' with blksize=512
2. File contains a directive to change blocksize to something else (e.g., 1024). This updates grub_pxe_blksize but doesn't change the parameters of the grub.cfg session.
3. File also contains a directive that causes grub to load some other file (say, an insmod or source command). This causes pxe.c to close grub.cfg for now, but the grub_pxe_data object sticks around.
4. Upon resuming read of grub.cfg, pxe opens with the new blocksize but reads with the old one.

I still think the solution is what I proposed originally (patch follows), which maintains the same PXE blocksize for a given file until it is finally closed by the requestor, no matter how many times PXE itself needs to open/close the TFTP session.

--- pxe-broken.c        2010-08-30 10:52:03.717580213 -0400
+++ pxe-fixed.c 2010-08-30 10:52:49.370957060 -0400
@@ -281,7 +281,7 @@
       o.gateway_ip = disk_data->gateway_ip;
       grub_strcpy ((char *)&o.filename[0], data->filename);
       o.tftp_port = grub_cpu_to_be16 (GRUB_PXE_TFTP_PORT);
-      o.packet_size = grub_pxe_blksize;
+      o.packet_size = data->block_size;
       grub_pxe_call (GRUB_PXENV_TFTP_OPEN, &o);
       if (o.status)
        {

Cheers,

--Ian

Re: Bug report: PXE blocksize

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 08/30/2010 04:52 PM, Turner, Ian wrote:
>> I don't see where you mean. The only reference to pxe_blksize I see in
>> pxefs_read is following:
>>       o.packet_size = grub_pxe_blksize;
>>       grub_pxe_call (GRUB_PXENV_TFTP_OPEN, &o);
>> Which is called only when reopening file. Could you retry pulling? Now
>> newreloc is merged into mainline so you can just pull mainline.
>>     
> You're right. The problem is more complex that I thought originally. I think it's something more like this:
>
> 1. Start loading a file 'grub.cfg' with blksize=512
> 2. File contains a directive to change blocksize to something else (e.g., 1024). This updates grub_pxe_blksize but doesn't change the parameters of the grub.cfg session.
> 3. File also contains a directive that causes grub to load some other file (say, an insmod or source command). This causes pxe.c to close grub.cfg for now, but the grub_pxe_data object sticks around.
> 4. Upon resuming read of grub.cfg, pxe opens with the new blocksize but reads with the old one.
>
> I still think the solution is what I proposed originally (patch follows), which maintains the same PXE blocksize for a given file until it is finally closed by the requestor, no matter how many times PXE itself needs to open/close the TFTP session.
>   
There are 2 problems with changing blocksize in the middle of the stream:
1) pn isn't recomputed. Easy to fix
2) bufio doesn't change the size. Difficult and ugly to fix.
So I applied your patch. Thanks
> --- pxe-broken.c        2010-08-30 10:52:03.717580213 -0400
> +++ pxe-fixed.c 2010-08-30 10:52:49.370957060 -0400
> @@ -281,7 +281,7 @@
>        o.gateway_ip = disk_data->gateway_ip;
>        grub_strcpy ((char *)&o.filename[0], data->filename);
>        o.tftp_port = grub_cpu_to_be16 (GRUB_PXE_TFTP_PORT);
> -      o.packet_size = grub_pxe_blksize;
> +      o.packet_size = data->block_size;
>        grub_pxe_call (GRUB_PXENV_TFTP_OPEN, &o);
>        if (o.status)
>         {
>
> Cheers,
>
> --Ian
>
>   
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]