Re: [PATCH RESEND] hw/net: fsl_etsec: Do not reject short frames

[Bin Meng]

On Tue, Feb 9, 2021 at 8:06 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Cc'ing libSLiRP

I am not sure whether my reply arrived to libSLiRP as I did not
subscribe to that list ...

>
> Hi Peter,
>
> On Tue, Feb 9, 2021 at 12:09 AM Peter Maydell <peter.maydell@linaro.org> 
> wrote:
> >
> > On Mon, 8 Feb 2021 at 14:53, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > As of today both slirp and tap networking do not pad short frames
> > > (e.g.: an ARP packet) to the minimum frame size of 60 bytes.
> > >
> > > If eTSEC is programmed to reject short frames, ARP requests will be
> > > dropped, preventing the guest from becoming visible on the network.
> > >
> > > The same issue was reported on e1000 and vmxenet3 before, see:
> > >
> > > commit 78aeb23eded2 ("e1000: Pad short frames to minimum size (60 bytes)")
> > > commit 40a87c6c9b11 ("vmxnet3: Pad short frames to minimum size (60 
> > > bytes)")
> >
> > How a short frame should be handled is ethernet device specific:
> > what is correct for one device model doesn't necessarily apply
> > to another.
>
> I digged some history about the above 2 commits and they are the same
> issue caused by slirp and tap networking, and workarounded in the
> ethernet controller models.
>
> >
> > > Ideally this should be fixed on the slirp/tap networking side to
> > > pad short frames to the minimum frame length, but I am not sure
> > > whether that's doable.
> >
> > It would be useful to investigate further exactly where these
> > short frames are coming from. If one guest is sending out short
> > frames, or we are doing tap networking and get a genuine short
> > frame from some external host then we should pass them to the
> > guest as short frames; if QEMU itself is generating frames (eg
> > from the 'fake' hosts in usermode networking) then it should be
> > generating valid frames, not bogus ones, and we should fix whatever
> > bit of code that is.
>
> From what I can tell it's the QEMU networking codes that generate such
> short frames.
>
> However it looks no one has ever attempted to fix that in the QEMU
> networking, instead the ethernet controller models are patched in the
> *receive* path, which is to pad such short frames to 60 bytes in e1000
> and vmxnet3.
>
> >
> > > This commit changes to codes to ignore the RCTRL_RSF setting and
> > > still allow receiving the short frame. The log message is updated
> > > to mention the reject short frames functionality is unimplemented.
> > >
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > ---
> > >
> > > RESEND using correct email address
> > >
> > >  hw/net/fsl_etsec/rings.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
> > > index 121415a..503b4d3 100644
> > > --- a/hw/net/fsl_etsec/rings.c
> > > +++ b/hw/net/fsl_etsec/rings.c
> > > @@ -502,10 +502,17 @@ ssize_t etsec_rx_ring_write(eTSEC *etsec, const 
> > > uint8_t *buf, size_t size)
> > >          return -1;
> > >      }
> > >
> > > +    /*
> > > +     * Both slirp and tap networking do not pad short frames
> > > +     * (e.g.: an ARP packet) to the minimum frame size of 60 bytes.
> > > +     *
> > > +     * If eTSEC is programmed to reject short frames, ARP requests
> > > +     * will be dropped, preventing the guest from becoming visible
> > > +     * on the network.
> > > +     */
> > >      if ((etsec->regs[RCTRL].value & RCTRL_RSF) && (size < 60)) {
> > >          /* CRC is not in the packet yet, so short frame is below 60 
> > > bytes */
> > > -        RING_DEBUG("%s: Drop short frame\n", __func__);
> > > -        return -1;
> > > +        RING_DEBUG("%s: Drop short frame not implemented\n", __func__);
> > >      }
> >
> > This doesn't look right. If the guest programs the device to
> > reject frames less than 60 bytes and then expects to recieve a
> > frame that's less than 60 bytes, that's a guest bug. If QEMU
> > itself is generating packets to send and they're short that sounds
> > like a bug elsewhere in QEMU.

Could anyone familiar with the QEMU networking have a look at the
implementations to solve this short frame issue once and for all?

Regards,
Bin