[PATCH 0/3] Net fix and improvements

[PATCH 0/3] Net fix and improvements

[Glenn Washburn]

v2 updates:
 * Use == NULL in conditionals
 * Update commit messages to have more context from previous cover letter

Glenn

Glenn Washburn (3):
  net: Unset grub_net_poll_cards_idle when net module has been unloaded
  net: Avoid unnecessary calls to grub_net_tcp_retransmit
  net/tcp: Only call grub_get_time_ms when there are sockets to
    potentially retransmit for

 grub-core/net/net.c | 5 +++--
 grub-core/net/tcp.c | 9 +++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

Range-diff:
1:  324f068453 ! 1:  76bbc12ed1 net: Unset grub_net_poll_cards_idle when net module has been unloaded
    @@ Metadata
      ## Commit message ##
         net: Unset grub_net_poll_cards_idle when net module has been unloaded
     
    +    This looks like it was a copy/paste error. If the net module is unloaded,
    +    grub_net_poll_cards_idle should be NULL so that GRUB does not try to call a
    +    function which now doesn't exist.
    +
      ## grub-core/net/net.c ##
     @@ grub-core/net/net.c: GRUB_MOD_FINI(net)
        grub_net_open = NULL;
2:  2e55f210c7 ! 2:  4efca58dab net: Avoid unnecessary calls to grub_net_tcp_retransmit
    @@ Commit message
     
         In grub_net_poll_cards_idle_real, only call grub_net_tcp_retransmit if there
         are network cards found. If there are no network card found, there can be no
    -    tcp sockets to transmit on.
    +    tcp sockets to transmit on. So no need to go through that logic.
     
      ## grub-core/net/net.c ##
     @@ grub-core/net/net.c: grub_net_poll_cards_idle_real (void)
    @@ grub-core/net/net.c: grub_net_poll_cards_idle_real (void)
            receive_packets (card, 0);
        }
     -  grub_net_tcp_retransmit ();
    -+  if (grub_net_cards)
    ++  if (grub_net_cards == NULL)
     +    grub_net_tcp_retransmit ();
      }
      
3:  7bef6e122f ! 3:  a6328b14cd net/tcp: Only call grub_get_time_ms when there are sockets to potentially retransmit for
    @@ Metadata
      ## Commit message ##
         net/tcp: Only call grub_get_time_ms when there are sockets to potentially retransmit for
     
    -    If there are no TCP sockets, this call to grub_get_time_ms is unneeded. This
    -    prevents a call into the firmware on some platforms.
    +    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 GRUB doesn't need to call
    +    grub_get_time_ms(), which does a call into firmware on powerpc-ieee1275,
    +    and probably other targets. So only call grub_get_time_ms() if there are tcp
    +    sockets.
    +
    +    Aside from improving performace, its also useful to stay out of the firmware
    +    as much as possible when debugging via QEMU because its a pain to get back
    +    in to GRUB execution. grub_net_tcp_retransmit() can get called very
    +    frequently via grub_net_poll_cards_idle() when GRUB is waiting for a
    +    keypress (grub_getkey_noblock() calls grub_net_poll_cards_idle()). This can
    +    be annoying when debugging an issue in GRUB on powerpc in QEMU with GDB when
    +    GRUB is waiting for a keypress because interrupting via GDB nearly always
    +    lands in the OpenBIOS firmware's milliseconds call.
     
      ## grub-core/net/tcp.c ##
     @@ grub-core/net/tcp.c: void
    @@ grub-core/net/tcp.c: void
     -  grub_uint64_t limit_time = ctime - TCP_RETRANSMISSION_TIMEOUT;
     +  grub_uint64_t ctime = 0, limit_time = 0;
     +
    -+  if (tcp_sockets)
    ++  if (tcp_sockets == NULL)
     +    {
     +      ctime = grub_get_time_ms ();
     +      limit_time = ctime - TCP_RETRANSMISSION_TIMEOUT;
-- 
2.27.0

[PATCH 1/3] net: Unset grub_net_poll_cards_idle when net module has been unloaded

[Glenn Washburn]

This looks like it was a copy/paste error. If the net module is unloaded,
grub_net_poll_cards_idle should be NULL so that GRUB does not try to call a
function which now doesn't exist.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/net/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 1b0e346014..3eac83d165 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1948,5 +1948,5 @@ GRUB_MOD_FINI(net)
   grub_net_open = NULL;
   grub_net_fini_hw (0);
   grub_loader_unregister_preboot_hook (fini_hnd);
-  grub_net_poll_cards_idle = grub_net_poll_cards_idle_real;
+  grub_net_poll_cards_idle = NULL;
 }
-- 
2.27.0

[PATCH 2/3] net: Avoid unnecessary calls to grub_net_tcp_retransmit

[Glenn Washburn]

In grub_net_poll_cards_idle_real, only call grub_net_tcp_retransmit if there
are network cards found. If there are no network card found, there can be no
tcp sockets to transmit on. So no need to go through that logic.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/net/net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 3eac83d165..11b39877b9 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1580,7 +1580,8 @@ grub_net_poll_cards_idle_real (void)
 	|| ctime >= card->last_poll + card->idle_poll_delay_ms)
       receive_packets (card, 0);
   }
-  grub_net_tcp_retransmit ();
+  if (grub_net_cards == NULL)
+    grub_net_tcp_retransmit ();
 }
 
 /*  Read from the packets list*/
-- 
2.27.0

[PATCH 3/3] net/tcp: Only call grub_get_time_ms when there are sockets to potentially retransmit for

[Glenn Washburn]

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 GRUB doesn't need to call
grub_get_time_ms(), which does a call into firmware on powerpc-ieee1275,
and probably other targets. So only call grub_get_time_ms() if there are tcp
sockets.

Aside from improving performace, its also useful to stay out of the firmware
as much as possible when debugging via QEMU because its a pain to get back
in to GRUB execution. grub_net_tcp_retransmit() can get called very
frequently via grub_net_poll_cards_idle() when GRUB is waiting for a
keypress (grub_getkey_noblock() calls grub_net_poll_cards_idle()). This can
be annoying when debugging an issue in GRUB on powerpc in QEMU with GDB when
GRUB is waiting for a keypress because interrupting via GDB nearly always
lands in the OpenBIOS firmware's milliseconds call.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/net/tcp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/grub-core/net/tcp.c b/grub-core/net/tcp.c
index 2004601212..48930fdfef 100644
--- a/grub-core/net/tcp.c
+++ b/grub-core/net/tcp.c
@@ -362,8 +362,13 @@ void
 grub_net_tcp_retransmit (void)
 {
   grub_net_tcp_socket_t sock;
-  grub_uint64_t ctime = grub_get_time_ms ();
-  grub_uint64_t limit_time = ctime - TCP_RETRANSMISSION_TIMEOUT;
+  grub_uint64_t ctime = 0, limit_time = 0;
+
+  if (tcp_sockets == NULL)
+    {
+      ctime = grub_get_time_ms ();
+      limit_time = ctime - TCP_RETRANSMISSION_TIMEOUT;
+    }
 
   FOR_TCP_SOCKETS (sock)
   {
-- 
2.27.0

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

[Daniel Kiper]

On Fri, Mar 18, 2022 at 01:51:30AM -0500, Glenn Washburn wrote:
> v2 updates:
>  * Use == NULL in conditionals
>  * Update commit messages to have more context from previous cover letter
>
> Glenn
>
> Glenn Washburn (3):
>   net: Unset grub_net_poll_cards_idle when net module has been unloaded
>   net: Avoid unnecessary calls to grub_net_tcp_retransmit
>   net/tcp: Only call grub_get_time_ms when there are sockets to
>     potentially retransmit for

For all patches Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Daniel

[PATCH 1/3] net: Unset grub_net_poll_cards_idle when net module has been unloaded

[Glenn Washburn]

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/net/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 4d3eb5c1a5..563dea9ec8 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1948,5 +1948,5 @@ GRUB_MOD_FINI(net)
   grub_net_open = NULL;
   grub_net_fini_hw (0);
   grub_loader_unregister_preboot_hook (fini_hnd);
-  grub_net_poll_cards_idle = grub_net_poll_cards_idle_real;
+  grub_net_poll_cards_idle = NULL;
 }
-- 
2.27.0