[v2] net/bnxt: remove compile-time option for IEEE 1588

Message ID 20230222182244.19294-1-ajit.khaparde@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series [v2] net/bnxt: remove compile-time option for IEEE 1588 |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Ajit Khaparde Feb. 22, 2023, 6:22 p.m. UTC
  The drivers are supposed to enable/disable features dynamically
instead of compile time options.
Remove the compile time option RTE_LIBRTE_IEEE1588.

This patch is an improved and bnxt specific version of the previous
submission which tried to remove the option tree wide, but was
completely disabling for bnxt PMD.

http://patchwork.dpdk.org/project/dpdk/patch/20230203132810.14187-1-thomas@monjalon.net/

Also add a devarg to enable/disable the setting,
invoked as for ex: "-a 000:0b:0d.0,ieee-1588=1".

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

---
v1->v2: Moved the check for ieee_1588 in Tx path.
---
 drivers/net/bnxt/bnxt.h        |  1 +
 drivers/net/bnxt/bnxt_ethdev.c | 62 ++++++++++++++++++++++++++++++----
 drivers/net/bnxt/bnxt_rxr.c    |  6 ----
 3 files changed, 57 insertions(+), 12 deletions(-)
  

Comments

Ajit Khaparde Feb. 22, 2023, 10:22 p.m. UTC | #1
On Wed, Feb 22, 2023 at 10:22 AM Ajit Khaparde
<ajit.khaparde@broadcom.com> wrote:
>
> The drivers are supposed to enable/disable features dynamically
> instead of compile time options.
> Remove the compile time option RTE_LIBRTE_IEEE1588.
>
> This patch is an improved and bnxt specific version of the previous
> submission which tried to remove the option tree wide, but was
> completely disabling for bnxt PMD.
>
> http://patchwork.dpdk.org/project/dpdk/patch/20230203132810.14187-1-thomas@monjalon.net/
>
> Also add a devarg to enable/disable the setting,
> invoked as for ex: "-a 000:0b:0d.0,ieee-1588=1".
>
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Patch applied to dpdk-next-net-brcm. Thanks

>
> ---
> v1->v2: Moved the check for ieee_1588 in Tx path.
> ---
>  drivers/net/bnxt/bnxt.h        |  1 +
>  drivers/net/bnxt/bnxt_ethdev.c | 62 ++++++++++++++++++++++++++++++----
>  drivers/net/bnxt/bnxt_rxr.c    |  6 ----
>  3 files changed, 57 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
> index c9aa45ed3b..f6eaaeb470 100644
> --- a/drivers/net/bnxt/bnxt.h
> +++ b/drivers/net/bnxt/bnxt.h
> @@ -832,6 +832,7 @@ struct bnxt {
>         uint32_t                hwrm_spec_code;
>
>         struct bnxt_led_info    *leds;
> +       uint8_t                 ieee_1588;
>         struct bnxt_ptp_cfg     *ptp_cfg;
>         uint16_t                vf_resv_strategy;
>         struct bnxt_ctx_mem_info        *ctx;
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 753e86b4b2..ef7b8859d9 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -97,6 +97,7 @@ static const struct rte_pci_id bnxt_pci_id_map[] = {
>  #define BNXT_DEVARG_REP_FC_R2F  "rep-fc-r2f"
>  #define BNXT_DEVARG_REP_FC_F2R  "rep-fc-f2r"
>  #define BNXT_DEVARG_APP_ID     "app-id"
> +#define BNXT_DEVARG_IEEE_1588  "ieee-1588"
>
>  static const char *const bnxt_dev_args[] = {
>         BNXT_DEVARG_REPRESENTOR,
> @@ -109,6 +110,7 @@ static const char *const bnxt_dev_args[] = {
>         BNXT_DEVARG_REP_FC_R2F,
>         BNXT_DEVARG_REP_FC_F2R,
>         BNXT_DEVARG_APP_ID,
> +       BNXT_DEVARG_IEEE_1588,
>         NULL
>  };
>
> @@ -117,6 +119,11 @@ static const char *const bnxt_dev_args[] = {
>   */
>  #define BNXT_DEVARG_APP_ID_INVALID(val)                        ((val) > 255)
>
> +/*
> + * ieee-1588 = an non-negative 8-bit number
> + */
> +#define BNXT_DEVARG_IEEE_1588_INVALID(val)                     ((val) > 255)
> +
>  /*
>   * flow_xstat == false to disable the feature
>   * flow_xstat == true to enable the feature
> @@ -1229,9 +1236,7 @@ bnxt_receive_function(struct rte_eth_dev *eth_dev)
>                 return bnxt_recv_pkts;
>         }
>
> -#if (defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)) && \
> -       !defined(RTE_LIBRTE_IEEE1588)
> -
> +#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
>         /* Vector mode receive cannot be enabled if scattered rx is in use. */
>         if (eth_dev->data->scattered_rx)
>                 goto use_scalar_rx;
> @@ -1260,6 +1265,9 @@ bnxt_receive_function(struct rte_eth_dev *eth_dev)
>                   RTE_ETH_RX_OFFLOAD_VLAN_FILTER))
>                 goto use_scalar_rx;
>
> +       if (bp->ieee_1588)
> +               goto use_scalar_rx;
> +
>  #if defined(RTE_ARCH_X86) && defined(CC_AVX2_SUPPORT)
>         if (rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_256 &&
>             rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2) == 1) {
> @@ -1300,8 +1308,7 @@ bnxt_transmit_function(struct rte_eth_dev *eth_dev)
>         if (BNXT_CHIP_SR2(bp))
>                 return bnxt_xmit_pkts;
>
> -#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) && \
> -       !defined(RTE_LIBRTE_IEEE1588)
> +#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
>         uint64_t offloads = eth_dev->data->dev_conf.txmode.offloads;
>
>         /*
> @@ -1310,7 +1317,7 @@ bnxt_transmit_function(struct rte_eth_dev *eth_dev)
>          */
>         if (eth_dev->data->scattered_rx ||
>             (offloads & ~RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) ||
> -           BNXT_TRUFLOW_EN(bp))
> +           BNXT_TRUFLOW_EN(bp) || bp->ieee_1588)
>                 goto use_scalar_tx;
>
>  #if defined(RTE_ARCH_X86) && defined(CC_AVX2_SUPPORT)
> @@ -5486,6 +5493,42 @@ bnxt_parse_devarg_app_id(__rte_unused const char *key,
>         return 0;
>  }
>
> +static int
> +bnxt_parse_devarg_ieee_1588(__rte_unused const char *key,
> +                           const char *value, void *opaque_arg)
> +{
> +       struct bnxt *bp = opaque_arg;
> +       unsigned long ieee_1588;
> +       char *end = NULL;
> +
> +       if (!value || !opaque_arg) {
> +               PMD_DRV_LOG(ERR,
> +                           "Invalid parameter passed to ieee-1588 "
> +                           "devargs.\n");
> +               return -EINVAL;
> +       }
> +
> +       ieee_1588 = strtoul(value, &end, 10);
> +       if (end == NULL || *end != '\0' ||
> +           (ieee_1588 == ULONG_MAX && errno == ERANGE)) {
> +               PMD_DRV_LOG(ERR,
> +                           "Invalid parameter passed to ieee_1588 "
> +                           "devargs.\n");
> +               return -EINVAL;
> +       }
> +
> +       if (BNXT_DEVARG_IEEE_1588_INVALID(ieee_1588)) {
> +               PMD_DRV_LOG(ERR, "Invalid ieee-1588(%d) devargs.\n",
> +                           (uint16_t)ieee_1588);
> +               return -EINVAL;
> +       }
> +
> +       bp->ieee_1588 = ieee_1588;
> +       PMD_DRV_LOG(INFO, "ieee-1588=%d feature enabled.\n", (uint16_t)ieee_1588);
> +
> +       return 0;
> +}
> +
>  static int
>  bnxt_parse_devarg_rep_is_pf(__rte_unused const char *key,
>                             const char *value, void *opaque_arg)
> @@ -5748,6 +5791,13 @@ bnxt_parse_dev_args(struct bnxt *bp, struct rte_devargs *devargs)
>         rte_kvargs_process(kvlist, BNXT_DEVARG_APP_ID,
>                            bnxt_parse_devarg_app_id, bp);
>
> +       /*
> +        * Handler for "ieee-1588" devarg.
> +        * Invoked as for ex: "-a 000:00:0d.0,ieee-1588=1"
> +        */
> +       rte_kvargs_process(kvlist, BNXT_DEVARG_IEEE_1588,
> +                          bnxt_parse_devarg_ieee_1588, bp);
> +
>         rte_kvargs_free(kvlist);
>         return ret;
>  }
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index 0eebddb05d..a067278dca 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -680,16 +680,13 @@ bnxt_set_ol_flags(struct bnxt_rx_ring_info *rxr, struct rx_pkt_cmpl *rxcmp,
>                 ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
>         }
>
> -#ifdef RTE_LIBRTE_IEEE1588
>         if (unlikely((flags_type & RX_PKT_CMPL_FLAGS_MASK) ==
>                      RX_PKT_CMPL_FLAGS_ITYPE_PTP_W_TIMESTAMP))
>                 ol_flags |= RTE_MBUF_F_RX_IEEE1588_PTP | RTE_MBUF_F_RX_IEEE1588_TMST;
> -#endif
>
>         mbuf->ol_flags = ol_flags;
>  }
>
> -#ifdef RTE_LIBRTE_IEEE1588
>  static void
>  bnxt_get_rx_ts_p5(struct bnxt *bp, uint32_t rx_ts_cmpl)
>  {
> @@ -716,7 +713,6 @@ bnxt_get_rx_ts_p5(struct bnxt *bp, uint32_t rx_ts_cmpl)
>         }
>         ptp->rx_timestamp = pkt_time;
>  }
> -#endif
>
>  static uint32_t
>  bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
> @@ -925,12 +921,10 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
>         mbuf->data_len = mbuf->pkt_len;
>         mbuf->port = rxq->port_id;
>
> -#ifdef RTE_LIBRTE_IEEE1588
>         if (unlikely((rte_le_to_cpu_16(rxcmp->flags_type) &
>                       RX_PKT_CMPL_FLAGS_MASK) ==
>                      RX_PKT_CMPL_FLAGS_ITYPE_PTP_W_TIMESTAMP))
>                 bnxt_get_rx_ts_p5(rxq->bp, rxcmp1->reorder);
> -#endif
>
>         if (cmp_type == CMPL_BASE_TYPE_RX_L2_V2) {
>                 bnxt_parse_csum_v2(mbuf, rxcmp1);
> --
> 2.37.1 (Apple Git-137.1)
>
  

Patch

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index c9aa45ed3b..f6eaaeb470 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -832,6 +832,7 @@  struct bnxt {
 	uint32_t		hwrm_spec_code;
 
 	struct bnxt_led_info	*leds;
+	uint8_t			ieee_1588;
 	struct bnxt_ptp_cfg     *ptp_cfg;
 	uint16_t		vf_resv_strategy;
 	struct bnxt_ctx_mem_info        *ctx;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 753e86b4b2..ef7b8859d9 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -97,6 +97,7 @@  static const struct rte_pci_id bnxt_pci_id_map[] = {
 #define BNXT_DEVARG_REP_FC_R2F  "rep-fc-r2f"
 #define BNXT_DEVARG_REP_FC_F2R  "rep-fc-f2r"
 #define BNXT_DEVARG_APP_ID	"app-id"
+#define BNXT_DEVARG_IEEE_1588	"ieee-1588"
 
 static const char *const bnxt_dev_args[] = {
 	BNXT_DEVARG_REPRESENTOR,
@@ -109,6 +110,7 @@  static const char *const bnxt_dev_args[] = {
 	BNXT_DEVARG_REP_FC_R2F,
 	BNXT_DEVARG_REP_FC_F2R,
 	BNXT_DEVARG_APP_ID,
+	BNXT_DEVARG_IEEE_1588,
 	NULL
 };
 
@@ -117,6 +119,11 @@  static const char *const bnxt_dev_args[] = {
  */
 #define BNXT_DEVARG_APP_ID_INVALID(val)			((val) > 255)
 
+/*
+ * ieee-1588 = an non-negative 8-bit number
+ */
+#define BNXT_DEVARG_IEEE_1588_INVALID(val)			((val) > 255)
+
 /*
  * flow_xstat == false to disable the feature
  * flow_xstat == true to enable the feature
@@ -1229,9 +1236,7 @@  bnxt_receive_function(struct rte_eth_dev *eth_dev)
 		return bnxt_recv_pkts;
 	}
 
-#if (defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)) && \
-	!defined(RTE_LIBRTE_IEEE1588)
-
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
 	/* Vector mode receive cannot be enabled if scattered rx is in use. */
 	if (eth_dev->data->scattered_rx)
 		goto use_scalar_rx;
@@ -1260,6 +1265,9 @@  bnxt_receive_function(struct rte_eth_dev *eth_dev)
 		  RTE_ETH_RX_OFFLOAD_VLAN_FILTER))
 		goto use_scalar_rx;
 
+	if (bp->ieee_1588)
+		goto use_scalar_rx;
+
 #if defined(RTE_ARCH_X86) && defined(CC_AVX2_SUPPORT)
 	if (rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_256 &&
 	    rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2) == 1) {
@@ -1300,8 +1308,7 @@  bnxt_transmit_function(struct rte_eth_dev *eth_dev)
 	if (BNXT_CHIP_SR2(bp))
 		return bnxt_xmit_pkts;
 
-#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) && \
-	!defined(RTE_LIBRTE_IEEE1588)
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
 	uint64_t offloads = eth_dev->data->dev_conf.txmode.offloads;
 
 	/*
@@ -1310,7 +1317,7 @@  bnxt_transmit_function(struct rte_eth_dev *eth_dev)
 	 */
 	if (eth_dev->data->scattered_rx ||
 	    (offloads & ~RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) ||
-	    BNXT_TRUFLOW_EN(bp))
+	    BNXT_TRUFLOW_EN(bp) || bp->ieee_1588)
 		goto use_scalar_tx;
 
 #if defined(RTE_ARCH_X86) && defined(CC_AVX2_SUPPORT)
@@ -5486,6 +5493,42 @@  bnxt_parse_devarg_app_id(__rte_unused const char *key,
 	return 0;
 }
 
+static int
+bnxt_parse_devarg_ieee_1588(__rte_unused const char *key,
+			    const char *value, void *opaque_arg)
+{
+	struct bnxt *bp = opaque_arg;
+	unsigned long ieee_1588;
+	char *end = NULL;
+
+	if (!value || !opaque_arg) {
+		PMD_DRV_LOG(ERR,
+			    "Invalid parameter passed to ieee-1588 "
+			    "devargs.\n");
+		return -EINVAL;
+	}
+
+	ieee_1588 = strtoul(value, &end, 10);
+	if (end == NULL || *end != '\0' ||
+	    (ieee_1588 == ULONG_MAX && errno == ERANGE)) {
+		PMD_DRV_LOG(ERR,
+			    "Invalid parameter passed to ieee_1588 "
+			    "devargs.\n");
+		return -EINVAL;
+	}
+
+	if (BNXT_DEVARG_IEEE_1588_INVALID(ieee_1588)) {
+		PMD_DRV_LOG(ERR, "Invalid ieee-1588(%d) devargs.\n",
+			    (uint16_t)ieee_1588);
+		return -EINVAL;
+	}
+
+	bp->ieee_1588 = ieee_1588;
+	PMD_DRV_LOG(INFO, "ieee-1588=%d feature enabled.\n", (uint16_t)ieee_1588);
+
+	return 0;
+}
+
 static int
 bnxt_parse_devarg_rep_is_pf(__rte_unused const char *key,
 			    const char *value, void *opaque_arg)
@@ -5748,6 +5791,13 @@  bnxt_parse_dev_args(struct bnxt *bp, struct rte_devargs *devargs)
 	rte_kvargs_process(kvlist, BNXT_DEVARG_APP_ID,
 			   bnxt_parse_devarg_app_id, bp);
 
+	/*
+	 * Handler for "ieee-1588" devarg.
+	 * Invoked as for ex: "-a 000:00:0d.0,ieee-1588=1"
+	 */
+	rte_kvargs_process(kvlist, BNXT_DEVARG_IEEE_1588,
+			   bnxt_parse_devarg_ieee_1588, bp);
+
 	rte_kvargs_free(kvlist);
 	return ret;
 }
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 0eebddb05d..a067278dca 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -680,16 +680,13 @@  bnxt_set_ol_flags(struct bnxt_rx_ring_info *rxr, struct rx_pkt_cmpl *rxcmp,
 		ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
 	}
 
-#ifdef RTE_LIBRTE_IEEE1588
 	if (unlikely((flags_type & RX_PKT_CMPL_FLAGS_MASK) ==
 		     RX_PKT_CMPL_FLAGS_ITYPE_PTP_W_TIMESTAMP))
 		ol_flags |= RTE_MBUF_F_RX_IEEE1588_PTP | RTE_MBUF_F_RX_IEEE1588_TMST;
-#endif
 
 	mbuf->ol_flags = ol_flags;
 }
 
-#ifdef RTE_LIBRTE_IEEE1588
 static void
 bnxt_get_rx_ts_p5(struct bnxt *bp, uint32_t rx_ts_cmpl)
 {
@@ -716,7 +713,6 @@  bnxt_get_rx_ts_p5(struct bnxt *bp, uint32_t rx_ts_cmpl)
 	}
 	ptp->rx_timestamp = pkt_time;
 }
-#endif
 
 static uint32_t
 bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
@@ -925,12 +921,10 @@  static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 	mbuf->data_len = mbuf->pkt_len;
 	mbuf->port = rxq->port_id;
 
-#ifdef RTE_LIBRTE_IEEE1588
 	if (unlikely((rte_le_to_cpu_16(rxcmp->flags_type) &
 		      RX_PKT_CMPL_FLAGS_MASK) ==
 		     RX_PKT_CMPL_FLAGS_ITYPE_PTP_W_TIMESTAMP))
 		bnxt_get_rx_ts_p5(rxq->bp, rxcmp1->reorder);
-#endif
 
 	if (cmp_type == CMPL_BASE_TYPE_RX_L2_V2) {
 		bnxt_parse_csum_v2(mbuf, rxcmp1);