Re: Ping: [PATCH v2] net/vmnet: Pad short Ethernet frames

[Phil Dennis-Jordan <lists@philjordan.eu>]

On Sun, 20 Oct 2024 at 05:17, William Hooper <wsh@wshooper.org> wrote:
>
> On Sat, Aug 17, 2024 at 11:33 PM William Hooper <wsh@wshooper.org> wrote:
> > At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> > host's ARP replies, to the minimum size (60 bytes before the frame check
> > sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> > drivers may drop them with "frame too short" errors.
> >
> > This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> > and net/slirp.c. Thanks to Bin Meng and Philippe Mathieu-Daudé for
> > reviewing an earlier version.
> >
> > Signed-off-by: William Hooper <wsh@wshooper.org>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> > ---
> >  net/vmnet-common.m | 22 +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> > index 30c4e53c13..bce1cc590d 100644
> > --- a/net/vmnet-common.m
> > +++ b/net/vmnet-common.m
> > @@ -18,6 +18,7 @@
> >  #include "qemu/error-report.h"
> >  #include "qapi/error.h"
> >  #include "sysemu/runstate.h"
> > +#include "net/eth.h"
> >
> >  #include <vmnet/vmnet.h>
> >  #include <dispatch/dispatch.h>
> > @@ -147,10 +148,25 @@ static int vmnet_read_packets(VmnetState *s)
> >   */
> >  static void vmnet_write_packets_to_qemu(VmnetState *s)
> >  {
> > +    uint8_t *pkt;
> > +    size_t pktsz;
> > +    uint8_t min_pkt[ETH_ZLEN];
> > +    size_t min_pktsz;
> > +
> >      while (s->packets_send_current_pos < s->packets_send_end_pos) {
> > -        ssize_t size = qemu_send_packet_async(&s->nc,
> > -                                      s->iov_buf[s->packets_send_current_pos].iov_base,
> > -                                      s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
> > +        pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
> > +        pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
> > +
> > +        if (net_peer_needs_padding(&s->nc)) {
> > +            min_pktsz = sizeof(min_pkt);
> > +
> > +            if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
> > +                pkt = min_pkt;
> > +                pktsz = min_pktsz;
> > +            }
> > +        }
> > +
> > +        ssize_t size = qemu_send_packet_async(&s->nc, pkt, pktsz,
> >                                        vmnet_send_completed);

Nit: Move the declaration of 'size' up to the function header with the
rest of the variables for consistency.

Apart from that:
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>


> >          if (size == 0) {
> > --
> > 2.37.1
>
> Ping?
>