Hi,
> -----Original Message-----
> From: Olivier Matz <olivier.m...@6wind.com>
> Sent: Thursday, October 31, 2019 17:51
> To: Slava Ovsiienko <viachesl...@mellanox.com>
> Cc: dev@dpdk.org; Matan Azrad <ma...@mellanox.com>; Raslan
> Darawsheh <rasl...@mellanox.com>; Thomas Monjalon
> <tho...@monjalon.net>; arybche...@solarflare.com; Ori Kam
> <or...@mellanox.com>
> Subject: Re: [PATCH v7 2/2] ethdev: move egress metadata to dynamic field
>
> Hi,
>
> On Thu, Oct 31, 2019 at 01:05:21PM +0000, Viacheslav Ovsiienko wrote:
> > The dynamic mbuf fields were introduced by [1]. The egress metadata is
> > good candidate to be moved from statically allocated field tx_metadata
> > to dynamic one. Because mbufs are used in half-duplex fashion only, it
> > is safe to share this dynamic field with ingress metadata.
> >
> > The shared dynamic field contains either egress (if application going
> > to transmit mbuf with tx_burst) or ingress (if mbuf is received with
> > rx_burst) metadata and can be accessed by RTE_FLOW_DYNF_METADATA()
> > macro or with
> > rte_flow_dynf_metadata_set() and rte_flow_dynf_metadata_get() helper
> > routines. PKT_TX_DYNF_METADATA/PKT_RX_DYNF_METADATA flag will be
> set
> > along with the data.
> >
> > The mbuf dynamic field must be registered by calling
> > rte_flow_dynf_metadata_register() prior accessing the data.
> >
> > The availability of dynamic mbuf metadata field can be checked with
> > rte_flow_dynf_metadata_avail() routine.
> >
> > DEV_TX_OFFLOAD_MATCH_METADATA offload and configuration flag is
> removed.
> > The metadata support in PMDs is engaged on dynamic field registration.
> >
> > Metadata feature is getting complex. We might have some set of actions
> > and items that might be supported by PMDs in multiple combinations,
> > the supported values and masks are the subjects to query by perfroming
> > trials (with rte_flow_validate).
> >
[snip]
> > +/* Mbuf dynamic flags for metadata. */
> > #define PKT_RX_DYNF_METADATA (rte_flow_dynf_metadata_mask)
> > +#define PKT_TX_DYNF_METADATA (rte_flow_dynf_metadata_mask)
>
> Should we have 2 defines pointing to the same mask? Shall we use
> PKT_DYNF_METADATA for both?
It is just a style issue, we have two sets of PKT_RX_xxxx and PKT_TX_xxx flags,
it just looks nice to use appropriate flags in RX/TX parts of datapath.
> >
> > __rte_experimental
> > static inline uint32_t
> > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> > index 8c51dc1..35df1c4 100644
> > --- a/lib/librte_mbuf/rte_mbuf.c
> > +++ b/lib/librte_mbuf/rte_mbuf.c
> > @@ -670,7 +670,6 @@ const char *rte_get_tx_ol_flag_name(uint64_t
> mask)
> > case PKT_TX_SEC_OFFLOAD: return "PKT_TX_SEC_OFFLOAD";
> > case PKT_TX_UDP_SEG: return "PKT_TX_UDP_SEG";
> > case PKT_TX_OUTER_UDP_CKSUM: return
> "PKT_TX_OUTER_UDP_CKSUM";
> > - case PKT_TX_METADATA: return "PKT_TX_METADATA";
> > default: return NULL;
> > }
> > }
> > @@ -707,7 +706,6 @@ const char *rte_get_tx_ol_flag_name(uint64_t
> mask)
> > { PKT_TX_SEC_OFFLOAD, PKT_TX_SEC_OFFLOAD, NULL },
> > { PKT_TX_UDP_SEG, PKT_TX_UDP_SEG, NULL },
> > { PKT_TX_OUTER_UDP_CKSUM,
> PKT_TX_OUTER_UDP_CKSUM, NULL },
> > - { PKT_TX_METADATA, PKT_TX_METADATA, NULL },
> > };
> > const char *name;
> > unsigned int i;
> > diff --git a/lib/librte_mbuf/rte_mbuf_core.h
> > b/lib/librte_mbuf/rte_mbuf_core.h index 3022701..edfc7e9 100644
> > --- a/lib/librte_mbuf/rte_mbuf_core.h
> > +++ b/lib/librte_mbuf/rte_mbuf_core.h
> > @@ -192,11 +192,6 @@
> > /* add new TX flags here, don't forget to update PKT_LAST_FREE */
> >
> > /**
> > - * Indicate that the metadata field in the mbuf is in use.
> > - */
> > -#define PKT_TX_METADATA (1ULL << 40)
> > -
> > -/**
>
> You should also update PKT_LAST_FREE just above.
>
Yes, my bad, will fix.
[snip]
With best regards, Slava
