mbox series

[v5,00/12] net: Pad short frames for network backends

Message ID 20210317062638.72626-1-bmeng.cn@gmail.com
Headers show
Series net: Pad short frames for network backends | expand

Message

Bin Meng March 17, 2021, 6:26 a.m. UTC
The minimum Ethernet frame length is 60 bytes. For short frames with
smaller length like ARP packets (only 42 bytes), on a real world NIC
it can choose either padding its length to the minimum required 60
bytes, or sending it out directly to the wire. Such behavior can be
hardcoded or controled by a register bit. Similarly on the receive
path, NICs can choose either dropping such short frames directly or
handing them over to software to handle.

On the other hand, for the network backends like SLiRP/TAP, they
don't expose a way to control the short frame behavior. As of today
they just send/receive data from/to the other end connected to them,
which means any sized packet is acceptable. So they can send and
receive short frames without any problem. It is observed that ARP
packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
these ARP packets to the other end which might be a NIC model that
does not allow short frames to pass through.

To provide better compatibility, for packets sent from QEMU network
backends like SLiRP/TAP, we change to pad short frames before sending
it out to the other end, if the other end does not forbid it via the
nc->do_not_pad flag. This ensures a backend as an Ethernet sender
does not violate the spec. But with this change, the behavior of
dropping short frames from SLiRP/TAP interfaces in the NIC model
cannot be emulated because it always receives a packet that is spec
complaint. The capability of sending short frames from NIC models is
still supported and short frames can still pass through SLiRP/TAP.

This series should be able to fix the issue as reported with some
NIC models before, that ARP requests get dropped, preventing the
guest from becoming visible on the network. It was workarounded in
these NIC models on the receive path, that when a short frame is
received, it is padded up to 60 bytes.

Changes in v5:
- minor update on commit message
- update the eth_pad_short_frame() comment

Changes in v4:
- change 'ethernet' to 'Ethernet'
- do not inline the helper
- check the padded buffer size to avoid buffer overflow
- squash slirp/tap commits into one

Changes in v3:
- use 'without' instead of 'sans'
- add a helper to pad short frames
- add a comment to 'do_not_pad'
- use the pad_short_frame() helper

Bin Meng (12):
  net: eth: Add a helper to pad a short Ethernet frame
  net: Add a 'do_not_pad" to NetClientState
  net: Pad short frames to minimum size before sending from SLiRP/TAP
  hw/net: virtio-net: Initialize nc->do_not_pad to true
  hw/net: e1000: Remove the logic of padding short frames in the receive
    path
  hw/net: vmxnet3: Remove the logic of padding short frames in the
    receive path
  hw/net: i82596: Remove the logic of padding short frames in the
    receive path
  hw/net: ne2000: Remove the logic of padding short frames in the
    receive path
  hw/net: pcnet: Remove the logic of padding short frames in the receive
    path
  hw/net: rtl8139: Remove the logic of padding short frames in the
    receive path
  hw/net: sungem: Remove the logic of padding short frames in the
    receive path
  hw/net: sunhme: Remove the logic of padding short frames in the
    receive path

 hw/net/e1000.c      | 11 +----------
 hw/net/i82596.c     | 18 ------------------
 hw/net/ne2000.c     | 12 ------------
 hw/net/pcnet.c      |  9 ---------
 hw/net/rtl8139.c    | 12 ------------
 hw/net/sungem.c     | 14 --------------
 hw/net/sunhme.c     | 11 -----------
 hw/net/virtio-net.c |  4 ++++
 hw/net/vmxnet3.c    | 10 ----------
 include/net/eth.h   | 17 +++++++++++++++++
 include/net/net.h   |  1 +
 net/eth.c           | 17 +++++++++++++++++
 net/slirp.c         | 10 ++++++++++
 net/tap-win32.c     | 10 ++++++++++
 net/tap.c           | 10 ++++++++++
 15 files changed, 70 insertions(+), 96 deletions(-)

Comments

Bin Meng March 22, 2021, 1:28 a.m. UTC | #1
On Wed, Mar 17, 2021 at 2:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> The minimum Ethernet frame length is 60 bytes. For short frames with
> smaller length like ARP packets (only 42 bytes), on a real world NIC
> it can choose either padding its length to the minimum required 60
> bytes, or sending it out directly to the wire. Such behavior can be
> hardcoded or controled by a register bit. Similarly on the receive
> path, NICs can choose either dropping such short frames directly or
> handing them over to software to handle.
>
> On the other hand, for the network backends like SLiRP/TAP, they
> don't expose a way to control the short frame behavior. As of today
> they just send/receive data from/to the other end connected to them,
> which means any sized packet is acceptable. So they can send and
> receive short frames without any problem. It is observed that ARP
> packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
> these ARP packets to the other end which might be a NIC model that
> does not allow short frames to pass through.
>
> To provide better compatibility, for packets sent from QEMU network
> backends like SLiRP/TAP, we change to pad short frames before sending
> it out to the other end, if the other end does not forbid it via the
> nc->do_not_pad flag. This ensures a backend as an Ethernet sender
> does not violate the spec. But with this change, the behavior of
> dropping short frames from SLiRP/TAP interfaces in the NIC model
> cannot be emulated because it always receives a packet that is spec
> complaint. The capability of sending short frames from NIC models is
> still supported and short frames can still pass through SLiRP/TAP.
>
> This series should be able to fix the issue as reported with some
> NIC models before, that ARP requests get dropped, preventing the
> guest from becoming visible on the network. It was workarounded in
> these NIC models on the receive path, that when a short frame is
> received, it is padded up to 60 bytes.
>
> Changes in v5:
> - minor update on commit message
> - update the eth_pad_short_frame() comment
>

Ping?
Jason Wang March 22, 2021, 7:08 a.m. UTC | #2
在 2021/3/17 下午2:26, Bin Meng 写道:
> The minimum Ethernet frame length is 60 bytes. For short frames with
> smaller length like ARP packets (only 42 bytes), on a real world NIC
> it can choose either padding its length to the minimum required 60
> bytes, or sending it out directly to the wire. Such behavior can be
> hardcoded or controled by a register bit. Similarly on the receive
> path, NICs can choose either dropping such short frames directly or
> handing them over to software to handle.
>
> On the other hand, for the network backends like SLiRP/TAP, they
> don't expose a way to control the short frame behavior. As of today
> they just send/receive data from/to the other end connected to them,
> which means any sized packet is acceptable. So they can send and
> receive short frames without any problem. It is observed that ARP
> packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
> these ARP packets to the other end which might be a NIC model that
> does not allow short frames to pass through.
>
> To provide better compatibility, for packets sent from QEMU network
> backends like SLiRP/TAP, we change to pad short frames before sending
> it out to the other end, if the other end does not forbid it via the
> nc->do_not_pad flag. This ensures a backend as an Ethernet sender
> does not violate the spec. But with this change, the behavior of
> dropping short frames from SLiRP/TAP interfaces in the NIC model
> cannot be emulated because it always receives a packet that is spec
> complaint. The capability of sending short frames from NIC models is
> still supported and short frames can still pass through SLiRP/TAP.
>
> This series should be able to fix the issue as reported with some
> NIC models before, that ARP requests get dropped, preventing the
> guest from becoming visible on the network. It was workarounded in
> these NIC models on the receive path, that when a short frame is
> received, it is padded up to 60 bytes.
>
> Changes in v5:
> - minor update on commit message
> - update the eth_pad_short_frame() comment
>
> Changes in v4:
> - change 'ethernet' to 'Ethernet'
> - do not inline the helper
> - check the padded buffer size to avoid buffer overflow
> - squash slirp/tap commits into one
>
> Changes in v3:
> - use 'without' instead of 'sans'
> - add a helper to pad short frames
> - add a comment to 'do_not_pad'
> - use the pad_short_frame() helper
>
> Bin Meng (12):
>    net: eth: Add a helper to pad a short Ethernet frame
>    net: Add a 'do_not_pad" to NetClientState
>    net: Pad short frames to minimum size before sending from SLiRP/TAP
>    hw/net: virtio-net: Initialize nc->do_not_pad to true
>    hw/net: e1000: Remove the logic of padding short frames in the receive
>      path
>    hw/net: vmxnet3: Remove the logic of padding short frames in the
>      receive path
>    hw/net: i82596: Remove the logic of padding short frames in the
>      receive path
>    hw/net: ne2000: Remove the logic of padding short frames in the
>      receive path
>    hw/net: pcnet: Remove the logic of padding short frames in the receive
>      path
>    hw/net: rtl8139: Remove the logic of padding short frames in the
>      receive path
>    hw/net: sungem: Remove the logic of padding short frames in the
>      receive path
>    hw/net: sunhme: Remove the logic of padding short frames in the
>      receive path
>
>   hw/net/e1000.c      | 11 +----------
>   hw/net/i82596.c     | 18 ------------------
>   hw/net/ne2000.c     | 12 ------------
>   hw/net/pcnet.c      |  9 ---------
>   hw/net/rtl8139.c    | 12 ------------
>   hw/net/sungem.c     | 14 --------------
>   hw/net/sunhme.c     | 11 -----------
>   hw/net/virtio-net.c |  4 ++++
>   hw/net/vmxnet3.c    | 10 ----------
>   include/net/eth.h   | 17 +++++++++++++++++
>   include/net/net.h   |  1 +
>   net/eth.c           | 17 +++++++++++++++++
>   net/slirp.c         | 10 ++++++++++
>   net/tap-win32.c     | 10 ++++++++++
>   net/tap.c           | 10 ++++++++++
>   15 files changed, 70 insertions(+), 96 deletions(-)


I've queued patch 1-4 for 6.0 and the reset for 6.1.

Thanks


>
Bin Meng May 14, 2021, 5:10 a.m. UTC | #3
Hi Jason,

On Mon, Mar 22, 2021 at 3:10 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/3/17 下午2:26, Bin Meng 写道:
> > The minimum Ethernet frame length is 60 bytes. For short frames with
> > smaller length like ARP packets (only 42 bytes), on a real world NIC
> > it can choose either padding its length to the minimum required 60
> > bytes, or sending it out directly to the wire. Such behavior can be
> > hardcoded or controled by a register bit. Similarly on the receive
> > path, NICs can choose either dropping such short frames directly or
> > handing them over to software to handle.
> >
> > On the other hand, for the network backends like SLiRP/TAP, they
> > don't expose a way to control the short frame behavior. As of today
> > they just send/receive data from/to the other end connected to them,
> > which means any sized packet is acceptable. So they can send and
> > receive short frames without any problem. It is observed that ARP
> > packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
> > these ARP packets to the other end which might be a NIC model that
> > does not allow short frames to pass through.
> >
> > To provide better compatibility, for packets sent from QEMU network
> > backends like SLiRP/TAP, we change to pad short frames before sending
> > it out to the other end, if the other end does not forbid it via the
> > nc->do_not_pad flag. This ensures a backend as an Ethernet sender
> > does not violate the spec. But with this change, the behavior of
> > dropping short frames from SLiRP/TAP interfaces in the NIC model
> > cannot be emulated because it always receives a packet that is spec
> > complaint. The capability of sending short frames from NIC models is
> > still supported and short frames can still pass through SLiRP/TAP.
> >
> > This series should be able to fix the issue as reported with some
> > NIC models before, that ARP requests get dropped, preventing the
> > guest from becoming visible on the network. It was workarounded in
> > these NIC models on the receive path, that when a short frame is
> > received, it is padded up to 60 bytes.
> >
> > Changes in v5:
> > - minor update on commit message
> > - update the eth_pad_short_frame() comment
> >
> > Changes in v4:
> > - change 'ethernet' to 'Ethernet'
> > - do not inline the helper
> > - check the padded buffer size to avoid buffer overflow
> > - squash slirp/tap commits into one
> >
> > Changes in v3:
> > - use 'without' instead of 'sans'
> > - add a helper to pad short frames
> > - add a comment to 'do_not_pad'
> > - use the pad_short_frame() helper
> >
> > Bin Meng (12):
> >    net: eth: Add a helper to pad a short Ethernet frame
> >    net: Add a 'do_not_pad" to NetClientState
> >    net: Pad short frames to minimum size before sending from SLiRP/TAP
> >    hw/net: virtio-net: Initialize nc->do_not_pad to true
> >    hw/net: e1000: Remove the logic of padding short frames in the receive
> >      path
> >    hw/net: vmxnet3: Remove the logic of padding short frames in the
> >      receive path
> >    hw/net: i82596: Remove the logic of padding short frames in the
> >      receive path
> >    hw/net: ne2000: Remove the logic of padding short frames in the
> >      receive path
> >    hw/net: pcnet: Remove the logic of padding short frames in the receive
> >      path
> >    hw/net: rtl8139: Remove the logic of padding short frames in the
> >      receive path
> >    hw/net: sungem: Remove the logic of padding short frames in the
> >      receive path
> >    hw/net: sunhme: Remove the logic of padding short frames in the
> >      receive path
> >
> >   hw/net/e1000.c      | 11 +----------
> >   hw/net/i82596.c     | 18 ------------------
> >   hw/net/ne2000.c     | 12 ------------
> >   hw/net/pcnet.c      |  9 ---------
> >   hw/net/rtl8139.c    | 12 ------------
> >   hw/net/sungem.c     | 14 --------------
> >   hw/net/sunhme.c     | 11 -----------
> >   hw/net/virtio-net.c |  4 ++++
> >   hw/net/vmxnet3.c    | 10 ----------
> >   include/net/eth.h   | 17 +++++++++++++++++
> >   include/net/net.h   |  1 +
> >   net/eth.c           | 17 +++++++++++++++++
> >   net/slirp.c         | 10 ++++++++++
> >   net/tap-win32.c     | 10 ++++++++++
> >   net/tap.c           | 10 ++++++++++
> >   15 files changed, 70 insertions(+), 96 deletions(-)
>
>
> I've queued patch 1-4 for 6.0 and the reset for 6.1.

It seems the reset has not been applied for 6.1?

Regards,
Bin