[v3,02/16] mbuf: add Rx timestamp flag and helpers
Checks
Commit Message
There is already a dynamic field for timestamp,
used only for Tx scheduling with the dedicated Tx offload flag.
The same field can be used for Rx timestamp filled by drivers.
A new dynamic flag is defined for Rx usage.
A new function wraps the registration of both field and Rx flag.
The type rte_mbuf_timestamp_t is defined for the API users.
After migrating all Rx timestamp usages, it will be possible
to remove the deprecated timestamp field.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
lib/librte_mbuf/rte_mbuf_dyn.c | 43 ++++++++++++++++++++++++++++++++++
lib/librte_mbuf/rte_mbuf_dyn.h | 33 ++++++++++++++++++++++----
lib/librte_mbuf/version.map | 1 +
3 files changed, 72 insertions(+), 5 deletions(-)
Comments
Hi Thomas,
On Tue, Nov 03, 2020 at 01:13:53AM +0100, Thomas Monjalon wrote:
> There is already a dynamic field for timestamp,
> used only for Tx scheduling with the dedicated Tx offload flag.
> The same field can be used for Rx timestamp filled by drivers.
>
> A new dynamic flag is defined for Rx usage.
> A new function wraps the registration of both field and Rx flag.
> The type rte_mbuf_timestamp_t is defined for the API users.
>
> After migrating all Rx timestamp usages, it will be possible
> to remove the deprecated timestamp field.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> lib/librte_mbuf/rte_mbuf_dyn.c | 43 ++++++++++++++++++++++++++++++++++
> lib/librte_mbuf/rte_mbuf_dyn.h | 33 ++++++++++++++++++++++----
> lib/librte_mbuf/version.map | 1 +
> 3 files changed, 72 insertions(+), 5 deletions(-)
>
> diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
> index 538a43f695..e279b23aea 100644
> --- a/lib/librte_mbuf/rte_mbuf_dyn.c
> +++ b/lib/librte_mbuf/rte_mbuf_dyn.c
> @@ -13,6 +13,7 @@
> #include <rte_errno.h>
> #include <rte_malloc.h>
> #include <rte_string_fns.h>
> +#include <rte_bitops.h>
> #include <rte_mbuf.h>
> #include <rte_mbuf_dyn.h>
>
> @@ -569,3 +570,45 @@ void rte_mbuf_dyn_dump(FILE *out)
>
> rte_mcfg_tailq_write_unlock();
> }
> +
> +static int
> +rte_mbuf_dyn_timestamp_register(int *field_offset, uint64_t *flag,
> + const char *direction, const char *flag_name)
> +{
> + static const struct rte_mbuf_dynfield field_desc = {
> + .name = RTE_MBUF_DYNFIELD_TIMESTAMP_NAME,
> + .size = sizeof(rte_mbuf_timestamp_t),
> + .align = __alignof__(rte_mbuf_timestamp_t),
> + };
> + struct rte_mbuf_dynflag flag_desc;
> + int offset;
> +
> + offset = rte_mbuf_dynfield_register(&field_desc);
> + if (offset < 0) {
> + RTE_LOG(ERR, MBUF,
> + "Failed to register mbuf field for timestamp\n");
> + return -1;
> + }
> + if (field_offset != NULL)
> + *field_offset = offset;
> +
> + strlcpy(flag_desc.name, flag_name, sizeof flag_desc.name);
The rest of the flag_desc structure is not initialized to 0 (the "flags"
field).
I suggest to do it at declaration:
struct rte_mbuf_dynflag flag_desc = { 0 };
> + offset = rte_mbuf_dynflag_register(&flag_desc);
> + if (offset < 0) {
> + RTE_LOG(ERR, MBUF,
> + "Failed to register mbuf flag for %s timestamp\n",
> + direction);
> + return -1;
> + }
> + if (flag != NULL)
> + *flag = RTE_BIT64(offset);
> +
> + return 0;
> +}
> +
> +int
> +rte_mbuf_dyn_rx_timestamp_register(int *field_offset, uint64_t *rx_flag)
> +{
> + return rte_mbuf_dyn_timestamp_register(field_offset, rx_flag,
> + "Rx", RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME);
> +}
> diff --git a/lib/librte_mbuf/rte_mbuf_dyn.h b/lib/librte_mbuf/rte_mbuf_dyn.h
> index 0ebac88b83..2e729ddaca 100644
> --- a/lib/librte_mbuf/rte_mbuf_dyn.h
> +++ b/lib/librte_mbuf/rte_mbuf_dyn.h
> @@ -258,13 +258,36 @@ void rte_mbuf_dyn_dump(FILE *out);
> * timestamp. The dynamic Tx timestamp flag tells whether the field contains
> * actual timestamp value for the packets being sent, this value can be
> * used by PMD to schedule packet sending.
> - *
> - * After PKT_RX_TIMESTAMP flag and fixed timestamp field deprecation
> - * and obsoleting, the dedicated Rx timestamp flag is supposed to be
> - * introduced and the shared dynamic timestamp field will be used
> - * to handle the timestamps on receiving datapath as well.
> */
> #define RTE_MBUF_DYNFIELD_TIMESTAMP_NAME "rte_dynfield_timestamp"
> +typedef uint64_t rte_mbuf_timestamp_t;
> +
> +/**
> + * Indicate that the timestamp field in the mbuf was filled by the driver.
> + */
> +#define RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME "rte_dynflag_rx_timestamp"
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Register dynamic mbuf field and flag for Rx timestamp.
> + *
> + * @param field_offset
> + * Pointer to the offset of the registered mbuf field, can be NULL.
> + * The same field is shared for Rx and Tx timestamp.
> + * @param rx_flag
> + * Pointer to the mask of the registered offload flag, can be NULL.
> + * @return
> + * 0 on success, -1 otherwise.
> + * Possible values for rte_errno:
> + * - EEXIST: already registered with different parameters.
> + * - EPERM: called from a secondary process.
> + * - ENOENT: no more field or flag available.
> + * - ENOMEM: allocation failure.
> + */
> +__rte_experimental
> +int rte_mbuf_dyn_rx_timestamp_register(int *field_offset, uint64_t *rx_flag);
>
> /**
> * When PMD sees the RTE_MBUF_DYNFLAG_TX_TIMESTAMP_NAME flag set on the
> diff --git a/lib/librte_mbuf/version.map b/lib/librte_mbuf/version.map
> index a011aaead3..0b66668bff 100644
> --- a/lib/librte_mbuf/version.map
> +++ b/lib/librte_mbuf/version.map
> @@ -42,6 +42,7 @@ EXPERIMENTAL {
> rte_mbuf_dynflag_register;
> rte_mbuf_dynflag_register_bitnum;
> rte_mbuf_dyn_dump;
> + rte_mbuf_dyn_rx_timestamp_register;
> rte_pktmbuf_copy;
> rte_pktmbuf_free_bulk;
> rte_pktmbuf_pool_create_extbuf;
> --
> 2.28.0
>
03/11/2020 10:33, Olivier Matz:
> On Tue, Nov 03, 2020 at 01:13:53AM +0100, Thomas Monjalon wrote:
> > +static int
> > +rte_mbuf_dyn_timestamp_register(int *field_offset, uint64_t *flag,
> > + const char *direction, const char *flag_name)
> > +{
> > + static const struct rte_mbuf_dynfield field_desc = {
> > + .name = RTE_MBUF_DYNFIELD_TIMESTAMP_NAME,
> > + .size = sizeof(rte_mbuf_timestamp_t),
> > + .align = __alignof__(rte_mbuf_timestamp_t),
> > + };
> > + struct rte_mbuf_dynflag flag_desc;
> > + int offset;
> > +
> > + offset = rte_mbuf_dynfield_register(&field_desc);
> > + if (offset < 0) {
> > + RTE_LOG(ERR, MBUF,
> > + "Failed to register mbuf field for timestamp\n");
> > + return -1;
> > + }
> > + if (field_offset != NULL)
> > + *field_offset = offset;
> > +
> > + strlcpy(flag_desc.name, flag_name, sizeof flag_desc.name);
>
> The rest of the flag_desc structure is not initialized to 0 (the "flags"
> field).
>
> I suggest to do it at declaration:
>
> struct rte_mbuf_dynflag flag_desc = { 0 };
Yes I forgot, thanks for catching.
@@ -13,6 +13,7 @@
#include <rte_errno.h>
#include <rte_malloc.h>
#include <rte_string_fns.h>
+#include <rte_bitops.h>
#include <rte_mbuf.h>
#include <rte_mbuf_dyn.h>
@@ -569,3 +570,45 @@ void rte_mbuf_dyn_dump(FILE *out)
rte_mcfg_tailq_write_unlock();
}
+
+static int
+rte_mbuf_dyn_timestamp_register(int *field_offset, uint64_t *flag,
+ const char *direction, const char *flag_name)
+{
+ static const struct rte_mbuf_dynfield field_desc = {
+ .name = RTE_MBUF_DYNFIELD_TIMESTAMP_NAME,
+ .size = sizeof(rte_mbuf_timestamp_t),
+ .align = __alignof__(rte_mbuf_timestamp_t),
+ };
+ struct rte_mbuf_dynflag flag_desc;
+ int offset;
+
+ offset = rte_mbuf_dynfield_register(&field_desc);
+ if (offset < 0) {
+ RTE_LOG(ERR, MBUF,
+ "Failed to register mbuf field for timestamp\n");
+ return -1;
+ }
+ if (field_offset != NULL)
+ *field_offset = offset;
+
+ strlcpy(flag_desc.name, flag_name, sizeof flag_desc.name);
+ offset = rte_mbuf_dynflag_register(&flag_desc);
+ if (offset < 0) {
+ RTE_LOG(ERR, MBUF,
+ "Failed to register mbuf flag for %s timestamp\n",
+ direction);
+ return -1;
+ }
+ if (flag != NULL)
+ *flag = RTE_BIT64(offset);
+
+ return 0;
+}
+
+int
+rte_mbuf_dyn_rx_timestamp_register(int *field_offset, uint64_t *rx_flag)
+{
+ return rte_mbuf_dyn_timestamp_register(field_offset, rx_flag,
+ "Rx", RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME);
+}
@@ -258,13 +258,36 @@ void rte_mbuf_dyn_dump(FILE *out);
* timestamp. The dynamic Tx timestamp flag tells whether the field contains
* actual timestamp value for the packets being sent, this value can be
* used by PMD to schedule packet sending.
- *
- * After PKT_RX_TIMESTAMP flag and fixed timestamp field deprecation
- * and obsoleting, the dedicated Rx timestamp flag is supposed to be
- * introduced and the shared dynamic timestamp field will be used
- * to handle the timestamps on receiving datapath as well.
*/
#define RTE_MBUF_DYNFIELD_TIMESTAMP_NAME "rte_dynfield_timestamp"
+typedef uint64_t rte_mbuf_timestamp_t;
+
+/**
+ * Indicate that the timestamp field in the mbuf was filled by the driver.
+ */
+#define RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME "rte_dynflag_rx_timestamp"
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Register dynamic mbuf field and flag for Rx timestamp.
+ *
+ * @param field_offset
+ * Pointer to the offset of the registered mbuf field, can be NULL.
+ * The same field is shared for Rx and Tx timestamp.
+ * @param rx_flag
+ * Pointer to the mask of the registered offload flag, can be NULL.
+ * @return
+ * 0 on success, -1 otherwise.
+ * Possible values for rte_errno:
+ * - EEXIST: already registered with different parameters.
+ * - EPERM: called from a secondary process.
+ * - ENOENT: no more field or flag available.
+ * - ENOMEM: allocation failure.
+ */
+__rte_experimental
+int rte_mbuf_dyn_rx_timestamp_register(int *field_offset, uint64_t *rx_flag);
/**
* When PMD sees the RTE_MBUF_DYNFLAG_TX_TIMESTAMP_NAME flag set on the
@@ -42,6 +42,7 @@ EXPERIMENTAL {
rte_mbuf_dynflag_register;
rte_mbuf_dynflag_register_bitnum;
rte_mbuf_dyn_dump;
+ rte_mbuf_dyn_rx_timestamp_register;
rte_pktmbuf_copy;
rte_pktmbuf_free_bulk;
rte_pktmbuf_pool_create_extbuf;