[dpdk-dev] [PATCH] nfp: add support for new metadata api

[dpdk-dev] [PATCH] nfp: add support for new metadata api

[Alejandro Lucero]

NFP is a smart programmable NIC and firmware is deployed for specific
system needs, like offloading OVS, vRouter, contrack or eBPF into the
hardware. This often requires to give metadata to the host within
packets delivered. Last NFP firmware implementations support richer
metadata api facilitating interaction between firmware and host code.

Old way of handling metadata needs to be still there for supporting
old firmware.

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 drivers/net/nfp/nfp_net.c      | 33 +++++++++++++++++++++++++++------
 drivers/net/nfp/nfp_net_ctrl.h |  6 ++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index cee8f63..69ae6d5 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1726,6 +1726,7 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 
 #define NFP_HASH_OFFSET      ((uint8_t *)mbuf->buf_addr + mbuf->data_off - 4)
 #define NFP_HASH_TYPE_OFFSET ((uint8_t *)mbuf->buf_addr + mbuf->data_off - 8)
+#define NFP_DESC_META_LEN(d) (d->rxd.meta_len_dd & PCIE_DESC_RX_META_LEN_MASK)
 
 /*
  * nfp_net_set_hash - Set mbuf hash data
@@ -1739,16 +1740,38 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 {
 	uint32_t hash;
 	uint32_t hash_type;
+	uint32_t meta_info;
+	uint8_t *meta_offset;
 	struct nfp_net_hw *hw = rxq->hw;
 
 	if (!(hw->ctrl & NFP_NET_CFG_CTRL_RSS))
 		return;
 
-	if (!(rxd->rxd.flags & PCIE_DESC_RX_RSS))
+	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) <= 3) {
+		if (!(rxd->rxd.flags & PCIE_DESC_RX_RSS))
+			return;
+
+		hash = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_OFFSET);
+		hash_type = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_TYPE_OFFSET);
+
+	} else if (NFP_DESC_META_LEN(rxd)) {
+		meta_offset = (uint8_t *)mbuf->buf_addr;
+		meta_info = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
+		meta_offset += 4;
+		/* NFP PMD just supports metadata for hashing */
+		switch (meta_info & NFP_NET_META_FIELD_MASK) {
+		case NFP_NET_META_HASH:
+			meta_info >>= NFP_NET_META_FIELD_SIZE;
+			hash = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
+			hash_type = meta_info && NFP_NET_META_FIELD_MASK;
+			break;
+		default:
+			/* Unsupported metadata can be a performance issue */
+			return;
+		}
+	} else {
 		return;
-
-	hash = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_OFFSET);
-	hash_type = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_TYPE_OFFSET);
+	}
 
 	mbuf->hash.rss = hash;
 	mbuf->ol_flags |= PKT_RX_RSS_HASH;
@@ -1774,8 +1797,6 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 	rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
 }
 
-#define NFP_DESC_META_LEN(d) (d->rxd.meta_len_dd & PCIE_DESC_RX_META_LEN_MASK)
-
 /*
  * RX path design:
  *
diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
index 2c50043..281205d 100644
--- a/drivers/net/nfp/nfp_net_ctrl.h
+++ b/drivers/net/nfp/nfp_net_ctrl.h
@@ -52,6 +52,12 @@
 /* Offset in Freelist buffer where packet starts on RX */
 #define NFP_NET_RX_OFFSET               32
 
+/* Prepend field types */
+#define NFP_NET_META_FIELD_SIZE         4
+#define NFP_NET_META_HASH               1 /* next field carries hash type */
+#define NFP_NET_META_MARK               2
+#define NFP_NET_META_FIELD_MASK         (0xf)
+
 /* Hash type pre-pended when a RSS hash was computed */
 #define NFP_NET_RSS_NONE                0
 #define NFP_NET_RSS_IPV4                1
-- 
1.9.1

Re: [dpdk-dev] [PATCH] nfp: add support for new metadata api

[Ferruh Yigit]

On 12/20/2016 2:13 PM, Alejandro Lucero wrote:
> NFP is a smart programmable NIC and firmware is deployed for specific
> system needs, like offloading OVS, vRouter, contrack or eBPF into the
> hardware. This often requires to give metadata to the host within
> packets delivered. Last NFP firmware implementations support richer
> metadata api facilitating interaction between firmware and host code.
> 
> Old way of handling metadata needs to be still there for supporting
> old firmware.
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH] nfp: add support for new metadata api

[Ferruh Yigit]

On 12/20/2016 2:13 PM, Alejandro Lucero wrote:
> NFP is a smart programmable NIC and firmware is deployed for specific
> system needs, like offloading OVS, vRouter, contrack or eBPF into the
> hardware. This often requires to give metadata to the host within
> packets delivered. Last NFP firmware implementations support richer
> metadata api facilitating interaction between firmware and host code.
> 
> Old way of handling metadata needs to be still there for supporting
> old firmware.
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> ---

<...>

> +
> +	} else if (NFP_DESC_META_LEN(rxd)) {
> +		meta_offset = (uint8_t *)mbuf->buf_addr;
> +		meta_info = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
> +		meta_offset += 4;
> +		/* NFP PMD just supports metadata for hashing */
> +		switch (meta_info & NFP_NET_META_FIELD_MASK) {
> +		case NFP_NET_META_HASH:
> +			meta_info >>= NFP_NET_META_FIELD_SIZE;
> +			hash = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
> +			hash_type = meta_info && NFP_NET_META_FIELD_MASK;

I already applied this patch but above "&&" looks wrong.
Most probably intention is "bitwise AND" (&), do you want me fix this as
"&" or remove the patch completely to replace with new version?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] nfp: add support for new metadata api

[Alejandro Lucero]

Hi Ferruh,

On Wed, Jan 4, 2017 at 3:15 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 12/20/2016 2:13 PM, Alejandro Lucero wrote:
> > NFP is a smart programmable NIC and firmware is deployed for specific
> > system needs, like offloading OVS, vRouter, contrack or eBPF into the
> > hardware. This often requires to give metadata to the host within
> > packets delivered. Last NFP firmware implementations support richer
> > metadata api facilitating interaction between firmware and host code.
> >
> > Old way of handling metadata needs to be still there for supporting
> > old firmware.
> >
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > ---
>
> <...>
>
> > +
> > +     } else if (NFP_DESC_META_LEN(rxd)) {
> > +             meta_offset = (uint8_t *)mbuf->buf_addr;
> > +             meta_info = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
> > +             meta_offset += 4;
> > +             /* NFP PMD just supports metadata for hashing */
> > +             switch (meta_info & NFP_NET_META_FIELD_MASK) {
> > +             case NFP_NET_META_HASH:
> > +                     meta_info >>= NFP_NET_META_FIELD_SIZE;
> > +                     hash = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
> > +                     hash_type = meta_info && NFP_NET_META_FIELD_MASK;
>
> I already applied this patch but above "&&" looks wrong.
> Most probably intention is "bitwise AND" (&), do you want me fix this as
> "&" or remove the patch completely to replace with new version?
>
>
Yes, that is wrong. I wonder how related tests did not fail. I'll check
that right now.

Maybe it is better to wait for another patch version or at least to be sure
that simple change is good enough.
Let me to peer into those tests and re-run them with that fix applied.


> Thanks,
> ferruh
>
>

Re: [dpdk-dev] [PATCH] nfp: add support for new metadata api

[Ferruh Yigit]

On 1/4/2017 2:43 PM, Alejandro Lucero wrote:
> Hi Ferruh,
> 
> On Wed, Jan 4, 2017 at 3:15 PM, Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 12/20/2016 2:13 PM, Alejandro Lucero wrote:
>     > NFP is a smart programmable NIC and firmware is deployed for specific
>     > system needs, like offloading OVS, vRouter, contrack or eBPF into the
>     > hardware. This often requires to give metadata to the host within
>     > packets delivered. Last NFP firmware implementations support richer
>     > metadata api facilitating interaction between firmware and host code.
>     >
>     > Old way of handling metadata needs to be still there for supporting
>     > old firmware.
>     >
>     > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com <mailto:alejandro.lucero@netronome.com>>
>     > ---
> 
>     <...>
> 
>     > +
>     > +     } else if (NFP_DESC_META_LEN(rxd)) {
>     > +             meta_offset = (uint8_t *)mbuf->buf_addr;
>     > +             meta_info = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
>     > +             meta_offset += 4;
>     > +             /* NFP PMD just supports metadata for hashing */
>     > +             switch (meta_info & NFP_NET_META_FIELD_MASK) {
>     > +             case NFP_NET_META_HASH:
>     > +                     meta_info >>= NFP_NET_META_FIELD_SIZE;
>     > +                     hash = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
>     > +                     hash_type = meta_info && NFP_NET_META_FIELD_MASK;
> 
>     I already applied this patch but above "&&" looks wrong.
>     Most probably intention is "bitwise AND" (&), do you want me fix this as
>     "&" or remove the patch completely to replace with new version?
> 
> 
> Yes, that is wrong. I wonder how related tests did not fail. I'll check
> that right now.
> 
> Maybe it is better to wait for another patch version or at least to be
> sure that simple change is good enough.
> Let me to peer into those tests and re-run them with that fix applied.

Removed from next-net, patchwork status updated as "Change Requested".

>  
> 
>     Thanks,
>     ferruh
> 
>