Re: [PATCH 0/3] Net fix and improvements

[Daniel Kiper]

Hey,

On Tue, Mar 08, 2022 at 03:20:17PM -0600, Glenn Washburn wrote:
> The first patch looks like it was a copy/paste error. If the net module is
> unloaded, grub_net_poll_cards_idle should be NULL so that a function in the
> net module which now doesn't exist.
>
> The second and third patches are for performance and were helpful when
> debugging GRUB. When the net module is loaded, even if there are no net
> cards found, grub_net_poll_cards_idle will call grub_net_tcp_retransmit()
> unconditionally. But there's no need to go through tcp retransmit logic if
> there aren't any cards that we could be retransmitting on.
>
> As for the third patch, if the machine has network cards found, but there are
> no tcp open sockets (because the user doesn't use the network to boot), then
> grub_net_tcp_retransmit() should be a noop. Thus is doesn't need to call
> grub_get_time_ms(), which does a call into firmware on powerpc-ieee1275. So
> only call grub_get_time_ms() if there are tcp sockets.
>
> Calls to grub_net_poll_cards_idle can happen a lot when GRUB is waiting for a
> keypress (grub_getkey_noblock calls grub_net_poll_cards_idle). I found this
> annoying when debugging an issue in GRUB with GDB and I found that nearly
> every time I interrupted GRUB with the GDB I was landing in OpenBIOS's
> milliseconds call and then I had to step out back into GRUB which could be
> a hundred or more instructions. This reduces the probability that interrupting
> GRUB lands in the firmware when GRUB is blocked waiting on a keypress.

In general I think this cover letter should be split into three parts and they
should go into relevant patches.

Additionally, if you check pointers for NULL please use "== NULL"
comparison explicitly instead of shortened version like "if (pointer)".

Otherwise patches LGTM.

Daniel