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

[Peter Maydell]

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. 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.

thanks
-- PMM