Re: [PATCH v6 0/8] net: Pad short frames for network backends

[Cédric Le Goater]

Hello Bin,

On 3/1/23 10:01, bmeng.cn@gmail.com wrote:
> From: Bin Meng <bmeng@tinylab.org>
>
> The minimum Ethernet frame length is 60 bytes. For short frames with
> smaller length like ARP packets (only 42 bytes), on a real world NIC
> it can choose either padding its length to the minimum required 60
> bytes, or sending it out directly to the wire. Such behavior can be
> hardcoded or controled by a register bit. Similarly on the receive
> path, NICs can choose either dropping such short frames directly or
> handing them over to software to handle.
>
> On the other hand, for the network backends like SLiRP/TAP, they
> don't expose a way to control the short frame behavior. As of today
> they just send/receive data from/to the other end connected to them,
> which means any sized packet is acceptable. So they can send and
> receive short frames without any problem. It is observed that ARP
> packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
> these ARP packets to the other end which might be a NIC model that
> does not allow short frames to pass through.
>
> To provide better compatibility, for packets sent from QEMU network
> backends like SLiRP/TAP, we change to pad short frames before sending
> it out to the other end, if the other end does not forbid it via the
> nc->do_not_pad flag. This ensures a backend as an Ethernet sender
> does not violate the spec. But with this change, the behavior of
> dropping short frames from SLiRP/TAP interfaces in the NIC model
> cannot be emulated because it always receives a packet that is spec
> complaint. The capability of sending short frames from NIC models is
> still supported and short frames can still pass through SLiRP/TAP.
>
> This series should be able to fix the issue as reported with some
> NIC models before, that ARP requests get dropped, preventing the
> guest from becoming visible on the network. It was workarounded in
> these NIC models on the receive path, that when a short frame is
> received, it is padded up to 60 bytes.

I guess we can drop this code in ftgmac100.c also then :

    /* TODO : Pad to minimum Ethernet frame length */
    /* handle small packets.  */
    if (size < 10) {
        qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped frame of %zd bytes\n",
                      __func__, size);
        return size;
    }

Correct ? No need to resend. I can take care of it.

Thanks,

C.

> Note this is a resend of the v5 [1]. Only the first 4 patches were
> applied in QEMU 6.0, and the reset was said to be queued for 6.1
> but for some reason they never landed in QEMU mainline.
>
> [1] 
> https://lore.kernel.org/qemu-devel/859cd26a-feb2-ed62-98d5-764841a468cf@redhat.com/
>
> Bin Meng (8):
>    hw/net: e1000: Remove the logic of padding short frames in the receive
>      path
>    hw/net: vmxnet3: Remove the logic of padding short frames in the
>      receive path
>    hw/net: i82596: Remove the logic of padding short frames in the
>      receive path
>    hw/net: ne2000: Remove the logic of padding short frames in the
>      receive path
>    hw/net: pcnet: Remove the logic of padding short frames in the receive
>      path
>    hw/net: rtl8139: Remove the logic of padding short frames in the
>      receive path
>    hw/net: sungem: Remove the logic of padding short frames in the
>      receive path
>    hw/net: sunhme: Remove the logic of padding short frames in the
>      receive path
>
>   hw/net/e1000.c   | 11 +----------
>   hw/net/i82596.c  | 18 ------------------
>   hw/net/ne2000.c  | 12 ------------
>   hw/net/pcnet.c   |  9 ---------
>   hw/net/rtl8139.c | 12 ------------
>   hw/net/sungem.c  | 14 --------------
>   hw/net/sunhme.c  | 11 -----------
>   hw/net/vmxnet3.c | 10 ----------
>   8 files changed, 1 insertion(+), 96 deletions(-)