Date: Tue, 19 Jan 2021 05:07:59 GMT From: Bryan Venteicher <bryanv@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 1cd1ed3f5dd5 - main - Revert: virtio: Support non-legacy network device and queue Message-ID: <202101190507.10J57xQP085536@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by bryanv: URL: https://cgit.FreeBSD.org/src/commit/?id=1cd1ed3f5dd50ca070908468a3816f570448427b commit 1cd1ed3f5dd50ca070908468a3816f570448427b Author: Bryan Venteicher <bryanv@FreeBSD.org> AuthorDate: 2021-01-19 04:55:23 +0000 Commit: Bryan Venteicher <bryanv@FreeBSD.org> CommitDate: 2021-01-19 04:55:23 +0000 Revert: virtio: Support non-legacy network device and queue And subsequent fix 576b099a. By adding the mergable header to the vtnet_rx_header structure, the size was increased by 2 bytes, breaking the alignment of this structure as described the in preceding comments. Furthermore, the mergable header does not belong the structure. With the mergable feature, the header is placed in line with the data, so there is no need for a separate segment, and misleading to follow the mergable header with any padding. The V1 header is effectively identical to mergable header, and the driver has long supported the mergable feature. Revert this so the later changes that add V1 support can show how V1 is derived from the existing mergable buffers support, and to facilitate a later MFC. Reviewed by: grehan (mentor) Differential Revision: https://reviews.freebsd.org/D27855 --- sys/dev/virtio/network/if_vtnet.c | 17 ++++++----------- sys/dev/virtio/network/if_vtnetvar.h | 14 ++++---------- sys/dev/virtio/virtio.c | 2 +- sys/dev/virtio/virtqueue.c | 1 - 4 files changed, 11 insertions(+), 23 deletions(-) diff --git a/sys/dev/virtio/network/if_vtnet.c b/sys/dev/virtio/network/if_vtnet.c index 2bfa72734f9e..e16570f7b4f1 100644 --- a/sys/dev/virtio/network/if_vtnet.c +++ b/sys/dev/virtio/network/if_vtnet.c @@ -640,13 +640,10 @@ vtnet_setup_features(struct vtnet_softc *sc) sc->vtnet_flags |= VTNET_FLAG_MAC; } - if (virtio_with_feature(dev, VIRTIO_NET_F_MRG_RXBUF)) + if (virtio_with_feature(dev, VIRTIO_NET_F_MRG_RXBUF)) { sc->vtnet_flags |= VTNET_FLAG_MRG_RXBUFS; - - if (virtio_with_feature(dev, VIRTIO_NET_F_MRG_RXBUF) || - virtio_with_feature(dev, VIRTIO_F_VERSION_1)) sc->vtnet_hdr_size = sizeof(struct virtio_net_hdr_mrg_rxbuf); - else + } else sc->vtnet_hdr_size = sizeof(struct virtio_net_hdr); if (sc->vtnet_flags & VTNET_FLAG_MRG_RXBUFS) @@ -1462,10 +1459,9 @@ vtnet_rxq_enqueue_buf(struct vtnet_rxq *rxq, struct mbuf *m) sglist_reset(sg); if ((sc->vtnet_flags & VTNET_FLAG_MRG_RXBUFS) == 0) { - MPASS(sc->vtnet_hdr_size == sizeof(rxhdr->vrh_uhdr.hdr) || - sc->vtnet_hdr_size == sizeof(rxhdr->vrh_uhdr.mhdr)); + MPASS(sc->vtnet_hdr_size == sizeof(struct virtio_net_hdr)); rxhdr = (struct vtnet_rx_header *) mdata; - sglist_append(sg, &rxhdr->vrh_uhdr, sc->vtnet_hdr_size); + sglist_append(sg, &rxhdr->vrh_hdr, sc->vtnet_hdr_size); offset = sizeof(struct vtnet_rx_header); } else offset = 0; @@ -1819,10 +1815,9 @@ vtnet_rxq_eof(struct vtnet_rxq *rxq) adjsz = sizeof(struct vtnet_rx_header); /* * Account for our pad inserted between the header - * and the actual start of the frame. This includes - * the unused num_buffers when using a legacy device. + * and the actual start of the frame. */ - len += adjsz - sc->vtnet_hdr_size; + len += VTNET_RX_HEADER_PAD; } else { mhdr = mtod(m, struct virtio_net_hdr_mrg_rxbuf *); nbufs = mhdr->num_buffers; diff --git a/sys/dev/virtio/network/if_vtnetvar.h b/sys/dev/virtio/network/if_vtnetvar.h index 01775cc186f2..24f2c354a298 100644 --- a/sys/dev/virtio/network/if_vtnetvar.h +++ b/sys/dev/virtio/network/if_vtnetvar.h @@ -219,20 +219,15 @@ struct vtnet_softc { * When mergeable buffers are not negotiated, the vtnet_rx_header structure * below is placed at the beginning of the mbuf data. Use 4 bytes of pad to * both keep the VirtIO header and the data non-contiguous and to keep the - * frame's payload 4 byte aligned. Note that non-legacy drivers still want - * room for a full mergeable buffer header. + * frame's payload 4 byte aligned. * * When mergeable buffers are negotiated, the host puts the VirtIO header in * the beginning of the first mbuf's data. */ #define VTNET_RX_HEADER_PAD 4 struct vtnet_rx_header { - union { - struct virtio_net_hdr hdr; - struct virtio_net_hdr_mrg_rxbuf mhdr; - } vrh_uhdr; - - char vrh_pad[VTNET_RX_HEADER_PAD]; + struct virtio_net_hdr vrh_hdr; + char vrh_pad[VTNET_RX_HEADER_PAD]; } __packed; /* @@ -301,8 +296,7 @@ CTASSERT(sizeof(struct vtnet_mac_filter) <= PAGE_SIZE); VIRTIO_NET_F_MRG_RXBUF | \ VIRTIO_NET_F_MQ | \ VIRTIO_RING_F_EVENT_IDX | \ - VIRTIO_RING_F_INDIRECT_DESC | \ - VIRTIO_F_VERSION_1) + VIRTIO_RING_F_INDIRECT_DESC) /* * The VIRTIO_NET_F_HOST_TSO[46] features permit us to send the host diff --git a/sys/dev/virtio/virtio.c b/sys/dev/virtio/virtio.c index a37f7b550c93..b6537e9305ea 100644 --- a/sys/dev/virtio/virtio.c +++ b/sys/dev/virtio/virtio.c @@ -78,7 +78,7 @@ static struct virtio_feature_desc virtio_common_feature_desc[] = { { VIRTIO_RING_F_INDIRECT_DESC, "RingIndirect" }, { VIRTIO_RING_F_EVENT_IDX, "EventIdx" }, { VIRTIO_F_BAD_FEATURE, "BadFeature" }, - { VIRTIO_F_VERSION_1, "Version1" }, + { 0, NULL } }; diff --git a/sys/dev/virtio/virtqueue.c b/sys/dev/virtio/virtqueue.c index 0273c2ad005b..e23d4d25c47f 100644 --- a/sys/dev/virtio/virtqueue.c +++ b/sys/dev/virtio/virtqueue.c @@ -142,7 +142,6 @@ virtqueue_filter_features(uint64_t features) mask = (1 << VIRTIO_TRANSPORT_F_START) - 1; mask |= VIRTIO_RING_F_INDIRECT_DESC; mask |= VIRTIO_RING_F_EVENT_IDX; - mask |= VIRTIO_F_VERSION_1; return (features & mask); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202101190507.10J57xQP085536>