On Fri, Jan 13, 2023 at 4:23 PM Alexander Duyck
<alexander.duyck@xxxxxxxxx> wrote:
>
> On Fri, Jan 13, 2023 at 3:37 PM Parav Pandit <parav@xxxxxxxxxx> wrote:
> >
> >
> > > From: Alexander H Duyck <alexander.duyck@xxxxxxxxx>
> > > Sent: Friday, January 13, 2023 6:24 PM
> > >
> > > On Sat, 2023-01-14 at 00:36 +0200, Parav Pandit wrote:
> > > > A smallest Ethernet frame defined by IEEE 802.3 is 60 bytes without
> > > > any preemble and CRC.
> > > >
> > > > Current code only checks for minimal 14 bytes of Ethernet header length.
> > > > Correct it to consider the minimum Ethernet frame length.
> > > >
> > > > Fixes: 296f96fcfc16 ("Net driver using virtio")
> > > > Signed-off-by: Parav Pandit <parav@xxxxxxxxxx>
> > > > ---
> > > > drivers/net/virtio_net.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index
> > > > 7723b2a49d8e..d45e140b6852 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -1248,7 +1248,7 @@ static void receive_buf(struct virtnet_info *vi,
> > > struct receive_queue *rq,
> > > > struct sk_buff *skb;
> > > > struct virtio_net_hdr_mrg_rxbuf *hdr;
> > > >
> > > > - if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> > > > + if (unlikely(len < vi->hdr_len + ETH_ZLEN)) {
> > > > pr_debug("%s: short packet %i\n", dev->name, len);
> > > > dev->stats.rx_length_errors++;
> > > > if (vi->mergeable_rx_bufs) {
> > >
> > > I'm not sure I agree with this change as packets are only 60B if they have gone
> > > across the wire as they are usually padded out on the transmit side. There may
> > > be cases where software routed packets may not be 60B.
> > >
> > Do you mean Linux kernel software? Any link to it would be helpful.
>
> The problem is there are several software paths involved and that is
> why I am wanting to be cautious. As I recall this would impact Qemu
> itself, DPDK, the Linux Kernel and several others if I am not
> mistaken. That is why I am tending to err on the side of caution as
> this is a pretty significant change.
>
> > > As such rather than changing out ETH_HLEN for ETH_ZLEN I wonder if we
> > > should look at maybe making this a "<=" comparison instead since that is the
> > > only case I can think of where the packet would end up being entirely empty
> > > after eth_type_trans is called and we would be passing an skb with length 0.
> >
> > I likely didn’t understand your comment.
> > This driver check is before creating the skb for the received packet.
> > So, purpose is to not even process the packet header or prepare the skb if it not an Ethernet frame.
> >
> > It is interesting to know when we get < 60B frame.
>
> If I recall, a UDPv4 frame can easily do it since Ethernet is 14B, IP
> header is 20, and UDP is only 8 so that only comes to 42B if I recall
> correctly. Similarly I think a TCPv4 Frame can be as small as 54B if
> you disable all the option headers.
>
> A quick and dirty test would be to run something like a netperf UDP_RR
> test. I know in the case of the network stack we see the transmits
> that go out are less than 60B until they are padded on xmit, usually
> by the device. My concern is wanting to make sure all those paths are
> covered before we assume that all the packets will be padded.
I was curious so I decided to try verifying things with a qemu w/ user
networking and virtio-net. From what I can tell it looks like it is
definitely not padding them out.
19:34:38.331376 IP (tos 0x0, ttl 64, id 31799, offset 0, flags [DF],
proto UDP (17), length 29)
localhost.localdomain.59579 > _gateway.52701: [udp sum ok] UDP, length 1
0x0000: 5255 0a00 0202 5254 0012 3456 0800 4500
0x0010: 001d 7c37 4000 4011 a688 0a00 020f 0a00
0x0020: 0202 e8bb cddd 0009 c331 6e
19:34:38.331431 IP (tos 0x0, ttl 64, id 45459, offset 0, flags [none],
proto UDP (17), length 29)
_gateway.52701 > localhost.localdomain.59579: [udp sum ok] UDP, length 1
0x0000: 5254 0012 3456 5255 0a00 0202 0800 4500
0x0010: 001d b193 0000 4011 b12c 0a00 0202 0a00
0x0020: 020f cddd e8bb 0009 c331 6e
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
