[v2,4/4] vhost: fix offload flags in Rx path

Message ID 20210429080438.15032-5-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series Offload flags fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot success github build: passed
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

David Marchand April 29, 2021, 8:04 a.m. UTC
  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.marchand@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(-)
  

Comments

Maxime Coquelin April 29, 2021, 1:30 p.m. UTC | #1
On 4/29/21 10:04 AM, David Marchand wrote:
> The vhost library current configures Tx offloading (PKT_TX_*) on any

s/current/currently/

> 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.marchand@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(-)

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
  
Maxime Coquelin April 29, 2021, 1:31 p.m. UTC | #2
On 4/29/21 3:30 PM, Maxime Coquelin wrote:
> 
> 
> On 4/29/21 10:04 AM, David Marchand wrote:
>> The vhost library current configures Tx offloading (PKT_TX_*) on any
> 
> s/current/currently/
> 
>> 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.marchand@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(-)
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 

As I understand it, this change kind of break the ABI, but it is
actually fixing a misuse of the mbuf API, so I think we should
take this patch.
  
Flavio Leitner April 29, 2021, 6:39 p.m. UTC | #3
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.marchand@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 <fbl@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
>
  
David Marchand April 29, 2021, 7:18 p.m. UTC | #4
On Thu, Apr 29, 2021 at 8:39 PM Flavio Leitner <fbl@sysclose.org> wrote:
> > -     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.

This forces a superfluous cksum in such a situation.
It can be fixed later if needed.

The virtio pmd rx side has the same behavior.


> Anyways, that's unusual and not the goal of this patch.
>
> Acked-by: Flavio Leitner <fbl@sysclose.org>

Thanks!
  
David Marchand April 29, 2021, 8:09 p.m. UTC | #5
On Thu, Apr 29, 2021 at 3:30 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> On 4/29/21 10:04 AM, David Marchand wrote:
> > The vhost library current configures Tx offloading (PKT_TX_*) on any
>
> s/current/currently/

Ok.

>
> > 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.marchand@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(-)


A release note update is missing.
  
David Marchand April 29, 2021, 8:21 p.m. UTC | #6
On Thu, Apr 29, 2021 at 3:31 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> On 4/29/21 3:30 PM, Maxime Coquelin 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")
>
> As I understand it, this change kind of break the ABI, but it is
> actually fixing a misuse of the mbuf API, so I think we should
> take this patch.

Indeed, this breaks the v21 ABI.

But the only usecase I can think of is an application using TSO /
checksum offloads *only* for traffic coming from vhost.
I say *only* for traffic coming from vhost, because to have this
application do TSO / checksum offloaing for traffic coming from a
physical port, it would comply with the mbuf API and set the PKT_TX_*
flags.

Apart from the example/vhost, I am not sure there is such an
application that only does v2v or v2p but _not_ p2v TSO / checksum
offloading.
(Note: I am unable to use this example... it seems unhappy with the
mlx5 port I use => FPE because this driver does not support vmdq o_O)


I see three options:
- fix the vhost library and break the ABI that only works in an
example (this current patch),
- maintain the v21 ABI
  * using symbol versioning, this adds no branch, recompiled
application use the new ABI, this can't be backported to 20.11,
  * keeping the current behavior by default, but introducing a new
flag that an application would pass to rte_vhost_driver_register().
This new flag triggers this current patch behavior but it would add an
additional branch per packets bulk in vhost dequeue path. This *could*
be backported to 20.11.
  
Maxime Coquelin April 30, 2021, 8:38 a.m. UTC | #7
Hi David,

On 4/29/21 10:21 PM, David Marchand wrote:
> On Thu, Apr 29, 2021 at 3:31 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>> On 4/29/21 3:30 PM, Maxime Coquelin 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")
>>
>> As I understand it, this change kind of break the ABI, but it is
>> actually fixing a misuse of the mbuf API, so I think we should
>> take this patch.
> 
> Indeed, this breaks the v21 ABI.
> 
> But the only usecase I can think of is an application using TSO /
> checksum offloads *only* for traffic coming from vhost.
> I say *only* for traffic coming from vhost, because to have this
> application do TSO / checksum offloaing for traffic coming from a
> physical port, it would comply with the mbuf API and set the PKT_TX_*
> flags.
> 
> Apart from the example/vhost, I am not sure there is such an
> application that only does v2v or v2p but _not_ p2v TSO / checksum
> offloading.
> (Note: I am unable to use this example... it seems unhappy with the
> mlx5 port I use => FPE because this driver does not support vmdq o_O)
> 
> 
> I see three options:
> - fix the vhost library and break the ABI that only works in an
> example (this current patch),
> - maintain the v21 ABI
>   * using symbol versioning, this adds no branch, recompiled
> application use the new ABI, this can't be backported to 20.11,
>   * keeping the current behavior by default, but introducing a new
> flag that an application would pass to rte_vhost_driver_register().
> This new flag triggers this current patch behavior but it would add an
> additional branch per packets bulk in vhost dequeue path. This *could*
> be backported to 20.11.

The flag option seems to be the best option, as it will not break ABI so
applications we don't know about using Vhost offloads won't be impacted
and can add support for the behaviour in a smooth way.

The hardest part with this solution is to find a proper name for that
flag...

Thanks,
Maxime
  

Patch

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index ff48ba270d..4b3df254ba 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -19,6 +19,7 @@ 
 #include <rte_log.h>
 #include <rte_string_fns.h>
 #include <rte_malloc.h>
+#include <rte_net.h>
 #include <rte_vhost.h>
 #include <rte_ip.h>
 #include <rte_tcp.h>
@@ -1032,33 +1033,34 @@  find_local_dest(struct vhost_dev *vdev, struct rte_mbuf *m,
 	return 0;
 }
 
-static uint16_t
-get_psd_sum(void *l3_hdr, uint64_t ol_flags)
-{
-	if (ol_flags & PKT_TX_IPV4)
-		return rte_ipv4_phdr_cksum(l3_hdr, ol_flags);
-	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
-		return rte_ipv6_phdr_cksum(l3_hdr, ol_flags);
-}
-
 static void virtio_tx_offload(struct rte_mbuf *m)
 {
+	struct rte_net_hdr_lens hdr_lens;
+	struct rte_ipv4_hdr *ipv4_hdr;
+	struct rte_tcp_hdr *tcp_hdr;
+	uint32_t ptype;
 	void *l3_hdr;
-	struct rte_ipv4_hdr *ipv4_hdr = NULL;
-	struct rte_tcp_hdr *tcp_hdr = NULL;
-	struct rte_ether_hdr *eth_hdr =
-		rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
-	l3_hdr = (char *)eth_hdr + m->l2_len;
+	ptype = rte_net_get_ptype(m, &hdr_lens, RTE_PTYPE_ALL_MASK);
+	m->l2_len = hdr_lens.l2_len;
+	m->l3_len = hdr_lens.l3_len;
+	m->l4_len = hdr_lens.l4_len;
 
-	if (m->ol_flags & PKT_TX_IPV4) {
+	l3_hdr = rte_pktmbuf_mtod_offset(m, void *, m->l2_len);
+	tcp_hdr = rte_pktmbuf_mtod_offset(m, struct rte_tcp_hdr *,
+		m->l2_len + m->l3_len);
+
+	m->ol_flags |= PKT_TX_TCP_SEG;
+	if ((ptype & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_IPV4) {
+		m->ol_flags |= PKT_TX_IPV4;
+		m->ol_flags |= PKT_TX_IP_CKSUM;
 		ipv4_hdr = l3_hdr;
 		ipv4_hdr->hdr_checksum = 0;
-		m->ol_flags |= PKT_TX_IP_CKSUM;
+		tcp_hdr->cksum = rte_ipv4_phdr_cksum(l3_hdr, m->ol_flags);
+	} else { /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
+		m->ol_flags |= PKT_TX_IPV6;
+		tcp_hdr->cksum = rte_ipv6_phdr_cksum(l3_hdr, m->ol_flags);
 	}
-
-	tcp_hdr = (struct rte_tcp_hdr *)((char *)l3_hdr + m->l3_len);
-	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
 }
 
 static __rte_always_inline void
@@ -1151,7 +1153,7 @@  virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 		m->vlan_tci = vlan_tag;
 	}
 
-	if (m->ol_flags & PKT_TX_TCP_SEG)
+	if (m->ol_flags & PKT_RX_LRO)
 		virtio_tx_offload(m);
 
 	tx_q->m_table[tx_q->len++] = m;
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index ff39878609..da15d11390 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -8,6 +8,7 @@ 
 
 #include <rte_mbuf.h>
 #include <rte_memcpy.h>
+#include <rte_net.h>
 #include <rte_ether.h>
 #include <rte_ip.h>
 #include <rte_vhost.h>
@@ -1827,105 +1828,74 @@  virtio_net_with_host_offload(struct virtio_net *dev)
 	return false;
 }
 
-static void
-parse_ethernet(struct rte_mbuf *m, uint16_t *l4_proto, void **l4_hdr)
-{
-	struct rte_ipv4_hdr *ipv4_hdr;
-	struct rte_ipv6_hdr *ipv6_hdr;
-	void *l3_hdr = NULL;
-	struct rte_ether_hdr *eth_hdr;
-	uint16_t ethertype;
-
-	eth_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
-
-	m->l2_len = sizeof(struct rte_ether_hdr);
-	ethertype = rte_be_to_cpu_16(eth_hdr->ether_type);
-
-	if (ethertype == RTE_ETHER_TYPE_VLAN) {
-		struct rte_vlan_hdr *vlan_hdr =
-			(struct rte_vlan_hdr *)(eth_hdr + 1);
-
-		m->l2_len += sizeof(struct rte_vlan_hdr);
-		ethertype = rte_be_to_cpu_16(vlan_hdr->eth_proto);
-	}
-
-	l3_hdr = (char *)eth_hdr + m->l2_len;
-
-	switch (ethertype) {
-	case RTE_ETHER_TYPE_IPV4:
-		ipv4_hdr = l3_hdr;
-		*l4_proto = ipv4_hdr->next_proto_id;
-		m->l3_len = rte_ipv4_hdr_len(ipv4_hdr);
-		*l4_hdr = (char *)l3_hdr + m->l3_len;
-		m->ol_flags |= PKT_TX_IPV4;
-		break;
-	case RTE_ETHER_TYPE_IPV6:
-		ipv6_hdr = l3_hdr;
-		*l4_proto = ipv6_hdr->proto;
-		m->l3_len = sizeof(struct rte_ipv6_hdr);
-		*l4_hdr = (char *)l3_hdr + m->l3_len;
-		m->ol_flags |= PKT_TX_IPV6;
-		break;
-	default:
-		m->l3_len = 0;
-		*l4_proto = 0;
-		*l4_hdr = NULL;
-		break;
-	}
-}
-
-static __rte_always_inline void
+static __rte_always_inline int
 vhost_dequeue_offload(struct virtio_net_hdr *hdr, struct rte_mbuf *m)
 {
-	uint16_t l4_proto = 0;
-	void *l4_hdr = NULL;
-	struct rte_tcp_hdr *tcp_hdr = NULL;
+	struct rte_net_hdr_lens hdr_lens;
+	uint32_t hdrlen, ptype;
+	int l4_supported = 0;
 
+	/* nothing to do */
 	if (hdr->flags == 0 && hdr->gso_type == VIRTIO_NET_HDR_GSO_NONE)
-		return;
-
-	parse_ethernet(m, &l4_proto, &l4_hdr);
-	if (hdr->flags == VIRTIO_NET_HDR_F_NEEDS_CSUM) {
-		if (hdr->csum_start == (m->l2_len + m->l3_len)) {
-			switch (hdr->csum_offset) {
-			case (offsetof(struct rte_tcp_hdr, cksum)):
-				if (l4_proto == IPPROTO_TCP)
-					m->ol_flags |= PKT_TX_TCP_CKSUM;
-				break;
-			case (offsetof(struct rte_udp_hdr, dgram_cksum)):
-				if (l4_proto == IPPROTO_UDP)
-					m->ol_flags |= PKT_TX_UDP_CKSUM;
-				break;
-			case (offsetof(struct rte_sctp_hdr, cksum)):
-				if (l4_proto == IPPROTO_SCTP)
-					m->ol_flags |= PKT_TX_SCTP_CKSUM;
-				break;
-			default:
-				break;
-			}
+		return 0;
+
+	m->ol_flags |= PKT_RX_IP_CKSUM_UNKNOWN;
+
+	ptype = rte_net_get_ptype(m, &hdr_lens, RTE_PTYPE_ALL_MASK);
+	m->packet_type = ptype;
+	if ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_TCP ||
+	    (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_UDP ||
+	    (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_SCTP)
+		l4_supported = 1;
+
+	if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		hdrlen = hdr_lens.l2_len + hdr_lens.l3_len + hdr_lens.l4_len;
+		if (hdr->csum_start <= hdrlen && l4_supported) {
+			m->ol_flags |= PKT_RX_L4_CKSUM_NONE;
+		} else {
+			/* Unknown proto or tunnel, do sw cksum. We can assume
+			 * the cksum field is in the first segment since the
+			 * buffers we provided to the host are large enough.
+			 * In case of SCTP, this will be wrong since it's a CRC
+			 * but there's nothing we can do.
+			 */
+			uint16_t csum = 0, off;
+
+			if (rte_raw_cksum_mbuf(m, hdr->csum_start,
+					rte_pktmbuf_pkt_len(m) - hdr->csum_start, &csum) < 0)
+				return -EINVAL;
+			if (likely(csum != 0xffff))
+				csum = ~csum;
+			off = hdr->csum_offset + hdr->csum_start;
+			if (rte_pktmbuf_data_len(m) >= off + 1)
+				*rte_pktmbuf_mtod_offset(m, uint16_t *, off) = csum;
 		}
+	} else if (hdr->flags & VIRTIO_NET_HDR_F_DATA_VALID && l4_supported) {
+		m->ol_flags |= PKT_RX_L4_CKSUM_GOOD;
 	}
 
-	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;
+			/* 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: