After the bug fix in the previous commit, the length and prev_buf_size
variables are identical, except that prev_buf_size is uint32_t and
length is uint16_t. We can therefore unify them. The only place where
the type makes a difference is that we will truncate the packet
at 64K when sending it; this commit preserves that behaviour
by using a local variable when doing the packet send.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Should the behaviour really be to truncate the packet at 64K
rather than flagging an error if the guest assembles descriptors
that combine to give a too-large packet?
---
 hw/net/npcm_gmac.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index 921327dd8ca..a0050a7725f 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -516,7 +516,6 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
     uint32_t desc_addr;
     struct NPCMGMACTxDesc tx_desc;
     uint32_t tx_buf_addr, tx_buf_len;
-    uint16_t length = 0;
     uint8_t *buf = tx_send_buffer;
     uint32_t prev_buf_size = 0;
     int csum = 0;
@@ -583,7 +582,6 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
                         __func__, tx_buf_addr);
             return;
         }
-        length += tx_buf_len;
         prev_buf_size += tx_buf_len;
 
         /* If not chained we'll have a second buffer. */
@@ -606,15 +604,18 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
                               __func__, tx_buf_addr);
                 return;
             }
-            length += tx_buf_len;
             prev_buf_size += tx_buf_len;
         }
         if (tx_desc.tdes1 & TX_DESC_TDES1_LAST_SEG_MASK) {
+            /*
+             * This will truncate the packet at 64K.
+             * TODO: find out if this is the correct behaviour.
+             */
+            uint16_t length = prev_buf_size;
             net_checksum_calculate(tx_send_buffer, length, csum);
             qemu_send_packet(qemu_get_queue(gmac->nic), tx_send_buffer, length);
             trace_npcm_gmac_packet_sent(DEVICE(gmac)->canonical_path, length);
             buf = tx_send_buffer;
-            length = 0;
             prev_buf_size = 0;
         }
 
-- 
2.43.0

On Mon, 14 Jul 2025 at 17:55, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> After the bug fix in the previous commit, the length and prev_buf_size
> variables are identical, except that prev_buf_size is uint32_t and
> length is uint16_t. We can therefore unify them. The only place where
> the type makes a difference is that we will truncate the packet
> at 64K when sending it; this commit preserves that behaviour
> by using a local variable when doing the packet send.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Should the behaviour really be to truncate the packet at 64K
> rather than flagging an error if the guest assembles descriptors
> that combine to give a too-large packet?

I see Jason has picked this up in a pullreq, which is OK given
this isn't a behaviour change -- but it would still be good
if somebody with access to the datasheet for this device could
confirm what the actual correct behaviour for attempts to tx
very large packets should be.

thanks
-- PMM