11.03.2016 19:28, Josef Bacik пишет:
> We were hitting a problem in production where our bios based machines would
> sometimes timeout while pulling down either the kernel/initrd. It turned out
> that sometimes we'd run out of transmit buffers and would then error out while
> sending packets. This is because we don't actually have an interrupt handler
> in
> PXE, we just poll the card when we want to receive. This works most of the
> time, but sometimes we end up with too many transmit interrupts pending and
> then
> we can't add new packets to the transmit buffers.
>
> So rework the whole ISR logic to be able to be called from both transmit and
> receive. If we get OUT_OF_RESOURCES while trying to transmit then we can go
> through and process the interrupts and hope that leaves us with space to retry
> the transmit. Unfortunately this puts us in a place where we can trip over a
> RECEIVE interrupt, so we have to process that interrupt and leave a netbuff
> behind for the next call into recv.
>
> With this patch we are now able to properly provision boxes suffering from
> this
> problem. Thanks,
>
> Signed-off-by: Josef Bacik <jba...@fb.com>
> ---
> grub-core/net/drivers/i386/pc/pxe.c | 155
> +++++++++++++++++++++++++-----------
> 1 file changed, 108 insertions(+), 47 deletions(-)
>
> diff --git a/grub-core/net/drivers/i386/pc/pxe.c
> b/grub-core/net/drivers/i386/pc/pxe.c
> index 3f4152d..57445b7 100644
> --- a/grub-core/net/drivers/i386/pc/pxe.c
> +++ b/grub-core/net/drivers/i386/pc/pxe.c
> @@ -166,16 +166,11 @@ grub_pxe_scan (void)
> return bangpxe;
> }
>
> -static struct grub_net_buff *
> -grub_pxe_recv (struct grub_net_card *dev __attribute__ ((unused)))
> -{
> - struct grub_pxe_undi_isr *isr;
> - static int in_progress = 0;
> - grub_uint8_t *ptr, *end;
> - struct grub_net_buff *buf;
> -
> - isr = (void *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
> +static int in_progress = 0;
>
You need to reset it in ->open or ->close.
> +static void
> +grub_pxe_process_isr (struct grub_pxe_undi_isr *isr)
> +{
> if (!in_progress)
> {
> grub_memset (isr, 0, sizeof (*isr));
> @@ -186,13 +181,14 @@ grub_pxe_recv (struct grub_net_card *dev __attribute__
> ((unused)))
> breaks on intel cards.
> */
> if (isr->status)
> - {
> - in_progress = 0;
> - return NULL;
> + {
> + grub_dprintf("net", "problem pulling isr %d\n", (int)isr->status);
"pxe" would be better for dedicated debugging, "net" is too generic. We
can also set debug="pxe net" if needed. Also PXE spec lists status
values as hex, would be better to print it this way too.
> + return;
> }
> grub_memset (isr, 0, sizeof (*isr));
> isr->func_flag = GRUB_PXE_ISR_IN_PROCESS;
> grub_pxe_call (GRUB_PXENV_UNDI_ISR, isr, pxe_rm_entry);
> + in_progress = 1;
> }
> else
> {
> @@ -200,18 +196,13 @@ grub_pxe_recv (struct grub_net_card *dev __attribute__
> ((unused)))
> isr->func_flag = GRUB_PXE_ISR_IN_GET_NEXT;
> grub_pxe_call (GRUB_PXENV_UNDI_ISR, isr, pxe_rm_entry);
> }
> +}
>
> - while (isr->func_flag != GRUB_PXE_ISR_OUT_RECEIVE)
> - {
> - if (isr->status || isr->func_flag == GRUB_PXE_ISR_OUT_DONE)
> - {
> - in_progress = 0;
> - return NULL;
> - }
> - grub_memset (isr, 0, sizeof (*isr));
> - isr->func_flag = GRUB_PXE_ISR_IN_GET_NEXT;
> - grub_pxe_call (GRUB_PXENV_UNDI_ISR, isr, pxe_rm_entry);
> - }
> +static struct grub_net_buff *
> +grub_pxe_make_net_buff (struct grub_pxe_undi_isr *isr)
> +{
> + struct grub_net_buff *buf;
> + grub_uint8_t *ptr, *end;
>
> buf = grub_netbuff_alloc (isr->frame_len + 2);
> if (!buf)
> @@ -230,48 +221,118 @@ grub_pxe_recv (struct grub_net_card *dev __attribute__
> ((unused)))
> ptr += isr->buffer_len;
> while (ptr < end)
> {
> - grub_memset (isr, 0, sizeof (*isr));
> - isr->func_flag = GRUB_PXE_ISR_IN_GET_NEXT;
> - grub_pxe_call (GRUB_PXENV_UNDI_ISR, isr, pxe_rm_entry);
> + grub_pxe_process_isr (isr);
> if (isr->status || isr->func_flag != GRUB_PXE_ISR_OUT_RECEIVE)
> {
> - in_progress = 1;
> + grub_dprintf("net", "half processed packet\n");
> grub_netbuff_free (buf);
> return NULL;
> }
> -
> grub_memcpy (ptr, LINEAR (isr->buffer), isr->buffer_len);
> ptr += isr->buffer_len;
> }
> - in_progress = 1;
> -
> return buf;
> }
>
> +static struct grub_net_buff *
> +grub_pxe_recv (struct grub_net_card *dev)
> +{
> + struct grub_pxe_undi_isr *isr;
> +
> + if (dev->data)
> + {
> + struct grub_net_buff *buf = dev->data;
> + grub_dprintf("net", "Pulling existing receive packet\n");
> + dev->data = NULL;
> + return buf;
> + }
> +
> + isr = (void *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
> + grub_pxe_process_isr (isr);
> + while (isr->func_flag != GRUB_PXE_ISR_OUT_RECEIVE)
> + {
> + if (isr->status || isr->func_flag == GRUB_PXE_ISR_OUT_DONE)
> + {
> + if (isr->status)
> + grub_dprintf("net", "error pulling next %d\n", (int)isr->status);
> + in_progress = 0;
> + return 0;
> + }
> + grub_memset (isr, 0, sizeof (*isr));
> + isr->func_flag = GRUB_PXE_ISR_IN_GET_NEXT;
> + grub_pxe_call (GRUB_PXENV_UNDI_ISR, isr, pxe_rm_entry);
> + }
> + return grub_pxe_make_net_buff (isr);
> +}
> +
> static grub_err_t
> -grub_pxe_send (struct grub_net_card *dev __attribute__ ((unused)),
> - struct grub_net_buff *pack)
> +grub_pxe_send (struct grub_net_card *dev, struct grub_net_buff *pack)
> {
> struct grub_pxe_undi_transmit *trans;
> struct grub_pxe_undi_tbd *tbd;
> char *buf;
>
> - trans = (void *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
> - grub_memset (trans, 0, sizeof (*trans));
> - tbd = (void *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR + 128);
> - grub_memset (tbd, 0, sizeof (*tbd));
> - buf = (void *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR + 256);
> - grub_memcpy (buf, pack->data, pack->tail - pack->data);
> -
> - trans->tbd = SEGOFS ((grub_addr_t) tbd);
> - trans->protocol = 0;
> - tbd->len = pack->tail - pack->data;
> - tbd->buf = SEGOFS ((grub_addr_t) buf);
> -
> - grub_pxe_call (GRUB_PXENV_UNDI_TRANSMIT, trans, pxe_rm_entry);
> - if (trans->status)
> - return grub_error (GRUB_ERR_IO, N_("couldn't send network packet"));
> + while (1)
> + {
> + int made_progress = 0;
> +
> + trans = (void *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
> + grub_memset (trans, 0, sizeof (*trans));
> + tbd = (void *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR + 128);
> + grub_memset (tbd, 0, sizeof (*tbd));
> + buf = (void *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR + 256);
> + grub_memcpy (buf, pack->data, pack->tail - pack->data);
> +
> + trans->tbd = SEGOFS ((grub_addr_t) tbd);
> + trans->protocol = 0;
> + tbd->len = pack->tail - pack->data;
> + tbd->buf = SEGOFS ((grub_addr_t) buf);
> +
> + grub_pxe_call (GRUB_PXENV_UNDI_TRANSMIT, trans, pxe_rm_entry);
> + if (!trans->status)
> + break;
This looks like the only terminating condition. What if hardware is
stuck and cannot make progress anymore and continues to return error here?
> + if (trans->status == GRUB_PXENV_STATUS_OUT_OF_RESOURCES)
> + {
> + struct grub_pxe_undi_isr *isr;
> +
> + grub_dprintf("net", "Out of transmit buffers, processing "
> + "interrupts\n");
> + /* Process any outstanding transmit interrupts. */
> + isr = (void *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
> + do
> + {
> + grub_pxe_process_isr (isr);
> + if (isr->status)
> + {
> + grub_dprintf("net", "process interrupts errored %d,"
> + "made_progress %d\n", (int)isr->status,
> made_progress);
> + if (made_progress)
> + break;
> + else
> + goto out_err;
> + }
> + if (isr->func_flag == GRUB_PXE_ISR_OUT_DONE)
> + in_progress = 0;
> + made_progress = 1;
> + } while (isr->func_flag == GRUB_PXE_ISR_OUT_TRANSMIT);
> +
> + /* If we had a receive interrupt in the queue we need to copy this
> + buffer out otherwise we'll lose it. */
> + if (isr->func_flag == GRUB_PXE_ISR_OUT_RECEIVE)
It probably should check isr->status, we come here also if we got an
error previously. Such packets are ignored everywhere else.
> + {
> + if (dev->data)
> + grub_dprintf("net", "dropping packet, already have a "
> + "pending packet.\n");
> + else
> + dev->data = grub_pxe_make_net_buff (isr);
> + }
> + }
> + else
> + goto out_err;
> + }
> return 0;
> +out_err:
> + return grub_error (GRUB_ERR_IO, N_("couldn't send network packet"));
> }
>
> static void
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
