Re: [PATCH] bug fix for efi network
On Thu, May 3, 2012 at 11:38 PM, Vladimir 'φ-coder/phcoder' Serbinenko
wrote:
> On 29.04.2012 17:05, Bean wrote:
>> + for (i = 0; i < 3; i++)
>> {
>> + grub_uint64_t limit_time;
>> +
>> + efi_call_3 (net->get_status, net, &int_status, 0);
>> +
>> + limit_time = grub_get_time_ms () + 5;
>> + for (;;)
>> + {
>> + st = efi_call_7 (net->transmit, net, 0, (pack->tail - pack->data),
>> + pack->data, NULL, NULL, NULL);
>> + if (st != GRUB_EFI_NOT_READY)
>> + break;
>> +
>> + if (limit_time < grub_get_time_ms ())
>> + {
>> + st = GRUB_EFI_TIMEOUT;
>> + break;
>> + }
>> + }
>> +
>> + if (st)
>> + goto quit;
>> +
>> void *txbuf = NULL;
>> - st = efi_call_3 (net->get_status, net, 0, &txbuf);
>> - if (st != GRUB_EFI_SUCCESS)
>> - return grub_error (GRUB_ERR_IO, N_("couldn't send network packet"));
>> - if (txbuf)
>> - return GRUB_ERR_NONE;
>> - if (limit_time < grub_get_time_ms ())
>> - return grub_error (GRUB_ERR_TIMEOUT, N_("couldn't send network
>> packet"));
>> + limit_time = grub_get_time_ms () + 5;
>> + for (;;)
>> + {
>> + st = efi_call_3 (net->get_status, net, &int_status, &txbuf);
>> +
>> + if (txbuf != NULL)
>> + break;
>> +
>> + if (limit_time < grub_get_time_ms ())
>> + {
>> + st = GRUB_EFI_TIMEOUT;
>> + break;
>> + }
>> + }
>> +
>> + quit:
>> + if (st != GRUB_EFI_TIMEOUT)
>> + break;
>> }
>> +
>> + return st;
>> }
> I see no justification as to why this part of patch so I can't know if
> it's something which is we absolutely need to commit or something that
> can wait.
Hi,
Yeah, this is not necessary, it's my custom code for the tftp service,
I just want to check if it makes a difference. In fact, the problem is
in the network code itself, not the low level driver.
--
Best wishes
Bean
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] bug fix for efi network
On 29.04.2012 17:05, Bean wrote:
> + for (i = 0; i < 3; i++)
> {
> + grub_uint64_t limit_time;
> +
> + efi_call_3 (net->get_status, net, &int_status, 0);
> +
> + limit_time = grub_get_time_ms () + 5;
> + for (;;)
> + {
> + st = efi_call_7 (net->transmit, net, 0, (pack->tail - pack->data),
> +pack->data, NULL, NULL, NULL);
> + if (st != GRUB_EFI_NOT_READY)
> + break;
> +
> + if (limit_time < grub_get_time_ms ())
> + {
> + st = GRUB_EFI_TIMEOUT;
> + break;
> + }
> + }
> +
> + if (st)
> + goto quit;
> +
>void *txbuf = NULL;
> - st = efi_call_3 (net->get_status, net, 0, &txbuf);
> - if (st != GRUB_EFI_SUCCESS)
> - return grub_error (GRUB_ERR_IO, N_("couldn't send network packet"));
> - if (txbuf)
> - return GRUB_ERR_NONE;
> - if (limit_time < grub_get_time_ms ())
> - return grub_error (GRUB_ERR_TIMEOUT, N_("couldn't send network
> packet"));
> + limit_time = grub_get_time_ms () + 5;
> + for (;;)
> + {
> + st = efi_call_3 (net->get_status, net, &int_status, &txbuf);
> +
> + if (txbuf != NULL)
> + break;
> +
> + if (limit_time < grub_get_time_ms ())
> + {
> + st = GRUB_EFI_TIMEOUT;
> + break;
> + }
> + }
> +
> +quit:
> + if (st != GRUB_EFI_TIMEOUT)
> + break;
> }
> +
> + return st;
> }
I see no justification as to why this part of patch so I can't know if
it's something which is we absolutely need to commit or something that
can wait.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] bug fix for efi network
On 29.04.2012 20:19, Bean wrote: > 2012/4/30 Vladimir 'φ-coder/phcoder' Serbinenko : >> On 29.04.2012 10:22, Bean wrote: >>> Hi, >>> >>> This patch fix a few bugs in efinet. >>> >>> It also change the tftp block size from 1024 to 8192, which would >>> result in HUGE speed difference. In my previous testing, the larger >>> the block size, the faster the speed. It can reach up to 60-70MB/s in >>> 1Gb ethernet when block size is about 60K, therefore it would be a >>> good idea to allow user to configure this parameter. The native tftp >>> service in UEFI use a default block size of 8192. >> 8192 is bigger than the MTU on most cards so it results in fragmentation >> or it emits jumbo packets. It's unclear how both network and server >> would react to this. >> Moreover the optimal packet size is in any case: >> k * (mtu - ip_header_size - udp_header_size - tftp_header_size) >> where k is some number. Only these number result in fully sized packets. >> k=1 result in best available option for non-fragmented packets. k > 1 >> can create some preload by abusing fragmentation. >> Note that some servers do preload by other means and k > 1 with such >> servers would result in slowdown. > Hi, > > I believe server would split the packets into multiple fragment, each > within mtu to pass through network, and client assemble them to get > the original packet. Large block would reduce the number of > handshakes, and since nic has buffer to cache multiple packets, it > allows network driver and nic card to work in parallel and thus hide > some of the network traffic. But the fragmentation/defragmentation is also something that routers do. While I'm pretty sure none of them crashes on such packets since the ping of death, some of them may mishandle such packets or reject them as an "attack attempt". While this is an interesting possibility to try out, we need more data as to its functioning in real-world scenarios. We would also need a way to reestablish connection with a small block if we notice that the packets aren't comming through or are corrupted. > Perhaps users with real UEFI implementation can try out different > block size setting and reports the result. > -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] bug fix for efi network
2012/4/30 Vladimir 'φ-coder/phcoder' Serbinenko : > On 29.04.2012 10:22, Bean wrote: >> Hi, >> >> This patch fix a few bugs in efinet. >> >> It also change the tftp block size from 1024 to 8192, which would >> result in HUGE speed difference. In my previous testing, the larger >> the block size, the faster the speed. It can reach up to 60-70MB/s in >> 1Gb ethernet when block size is about 60K, therefore it would be a >> good idea to allow user to configure this parameter. The native tftp >> service in UEFI use a default block size of 8192. > 8192 is bigger than the MTU on most cards so it results in fragmentation > or it emits jumbo packets. It's unclear how both network and server > would react to this. > Moreover the optimal packet size is in any case: > k * (mtu - ip_header_size - udp_header_size - tftp_header_size) > where k is some number. Only these number result in fully sized packets. > k=1 result in best available option for non-fragmented packets. k > 1 > can create some preload by abusing fragmentation. > Note that some servers do preload by other means and k > 1 with such > servers would result in slowdown. Hi, I believe server would split the packets into multiple fragment, each within mtu to pass through network, and client assemble them to get the original packet. Large block would reduce the number of handshakes, and since nic has buffer to cache multiple packets, it allows network driver and nic card to work in parallel and thus hide some of the network traffic. Perhaps users with real UEFI implementation can try out different block size setting and reports the result. -- Best wishes Bean ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] bug fix for efi network
On 29.04.2012 10:22, Bean wrote: > Hi, > > This patch fix a few bugs in efinet. > > It also change the tftp block size from 1024 to 8192, which would > result in HUGE speed difference. In my previous testing, the larger > the block size, the faster the speed. It can reach up to 60-70MB/s in > 1Gb ethernet when block size is about 60K, therefore it would be a > good idea to allow user to configure this parameter. The native tftp > service in UEFI use a default block size of 8192. 8192 is bigger than the MTU on most cards so it results in fragmentation or it emits jumbo packets. It's unclear how both network and server would react to this. Moreover the optimal packet size is in any case: k * (mtu - ip_header_size - udp_header_size - tftp_header_size) where k is some number. Only these number result in fully sized packets. k=1 result in best available option for non-fragmented packets. k > 1 can create some preload by abusing fragmentation. Note that some servers do preload by other means and k > 1 with such servers would result in slowdown. > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] bug fix for efi network
Hi,
Update: I also rewrite the send_card_buffer function. Also add support
for tftp_block_size environment variable which allows user to specify
customized block size (default is 8192).
for example:
set tftp_block_size=16384
testspeed (tftp)/imagefile
When testing in virtual machine, try not to set block size too high
since it could cause timeouts, this is not an issue for real machine
since it normally have more nic buffer.
--
Best wishes
Bean
=== modified file 'grub-core/net/drivers/efi/efinet.c'
--- grub-core/net/drivers/efi/efinet.c 2012-03-10 19:41:28 +
+++ grub-core/net/drivers/efi/efinet.c 2012-04-29 14:44:08 +
@@ -36,22 +36,55 @@
{
grub_efi_status_t st;
grub_efi_simple_network_t *net = dev->efi_net;
- grub_uint64_t limit_time = grub_get_time_ms () + 4000;
- st = efi_call_7 (net->transmit, net, 0, (pack->tail - pack->data),
- pack->data, NULL, NULL, NULL);
- if (st != GRUB_EFI_SUCCESS)
-return grub_error (GRUB_ERR_IO, N_("couldn't send network packet"));
- while (1)
+ grub_uint32_t int_status;
+ int i;
+
+ for (i = 0; i < 3; i++)
{
+ grub_uint64_t limit_time;
+
+ efi_call_3 (net->get_status, net, &int_status, 0);
+
+ limit_time = grub_get_time_ms () + 5;
+ for (;;)
+ {
+ st = efi_call_7 (net->transmit, net, 0, (pack->tail - pack->data),
+ pack->data, NULL, NULL, NULL);
+ if (st != GRUB_EFI_NOT_READY)
+ break;
+
+ if (limit_time < grub_get_time_ms ())
+ {
+ st = GRUB_EFI_TIMEOUT;
+ break;
+ }
+ }
+
+ if (st)
+ goto quit;
+
void *txbuf = NULL;
- st = efi_call_3 (net->get_status, net, 0, &txbuf);
- if (st != GRUB_EFI_SUCCESS)
- return grub_error (GRUB_ERR_IO, N_("couldn't send network packet"));
- if (txbuf)
- return GRUB_ERR_NONE;
- if (limit_time < grub_get_time_ms ())
- return grub_error (GRUB_ERR_TIMEOUT, N_("couldn't send network
packet"));
+ limit_time = grub_get_time_ms () + 5;
+ for (;;)
+ {
+ st = efi_call_3 (net->get_status, net, &int_status, &txbuf);
+
+ if (txbuf != NULL)
+ break;
+
+ if (limit_time < grub_get_time_ms ())
+ {
+ st = GRUB_EFI_TIMEOUT;
+ break;
+ }
+ }
+
+quit:
+ if (st != GRUB_EFI_TIMEOUT)
+ break;
}
+
+ return st;
}
static struct grub_net_buff *
@@ -63,14 +96,13 @@
grub_efi_uintn_t bufsize = 1536;
struct grub_net_buff *nb;
- nb = grub_netbuff_alloc (bufsize);
+ nb = grub_netbuff_alloc (bufsize + 2);
if (!nb)
return NULL;
/* Reserve 2 bytes so that 2 + 14/18 bytes of ethernet header is divisible
by 4. So that IP header is aligned on 4 bytes. */
- grub_netbuff_reserve (nb, 2);
- if (!nb)
+ if (grub_netbuff_reserve (nb, 2))
{
grub_netbuff_free (nb);
return NULL;
@@ -84,14 +116,13 @@
bufsize = ALIGN_UP (bufsize, 32);
- nb = grub_netbuff_alloc (bufsize);
+ nb = grub_netbuff_alloc (bufsize + 2);
if (!nb)
return NULL;
/* Reserve 2 bytes so that 2 + 14/18 bytes of ethernet header is
divisible
by 4. So that IP header is aligned on 4 bytes. */
- grub_netbuff_reserve (nb, 2);
- if (!nb)
+ if (grub_netbuff_reserve (nb, 2))
{
grub_netbuff_free (nb);
return NULL;
=== modified file 'grub-core/net/tftp.c'
--- grub-core/net/tftp.c2012-02-12 18:11:06 +
+++ grub-core/net/tftp.c2012-04-29 14:31:12 +
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
GRUB_MOD_LICENSE ("GPLv3+");
@@ -288,6 +289,7 @@
grub_err_t err;
grub_uint8_t *nbd;
grub_net_network_level_address_t addr;
+ const char *block_size;
data = grub_zalloc (sizeof (*data));
if (!data)
@@ -320,9 +322,12 @@
rrqlen += grub_strlen ("blksize") + 1;
rrq += grub_strlen ("blksize") + 1;
- grub_strcpy (rrq, "1024");
- rrqlen += grub_strlen ("1024") + 1;
- rrq += grub_strlen ("1024") + 1;
+ block_size = grub_env_get ("tftp_block_size");
+ if (block_size == NULL)
+block_size = "8192";
+ grub_strcpy (rrq, block_size);
+ rrqlen += grub_strlen (block_size) + 1;
+ rrq += grub_strlen (block_size) + 1;
grub_strcpy (rrq, "tsize");
rrqlen += grub_strlen ("tsize") + 1;
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] bug fix for efi network
Hi,
Sorry, the previous patch missed a similar bug.
On Sun, Apr 29, 2012 at 4:22 PM, Bean wrote:
> Hi,
>
> This patch fix a few bugs in efinet.
>
> It also change the tftp block size from 1024 to 8192, which would
> result in HUGE speed difference. In my previous testing, the larger
> the block size, the faster the speed. It can reach up to 60-70MB/s in
> 1Gb ethernet when block size is about 60K, therefore it would be a
> good idea to allow user to configure this parameter. The native tftp
> service in UEFI use a default block size of 8192.
>
> --
> Best wishes
> Bean
--
Best wishes
Bean
=== modified file 'grub-core/net/drivers/efi/efinet.c'
--- grub-core/net/drivers/efi/efinet.c 2012-03-10 19:41:28 +
+++ grub-core/net/drivers/efi/efinet.c 2012-04-29 08:23:58 +
@@ -63,14 +63,13 @@
grub_efi_uintn_t bufsize = 1536;
struct grub_net_buff *nb;
- nb = grub_netbuff_alloc (bufsize);
+ nb = grub_netbuff_alloc (bufsize + 2);
if (!nb)
return NULL;
/* Reserve 2 bytes so that 2 + 14/18 bytes of ethernet header is divisible
by 4. So that IP header is aligned on 4 bytes. */
- grub_netbuff_reserve (nb, 2);
- if (!nb)
+ if (grub_netbuff_reserve (nb, 2))
{
grub_netbuff_free (nb);
return NULL;
@@ -84,14 +83,13 @@
bufsize = ALIGN_UP (bufsize, 32);
- nb = grub_netbuff_alloc (bufsize);
+ nb = grub_netbuff_alloc (bufsize + 2);
if (!nb)
return NULL;
/* Reserve 2 bytes so that 2 + 14/18 bytes of ethernet header is
divisible
by 4. So that IP header is aligned on 4 bytes. */
- grub_netbuff_reserve (nb, 2);
- if (!nb)
+ if (grub_netbuff_reserve (nb, 2))
{
grub_netbuff_free (nb);
return NULL;
=== modified file 'grub-core/net/tftp.c'
--- grub-core/net/tftp.c2012-02-12 18:11:06 +
+++ grub-core/net/tftp.c2012-04-29 08:10:29 +
@@ -320,9 +320,9 @@
rrqlen += grub_strlen ("blksize") + 1;
rrq += grub_strlen ("blksize") + 1;
- grub_strcpy (rrq, "1024");
- rrqlen += grub_strlen ("1024") + 1;
- rrq += grub_strlen ("1024") + 1;
+ grub_strcpy (rrq, "8192");
+ rrqlen += grub_strlen ("8192") + 1;
+ rrq += grub_strlen ("8192") + 1;
grub_strcpy (rrq, "tsize");
rrqlen += grub_strlen ("tsize") + 1;
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
