On 8/18/26 4:53 PM, Zhuoying Cai wrote:
The virtio-net device used a fixed header size that did not account for
the num_buffers field used in VirtIO 1.0 or for the mergeable receive
buffers feature.
Use dynamic header sizing: 10 bytes for legacy mode and 12 bytes for
VirtIO 1.0 or when VIRTIO_NET_F_MRG_RXBUF is enabled. This ensures
correct packet handling across different VirtIO configurations.
Signed-off-by: Zhuoying Cai <[email protected]>
---
pc-bios/s390-ccw/virtio-net.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index 0ee51653ab..3a9ae789cf 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -20,6 +20,7 @@
#include "s390-ccw.h"
#include "virtio.h"
#include "virtio-ccw.h"
+#include "virtio-pci.h"
#include "s390-time.h"
#include "helper.h"
@@ -28,10 +29,15 @@
#endif
#define VIRTIO_NET_F_MAC_BIT (1 << 5)
+#define VIRTIO_NET_F_MRG_RXBUF_BIT (1 << 15)
#define VQ_RX 0 /* Receive queue */
#define VQ_TX 1 /* Transmit queue */
+/* Header sizes for different modes */
+#define VIRTIO_NET_HDR_SIZE_LEGACY 10 /* Without num_buffers */
+#define VIRTIO_NET_HDR_SIZE_V1 12 /* With num_buffers */
+
struct VirtioNetHdr {
uint8_t flags;
uint8_t gso_type;
@@ -39,11 +45,12 @@ struct VirtioNetHdr {
uint16_t gso_size;
uint16_t csum_start;
uint16_t csum_offset;
- /*uint16_t num_buffers;*/ /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
+ uint16_t num_buffers; /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
};
typedef struct VirtioNetHdr VirtioNetHdr;
static uint16_t rx_last_idx; /* Last index in receive queue "used" ring */
+static int virtio_net_hdr_size;
int virtio_net_init(void *mac_addr)
{
@@ -62,12 +69,17 @@ int virtio_net_init(void *mac_addr)
return -1;
}
+ virtio_net_hdr_size = ((vdev->guest_features[1] & VIRTIO_F_VERSION_1) ||
+ (vdev->guest_features[0] &
VIRTIO_NET_F_MRG_RXBUF_BIT))
+ ? VIRTIO_NET_HDR_SIZE_V1
+ : VIRTIO_NET_HDR_SIZE_LEGACY;
+
memcpy(mac_addr, vdev->config.net.mac, ETH_ALEN);
for (i = 0; i < 64; i++) {
- buf = malloc(ETH_MTU_SIZE + sizeof(VirtioNetHdr));
+ buf = malloc(ETH_MTU_SIZE + virtio_net_hdr_size);
IPL_assert(buf != NULL, "Can not allocate memory for receive
buffers");
- vring_send_buf(rxvq, buf, ETH_MTU_SIZE + sizeof(VirtioNetHdr),
+ vring_send_buf(rxvq, buf, ETH_MTU_SIZE + virtio_net_hdr_size,
VRING_DESC_F_WRITE);
}
vring_notify(rxvq);
@@ -82,9 +94,9 @@ int send(int fd, const void *buf, int len, int flags)
VRing *txvq = &vdev->vrings[VQ_TX];
/* Set up header - we do not use anything special, so simply clear it */
- memset(&tx_hdr, 0, sizeof(tx_hdr));
+ memset(&tx_hdr, 0, virtio_net_hdr_size);
I think you want to leave this line as-is. There's no harm in cleaning
the entire struct's memory even if we never end up using the final
field. But only partially cleaning the struct looks weird and could
potentially cause problems if subsequent code changes introduce code
that attempts to read uninitialized data from num_buffers later.
With that change made:
Reviewed-by: Jason J. Herne <[email protected]>
