Re: [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect

[Jason Wang]

On 2020/4/24 上午7:16, Philippe Mathieu-Daudé wrote:
When a frame lenght is incorrect, set the RDES0 'Error Summary'
and 'Frame too long' bits. Then stop the receive process and
trigger an abnormal interrupt. See [4.3.5 Receive Process].

Cc: Li Qiang <liq...@gmail.com>
Cc: Li Qiang <pangpei...@antfin.com>
Cc: Ziming Zhang <ezrak...@gmail.com>
Cc: Jason Wang <jasow...@redhat.com>
Cc: Prasad J Pandit <p...@fedoraproject.org>
Fixes: 8ffb7265af ("check frame size and r/w data length")
Buglink: https://bugs.launchpad.net/bugs/1874539
Reported-by: Helge Deller <del...@gmx.de>
Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>


Hi Philippe:

It's still unclear to me that how this fixes the stuck. Did you mean 
guest trigger the error condition and then recvoer by abnormal interrupt?

If yes, this sounds still like a bug somewhere in the code.

Thanks


---
  hw/net/tulip.c | 16 ++++++++++++----
  1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 470f635acb..671f79b6f4 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -158,7 +158,7 @@ static void tulip_next_rx_descriptor(TULIPState *s,
      s->current_rx_desc &= ~3ULL;
  }
  
-static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
+static int tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
  {
      int len1 = (desc->control >> RDES1_BUF1_SIZE_SHIFT) & 
RDES1_BUF1_SIZE_MASK;
      int len2 = (desc->control >> RDES1_BUF2_SIZE_SHIFT) & 
RDES1_BUF2_SIZE_MASK;
@@ -177,7 +177,8 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct 
tulip_descriptor *desc)
                            "(ofs: %u, len:%d, size:%zu)\n",
                            __func__, s->rx_frame_len, len,
                            sizeof(s->rx_frame));
-            return;
+            s->rx_frame_len = 0;
+            return -1;
          }
          pci_dma_write(&s->dev, desc->buf_addr1, s->rx_frame +
              (s->rx_frame_size - s->rx_frame_len), len);
@@ -197,12 +198,15 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct 
tulip_descriptor *desc)
                            "(ofs: %u, len:%d, size:%zu)\n",
                            __func__, s->rx_frame_len, len,
                            sizeof(s->rx_frame));
-            return;
+            s->rx_frame_len = 0;
+            return -1;
          }
          pci_dma_write(&s->dev, desc->buf_addr2, s->rx_frame +
              (s->rx_frame_size - s->rx_frame_len), len);
          s->rx_frame_len -= len;
      }
+
+    return 0;
  }
  
  static bool tulip_filter_address(TULIPState *s, const uint8_t *addr)
@@ -274,7 +278,11 @@ static ssize_t tulip_receive(TULIPState *s, const uint8_t 
*buf, size_t size)
              s->rx_frame_len = s->rx_frame_size;
          }
  
-        tulip_copy_rx_bytes(s, &desc);
+        if (tulip_copy_rx_bytes(s, &desc)) {
+            desc.status |= RDES0_ES | RDES0_TL; /* Error: frame too long */
+            s->csr[5] |= CSR5_RPS; /* Receive process stopped */
+            tulip_update_int(s);
+        }
  
          if (!s->rx_frame_len) {
              desc.status |= s->rx_status;