qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH v3 02/10] net: Pad short frames to minimum size before se


From: Jason Wang
Subject: Re: [RFC PATCH v3 02/10] net: Pad short frames to minimum size before send from SLiRP/TAP
Date: Fri, 12 Mar 2021 15:02:13 +0800
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.16; rv:78.0) Gecko/20100101 Thunderbird/78.8.1


On 2021/3/12 2:53 下午, Bin Meng wrote:
On Fri, Mar 12, 2021 at 2:50 PM Jason Wang <jasowang@redhat.com> wrote:

On 2021/3/12 2:28 下午, Bin Meng wrote:
On Fri, Mar 12, 2021 at 2:23 PM Jason Wang <jasowang@redhat.com> wrote:
On 2021/3/11 6:27 下午, Bin Meng wrote:
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".
Is it? You did it at net core instead of netdevs if I read the code
correctly.

Literally I don't see Peter requested option (1) to be done in net
core or net devs.

If doing it in netdevs, the following codes need to be duplicated in
both SLiRP and TAP codes.

if (sender->info->type == NET_CLIENT_DRIVER_USER ||
      sender->info->type == NET_CLIENT_DRIVER_TAP) {
      do the short frames padding;
}

So my understanding is that it's better to be done at netdev where we
know whether it's a ethernet dev, core should be protocol independent.
OK, will change to pad short frames in SLiRP and TAP codes in the next version.

Regards,
Bin


And we probably need a need_padding in the NetClientState, and only do the padding if it was required by the peer.

Then we can say netdevs and virtio-net doesn't need padding.

Thanks





reply via email to

[Prev in Thread] Current Thread [Next in Thread]