diff mbox series

[v2,08/15] net/bnxt: switch CFA code to dynamic mbuf field

Message ID 20201026222013.2147904-9-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series remove mbuf userdata |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Thomas Monjalon Oct. 26, 2020, 10:20 p.m. UTC 
  The CFA code from mark was stored in the deprecated mbuf field udata64.
It is moved to a dynamic field in order to allow removal of udata64.

Note: the new field has 32 bits, smaller than udata64.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/bnxt/bnxt_ethdev.c  | 19 +++++++++++++++++++
 drivers/net/bnxt/bnxt_rxr.c     |  2 +-
 drivers/net/bnxt/bnxt_rxr.h     |  5 +++++
 drivers/net/bnxt/rte_pmd_bnxt.h |  3 +++
 4 files changed, 28 insertions(+), 1 deletion(-)

Comments

Ajit Khaparde Oct. 27, 2020, 4:44 a.m. UTC | #1 
On Mon, Oct 26, 2020 at 3:20 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> The CFA code from mark was stored in the deprecated mbuf field udata64.
> It is moved to a dynamic field in order to allow removal of udata64.
>
> Note: the new field has 32 bits, smaller than udata64.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

>
> ---
>  drivers/net/bnxt/bnxt_ethdev.c  | 19 +++++++++++++++++++
>  drivers/net/bnxt/bnxt_rxr.c     |  2 +-
>  drivers/net/bnxt/bnxt_rxr.h     |  5 +++++
>  drivers/net/bnxt/rte_pmd_bnxt.h |  3 +++
>  4 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 1c7d1b758d..1090d28341 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -31,6 +31,7 @@
>  #include "bnxt_nvm_defs.h"
>  #include "bnxt_tf_common.h"
>  #include "ulp_flow_db.h"
> +#include "rte_pmd_bnxt.h"
>
>  #define DRV_MODULE_NAME                "bnxt"
>  static const char bnxt_version[] =
> @@ -163,6 +164,8 @@ static const char *const bnxt_dev_args[] = {
>   */
>  #define BNXT_DEVARG_REP_FC_F2R_INVALID(rep_fc_f2r)     ((rep_fc_f2r) > 1)
>
> +int bnxt_cfa_code_dynfield_offset;
> +
>  /*
>   * max_num_kflows must be >= 32
>   * and must be a power-of-2 supported value
> @@ -6021,6 +6024,22 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
>             pci_dev->id.device_id == BROADCOM_DEV_ID_58802_VF)
>                 bp->flags |= BNXT_FLAG_STINGRAY;
>
> +       if (BNXT_TRUFLOW_EN(bp)) {
> +               /* extra mbuf field is required to store CFA code from mark */
> +               static const struct rte_mbuf_dynfield bnxt_cfa_code_dynfield_desc = {
> +                       .name = RTE_PMD_BNXT_CFA_CODE_DYNFIELD_NAME,
> +                       .size = sizeof(BNXT_CFA_CODE_DYNFIELD_TYPE),
> +                       .align = __alignof__(BNXT_CFA_CODE_DYNFIELD_TYPE),
> +               };
> +               bnxt_cfa_code_dynfield_offset =
> +                       rte_mbuf_dynfield_register(&bnxt_cfa_code_dynfield_desc);
> +               if (bnxt_cfa_code_dynfield_offset < 0) {
> +                       PMD_DRV_LOG(ERR,
> +                           "Failed to register mbuf field for TruFlow mark\n");
> +                       return -rte_errno;
> +               }
> +       }
> +
>         rc = bnxt_init_board(eth_dev);
>         if (rc) {
>                 PMD_DRV_LOG(ERR,
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index 039217fa60..7156ce7dd8 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -606,7 +606,7 @@ bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
>                         return mark_id;
>                 /* Got the mark, write it to the mbuf and return */
>                 mbuf->hash.fdir.hi = mark_id;
> -               mbuf->udata64 = (cfa_code & 0xffffffffull) << 32;
> +               BNXT_CFA_CODE_DYNFIELD(mbuf) = cfa_code & 0xffffffffull;
>                 mbuf->hash.fdir.id = rxcmp1->cfa_code;
>                 mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID;
>                 return mark_id;
> diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
> index b874e54a8c..e01d3e4f18 100644
> --- a/drivers/net/bnxt/bnxt_rxr.h
> +++ b/drivers/net/bnxt/bnxt_rxr.h
> @@ -95,6 +95,11 @@ void bnxt_set_mark_in_mbuf(struct bnxt *bp,
>                            struct rx_pkt_cmpl_hi *rxcmp1,
>                            struct rte_mbuf *mbuf);
>
> +extern int bnxt_cfa_code_dynfield_offset;
> +#define BNXT_CFA_CODE_DYNFIELD_TYPE uint32_t
> +#define BNXT_CFA_CODE_DYNFIELD(mbuf) (*RTE_MBUF_DYNFIELD(mbuf, \
> +               bnxt_cfa_code_dynfield_offset, BNXT_CFA_CODE_DYNFIELD_TYPE *))
> +
>  #define BNXT_RX_META_CFA_CODE_SHIFT            19
>  #define BNXT_CFA_CODE_META_SHIFT               16
>  #define BNXT_RX_META_CFA_CODE_INT_ACT_REC_BIT  0x8000000
> diff --git a/drivers/net/bnxt/rte_pmd_bnxt.h b/drivers/net/bnxt/rte_pmd_bnxt.h
> index 81d0d0e032..8d3303bb0f 100644
> --- a/drivers/net/bnxt/rte_pmd_bnxt.h
> +++ b/drivers/net/bnxt/rte_pmd_bnxt.h
> @@ -9,6 +9,9 @@
>  #include <rte_ethdev.h>
>  #include <rte_ether.h>
>
> +/* mbuf dynfield where CFA code is stored */
> +#define RTE_PMD_BNXT_CFA_CODE_DYNFIELD_NAME "rte_net_bnxt_dynfield_cfa_code"
> +
>  /*
>   * Response sent back to the caller after callback
>   */
> --
> 2.28.0
>
Olivier Matz Oct. 27, 2020, 10:31 a.m. UTC | #2 
On Mon, Oct 26, 2020 at 11:20:06PM +0100, Thomas Monjalon wrote:
> The CFA code from mark was stored in the deprecated mbuf field udata64.
> It is moved to a dynamic field in order to allow removal of udata64.
> 
> Note: the new field has 32 bits, smaller than udata64.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

How is it used by an application?
It seems that the cfa code is not always set. Is there a flag to
know when the field is valid?

Also, for ice and iavf PMDs, we were discussing what is the better
way to export the offset.

Note that there is an open discussion about how to export dynamic
fields offset for PMDs:
http://inbox.dpdk.org/dev/20201025071352.221953-1-haiyue.wang@intel.com/T/#mab79dd5cbb7d199a33515b7456dfe1831cf473bc



> ---
>  drivers/net/bnxt/bnxt_ethdev.c  | 19 +++++++++++++++++++
>  drivers/net/bnxt/bnxt_rxr.c     |  2 +-
>  drivers/net/bnxt/bnxt_rxr.h     |  5 +++++
>  drivers/net/bnxt/rte_pmd_bnxt.h |  3 +++
>  4 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 1c7d1b758d..1090d28341 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -31,6 +31,7 @@
>  #include "bnxt_nvm_defs.h"
>  #include "bnxt_tf_common.h"
>  #include "ulp_flow_db.h"
> +#include "rte_pmd_bnxt.h"
>  
>  #define DRV_MODULE_NAME		"bnxt"
>  static const char bnxt_version[] =
> @@ -163,6 +164,8 @@ static const char *const bnxt_dev_args[] = {
>   */
>  #define BNXT_DEVARG_REP_FC_F2R_INVALID(rep_fc_f2r)	((rep_fc_f2r) > 1)
>  
> +int bnxt_cfa_code_dynfield_offset;
> +
>  /*
>   * max_num_kflows must be >= 32
>   * and must be a power-of-2 supported value
> @@ -6021,6 +6024,22 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
>  	    pci_dev->id.device_id == BROADCOM_DEV_ID_58802_VF)
>  		bp->flags |= BNXT_FLAG_STINGRAY;
>  
> +	if (BNXT_TRUFLOW_EN(bp)) {
> +		/* extra mbuf field is required to store CFA code from mark */
> +		static const struct rte_mbuf_dynfield bnxt_cfa_code_dynfield_desc = {
> +			.name = RTE_PMD_BNXT_CFA_CODE_DYNFIELD_NAME,
> +			.size = sizeof(BNXT_CFA_CODE_DYNFIELD_TYPE),
> +			.align = __alignof__(BNXT_CFA_CODE_DYNFIELD_TYPE),
> +		};
> +		bnxt_cfa_code_dynfield_offset =
> +			rte_mbuf_dynfield_register(&bnxt_cfa_code_dynfield_desc);
> +		if (bnxt_cfa_code_dynfield_offset < 0) {
> +			PMD_DRV_LOG(ERR,
> +			    "Failed to register mbuf field for TruFlow mark\n");
> +			return -rte_errno;
> +		}
> +	}
> +
>  	rc = bnxt_init_board(eth_dev);
>  	if (rc) {
>  		PMD_DRV_LOG(ERR,
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index 039217fa60..7156ce7dd8 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -606,7 +606,7 @@ bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
>  			return mark_id;
>  		/* Got the mark, write it to the mbuf and return */
>  		mbuf->hash.fdir.hi = mark_id;
> -		mbuf->udata64 = (cfa_code & 0xffffffffull) << 32;
> +		BNXT_CFA_CODE_DYNFIELD(mbuf) = cfa_code & 0xffffffffull;
>  		mbuf->hash.fdir.id = rxcmp1->cfa_code;
>  		mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID;
>  		return mark_id;
> diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
> index b874e54a8c..e01d3e4f18 100644
> --- a/drivers/net/bnxt/bnxt_rxr.h
> +++ b/drivers/net/bnxt/bnxt_rxr.h
> @@ -95,6 +95,11 @@ void bnxt_set_mark_in_mbuf(struct bnxt *bp,
>  			   struct rx_pkt_cmpl_hi *rxcmp1,
>  			   struct rte_mbuf *mbuf);
>  
> +extern int bnxt_cfa_code_dynfield_offset;
> +#define BNXT_CFA_CODE_DYNFIELD_TYPE uint32_t
> +#define BNXT_CFA_CODE_DYNFIELD(mbuf) (*RTE_MBUF_DYNFIELD(mbuf, \
> +		bnxt_cfa_code_dynfield_offset, BNXT_CFA_CODE_DYNFIELD_TYPE *))
> +
>  #define BNXT_RX_META_CFA_CODE_SHIFT		19
>  #define BNXT_CFA_CODE_META_SHIFT		16
>  #define BNXT_RX_META_CFA_CODE_INT_ACT_REC_BIT	0x8000000
> diff --git a/drivers/net/bnxt/rte_pmd_bnxt.h b/drivers/net/bnxt/rte_pmd_bnxt.h
> index 81d0d0e032..8d3303bb0f 100644
> --- a/drivers/net/bnxt/rte_pmd_bnxt.h
> +++ b/drivers/net/bnxt/rte_pmd_bnxt.h
> @@ -9,6 +9,9 @@
>  #include <rte_ethdev.h>
>  #include <rte_ether.h>
>  
> +/* mbuf dynfield where CFA code is stored */
> +#define RTE_PMD_BNXT_CFA_CODE_DYNFIELD_NAME "rte_net_bnxt_dynfield_cfa_code"
> +
>  /*
>   * Response sent back to the caller after callback
>   */
> -- 
> 2.28.0
>
Thomas Monjalon Oct. 27, 2020, 4:22 p.m. UTC | #3 
27/10/2020 11:31, Olivier Matz:
> On Mon, Oct 26, 2020 at 11:20:06PM +0100, Thomas Monjalon wrote:
> > The CFA code from mark was stored in the deprecated mbuf field udata64.
> > It is moved to a dynamic field in order to allow removal of udata64.
> > 
> > Note: the new field has 32 bits, smaller than udata64.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> How is it used by an application?
> It seems that the cfa code is not always set. Is there a flag to
> know when the field is valid?

I don't know, and it is PMD-specific,
but this behaviour is equivalent with what was before.

> Also, for ice and iavf PMDs, we were discussing what is the better
> way to export the offset.
> 
> Note that there is an open discussion about how to export dynamic
> fields offset for PMDs:
> http://inbox.dpdk.org/dev/20201025071352.221953-1-haiyue.wang@intel.com/T/#mab79dd5cbb7d199a33515b7456dfe1831cf473bc

In general I think it is better not exporting the offset,
but keep a local copy of it.
diff mbox series

Patch

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 1c7d1b758d..1090d28341 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -31,6 +31,7 @@ 
 #include "bnxt_nvm_defs.h"
 #include "bnxt_tf_common.h"
 #include "ulp_flow_db.h"
+#include "rte_pmd_bnxt.h"
 
 #define DRV_MODULE_NAME		"bnxt"
 static const char bnxt_version[] =
@@ -163,6 +164,8 @@  static const char *const bnxt_dev_args[] = {
  */
 #define BNXT_DEVARG_REP_FC_F2R_INVALID(rep_fc_f2r)	((rep_fc_f2r) > 1)
 
+int bnxt_cfa_code_dynfield_offset;
+
 /*
  * max_num_kflows must be >= 32
  * and must be a power-of-2 supported value
@@ -6021,6 +6024,22 @@  bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 	    pci_dev->id.device_id == BROADCOM_DEV_ID_58802_VF)
 		bp->flags |= BNXT_FLAG_STINGRAY;
 
+	if (BNXT_TRUFLOW_EN(bp)) {
+		/* extra mbuf field is required to store CFA code from mark */
+		static const struct rte_mbuf_dynfield bnxt_cfa_code_dynfield_desc = {
+			.name = RTE_PMD_BNXT_CFA_CODE_DYNFIELD_NAME,
+			.size = sizeof(BNXT_CFA_CODE_DYNFIELD_TYPE),
+			.align = __alignof__(BNXT_CFA_CODE_DYNFIELD_TYPE),
+		};
+		bnxt_cfa_code_dynfield_offset =
+			rte_mbuf_dynfield_register(&bnxt_cfa_code_dynfield_desc);
+		if (bnxt_cfa_code_dynfield_offset < 0) {
+			PMD_DRV_LOG(ERR,
+			    "Failed to register mbuf field for TruFlow mark\n");
+			return -rte_errno;
+		}
+	}
+
 	rc = bnxt_init_board(eth_dev);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 039217fa60..7156ce7dd8 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -606,7 +606,7 @@  bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
 			return mark_id;
 		/* Got the mark, write it to the mbuf and return */
 		mbuf->hash.fdir.hi = mark_id;
-		mbuf->udata64 = (cfa_code & 0xffffffffull) << 32;
+		BNXT_CFA_CODE_DYNFIELD(mbuf) = cfa_code & 0xffffffffull;
 		mbuf->hash.fdir.id = rxcmp1->cfa_code;
 		mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID;
 		return mark_id;
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index b874e54a8c..e01d3e4f18 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -95,6 +95,11 @@  void bnxt_set_mark_in_mbuf(struct bnxt *bp,
 			   struct rx_pkt_cmpl_hi *rxcmp1,
 			   struct rte_mbuf *mbuf);
 
+extern int bnxt_cfa_code_dynfield_offset;
+#define BNXT_CFA_CODE_DYNFIELD_TYPE uint32_t
+#define BNXT_CFA_CODE_DYNFIELD(mbuf) (*RTE_MBUF_DYNFIELD(mbuf, \
+		bnxt_cfa_code_dynfield_offset, BNXT_CFA_CODE_DYNFIELD_TYPE *))
+
 #define BNXT_RX_META_CFA_CODE_SHIFT		19
 #define BNXT_CFA_CODE_META_SHIFT		16
 #define BNXT_RX_META_CFA_CODE_INT_ACT_REC_BIT	0x8000000
diff --git a/drivers/net/bnxt/rte_pmd_bnxt.h b/drivers/net/bnxt/rte_pmd_bnxt.h
index 81d0d0e032..8d3303bb0f 100644
--- a/drivers/net/bnxt/rte_pmd_bnxt.h
+++ b/drivers/net/bnxt/rte_pmd_bnxt.h
@@ -9,6 +9,9 @@ 
 #include <rte_ethdev.h>
 #include <rte_ether.h>
 
+/* mbuf dynfield where CFA code is stored */
+#define RTE_PMD_BNXT_CFA_CODE_DYNFIELD_NAME "rte_net_bnxt_dynfield_cfa_code"
+
 /*
  * Response sent back to the caller after callback
  */