qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Bin Meng <bmeng.cn@gmail.com>
Cc: "Dmitry Fleytman" <dmitry.fleytman@gmail.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Bin Meng" <bin.meng@windriver.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [RFC PATCH v3 02/10] net: Pad short frames to minimum size before send from SLiRP/TAP
Date: Thu, 11 Mar 2021 10:22:07 +0000	[thread overview]
Message-ID: <CAFEAcA8gwa2NGF2s3f=hO+EaVSJNDJrKz7xG60eSm68-CXf-mw@mail.gmail.com> (raw)
In-Reply-To: <CAEUhbmW1pz0=TgwF12j5pQUaCGSLPumb5-yiy32PKfdTvvdpVQ@mail.gmail.com>

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.

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

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.

thanks
-- PMM


  reply	other threads:[~2021-03-11 10:23 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-03 19:11 [RFC PATCH v3 00/10] net: Handle short frames for SLiRP/TAP interfaces Philippe Mathieu-Daudé
2021-03-03 19:11 ` [RFC PATCH v3 01/10] net: Use 'struct iovec' in qemu_send_packet_async_with_flags() Philippe Mathieu-Daudé
2021-03-03 19:11 ` [RFC PATCH v3 02/10] net: Pad short frames to minimum size before send from SLiRP/TAP Philippe Mathieu-Daudé
2021-03-08  3:48   ` Jason Wang
2021-03-08  4:12     ` Bin Meng
2021-03-08 10:22     ` Peter Maydell
2021-03-09  8:23       ` Jason Wang
2021-03-09  8:35         ` Bin Meng
2021-03-09  8:57           ` Jason Wang
2021-03-09  9:00             ` Bin Meng
2021-03-09  9:01               ` Bin Meng
2021-03-09 10:13                 ` Peter Maydell
2021-03-09 10:17                   ` Bin Meng
2021-03-09 12:30                   ` Yan Vugenfirer
2021-03-09 12:33                     ` Bin Meng
2021-03-12  6:25                     ` Jason Wang
2021-03-11  3:01                   ` Jason Wang
2021-03-11  3:12                     ` Bin Meng
2021-03-11  3:33                       ` Jason Wang
2021-03-11  7:35                         ` Bin Meng
2021-03-11  9:43                     ` Peter Maydell
2021-03-11  9:58                       ` Bin Meng
2021-03-11 10:22                         ` Peter Maydell [this message]
2021-03-11 10:27                           ` Bin Meng
2021-03-12  6:22                             ` Jason Wang
2021-03-12  6:28                               ` Bin Meng
2021-03-12  6:50                                 ` Jason Wang
2021-03-12  6:53                                   ` Bin Meng
2021-03-12  7:02                                     ` Jason Wang
2021-03-03 19:11 ` [RFC PATCH v3 03/10] hw/net: e1000: Remove the logic of padding short frames in the receive path Philippe Mathieu-Daudé
2021-03-03 19:11 ` [RFC PATCH v3 04/10] hw/net: vmxnet3: " Philippe Mathieu-Daudé
2021-03-03 19:12 ` [RFC PATCH v3 05/10] hw/net: i82596: " Philippe Mathieu-Daudé
2021-03-03 19:12 ` [RFC PATCH v3 06/10] hw/net: ne2000: " Philippe Mathieu-Daudé
2021-03-03 19:12 ` [RFC PATCH v3 07/10] hw/net: pcnet: " Philippe Mathieu-Daudé
2021-03-03 19:12 ` [RFC PATCH v3 08/10] hw/net: rtl8139: " Philippe Mathieu-Daudé
2021-03-03 19:12 ` [RFC PATCH v3 09/10] hw/net: sungem: " Philippe Mathieu-Daudé
2021-03-03 19:12 ` [RFC PATCH v3 10/10] hw/net: sunhme: " Philippe Mathieu-Daudé
2021-03-08  1:51 ` [RFC PATCH v3 00/10] net: Handle short frames for SLiRP/TAP interfaces Bin Meng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFEAcA8gwa2NGF2s3f=hO+EaVSJNDJrKz7xG60eSm68-CXf-mw@mail.gmail.com' \
    --to=peter.maydell@linaro.org \
    --cc=bin.meng@windriver.com \
    --cc=bmeng.cn@gmail.com \
    --cc=dmitry.fleytman@gmail.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).