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 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> > On 2021/3/9 6:13 下午, Peter Maydell wrote:
> > > On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >> Ah, so we want this:
> > >>
> > >> if (sender->info->type != NET_CLIENT_DRIVER_NIC)
> > >>
> > >> correct?
>
> > > No, option (2) is "always pad short packets regardless of
> > > sender->info->type". Even if a NIC driver sends out a short
> > > packet, we want to pad it, because we might be feeding it to
> > > something that assumes it does not see short packets.
> >
> > So I'm not sure this is correct. There're NIC that has its own logic
> > that choose whether to pad the frame during TX (e.g e1000).
>
> Yes; that would be dead-code if we go for "always pad", the same
> as the code in devices to handle incoming short packets would also
> be dead.
>
> > 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.

> You've said earlier that we want (2). That's also
> a coherent position. A mix of the two doesn't seem to
> me to be very workable.

Regards,
Bin