On Thu, Apr 29, 2021 at 10:04:38AM +0200, David Marchand wrote:
> The vhost library current configures Tx offloading (PKT_TX_*) on any
> packet received from a guest virtio device which asks for some offloading.
>
> This is problematic, as Tx offloading is something that the application
> must ask for: the application needs to configure devices
> to support every used offloads (ip, tcp checksumming, tso..), and the
> various l2/l3/l4 lengths must be set following any processing that
> happened in the application itself.
>
> On the other hand, the received packets are not marked wrt current
> packet l3/l4 checksumming info.
>
> Copy virtio rx processing to fix those offload flags but accepting
> VIRTIO_NET_HDR_GSO_ECN and VIRTIO_NET_HDR_GSO_UDP too.
>
> The vhost example has been updated accordingly: TSO is applied to any
> packet marked LRO.
>
> Fixes: 859b480d5afd ("vhost: add guest offload setting")
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> Changes since v1:
> - updated vhost example,
> - restored VIRTIO_NET_HDR_GSO_ECN and VIRTIO_NET_HDR_GSO_UDP support,
> - restored log on buggy offload request,
>
> ---
> examples/vhost/main.c | 42 +++++++------
> lib/vhost/virtio_net.c | 139 +++++++++++++++++------------------------
> 2 files changed, 78 insertions(+), 103 deletions(-)
>
[...]
> - if (l4_hdr && hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> + /* GSO request, save required information in mbuf */
> + if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> + /* Check unsupported modes */
> + if (hdr->gso_size == 0)
> + return -EINVAL;
> +
> switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> case VIRTIO_NET_HDR_GSO_TCPV4:
> case VIRTIO_NET_HDR_GSO_TCPV6:
> - tcp_hdr = l4_hdr;
> - m->ol_flags |= PKT_TX_TCP_SEG;
> - m->tso_segsz = hdr->gso_size;
> - m->l4_len = (tcp_hdr->data_off & 0xf0) >> 2;
> - break;
> case VIRTIO_NET_HDR_GSO_UDP:
> - m->ol_flags |= PKT_TX_UDP_SEG;
> + m->ol_flags |= PKT_RX_LRO | PKT_RX_L4_CKSUM_NONE;
My understanding of the virtio 1.1 spec is that GSO can be
used independently of CSUM. There is nothing preventing to
send a fully checksummed TSO packet.
Anyways, that's unusual and not the goal of this patch.
Acked-by: Flavio Leitner <f...@sysclose.org>
fbl
> + /* Update mss lengths in mbuf */
> m->tso_segsz = hdr->gso_size;
> - m->l4_len = sizeof(struct rte_udp_hdr);
> break;
> default:
> VHOST_LOG_DATA(WARNING,
> "unsupported gso type %u.\n", hdr->gso_type);
> - break;
> + return -EINVAL;
> }
> }
> +
> + return 0;
> }
>
> static __rte_noinline void
> @@ -2084,8 +2054,11 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
> prev->data_len = mbuf_offset;
> m->pkt_len += mbuf_offset;
>
> - if (hdr)
> - vhost_dequeue_offload(hdr, m);
> + if (hdr && vhost_dequeue_offload(hdr, m) < 0) {
> + VHOST_LOG_DATA(ERR, "Packet with invalid offloads.\n");
> + error = -1;
> + goto out;
> + }
>
> out:
>
> --
> 2.23.0
>
--
fbl
