[dpdk-dev] [PATCH v1] net/mlx4: improve Rx packet type offloads report

[dpdk-dev] [PATCH v1] net/mlx4: improve Rx packet type offloads report

[Moti Haimovsky]

This patch improves Rx packet type offload report in case the device is
a virtual function device. L2 tunnel indications are not reported by
those devices and therefore should not be checked by the PMD.

Fixes: aee4a03fee4f ("net/mlx4: enhance Rx packet type offloads")

Signed-off-by: Moti Haimovsky <motih@mellanox.com>
---
 drivers/net/mlx4/mlx4.c      | 2 ++
 drivers/net/mlx4/mlx4.h      | 1 +
 drivers/net/mlx4/mlx4_rxq.c  | 1 +
 drivers/net/mlx4/mlx4_rxtx.c | 9 ++++++---
 drivers/net/mlx4/mlx4_rxtx.h | 1 +
 5 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index f9e4f9d..efff65d 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -573,6 +573,8 @@ struct mlx4_conf {
 			 PCI_DEVICE_ID_MELLANOX_CONNECTX3PRO);
 		DEBUG("L2 tunnel checksum offloads are %ssupported",
 		      (priv->hw_csum_l2tun ? "" : "not "));
+		/* VFs are not configured to offload L2 tunnels */
+		priv->hw_l2tun_offload = !vf;
 		/* Configure the first MAC address by default. */
 		if (mlx4_get_mac(priv, &mac.addr_bytes)) {
 			ERROR("cannot get MAC address, is mlx4_en loaded?"
diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
index 3aeef87..cbb8628 100644
--- a/drivers/net/mlx4/mlx4.h
+++ b/drivers/net/mlx4/mlx4.h
@@ -128,6 +128,7 @@ struct priv {
 	uint32_t isolated:1; /**< Toggle isolated mode. */
 	uint32_t hw_csum:1; /* Checksum offload is supported. */
 	uint32_t hw_csum_l2tun:1; /* Checksum support for L2 tunnels. */
+	uint32_t hw_l2tun_offload:1; /**< L2 tunnel offload is configured. */
 	struct rte_intr_handle intr_handle; /**< Port interrupt handle. */
 	struct mlx4_drop *drop; /**< Shared resources for drop flow rules. */
 	LIST_HEAD(, mlx4_rss) rss; /**< Shared targets for Rx flow rules. */
diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index 8b97a89..802be84 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -750,6 +750,7 @@ struct mlx4_rss *
 			 dev->data->dev_conf.rxmode.hw_ip_checksum),
 		.csum_l2tun = (priv->hw_csum_l2tun &&
 			       dev->data->dev_conf.rxmode.hw_ip_checksum),
+		.l2tun_offload = priv->hw_l2tun_offload,
 		.stats = {
 			.idx = idx,
 		},
diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c
index 3985e06..1131d56 100644
--- a/drivers/net/mlx4/mlx4_rxtx.c
+++ b/drivers/net/mlx4/mlx4_rxtx.c
@@ -37,6 +37,7 @@
  */
 
 #include <assert.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <string.h>
 
@@ -751,7 +752,8 @@ struct pv {
  *   Packet type for struct rte_mbuf.
  */
 static inline uint32_t
-rxq_cq_to_pkt_type(volatile struct mlx4_cqe *cqe)
+rxq_cq_to_pkt_type(volatile struct mlx4_cqe *cqe,
+		   uint32_t l2tun_offload)
 {
 	uint8_t idx = 0;
 	uint32_t pinfo = rte_be_to_cpu_32(cqe->vlan_my_qpn);
@@ -762,7 +764,7 @@ struct pv {
 	 *  bit[7] - MLX4_CQE_L2_TUNNEL
 	 *  bit[6] - MLX4_CQE_L2_TUNNEL_IPV4
 	 */
-	if (!(pinfo & MLX4_CQE_L2_VLAN_MASK) && (pinfo & MLX4_CQE_L2_TUNNEL))
+	if (l2tun_offload && (pinfo & MLX4_CQE_L2_TUNNEL))
 		idx |= ((pinfo & MLX4_CQE_L2_TUNNEL) >> 20) |
 		       ((pinfo & MLX4_CQE_L2_TUNNEL_IPV4) >> 19);
 	/*
@@ -960,7 +962,8 @@ struct pv {
 			}
 			pkt = seg;
 			/* Update packet information. */
-			pkt->packet_type = rxq_cq_to_pkt_type(cqe);
+			pkt->packet_type =
+				rxq_cq_to_pkt_type(cqe, rxq->l2tun_offload);
 			pkt->ol_flags = 0;
 			pkt->pkt_len = len;
 			if (rxq->csum | rxq->csum_l2tun) {
diff --git a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h
index 4acad80..463df2b 100644
--- a/drivers/net/mlx4/mlx4_rxtx.h
+++ b/drivers/net/mlx4/mlx4_rxtx.h
@@ -80,6 +80,7 @@ struct rxq {
 	volatile uint32_t *rq_db; /**< RQ doorbell record. */
 	uint32_t csum:1; /**< Enable checksum offloading. */
 	uint32_t csum_l2tun:1; /**< Same for L2 tunnels. */
+	uint32_t l2tun_offload:1; /**< L2 tunnel offload is enabled. */
 	struct mlx4_cq mcq;  /**< Info for directly manipulating the CQ. */
 	struct mlx4_rxq_stats stats; /**< Rx queue counters. */
 	unsigned int socket; /**< CPU socket ID for allocations. */
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v1] net/mlx4: improve Rx packet type offloads report

[Adrien Mazarguil]

Hi Moti,

On Wed, Nov 08, 2017 at 02:02:45PM +0200, Moti Haimovsky wrote:
> This patch improves Rx packet type offload report in case the device is
> a virtual function device. L2 tunnel indications are not reported by
> those devices and therefore should not be checked by the PMD.
> 
> Fixes: aee4a03fee4f ("net/mlx4: enhance Rx packet type offloads")
> 
> Signed-off-by: Moti Haimovsky <motih@mellanox.com>

Does "not reporting them" cause any issue? Is this patch adding anything on
top of checking they can't be reported before not reporting them either?

Otherwise this additional unnecessary check may cause a minor performance
regression for no good reason.

I think this patch should really update mlx4_dev_supported_ptypes_get()
(control path) instead of rxq_cq_to_pkt_type() (data path). What's your
opinion?

A few other suggestions, see below.

> ---
>  drivers/net/mlx4/mlx4.c      | 2 ++
>  drivers/net/mlx4/mlx4.h      | 1 +
>  drivers/net/mlx4/mlx4_rxq.c  | 1 +
>  drivers/net/mlx4/mlx4_rxtx.c | 9 ++++++---
>  drivers/net/mlx4/mlx4_rxtx.h | 1 +
>  5 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index f9e4f9d..efff65d 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -573,6 +573,8 @@ struct mlx4_conf {
>  			 PCI_DEVICE_ID_MELLANOX_CONNECTX3PRO);
>  		DEBUG("L2 tunnel checksum offloads are %ssupported",
>  		      (priv->hw_csum_l2tun ? "" : "not "));
> +		/* VFs are not configured to offload L2 tunnels */
> +		priv->hw_l2tun_offload = !vf;

Seeing how you're adding a new bit to this field, is hw_l2tun_offload really
different from hw_csum_l2tun? 

Can you get inner VXLAN checksums if the packet can't be recognized as VXLAN
in the first place? I don't think so.

Perhaps hw_csum_l2tun should be renamed, however in my opinion you could
simply update the hw_csum_l2tun assignment with a vf check and use that.

>  		/* Configure the first MAC address by default. */
>  		if (mlx4_get_mac(priv, &mac.addr_bytes)) {
>  			ERROR("cannot get MAC address, is mlx4_en loaded?"
> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
> index 3aeef87..cbb8628 100644
> --- a/drivers/net/mlx4/mlx4.h
> +++ b/drivers/net/mlx4/mlx4.h
> @@ -128,6 +128,7 @@ struct priv {
>  	uint32_t isolated:1; /**< Toggle isolated mode. */
>  	uint32_t hw_csum:1; /* Checksum offload is supported. */
>  	uint32_t hw_csum_l2tun:1; /* Checksum support for L2 tunnels. */
> +	uint32_t hw_l2tun_offload:1; /**< L2 tunnel offload is configured. */

This change would become unnecessary.

>  	struct rte_intr_handle intr_handle; /**< Port interrupt handle. */
>  	struct mlx4_drop *drop; /**< Shared resources for drop flow rules. */
>  	LIST_HEAD(, mlx4_rss) rss; /**< Shared targets for Rx flow rules. */
> diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
> index 8b97a89..802be84 100644
> --- a/drivers/net/mlx4/mlx4_rxq.c
> +++ b/drivers/net/mlx4/mlx4_rxq.c
> @@ -750,6 +750,7 @@ struct mlx4_rss *
>  			 dev->data->dev_conf.rxmode.hw_ip_checksum),
>  		.csum_l2tun = (priv->hw_csum_l2tun &&
>  			       dev->data->dev_conf.rxmode.hw_ip_checksum),
> +		.l2tun_offload = priv->hw_l2tun_offload,

Assuming a data path change is really needed, this one could likely stay
since it doesn't depend on the user enabling checksums.

>  		.stats = {
>  			.idx = idx,
>  		},
> diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c
> index 3985e06..1131d56 100644
> --- a/drivers/net/mlx4/mlx4_rxtx.c
> +++ b/drivers/net/mlx4/mlx4_rxtx.c
> @@ -37,6 +37,7 @@
>   */
>  
>  #include <assert.h>
> +#include <stdbool.h>

What for?

>  #include <stdint.h>
>  #include <string.h>
>  
> @@ -751,7 +752,8 @@ struct pv {
>   *   Packet type for struct rte_mbuf.
>   */
>  static inline uint32_t
> -rxq_cq_to_pkt_type(volatile struct mlx4_cqe *cqe)
> +rxq_cq_to_pkt_type(volatile struct mlx4_cqe *cqe,
> +		   uint32_t l2tun_offload)
>  {
>  	uint8_t idx = 0;
>  	uint32_t pinfo = rte_be_to_cpu_32(cqe->vlan_my_qpn);
> @@ -762,7 +764,7 @@ struct pv {
>  	 *  bit[7] - MLX4_CQE_L2_TUNNEL
>  	 *  bit[6] - MLX4_CQE_L2_TUNNEL_IPV4
>  	 */
> -	if (!(pinfo & MLX4_CQE_L2_VLAN_MASK) && (pinfo & MLX4_CQE_L2_TUNNEL))
> +	if (l2tun_offload && (pinfo & MLX4_CQE_L2_TUNNEL))
>  		idx |= ((pinfo & MLX4_CQE_L2_TUNNEL) >> 20) |
>  		       ((pinfo & MLX4_CQE_L2_TUNNEL_IPV4) >> 19);
>  	/*
> @@ -960,7 +962,8 @@ struct pv {
>  			}
>  			pkt = seg;
>  			/* Update packet information. */
> -			pkt->packet_type = rxq_cq_to_pkt_type(cqe);
> +			pkt->packet_type =
> +				rxq_cq_to_pkt_type(cqe, rxq->l2tun_offload);
>  			pkt->ol_flags = 0;
>  			pkt->pkt_len = len;
>  			if (rxq->csum | rxq->csum_l2tun) {
> diff --git a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h
> index 4acad80..463df2b 100644
> --- a/drivers/net/mlx4/mlx4_rxtx.h
> +++ b/drivers/net/mlx4/mlx4_rxtx.h
> @@ -80,6 +80,7 @@ struct rxq {
>  	volatile uint32_t *rq_db; /**< RQ doorbell record. */
>  	uint32_t csum:1; /**< Enable checksum offloading. */
>  	uint32_t csum_l2tun:1; /**< Same for L2 tunnels. */
> +	uint32_t l2tun_offload:1; /**< L2 tunnel offload is enabled. */
>  	struct mlx4_cq mcq;  /**< Info for directly manipulating the CQ. */
>  	struct mlx4_rxq_stats stats; /**< Rx queue counters. */
>  	unsigned int socket; /**< CPU socket ID for allocations. */
> -- 
> 1.8.3.1
> 

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH v1] net/mlx4: improve Rx packet type offloads report

[Mordechay Haimovsky]

Inline

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Wednesday, November 8, 2017 4:58 PM
> To: Mordechay Haimovsky <motih@mellanox.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v1] net/mlx4: improve Rx packet type offloads report
> 
> Hi Moti,
> 
> On Wed, Nov 08, 2017 at 02:02:45PM +0200, Moti Haimovsky wrote:
> > This patch improves Rx packet type offload report in case the device
> > is a virtual function device. L2 tunnel indications are not reported
> > by those devices and therefore should not be checked by the PMD.
> >
> > Fixes: aee4a03fee4f ("net/mlx4: enhance Rx packet type offloads")
> >
> > Signed-off-by: Moti Haimovsky <motih@mellanox.com>
> 
> Does "not reporting them" cause any issue? Is this patch adding anything on
> top of checking they can't be reported before not reporting them either?
> 
> Otherwise this additional unnecessary check may cause a minor performance
> regression for no good reason.
> 
In VFs (sriov VF devices) we see that the L2 tunnel flag is set which causes a complete misinterpretation of the packet type being received.
This happens since the tunnel_mode is not set to 0x7 by the driver and therefore has meaningless value in such case and should be ignored.
And yes, performance-wise there is probably an impact.
Another approach which will not affect performance can be to init the mlx4_ptype_table according to the device at hand (i.e. per-device table).
This however will require some effort :)

> I think this patch should really update mlx4_dev_supported_ptypes_get()
> (control path) instead of rxq_cq_to_pkt_type() (data path). What's your
> opinion?
> 
> A few other suggestions, see below.
> 
> > ---
> >  drivers/net/mlx4/mlx4.c      | 2 ++
> >  drivers/net/mlx4/mlx4.h      | 1 +
> >  drivers/net/mlx4/mlx4_rxq.c  | 1 +
> >  drivers/net/mlx4/mlx4_rxtx.c | 9 ++++++---
> > drivers/net/mlx4/mlx4_rxtx.h | 1 +
> >  5 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index
> > f9e4f9d..efff65d 100644
> > --- a/drivers/net/mlx4/mlx4.c
> > +++ b/drivers/net/mlx4/mlx4.c
> > @@ -573,6 +573,8 @@ struct mlx4_conf {
> >  			 PCI_DEVICE_ID_MELLANOX_CONNECTX3PRO);
> >  		DEBUG("L2 tunnel checksum offloads are %ssupported",
> >  		      (priv->hw_csum_l2tun ? "" : "not "));
> > +		/* VFs are not configured to offload L2 tunnels */
> > +		priv->hw_l2tun_offload = !vf;
> 
> Seeing how you're adding a new bit to this field, is hw_l2tun_offload really
> different from hw_csum_l2tun?
> 
> Can you get inner VXLAN checksums if the packet can't be recognized as
> VXLAN in the first place? I don't think so.
> 
> Perhaps hw_csum_l2tun should be renamed, however in my opinion you
> could simply update the hw_csum_l2tun assignment with a vf check and use
> that.
> 
> >  		/* Configure the first MAC address by default. */
> >  		if (mlx4_get_mac(priv, &mac.addr_bytes)) {
> >  			ERROR("cannot get MAC address, is mlx4_en
> loaded?"
> > diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h index
> > 3aeef87..cbb8628 100644
> > --- a/drivers/net/mlx4/mlx4.h
> > +++ b/drivers/net/mlx4/mlx4.h
> > @@ -128,6 +128,7 @@ struct priv {
> >  	uint32_t isolated:1; /**< Toggle isolated mode. */
> >  	uint32_t hw_csum:1; /* Checksum offload is supported. */
> >  	uint32_t hw_csum_l2tun:1; /* Checksum support for L2 tunnels. */
> > +	uint32_t hw_l2tun_offload:1; /**< L2 tunnel offload is configured.
> > +*/
> 
> This change would become unnecessary.
> 
> >  	struct rte_intr_handle intr_handle; /**< Port interrupt handle. */
> >  	struct mlx4_drop *drop; /**< Shared resources for drop flow rules.
> */
> >  	LIST_HEAD(, mlx4_rss) rss; /**< Shared targets for Rx flow rules. */
> > diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
> > index 8b97a89..802be84 100644
> > --- a/drivers/net/mlx4/mlx4_rxq.c
> > +++ b/drivers/net/mlx4/mlx4_rxq.c
> > @@ -750,6 +750,7 @@ struct mlx4_rss *
> >  			 dev->data->dev_conf.rxmode.hw_ip_checksum),
> >  		.csum_l2tun = (priv->hw_csum_l2tun &&
> >  			       dev->data-
> >dev_conf.rxmode.hw_ip_checksum),
> > +		.l2tun_offload = priv->hw_l2tun_offload,
> 
> Assuming a data path change is really needed, this one could likely stay since
> it doesn't depend on the user enabling checksums.
> 
> >  		.stats = {
> >  			.idx = idx,
> >  		},
> > diff --git a/drivers/net/mlx4/mlx4_rxtx.c
> > b/drivers/net/mlx4/mlx4_rxtx.c index 3985e06..1131d56 100644
> > --- a/drivers/net/mlx4/mlx4_rxtx.c
> > +++ b/drivers/net/mlx4/mlx4_rxtx.c
> > @@ -37,6 +37,7 @@
> >   */
> >
> >  #include <assert.h>
> > +#include <stdbool.h>
> 
> What for?
> 
> >  #include <stdint.h>
> >  #include <string.h>
> >
> > @@ -751,7 +752,8 @@ struct pv {
> >   *   Packet type for struct rte_mbuf.
> >   */
> >  static inline uint32_t
> > -rxq_cq_to_pkt_type(volatile struct mlx4_cqe *cqe)
> > +rxq_cq_to_pkt_type(volatile struct mlx4_cqe *cqe,
> > +		   uint32_t l2tun_offload)
> >  {
> >  	uint8_t idx = 0;
> >  	uint32_t pinfo = rte_be_to_cpu_32(cqe->vlan_my_qpn);
> > @@ -762,7 +764,7 @@ struct pv {
> >  	 *  bit[7] - MLX4_CQE_L2_TUNNEL
> >  	 *  bit[6] - MLX4_CQE_L2_TUNNEL_IPV4
> >  	 */
> > -	if (!(pinfo & MLX4_CQE_L2_VLAN_MASK) && (pinfo &
> MLX4_CQE_L2_TUNNEL))
> > +	if (l2tun_offload && (pinfo & MLX4_CQE_L2_TUNNEL))
> >  		idx |= ((pinfo & MLX4_CQE_L2_TUNNEL) >> 20) |
> >  		       ((pinfo & MLX4_CQE_L2_TUNNEL_IPV4) >> 19);
> >  	/*
> > @@ -960,7 +962,8 @@ struct pv {
> >  			}
> >  			pkt = seg;
> >  			/* Update packet information. */
> > -			pkt->packet_type = rxq_cq_to_pkt_type(cqe);
> > +			pkt->packet_type =
> > +				rxq_cq_to_pkt_type(cqe, rxq-
> >l2tun_offload);
> >  			pkt->ol_flags = 0;
> >  			pkt->pkt_len = len;
> >  			if (rxq->csum | rxq->csum_l2tun) { diff --git
> > a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h index
> > 4acad80..463df2b 100644
> > --- a/drivers/net/mlx4/mlx4_rxtx.h
> > +++ b/drivers/net/mlx4/mlx4_rxtx.h
> > @@ -80,6 +80,7 @@ struct rxq {
> >  	volatile uint32_t *rq_db; /**< RQ doorbell record. */
> >  	uint32_t csum:1; /**< Enable checksum offloading. */
> >  	uint32_t csum_l2tun:1; /**< Same for L2 tunnels. */
> > +	uint32_t l2tun_offload:1; /**< L2 tunnel offload is enabled. */
> >  	struct mlx4_cq mcq;  /**< Info for directly manipulating the CQ. */
> >  	struct mlx4_rxq_stats stats; /**< Rx queue counters. */
> >  	unsigned int socket; /**< CPU socket ID for allocations. */
> > --
> > 1.8.3.1
> >
> 
> --
> Adrien Mazarguil
> 6WIND

Re: [dpdk-dev] [PATCH v1] net/mlx4: improve Rx packet type offloads report

[Adrien Mazarguil]

On Wed, Nov 08, 2017 at 03:33:57PM +0000, Mordechay Haimovsky wrote:
> Inline
> 
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Wednesday, November 8, 2017 4:58 PM
> > To: Mordechay Haimovsky <motih@mellanox.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH v1] net/mlx4: improve Rx packet type offloads report
> > 
> > Hi Moti,
> > 
> > On Wed, Nov 08, 2017 at 02:02:45PM +0200, Moti Haimovsky wrote:
> > > This patch improves Rx packet type offload report in case the device
> > > is a virtual function device. L2 tunnel indications are not reported
> > > by those devices and therefore should not be checked by the PMD.
> > >
> > > Fixes: aee4a03fee4f ("net/mlx4: enhance Rx packet type offloads")
> > >
> > > Signed-off-by: Moti Haimovsky <motih@mellanox.com>
> > 
> > Does "not reporting them" cause any issue? Is this patch adding anything on
> > top of checking they can't be reported before not reporting them either?
> > 
> > Otherwise this additional unnecessary check may cause a minor performance
> > regression for no good reason.
> > 
> In VFs (sriov VF devices) we see that the L2 tunnel flag is set which causes a complete misinterpretation of the packet type being received.
> This happens since the tunnel_mode is not set to 0x7 by the driver and therefore has meaningless value in such case and should be ignored.

OK, please mention this in the commit log then, otherwise it doesn't look
like this patch addresses anything.

> And yes, performance-wise there is probably an impact.
> Another approach which will not affect performance can be to init the mlx4_ptype_table according to the device at hand (i.e. per-device table).
> This however will require some effort :)

My remaining comments still stand, particularly the need to update
mlx4_dev_supported_ptypes_get() as well.

> 
> > I think this patch should really update mlx4_dev_supported_ptypes_get()
> > (control path) instead of rxq_cq_to_pkt_type() (data path). What's your
> > opinion?
> > 
> > A few other suggestions, see below.
> > 
> > > ---
> > >  drivers/net/mlx4/mlx4.c      | 2 ++
> > >  drivers/net/mlx4/mlx4.h      | 1 +
> > >  drivers/net/mlx4/mlx4_rxq.c  | 1 +
> > >  drivers/net/mlx4/mlx4_rxtx.c | 9 ++++++---
> > > drivers/net/mlx4/mlx4_rxtx.h | 1 +
> > >  5 files changed, 11 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index
> > > f9e4f9d..efff65d 100644
> > > --- a/drivers/net/mlx4/mlx4.c
> > > +++ b/drivers/net/mlx4/mlx4.c
> > > @@ -573,6 +573,8 @@ struct mlx4_conf {
> > >  			 PCI_DEVICE_ID_MELLANOX_CONNECTX3PRO);
> > >  		DEBUG("L2 tunnel checksum offloads are %ssupported",
> > >  		      (priv->hw_csum_l2tun ? "" : "not "));
> > > +		/* VFs are not configured to offload L2 tunnels */
> > > +		priv->hw_l2tun_offload = !vf;
> > 
> > Seeing how you're adding a new bit to this field, is hw_l2tun_offload really
> > different from hw_csum_l2tun?
> > 
> > Can you get inner VXLAN checksums if the packet can't be recognized as
> > VXLAN in the first place? I don't think so.
> > 
> > Perhaps hw_csum_l2tun should be renamed, however in my opinion you
> > could simply update the hw_csum_l2tun assignment with a vf check and use
> > that.
> > 
> > >  		/* Configure the first MAC address by default. */
> > >  		if (mlx4_get_mac(priv, &mac.addr_bytes)) {
> > >  			ERROR("cannot get MAC address, is mlx4_en
> > loaded?"
> > > diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h index
> > > 3aeef87..cbb8628 100644
> > > --- a/drivers/net/mlx4/mlx4.h
> > > +++ b/drivers/net/mlx4/mlx4.h
> > > @@ -128,6 +128,7 @@ struct priv {
> > >  	uint32_t isolated:1; /**< Toggle isolated mode. */
> > >  	uint32_t hw_csum:1; /* Checksum offload is supported. */
> > >  	uint32_t hw_csum_l2tun:1; /* Checksum support for L2 tunnels. */
> > > +	uint32_t hw_l2tun_offload:1; /**< L2 tunnel offload is configured.
> > > +*/
> > 
> > This change would become unnecessary.
> > 
> > >  	struct rte_intr_handle intr_handle; /**< Port interrupt handle. */
> > >  	struct mlx4_drop *drop; /**< Shared resources for drop flow rules.
> > */
> > >  	LIST_HEAD(, mlx4_rss) rss; /**< Shared targets for Rx flow rules. */
> > > diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
> > > index 8b97a89..802be84 100644
> > > --- a/drivers/net/mlx4/mlx4_rxq.c
> > > +++ b/drivers/net/mlx4/mlx4_rxq.c
> > > @@ -750,6 +750,7 @@ struct mlx4_rss *
> > >  			 dev->data->dev_conf.rxmode.hw_ip_checksum),
> > >  		.csum_l2tun = (priv->hw_csum_l2tun &&
> > >  			       dev->data-
> > >dev_conf.rxmode.hw_ip_checksum),
> > > +		.l2tun_offload = priv->hw_l2tun_offload,
> > 
> > Assuming a data path change is really needed, this one could likely stay since
> > it doesn't depend on the user enabling checksums.
> > 
> > >  		.stats = {
> > >  			.idx = idx,
> > >  		},
> > > diff --git a/drivers/net/mlx4/mlx4_rxtx.c
> > > b/drivers/net/mlx4/mlx4_rxtx.c index 3985e06..1131d56 100644
> > > --- a/drivers/net/mlx4/mlx4_rxtx.c
> > > +++ b/drivers/net/mlx4/mlx4_rxtx.c
> > > @@ -37,6 +37,7 @@
> > >   */
> > >
> > >  #include <assert.h>
> > > +#include <stdbool.h>
> > 
> > What for?
> > 
> > >  #include <stdint.h>
> > >  #include <string.h>
> > >
> > > @@ -751,7 +752,8 @@ struct pv {
> > >   *   Packet type for struct rte_mbuf.
> > >   */
> > >  static inline uint32_t
> > > -rxq_cq_to_pkt_type(volatile struct mlx4_cqe *cqe)
> > > +rxq_cq_to_pkt_type(volatile struct mlx4_cqe *cqe,
> > > +		   uint32_t l2tun_offload)
> > >  {
> > >  	uint8_t idx = 0;
> > >  	uint32_t pinfo = rte_be_to_cpu_32(cqe->vlan_my_qpn);
> > > @@ -762,7 +764,7 @@ struct pv {
> > >  	 *  bit[7] - MLX4_CQE_L2_TUNNEL
> > >  	 *  bit[6] - MLX4_CQE_L2_TUNNEL_IPV4
> > >  	 */
> > > -	if (!(pinfo & MLX4_CQE_L2_VLAN_MASK) && (pinfo &
> > MLX4_CQE_L2_TUNNEL))
> > > +	if (l2tun_offload && (pinfo & MLX4_CQE_L2_TUNNEL))
> > >  		idx |= ((pinfo & MLX4_CQE_L2_TUNNEL) >> 20) |
> > >  		       ((pinfo & MLX4_CQE_L2_TUNNEL_IPV4) >> 19);
> > >  	/*
> > > @@ -960,7 +962,8 @@ struct pv {
> > >  			}
> > >  			pkt = seg;
> > >  			/* Update packet information. */
> > > -			pkt->packet_type = rxq_cq_to_pkt_type(cqe);
> > > +			pkt->packet_type =
> > > +				rxq_cq_to_pkt_type(cqe, rxq-
> > >l2tun_offload);
> > >  			pkt->ol_flags = 0;
> > >  			pkt->pkt_len = len;
> > >  			if (rxq->csum | rxq->csum_l2tun) { diff --git
> > > a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h index
> > > 4acad80..463df2b 100644
> > > --- a/drivers/net/mlx4/mlx4_rxtx.h
> > > +++ b/drivers/net/mlx4/mlx4_rxtx.h
> > > @@ -80,6 +80,7 @@ struct rxq {
> > >  	volatile uint32_t *rq_db; /**< RQ doorbell record. */
> > >  	uint32_t csum:1; /**< Enable checksum offloading. */
> > >  	uint32_t csum_l2tun:1; /**< Same for L2 tunnels. */
> > > +	uint32_t l2tun_offload:1; /**< L2 tunnel offload is enabled. */
> > >  	struct mlx4_cq mcq;  /**< Info for directly manipulating the CQ. */
> > >  	struct mlx4_rxq_stats stats; /**< Rx queue counters. */
> > >  	unsigned int socket; /**< CPU socket ID for allocations. */
> > > --
> > > 1.8.3.1
> > >
> > 
> > --
> > Adrien Mazarguil
> > 6WIND

-- 
Adrien Mazarguil
6WIND