Re: [RFC PATCH v3 02/10] net: Pad short frames to minimum size before send from SLiRP/TAP

[Bin Meng]

On Thu, Mar 11, 2021 at 6:22 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 11 Mar 2021 at 09:58, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> 
> > wrote:
> > >
> > > On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote:
> > > > And after a discussion 10 years ago [1]. Michael (cced) seems to want to
> > > > keep the padding logic in the NIC itself (probably with a generic helper
> > > > in the core). Since 1) the padding is only required for ethernet 2)
> > > > virito-net doesn't need that (it can pass incomplete packet by design).
> > >
> > > Like I said, we need to decide; either:
> > >
> > >  (1) we do want to support short packets in the net core:
> > > every sender needs to either pad, or to have some flag to say
> > > "my implementation can't pad, please can the net core do it for me",
> > > unless they are deliberately sending a short packet. Every
> > > receiver needs to be able to cope with short packets, at least
> > > in the sense of not crashing (they should report them as a rx
> > > error if they have that kind of error reporting status register).
> > > I think we have mostly implemented our NIC models this way.
> > >
> > >  (2) we simply don't support short packets in the net core:
> > > nobody (not NICs, not network backends) needs to pad, because
> > > they can rely on the core to do it. Some existing senders and
> > > receivers may have now-dead code to do their own padding which
> > > could be removed.
> > >
> > > MST is advocating for (1) in that old thread. That's a coherent
> > > position.
> >
> > But it's a wrong approach. As Edgar and Stefan also said in the old
> > discussion thread, padding in the RX is wrong as real world NICs don't
> > do this.
>
> Neither option (1) nor option (2) involve padding in RX.

Correct. What I referred to is the current approach used in many NIC
modes, which is wrong, and we have to correct this.

>
> Option (1) is:
>  * no NIC implementation pads on TX, except as defined
>    by whatever NIC-specific config registers or h/w behaviour
>    might require (ie if the guest wants to send a short packet
>    it can do that)
>  * non-NIC sources like slirp need to pad on TX unless they're
>    deliberately trying to send a short packet
>  * all receivers of packets need to cope with being given a
>    short packet; this is usually going to mean "flag it to the
>    guest as an RX error", but exact behaviour is NIC-dependent
>

My patch series in RFC v2/v3 does almost exactly this option (1),
except "flag it to the guest as an RX error".

> Option (2) is:
>  * the net core code pads any packet that goes through it
>  * no NIC implementation needs to pad on TX (it is harmless if they do)
>  * non-NIC sources don't need to pad on TX
>  * no receivers of packets need to cope with being given short packets
>
> Option 1 is what the real world does. Option 2 is a simplification
> which throws away the ability to emulate handling of short packets,
> in exchange for not having to sort out senders like slirp and not
> having to be so careful about short-packet handling in NIC models.
>
> If MST is correct that some use cases require short-packet support,
> then we need to go for option 1, I think.

Regards,
Bin