[dpdk-dev] [PATCH] nfp: support new medatada api

[Alejandro Lucero]

On Fri, Aug 18, 2017 at 4:51 PM, Ferruh Yigit <ferruh.yigit at intel.com>
wrote:

> On 8/11/2017 3:30 PM, Alejandro Lucero wrote:
> > We need to support how metadata was handled and the new api, which will
> > allow to work with different metadata types and data dynamically.
>
> What is the API mentioned?
>
>
This is a internal NFP firmware API for handling metadata. I will mention
this in next version.


> >
> > Although this patch just supports one type handled by the PMD. Future
> uses
> > regarding firmware apps will extend this support.
> >
> > Signed-off-by: Alejandro Lucero <alejandro.lucero at netronome.com>
> > ---
> >  drivers/net/nfp/nfp_net.c      | 37 ++++++++++++++++++++++++++++++
> +------
> >  drivers/net/nfp/nfp_net_ctrl.h |  5 +++++
> >  drivers/net/nfp/nfp_net_pmd.h  |  2 ++
> >  3 files changed, 38 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> > index c9895a2..5ea55c7 100644
> > --- a/drivers/net/nfp/nfp_net.c
> > +++ b/drivers/net/nfp/nfp_net.c
> > @@ -1734,6 +1734,8 @@ 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
> >   *
> > @@ -1744,18 +1746,41 @@ static void nfp_net_read_mac(struct nfp_net_hw
> *hw)
> >  nfp_net_set_hash(struct nfp_net_rxq *rxq, struct nfp_net_rx_desc *rxd,
> >                struct rte_mbuf *mbuf)
> >  {
> > -     uint32_t hash;
> > -     uint32_t hash_type;
> >       struct nfp_net_hw *hw = rxq->hw;
> > +     uint8_t *meta_offset;
> > +     uint32_t meta_info;
> > +     uint32_t hash = 0;
> > +     uint32_t hash_type = 0;
> >
> >       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) {
>
> It seems metadata parsing is valid for MAJOR_VERSION > 3, does it make
> sense to mention from this in commit log?
>

That is a good idea. Thanks


>
> > +             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 + mbuf->data_off;
>
> It is possible to use rte_pktmbuf_mtod_offset(), result will be same tough.
>
>
I will change it.


> > +             meta_offset -= NFP_DESC_META_LEN(rxd);
> > +             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;
>
> META_FIELD already masked out above, right? Is this META_FIELD again or
> just reused macro because of it has same value?
>
>
meta_info is changing after it is masked out the first time. Metadata is
described with at least two fields, but it can have more depending on the
metadata type. So NFP_NET_META_FIELD_SIZE is used for adjusting which field
is going to be processed. I guess adding this same description will help.
so I will do that in next patch version.


> > +                     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;
> > diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_
> ctrl.h
> > index 2c50043..becfabd 100644
> > --- a/drivers/net/nfp/nfp_net_ctrl.h
> > +++ b/drivers/net/nfp/nfp_net_ctrl.h
> > @@ -52,6 +52,11 @@
> >  /* 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
>
> META_MARK seems not used.
>
>
Not yet for the PMD. I will remove it.


> > +
> >  /* Hash type pre-pended when a RSS hash was computed */
> >  #define NFP_NET_RSS_NONE                0
> >  #define NFP_NET_RSS_IPV4                1
> > diff --git a/drivers/net/nfp/nfp_net_pmd.h
> b/drivers/net/nfp/nfp_net_pmd.h
> > index c6bddaa..df22fa1 100644
> > --- a/drivers/net/nfp/nfp_net_pmd.h
> > +++ b/drivers/net/nfp/nfp_net_pmd.h
> > @@ -283,6 +283,8 @@ struct nfp_net_txq {
> >  #define PCIE_DESC_RX_UDP_CSUM_OK        (1 <<  1)
> >  #define PCIE_DESC_RX_VLAN               (1 <<  0)
> >
> > +#define NFP_NET_META_FIELD_MASK (0xf)
>
> Can represent as following to stress the relation:
> #define NFP_NET_META_FIELD_MASK ((1 << NFP_NET_META_FIELD_SIZE) - 1)
>

this makes sense. Thanks


>
> > +
> >  struct nfp_net_rx_desc {
> >       union {
> >               /* Freelist descriptor */
> >
>
>