DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
@ 2017-11-27 12:29 Kirill Rybalchenko
  2017-12-04 17:43 ` Adrien Mazarguil
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Kirill Rybalchenko @ 2017-11-27 12:29 UTC (permalink / raw)
  To: dev
  Cc: jingjing.wu, beilei.xing, johndale, neescoba, adrien.mazarguil,
	nelio.laranjeiro, yskoh, wenzhuo.lu, konstantin.ananyev,
	kirill.rybalchenko, andrey.chilikin

Increase the internal limit for flow types from 32 to 64
to support future flow type extensions.
Change type of variables from uint32_t[] to uint64_t[]:
  rte_eth_fdir_info.flow_types_mask
  rte_eth_hash_global_conf.sym_hash_enable_mask
  rte_eth_hash_global_conf.valid_bit_mask

This modification affects the following components:
  net/i40e
  net/enic
  net/mlx5
  net/ixgbe
  app/testpmd

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 app/test-pmd/cmdline.c          | 22 +++++++++++-----------
 drivers/net/enic/enic_clsf.c    |  6 +++++-
 drivers/net/i40e/i40e_ethdev.c  | 38 +++++++++++++++++++-------------------
 drivers/net/i40e/i40e_fdir.c    | 25 ++++++++++++++-----------
 drivers/net/ixgbe/ixgbe_fdir.c  | 22 ++++++++++++----------
 drivers/net/mlx5/mlx5_flow.c    |  4 +++-
 lib/librte_ether/rte_eth_ctrl.h | 12 ++++++------
 7 files changed, 70 insertions(+), 59 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f71d963..3e57715 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -10681,7 +10681,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 	struct rte_eth_fdir_info fdir_info;
 	struct rte_eth_fdir_flex_mask flex_mask;
 	struct rte_port *port;
-	uint32_t flow_type_mask;
+	uint64_t flow_type_mask;
 	uint16_t i;
 	int ret;
 
@@ -10734,7 +10734,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 			return;
 		}
 		for (i = RTE_ETH_FLOW_UNKNOWN; i < RTE_ETH_FLOW_MAX; i++) {
-			if (flow_type_mask & (1 << i)) {
+			if (flow_type_mask & (1ULL << i)) {
 				flex_mask.flow_type = i;
 				fdir_set_flex_mask(res->port_id, &flex_mask);
 			}
@@ -10743,7 +10743,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 		return;
 	}
 	flex_mask.flow_type = str2flowtype(res->flow_type);
-	if (!(flow_type_mask & (1 << flex_mask.flow_type))) {
+	if (!(flow_type_mask & (1ULL << flex_mask.flow_type))) {
 		printf("Flow type %s not supported on port %d\n",
 				res->flow_type, res->port_id);
 		return;
@@ -11105,10 +11105,10 @@ cmd_get_hash_global_config_parsed(void *parsed_result,
 	}
 
 	for (i = 0; i < RTE_ETH_FLOW_MAX; i++) {
-		idx = i / UINT32_BIT;
-		offset = i % UINT32_BIT;
+		idx = i / UINT64_BIT;
+		offset = i % UINT64_BIT;
 		if (!(info.info.global_conf.valid_bit_mask[idx] &
-						(1UL << offset)))
+						(1ULL << offset)))
 			continue;
 		str = flowtype_to_str(i);
 		if (!str)
@@ -11116,7 +11116,7 @@ cmd_get_hash_global_config_parsed(void *parsed_result,
 		printf("Symmetric hash is %s globally for flow type %s "
 							"by port %d\n",
 			((info.info.global_conf.sym_hash_enable_mask[idx] &
-			(1UL << offset)) ? "enabled" : "disabled"), str,
+			(1ULL << offset)) ? "enabled" : "disabled"), str,
 							res->port_id);
 	}
 }
@@ -11177,12 +11177,12 @@ cmd_set_hash_global_config_parsed(void *parsed_result,
 			RTE_ETH_HASH_FUNCTION_DEFAULT;
 
 	ftype = str2flowtype(res->flow_type);
-	idx = ftype / (CHAR_BIT * sizeof(uint32_t));
-	offset = ftype % (CHAR_BIT * sizeof(uint32_t));
-	info.info.global_conf.valid_bit_mask[idx] |= (1UL << offset);
+	idx = ftype / UINT64_BIT;
+	offset = ftype % UINT64_BIT;
+	info.info.global_conf.valid_bit_mask[idx] |= (1ULL << offset);
 	if (!strcmp(res->enable, "enable"))
 		info.info.global_conf.sym_hash_enable_mask[idx] |=
-						(1UL << offset);
+						(1ULL << offset);
 	ret = rte_eth_dev_filter_ctrl(res->port_id, RTE_ETH_FILTER_HASH,
 					RTE_ETH_FILTER_SET, &info);
 	if (ret < 0)
diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c
index 9b46142..c5644cb 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -74,8 +74,12 @@ void enic_fdir_stats_get(struct enic *enic, struct rte_eth_fdir_stats *stats)
 
 void enic_fdir_info_get(struct enic *enic, struct rte_eth_fdir_info *info)
 {
+	uint32_t i;
+
 	info->mode = (enum rte_fdir_mode)enic->fdir.modes;
-	info->flow_types_mask[0] = enic->fdir.types_mask;
+	info->flow_types_mask[0] = (uint64_t)enic->fdir.types_mask;
+	for (i = 1; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
+		info->flow_types_mask[i] = 0ULL;
 }
 
 void enic_fdir_info(struct enic *enic)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 811cc9f..78744c2 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -7984,14 +7984,17 @@ i40e_get_hash_filter_global_config(struct i40e_hw *hw,
 		(reg & I40E_GLQF_CTL_HTOEP_MASK) ? "Toeplitz" : "Simple XOR");
 
 	/*
-	 * We work only with lowest 32 bits which is not correct, but to work
-	 * properly the valid_bit_mask size should be increased up to 64 bits
-	 * and this will brake ABI. This modification will be done in next
-	 * release
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
+	 * be checked.
 	 */
-	g_cfg->valid_bit_mask[0] = (uint32_t)adapter->flow_types_mask;
+	for (i = 1; i < RTE_SYM_HASH_MASK_ARRAY_SIZE; i++) {
+		g_cfg->valid_bit_mask[i] = 0ULL;
+		g_cfg->sym_hash_enable_mask[i] = 0ULL;
+	}
 
-	for (i = RTE_ETH_FLOW_UNKNOWN + 1; i < UINT32_BIT; i++) {
+	g_cfg->valid_bit_mask[0] = adapter->flow_types_mask;
+
+	for (i = RTE_ETH_FLOW_UNKNOWN + 1; i < UINT64_BIT; i++) {
 		if (!adapter->pctypes_tbl[i])
 			continue;
 		for (j = I40E_FILTER_PCTYPE_INVALID + 1;
@@ -8000,7 +8003,7 @@ i40e_get_hash_filter_global_config(struct i40e_hw *hw,
 				reg = i40e_read_rx_ctl(hw, I40E_GLQF_HSYM(j));
 				if (reg & I40E_GLQF_HSYM_SYMH_ENA_MASK) {
 					g_cfg->sym_hash_enable_mask[0] |=
-								(1UL << i);
+								(1ULL << i);
 				}
 			}
 		}
@@ -8014,7 +8017,7 @@ i40e_hash_global_config_check(const struct i40e_adapter *adapter,
 			      const struct rte_eth_hash_global_conf *g_cfg)
 {
 	uint32_t i;
-	uint32_t mask0, i40e_mask = adapter->flow_types_mask;
+	uint64_t mask0, i40e_mask = adapter->flow_types_mask;
 
 	if (g_cfg->hash_func != RTE_ETH_HASH_FUNCTION_TOEPLITZ &&
 		g_cfg->hash_func != RTE_ETH_HASH_FUNCTION_SIMPLE_XOR &&
@@ -8025,7 +8028,7 @@ i40e_hash_global_config_check(const struct i40e_adapter *adapter,
 	}
 
 	/*
-	 * As i40e supports less than 32 flow types, only first 32 bits need to
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
 	 * be checked.
 	 */
 	mask0 = g_cfg->valid_bit_mask[0];
@@ -8061,23 +8064,20 @@ i40e_set_hash_filter_global_config(struct i40e_hw *hw,
 	int ret;
 	uint16_t i, j;
 	uint32_t reg;
-	/*
-	 * We work only with lowest 32 bits which is not correct, but to work
-	 * properly the valid_bit_mask size should be increased up to 64 bits
-	 * and this will brake ABI. This modification will be done in next
-	 * release
-	 */
-	uint32_t mask0 = g_cfg->valid_bit_mask[0] &
-					(uint32_t)adapter->flow_types_mask;
+	uint64_t mask0 = g_cfg->valid_bit_mask[0] & adapter->flow_types_mask;
 
 	/* Check the input parameters */
 	ret = i40e_hash_global_config_check(adapter, g_cfg);
 	if (ret < 0)
 		return ret;
 
-	for (i = RTE_ETH_FLOW_UNKNOWN + 1; mask0 && i < UINT32_BIT; i++) {
+	/*
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
+	 * be configured.
+	 */
+	for (i = RTE_ETH_FLOW_UNKNOWN + 1; mask0 && i < UINT64_BIT; i++) {
 		if (mask0 & (1UL << i)) {
-			reg = (g_cfg->sym_hash_enable_mask[0] & (1UL << i)) ?
+			reg = (g_cfg->sym_hash_enable_mask[0] & (1ULL << i)) ?
 					I40E_GLQF_HSYM_SYMH_ENA_MASK : 0;
 
 			for (j = I40E_FILTER_PCTYPE_INVALID + 1;
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 3d7170d..fbc46ee 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -95,17 +95,17 @@
 #define I40E_COUNTER_INDEX_FDIR(pf_id)   (0 + (pf_id) * I40E_COUNTER_PF)
 
 #define I40E_FDIR_FLOWS ( \
-	(1 << RTE_ETH_FLOW_FRAG_IPV4) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
-	(1 << RTE_ETH_FLOW_FRAG_IPV6) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER) | \
-	(1 << RTE_ETH_FLOW_L2_PAYLOAD))
+	(1ULL << RTE_ETH_FLOW_FRAG_IPV4) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_FRAG_IPV6) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_L2_PAYLOAD))
 
 static int i40e_fdir_filter_programming(struct i40e_pf *pf,
 			enum i40e_filter_pctype pctype,
@@ -2020,6 +2020,7 @@ i40e_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir)
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	uint16_t num_flex_set = 0;
 	uint16_t num_flex_mask = 0;
+	uint16_t i;
 
 	if (dev->data->dev_conf.fdir_conf.mode == RTE_FDIR_MODE_PERFECT)
 		fdir->mode = RTE_FDIR_MODE_PERFECT;
@@ -2032,6 +2033,8 @@ i40e_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir)
 		(uint32_t)hw->func_caps.fd_filters_best_effort;
 	fdir->max_flexpayload = I40E_FDIR_MAX_FLEX_LEN;
 	fdir->flow_types_mask[0] = I40E_FDIR_FLOWS;
+	for (i = 1; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
+		fdir->flow_types_mask[i] = 0ULL;
 	fdir->flex_payload_unit = sizeof(uint16_t);
 	fdir->flex_bitmask_unit = sizeof(uint16_t);
 	fdir->max_flex_payload_segment_num = I40E_MAX_FLXPLD_FIED;
diff --git a/drivers/net/ixgbe/ixgbe_fdir.c b/drivers/net/ixgbe/ixgbe_fdir.c
index 9281dc1..6870a57 100644
--- a/drivers/net/ixgbe/ixgbe_fdir.c
+++ b/drivers/net/ixgbe/ixgbe_fdir.c
@@ -70,14 +70,14 @@
 #define IXGBE_FDIRCMD_CMD_INTERVAL_US   10
 
 #define IXGBE_FDIR_FLOW_TYPES ( \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER))
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER))
 
 #define IPV6_ADDR_TO_MASK(ipaddr, ipv6m) do { \
 	uint8_t ipv6_addr[16]; \
@@ -1435,7 +1435,7 @@ ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_hw_fdir_info *info =
 			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
-	uint32_t fdirctrl, max_num;
+	uint32_t fdirctrl, max_num, i;
 	uint8_t offset;
 
 	fdirctrl = IXGBE_READ_REG(hw, IXGBE_FDIRCTRL);
@@ -1467,9 +1467,11 @@ ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info
 
 	if (fdir_info->mode == RTE_FDIR_MODE_PERFECT_MAC_VLAN ||
 	    fdir_info->mode == RTE_FDIR_MODE_PERFECT_TUNNEL)
-		fdir_info->flow_types_mask[0] = 0;
+		fdir_info->flow_types_mask[0] = 0ULL;
 	else
 		fdir_info->flow_types_mask[0] = IXGBE_FDIR_FLOW_TYPES;
+	for (i = 1; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
+		fdir_info->flow_types_mask[i] = 0ULL;
 
 	fdir_info->flex_payload_unit = sizeof(uint16_t);
 	fdir_info->max_flex_payload_segment_num = 1;
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index f32dfdd..84927ff 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2987,6 +2987,7 @@ priv_fdir_filter_flush(struct priv *priv)
 static void
 priv_fdir_info_get(struct priv *priv, struct rte_eth_fdir_info *fdir_info)
 {
+	uint32_t i;
 	struct rte_eth_fdir_masks *mask =
 		&priv->dev->data->dev_conf.fdir_conf.mask;
 
@@ -2994,7 +2995,8 @@ priv_fdir_info_get(struct priv *priv, struct rte_eth_fdir_info *fdir_info)
 	fdir_info->guarant_spc = 0;
 	rte_memcpy(&fdir_info->mask, mask, sizeof(fdir_info->mask));
 	fdir_info->max_flexpayload = 0;
-	fdir_info->flow_types_mask[0] = 0;
+	for (i = 0; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
+		info->flow_types_mask[i] = 0ULL;
 	fdir_info->flex_payload_unit = 0;
 	fdir_info->max_flex_payload_segment_num = 0;
 	fdir_info->flex_payload_limit = 0;
diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 8386904..4f24a93 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -691,9 +691,9 @@ enum rte_fdir_mode {
 	RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode - tunnel. */
 };
 
-#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
+#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
 #define RTE_FLOW_MASK_ARRAY_SIZE \
-	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
+	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
 
 /**
  * A structure used to get the information of flow director filter.
@@ -710,7 +710,7 @@ struct rte_eth_fdir_info {
 	uint32_t guarant_spc; /**< Guaranteed spaces.*/
 	uint32_t best_spc; /**< Best effort spaces.*/
 	/** Bit mask for every supported flow type. */
-	uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
+	uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
 	uint32_t max_flexpayload; /**< Total flex payload in bytes. */
 	/** Flexible payload unit in bytes. Size and alignments of all flex
 	    payload segments should be multiplies of this value. */
@@ -803,7 +803,7 @@ enum rte_eth_hash_function {
 };
 
 #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
-	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
+	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
 /**
  * A structure used to set or get global hash function configurations which
  * include symmetric hash enable per flow type and hash function type.
@@ -816,9 +816,9 @@ enum rte_eth_hash_function {
 struct rte_eth_hash_global_conf {
 	enum rte_eth_hash_function hash_func; /**< Hash function type */
 	/** Bit mask for symmetric hash enable per flow type */
-	uint32_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
+	uint64_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
 	/** Bit mask indicates if the corresponding bit is valid */
-	uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
+	uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
 };
 
 /**
-- 
2.5.5

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
  2017-11-27 12:29 [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64 Kirill Rybalchenko
@ 2017-12-04 17:43 ` Adrien Mazarguil
  2018-01-09 15:16   ` Rybalchenko, Kirill
  2018-01-09 14:30 ` Zhang, Helin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Adrien Mazarguil @ 2017-12-04 17:43 UTC (permalink / raw)
  To: Kirill Rybalchenko
  Cc: dev, jingjing.wu, beilei.xing, johndale, neescoba,
	nelio.laranjeiro, yskoh, wenzhuo.lu, konstantin.ananyev,
	andrey.chilikin

Hi Kirill,

On Mon, Nov 27, 2017 at 12:29:47PM +0000, Kirill Rybalchenko wrote:
> Increase the internal limit for flow types from 32 to 64
> to support future flow type extensions.
> Change type of variables from uint32_t[] to uint64_t[]:
>   rte_eth_fdir_info.flow_types_mask
>   rte_eth_hash_global_conf.sym_hash_enable_mask
>   rte_eth_hash_global_conf.valid_bit_mask
> 
> This modification affects the following components:
>   net/i40e
>   net/enic
>   net/mlx5
>   net/ixgbe
>   app/testpmd
> 
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>

Can you elaborate a bit on the need for these changes?
Have you considered implementing those future extensions through rte_flow
instead?

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
  2017-11-27 12:29 [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64 Kirill Rybalchenko
  2017-12-04 17:43 ` Adrien Mazarguil
@ 2018-01-09 14:30 ` Zhang, Helin
  2018-01-10  6:51 ` Xing, Beilei
  2018-01-15 16:58 ` [dpdk-dev] [PATCH v2] " Kirill Rybalchenko
  3 siblings, 0 replies; 19+ messages in thread
From: Zhang, Helin @ 2018-01-09 14:30 UTC (permalink / raw)
  To: Xing, Beilei, johndale, neescoba, adrien.mazarguil,
	nelio.laranjeiro, yskoh, Lu, Wenzhuo, Ananyev, Konstantin, Zhang,
	Qi Z
  Cc: Rybalchenko, Kirill, dev

Hi maintainers

What is your comments on this patch?
As it affect Intel, ENIC and MLX NICs, I'd like to see the ACK from the maintainers. Then I can apply the patch.

Hi Kirill

I already saw a comments. Could you help to address it from 6wind.

Regards,
Helin

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Kirill Rybalchenko
> Sent: Monday, November 27, 2017 8:30 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Xing, Beilei; johndale@cisco.com; neescoba@cisco.com;
> adrien.mazarguil@6wind.com; nelio.laranjeiro@6wind.com;
> yskoh@mellanox.com; Lu, Wenzhuo; Ananyev, Konstantin; Rybalchenko, Kirill;
> Chilikin, Andrey
> Subject: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
> 
> Increase the internal limit for flow types from 32 to 64 to support future flow
> type extensions.
> Change type of variables from uint32_t[] to uint64_t[]:
>   rte_eth_fdir_info.flow_types_mask
>   rte_eth_hash_global_conf.sym_hash_enable_mask
>   rte_eth_hash_global_conf.valid_bit_mask
> 
> This modification affects the following components:
>   net/i40e
>   net/enic
>   net/mlx5
>   net/ixgbe
>   app/testpmd
> 
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
  2017-12-04 17:43 ` Adrien Mazarguil
@ 2018-01-09 15:16   ` Rybalchenko, Kirill
  2018-01-10 13:50     ` Thomas Monjalon
  2018-01-16 11:13     ` Adrien Mazarguil
  0 siblings, 2 replies; 19+ messages in thread
From: Rybalchenko, Kirill @ 2018-01-09 15:16 UTC (permalink / raw)
  To: Adrien Mazarguil
  Cc: dev, Wu, Jingjing, Xing, Beilei, johndale, neescoba,
	nelio.laranjeiro, yskoh, Lu, Wenzhuo, Ananyev, Konstantin,
	Chilikin, Andrey



> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Monday 4 December 2017 17:43
> To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; johndale@cisco.com; neescoba@cisco.com;
> nelio.laranjeiro@6wind.com; yskoh@mellanox.com; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Chilikin, Andrey
> <andrey.chilikin@intel.com>
> Subject: Re: [PATCH] ethdev: increase flow type limit from 32 to 64
> 
> Hi Kirill,
> 
> On Mon, Nov 27, 2017 at 12:29:47PM +0000, Kirill Rybalchenko wrote:
> > Increase the internal limit for flow types from 32 to 64 to support
> > future flow type extensions.
> > Change type of variables from uint32_t[] to uint64_t[]:
> >   rte_eth_fdir_info.flow_types_mask
> >   rte_eth_hash_global_conf.sym_hash_enable_mask
> >   rte_eth_hash_global_conf.valid_bit_mask
> >
> > This modification affects the following components:
> >   net/i40e
> >   net/enic
> >   net/mlx5
> >   net/ixgbe
> >   app/testpmd
> >
> > Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> 
> Can you elaborate a bit on the need for these changes?
> Have you considered implementing those future extensions through
> rte_flow instead?

Hi Adrien, this is not a new feature but rather fix of existing limitation.
In current implementation the symmetric hash mask and flow mask are
represented by 32-bit variable, while hardware bitmask has 64 bits.
Unfortunately, this modification changes ABI of the library as it changes size
of rte_eth_fdir_info structure. All related PMDs (listed above) had to be modified
accordingly.  

> 
> --
> Adrien Mazarguil
> 6WIND

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
  2017-11-27 12:29 [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64 Kirill Rybalchenko
  2017-12-04 17:43 ` Adrien Mazarguil
  2018-01-09 14:30 ` Zhang, Helin
@ 2018-01-10  6:51 ` Xing, Beilei
  2018-01-15 16:58 ` [dpdk-dev] [PATCH v2] " Kirill Rybalchenko
  3 siblings, 0 replies; 19+ messages in thread
From: Xing, Beilei @ 2018-01-10  6:51 UTC (permalink / raw)
  To: Rybalchenko, Kirill, dev
  Cc: Wu, Jingjing, johndale, neescoba, adrien.mazarguil,
	nelio.laranjeiro, yskoh, Lu, Wenzhuo, Ananyev, Konstantin,
	Chilikin, Andrey



> -----Original Message-----
> From: Rybalchenko, Kirill
> Sent: Monday, November 27, 2017 8:30 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; johndale@cisco.com; neescoba@cisco.com;
> adrien.mazarguil@6wind.com; nelio.laranjeiro@6wind.com;
> yskoh@mellanox.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Rybalchenko, Kirill
> <kirill.rybalchenko@intel.com>; Chilikin, Andrey <andrey.chilikin@intel.com>
> Subject: [PATCH] ethdev: increase flow type limit from 32 to 64
> 
> Increase the internal limit for flow types from 32 to 64 to support future flow
> type extensions.
> Change type of variables from uint32_t[] to uint64_t[]:
>   rte_eth_fdir_info.flow_types_mask
>   rte_eth_hash_global_conf.sym_hash_enable_mask
>   rte_eth_hash_global_conf.valid_bit_mask
> 
> This modification affects the following components:
>   net/i40e
>   net/enic
>   net/mlx5
>   net/ixgbe
>   app/testpmd
> 
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>

Reviewed-by: Beilei Xing <beilei.xing@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
  2018-01-09 15:16   ` Rybalchenko, Kirill
@ 2018-01-10 13:50     ` Thomas Monjalon
  2018-01-16 11:13     ` Adrien Mazarguil
  1 sibling, 0 replies; 19+ messages in thread
From: Thomas Monjalon @ 2018-01-10 13:50 UTC (permalink / raw)
  To: Rybalchenko, Kirill
  Cc: dev, Adrien Mazarguil, Wu, Jingjing, Xing, Beilei, johndale,
	neescoba, nelio.laranjeiro, yskoh, Lu, Wenzhuo, Ananyev,
	Konstantin, Chilikin, Andrey

Hi,

09/01/2018 16:16, Rybalchenko, Kirill:
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > On Mon, Nov 27, 2017 at 12:29:47PM +0000, Kirill Rybalchenko wrote:
> > > Increase the internal limit for flow types from 32 to 64 to support
> > > future flow type extensions.
> > > Change type of variables from uint32_t[] to uint64_t[]:
> > >   rte_eth_fdir_info.flow_types_mask
> > >   rte_eth_hash_global_conf.sym_hash_enable_mask
> > >   rte_eth_hash_global_conf.valid_bit_mask
> > >
> > > This modification affects the following components:
> > >   net/i40e
> > >   net/enic
> > >   net/mlx5
> > >   net/ixgbe
> > >   app/testpmd
> > >
> > > Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> > 
> > Can you elaborate a bit on the need for these changes?
> > Have you considered implementing those future extensions through
> > rte_flow instead?
> 
> Hi Adrien, this is not a new feature but rather fix of existing limitation.
> In current implementation the symmetric hash mask and flow mask are
> represented by 32-bit variable, while hardware bitmask has 64 bits.
> Unfortunately, this modification changes ABI of the library as it changes size
> of rte_eth_fdir_info structure. All related PMDs (listed above) had to be modified
> accordingly.  

This ABI change is not announced.
Unfortunately it will have to wait the release 18.05.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-dev] [PATCH v2] ethdev: increase flow type limit from 32 to 64
  2017-11-27 12:29 [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64 Kirill Rybalchenko
                   ` (2 preceding siblings ...)
  2018-01-10  6:51 ` Xing, Beilei
@ 2018-01-15 16:58 ` Kirill Rybalchenko
  2018-01-15 17:33   ` [dpdk-dev] [PATCH v3] " Kirill Rybalchenko
  3 siblings, 1 reply; 19+ messages in thread
From: Kirill Rybalchenko @ 2018-01-15 16:58 UTC (permalink / raw)
  To: dev; +Cc: kirill.rybalchenko, andrey.chilikin, thomas, ferruh.yigit

Increase the internal limit for flow types from 32 to 64
to support future flow type extensions.
Change type of variables from uint32_t[] to uint64_t[]:
rte_eth_fdir_info.flow_types_mask
rte_eth_hash_global_conf.sym_hash_enable_mask
rte_eth_hash_global_conf.valid_bit_mask

This modification affects the following components:
net/i40e
net/ixgbe
app/testpmd

v2:
implement versioning of rte_eth_dev_filter_ctrl function
for ABI backward compatibility with version 17.11 and older

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 app/test-pmd/cmdline.c                  |  22 ++---
 drivers/net/i40e/i40e_ethdev.c          |  38 ++++----
 drivers/net/i40e/i40e_fdir.c            |  25 ++---
 drivers/net/ixgbe/ixgbe_fdir.c          |  22 +++--
 lib/librte_ether/rte_eth_ctrl.h         |  12 +--
 lib/librte_ether/rte_ethdev.c           | 157 +++++++++++++++++++++++++++++++-
 lib/librte_ether/rte_ethdev_version.map |   7 ++
 7 files changed, 224 insertions(+), 59 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 6d71a5f..964d4ed 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -10803,7 +10803,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 	struct rte_eth_fdir_info fdir_info;
 	struct rte_eth_fdir_flex_mask flex_mask;
 	struct rte_port *port;
-	uint32_t flow_type_mask;
+	uint64_t flow_type_mask;
 	uint16_t i;
 	int ret;
 
@@ -10856,7 +10856,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 			return;
 		}
 		for (i = RTE_ETH_FLOW_UNKNOWN; i < RTE_ETH_FLOW_MAX; i++) {
-			if (flow_type_mask & (1 << i)) {
+			if (flow_type_mask & (1ULL << i)) {
 				flex_mask.flow_type = i;
 				fdir_set_flex_mask(res->port_id, &flex_mask);
 			}
@@ -10865,7 +10865,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 		return;
 	}
 	flex_mask.flow_type = str2flowtype(res->flow_type);
-	if (!(flow_type_mask & (1 << flex_mask.flow_type))) {
+	if (!(flow_type_mask & (1ULL << flex_mask.flow_type))) {
 		printf("Flow type %s not supported on port %d\n",
 				res->flow_type, res->port_id);
 		return;
@@ -11227,10 +11227,10 @@ cmd_get_hash_global_config_parsed(void *parsed_result,
 	}
 
 	for (i = 0; i < RTE_ETH_FLOW_MAX; i++) {
-		idx = i / UINT32_BIT;
-		offset = i % UINT32_BIT;
+		idx = i / UINT64_BIT;
+		offset = i % UINT64_BIT;
 		if (!(info.info.global_conf.valid_bit_mask[idx] &
-						(1UL << offset)))
+						(1ULL << offset)))
 			continue;
 		str = flowtype_to_str(i);
 		if (!str)
@@ -11238,7 +11238,7 @@ cmd_get_hash_global_config_parsed(void *parsed_result,
 		printf("Symmetric hash is %s globally for flow type %s "
 							"by port %d\n",
 			((info.info.global_conf.sym_hash_enable_mask[idx] &
-			(1UL << offset)) ? "enabled" : "disabled"), str,
+			(1ULL << offset)) ? "enabled" : "disabled"), str,
 							res->port_id);
 	}
 }
@@ -11299,12 +11299,12 @@ cmd_set_hash_global_config_parsed(void *parsed_result,
 			RTE_ETH_HASH_FUNCTION_DEFAULT;
 
 	ftype = str2flowtype(res->flow_type);
-	idx = ftype / (CHAR_BIT * sizeof(uint32_t));
-	offset = ftype % (CHAR_BIT * sizeof(uint32_t));
-	info.info.global_conf.valid_bit_mask[idx] |= (1UL << offset);
+	idx = ftype / UINT64_BIT;
+	offset = ftype % UINT64_BIT;
+	info.info.global_conf.valid_bit_mask[idx] |= (1ULL << offset);
 	if (!strcmp(res->enable, "enable"))
 		info.info.global_conf.sym_hash_enable_mask[idx] |=
-						(1UL << offset);
+						(1ULL << offset);
 	ret = rte_eth_dev_filter_ctrl(res->port_id, RTE_ETH_FILTER_HASH,
 					RTE_ETH_FILTER_SET, &info);
 	if (ret < 0)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7796e9e..d0473f0 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -8134,14 +8134,17 @@ i40e_get_hash_filter_global_config(struct i40e_hw *hw,
 		(reg & I40E_GLQF_CTL_HTOEP_MASK) ? "Toeplitz" : "Simple XOR");
 
 	/*
-	 * We work only with lowest 32 bits which is not correct, but to work
-	 * properly the valid_bit_mask size should be increased up to 64 bits
-	 * and this will brake ABI. This modification will be done in next
-	 * release
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
+	 * be checked.
 	 */
-	g_cfg->valid_bit_mask[0] = (uint32_t)adapter->flow_types_mask;
+	for (i = 1; i < RTE_SYM_HASH_MASK_ARRAY_SIZE; i++) {
+		g_cfg->valid_bit_mask[i] = 0ULL;
+		g_cfg->sym_hash_enable_mask[i] = 0ULL;
+	}
 
-	for (i = RTE_ETH_FLOW_UNKNOWN + 1; i < UINT32_BIT; i++) {
+	g_cfg->valid_bit_mask[0] = adapter->flow_types_mask;
+
+	for (i = RTE_ETH_FLOW_UNKNOWN + 1; i < UINT64_BIT; i++) {
 		if (!adapter->pctypes_tbl[i])
 			continue;
 		for (j = I40E_FILTER_PCTYPE_INVALID + 1;
@@ -8150,7 +8153,7 @@ i40e_get_hash_filter_global_config(struct i40e_hw *hw,
 				reg = i40e_read_rx_ctl(hw, I40E_GLQF_HSYM(j));
 				if (reg & I40E_GLQF_HSYM_SYMH_ENA_MASK) {
 					g_cfg->sym_hash_enable_mask[0] |=
-								(1UL << i);
+								(1ULL << i);
 				}
 			}
 		}
@@ -8164,7 +8167,7 @@ i40e_hash_global_config_check(const struct i40e_adapter *adapter,
 			      const struct rte_eth_hash_global_conf *g_cfg)
 {
 	uint32_t i;
-	uint32_t mask0, i40e_mask = adapter->flow_types_mask;
+	uint64_t mask0, i40e_mask = adapter->flow_types_mask;
 
 	if (g_cfg->hash_func != RTE_ETH_HASH_FUNCTION_TOEPLITZ &&
 		g_cfg->hash_func != RTE_ETH_HASH_FUNCTION_SIMPLE_XOR &&
@@ -8175,7 +8178,7 @@ i40e_hash_global_config_check(const struct i40e_adapter *adapter,
 	}
 
 	/*
-	 * As i40e supports less than 32 flow types, only first 32 bits need to
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
 	 * be checked.
 	 */
 	mask0 = g_cfg->valid_bit_mask[0];
@@ -8211,23 +8214,20 @@ i40e_set_hash_filter_global_config(struct i40e_hw *hw,
 	int ret;
 	uint16_t i, j;
 	uint32_t reg;
-	/*
-	 * We work only with lowest 32 bits which is not correct, but to work
-	 * properly the valid_bit_mask size should be increased up to 64 bits
-	 * and this will brake ABI. This modification will be done in next
-	 * release
-	 */
-	uint32_t mask0 = g_cfg->valid_bit_mask[0] &
-					(uint32_t)adapter->flow_types_mask;
+	uint64_t mask0 = g_cfg->valid_bit_mask[0] & adapter->flow_types_mask;
 
 	/* Check the input parameters */
 	ret = i40e_hash_global_config_check(adapter, g_cfg);
 	if (ret < 0)
 		return ret;
 
-	for (i = RTE_ETH_FLOW_UNKNOWN + 1; mask0 && i < UINT32_BIT; i++) {
+	/*
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
+	 * be configured.
+	 */
+	for (i = RTE_ETH_FLOW_UNKNOWN + 1; mask0 && i < UINT64_BIT; i++) {
 		if (mask0 & (1UL << i)) {
-			reg = (g_cfg->sym_hash_enable_mask[0] & (1UL << i)) ?
+			reg = (g_cfg->sym_hash_enable_mask[0] & (1ULL << i)) ?
 					I40E_GLQF_HSYM_SYMH_ENA_MASK : 0;
 
 			for (j = I40E_FILTER_PCTYPE_INVALID + 1;
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 906c204..3a9e656 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -66,17 +66,17 @@
 #define I40E_COUNTER_INDEX_FDIR(pf_id)   (0 + (pf_id) * I40E_COUNTER_PF)
 
 #define I40E_FDIR_FLOWS ( \
-	(1 << RTE_ETH_FLOW_FRAG_IPV4) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
-	(1 << RTE_ETH_FLOW_FRAG_IPV6) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER) | \
-	(1 << RTE_ETH_FLOW_L2_PAYLOAD))
+	(1ULL << RTE_ETH_FLOW_FRAG_IPV4) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_FRAG_IPV6) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_L2_PAYLOAD))
 
 static int i40e_fdir_filter_programming(struct i40e_pf *pf,
 			enum i40e_filter_pctype pctype,
@@ -1999,6 +1999,7 @@ i40e_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir)
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	uint16_t num_flex_set = 0;
 	uint16_t num_flex_mask = 0;
+	uint16_t i;
 
 	if (dev->data->dev_conf.fdir_conf.mode == RTE_FDIR_MODE_PERFECT)
 		fdir->mode = RTE_FDIR_MODE_PERFECT;
@@ -2011,6 +2012,8 @@ i40e_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir)
 		(uint32_t)hw->func_caps.fd_filters_best_effort;
 	fdir->max_flexpayload = I40E_FDIR_MAX_FLEX_LEN;
 	fdir->flow_types_mask[0] = I40E_FDIR_FLOWS;
+	for (i = 1; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
+		fdir->flow_types_mask[i] = 0ULL;
 	fdir->flex_payload_unit = sizeof(uint16_t);
 	fdir->flex_bitmask_unit = sizeof(uint16_t);
 	fdir->max_flex_payload_segment_num = I40E_MAX_FLXPLD_FIED;
diff --git a/drivers/net/ixgbe/ixgbe_fdir.c b/drivers/net/ixgbe/ixgbe_fdir.c
index 551580c..236ab8c 100644
--- a/drivers/net/ixgbe/ixgbe_fdir.c
+++ b/drivers/net/ixgbe/ixgbe_fdir.c
@@ -41,14 +41,14 @@
 #define IXGBE_FDIRCMD_CMD_INTERVAL_US   10
 
 #define IXGBE_FDIR_FLOW_TYPES ( \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER))
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER))
 
 #define IPV6_ADDR_TO_MASK(ipaddr, ipv6m) do { \
 	uint8_t ipv6_addr[16]; \
@@ -1407,7 +1407,7 @@ ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_hw_fdir_info *info =
 			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
-	uint32_t fdirctrl, max_num;
+	uint32_t fdirctrl, max_num, i;
 	uint8_t offset;
 
 	fdirctrl = IXGBE_READ_REG(hw, IXGBE_FDIRCTRL);
@@ -1439,9 +1439,11 @@ ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info
 
 	if (fdir_info->mode == RTE_FDIR_MODE_PERFECT_MAC_VLAN ||
 	    fdir_info->mode == RTE_FDIR_MODE_PERFECT_TUNNEL)
-		fdir_info->flow_types_mask[0] = 0;
+		fdir_info->flow_types_mask[0] = 0ULL;
 	else
 		fdir_info->flow_types_mask[0] = IXGBE_FDIR_FLOW_TYPES;
+	for (i = 1; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
+		fdir_info->flow_types_mask[i] = 0ULL;
 
 	fdir_info->flex_payload_unit = sizeof(uint16_t);
 	fdir_info->max_flex_payload_segment_num = 1;
diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 7991efa..668f59a 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -662,9 +662,9 @@ enum rte_fdir_mode {
 	RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode - tunnel. */
 };
 
-#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
+#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
 #define RTE_FLOW_MASK_ARRAY_SIZE \
-	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
+	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
 
 /**
  * A structure used to get the information of flow director filter.
@@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
 	uint32_t guarant_spc; /**< Guaranteed spaces.*/
 	uint32_t best_spc; /**< Best effort spaces.*/
 	/** Bit mask for every supported flow type. */
-	uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
+	uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
 	uint32_t max_flexpayload; /**< Total flex payload in bytes. */
 	/** Flexible payload unit in bytes. Size and alignments of all flex
 	    payload segments should be multiplies of this value. */
@@ -774,7 +774,7 @@ enum rte_eth_hash_function {
 };
 
 #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
-	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
+	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
 /**
  * A structure used to set or get global hash function configurations which
  * include symmetric hash enable per flow type and hash function type.
@@ -787,9 +787,9 @@ enum rte_eth_hash_function {
 struct rte_eth_hash_global_conf {
 	enum rte_eth_hash_function hash_func; /**< Hash function type */
 	/** Bit mask for symmetric hash enable per flow type */
-	uint32_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
+	uint64_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
 	/** Bit mask indicates if the corresponding bit is valid */
-	uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
+	uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
 };
 
 /**
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index b349599..d30eabc 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -34,6 +34,7 @@
 #include <rte_errno.h>
 #include <rte_spinlock.h>
 #include <rte_string_fns.h>
+#include <rte_compat.h>
 
 #include "rte_ether.h"
 #include "rte_ethdev.h"
@@ -3148,8 +3149,154 @@ rte_eth_dev_filter_supported(uint16_t port_id,
 }
 
 int
-rte_eth_dev_filter_ctrl(uint16_t port_id, enum rte_filter_type filter_type,
-		       enum rte_filter_op filter_op, void *arg)
+rte_eth_dev_filter_ctrl_v22(uint16_t port_id,
+			    enum rte_filter_type filter_type,
+			    enum rte_filter_op filter_op, void *arg);
+
+int
+rte_eth_dev_filter_ctrl_v22(uint16_t port_id,
+			    enum rte_filter_type filter_type,
+			    enum rte_filter_op filter_op, void *arg)
+{
+	struct rte_eth_fdir_info_v22 {
+		enum rte_fdir_mode mode;
+		struct rte_eth_fdir_masks mask;
+		struct rte_eth_fdir_flex_conf flex_conf;
+		uint32_t guarant_spc;
+		uint32_t best_spc;
+		uint32_t flow_types_mask[1];
+		uint32_t max_flexpayload;
+		uint32_t flex_payload_unit;
+		uint32_t max_flex_payload_segment_num;
+		uint16_t flex_payload_limit;
+		uint32_t flex_bitmask_unit;
+		uint32_t max_flex_bitmask_num;
+	};
+
+	struct rte_eth_hash_global_conf_v22 {
+		enum rte_eth_hash_function hash_func;
+		uint32_t sym_hash_enable_mask[1];
+		uint32_t valid_bit_mask[1];
+	};
+
+	struct rte_eth_hash_filter_info_v22 {
+		enum rte_eth_hash_filter_info_type info_type;
+		union {
+			uint8_t enable;
+			struct rte_eth_hash_global_conf_v22 global_conf;
+			struct rte_eth_input_set_conf input_set_conf;
+		} info;
+	};
+
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
+	if (filter_op == RTE_ETH_FILTER_INFO) {
+		int retval;
+		struct rte_eth_fdir_info_v22 *fdir_info_v22;
+		struct rte_eth_fdir_info fdir_info;
+
+		fdir_info_v22 = (struct rte_eth_fdir_info_v22 *)arg;
+
+		retval = (*dev->dev_ops->filter_ctrl)(dev, filter_type,
+			  filter_op, (void *)&fdir_info);
+		fdir_info_v22->mode = fdir_info.mode;
+		fdir_info_v22->mask = fdir_info.mask;
+		fdir_info_v22->flex_conf = fdir_info.flex_conf;
+		fdir_info_v22->guarant_spc = fdir_info.guarant_spc;
+		fdir_info_v22->best_spc = fdir_info.best_spc;
+		fdir_info_v22->flow_types_mask[0] =
+			(uint32_t)fdir_info.flow_types_mask[0];
+		fdir_info_v22->max_flexpayload = fdir_info.max_flexpayload;
+		fdir_info_v22->flex_payload_unit = fdir_info.flex_payload_unit;
+		fdir_info_v22->max_flex_payload_segment_num =
+			fdir_info.max_flex_payload_segment_num;
+		fdir_info_v22->flex_payload_limit =
+			fdir_info.flex_payload_limit;
+		fdir_info_v22->flex_bitmask_unit = fdir_info.flex_bitmask_unit;
+		fdir_info_v22->max_flex_bitmask_num =
+			fdir_info.max_flex_bitmask_num;
+		return retval;
+	} else if (filter_op == RTE_ETH_FILTER_GET) {
+		int retval;
+		struct rte_eth_hash_filter_info filter_info;
+		struct rte_eth_hash_filter_info_v22 *filter_info_v22 =
+			(struct rte_eth_hash_filter_info_v22 *)arg;
+
+		filter_info.info_type = filter_info_v22->info_type;
+		retval = (*dev->dev_ops->filter_ctrl)(dev, filter_type,
+			  filter_op, (void *)&filter_info);
+
+		switch (filter_info_v22->info_type) {
+		case RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT:
+			filter_info_v22->info.enable = filter_info.info.enable;
+			break;
+		case RTE_ETH_HASH_FILTER_GLOBAL_CONFIG:
+			filter_info_v22->info.global_conf.hash_func =
+				filter_info.info.global_conf.hash_func;
+			filter_info_v22->
+			info.global_conf.sym_hash_enable_mask[0] =
+				(uint32_t)filter_info.info.
+				global_conf.sym_hash_enable_mask[0];
+			filter_info_v22->info.global_conf.valid_bit_mask[0] =
+				(uint32_t)filter_info.info.global_conf.
+				valid_bit_mask[0];
+			break;
+		case RTE_ETH_HASH_FILTER_INPUT_SET_SELECT:
+			filter_info_v22->info.input_set_conf =
+				filter_info.info.input_set_conf;
+			break;
+		default:
+			break;
+		}
+		return retval;
+	} else if (filter_op == RTE_ETH_FILTER_SET) {
+		struct rte_eth_hash_filter_info filter_info;
+		struct rte_eth_hash_filter_info_v22 *filter_info_v22 =
+			(struct rte_eth_hash_filter_info_v22 *)arg;
+
+		filter_info.info_type = filter_info_v22->info_type;
+		switch (filter_info_v22->info_type) {
+		case RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT:
+			filter_info.info.enable = filter_info_v22->info.enable;
+			break;
+		case RTE_ETH_HASH_FILTER_GLOBAL_CONFIG:
+			filter_info.info.global_conf.hash_func =
+				filter_info_v22->info.global_conf.hash_func;
+			filter_info.info.global_conf.sym_hash_enable_mask[0] =
+				(uint32_t)filter_info_v22->
+				info.global_conf.sym_hash_enable_mask[0];
+			filter_info.info.global_conf.valid_bit_mask[0] =
+				(uint32_t)filter_info_v22->
+				info.global_conf.valid_bit_mask[0];
+			break;
+		case RTE_ETH_HASH_FILTER_INPUT_SET_SELECT:
+			filter_info.info.input_set_conf =
+				filter_info_v22->info.input_set_conf;
+			break;
+		default:
+			break;
+		}
+		return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op,
+						    (void *)&filter_info);
+	} else
+		return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op,
+						    arg);
+}
+VERSION_SYMBOL(rte_eth_dev_filter_ctrl, _v22, 2.2);
+
+int
+rte_eth_dev_filter_ctrl_v1802(uint16_t port_id,
+			      enum rte_filter_type filter_type,
+			      enum rte_filter_op filter_op, void *arg);
+
+int
+rte_eth_dev_filter_ctrl_v1802(uint16_t port_id,
+			      enum rte_filter_type filter_type,
+			      enum rte_filter_op filter_op, void *arg)
 {
 	struct rte_eth_dev *dev;
 
@@ -3159,6 +3306,12 @@ rte_eth_dev_filter_ctrl(uint16_t port_id, enum rte_filter_type filter_type,
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
 	return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op, arg);
 }
+BIND_DEFAULT_SYMBOL(rte_eth_dev_filter_ctrl, _v1802, 18.02);
+MAP_STATIC_SYMBOL(int rte_eth_dev_filter_ctrl(uint16_t port_id,
+		  enum rte_filter_type filter_type,
+		  enum rte_filter_op filter_op, void *arg),
+		  rte_eth_dev_filter_ctrl_v1802);
+
 
 void *
 rte_eth_add_rx_callback(uint16_t port_id, uint16_t queue_id,
diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
index e9681ac..2906a18 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -198,6 +198,13 @@ DPDK_17.11 {
 
 } DPDK_17.08;
 
+DPDK_18.02 {
+	global:
+
+	rte_eth_dev_filter_ctrl;
+
+} DPDK_17.11;
+
 EXPERIMENTAL {
 	global:
 
-- 
2.5.5

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-15 16:58 ` [dpdk-dev] [PATCH v2] " Kirill Rybalchenko
@ 2018-01-15 17:33   ` Kirill Rybalchenko
  2018-01-15 21:27     ` Thomas Monjalon
  2018-01-17 16:56     ` Ferruh Yigit
  0 siblings, 2 replies; 19+ messages in thread
From: Kirill Rybalchenko @ 2018-01-15 17:33 UTC (permalink / raw)
  To: dev; +Cc: kirill.rybalchenko, andrey.chilikin, thomas, ferruh.yigit

Increase the internal limit for flow types from 32 to 64
to support future flow type extensions.
Change type of variables from uint32_t[] to uint64_t[]:
rte_eth_fdir_info.flow_types_mask
rte_eth_hash_global_conf.sym_hash_enable_mask
rte_eth_hash_global_conf.valid_bit_mask

This modification affects the following components:
net/i40e
net/ixgbe
app/testpmd

v2:
implement versioning of rte_eth_dev_filter_ctrl function
for ABI backward compatibility with version 17.11 and older

v3:
fix code style warnings

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 app/test-pmd/cmdline.c                  |  22 ++---
 drivers/net/i40e/i40e_ethdev.c          |  38 ++++----
 drivers/net/i40e/i40e_fdir.c            |  25 ++---
 drivers/net/ixgbe/ixgbe_fdir.c          |  22 +++--
 lib/librte_ether/rte_eth_ctrl.h         |  12 +--
 lib/librte_ether/rte_ethdev.c           | 156 +++++++++++++++++++++++++++++++-
 lib/librte_ether/rte_ethdev_version.map |   7 ++
 7 files changed, 223 insertions(+), 59 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 6d71a5f..964d4ed 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -10803,7 +10803,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 	struct rte_eth_fdir_info fdir_info;
 	struct rte_eth_fdir_flex_mask flex_mask;
 	struct rte_port *port;
-	uint32_t flow_type_mask;
+	uint64_t flow_type_mask;
 	uint16_t i;
 	int ret;
 
@@ -10856,7 +10856,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 			return;
 		}
 		for (i = RTE_ETH_FLOW_UNKNOWN; i < RTE_ETH_FLOW_MAX; i++) {
-			if (flow_type_mask & (1 << i)) {
+			if (flow_type_mask & (1ULL << i)) {
 				flex_mask.flow_type = i;
 				fdir_set_flex_mask(res->port_id, &flex_mask);
 			}
@@ -10865,7 +10865,7 @@ cmd_flow_director_flex_mask_parsed(void *parsed_result,
 		return;
 	}
 	flex_mask.flow_type = str2flowtype(res->flow_type);
-	if (!(flow_type_mask & (1 << flex_mask.flow_type))) {
+	if (!(flow_type_mask & (1ULL << flex_mask.flow_type))) {
 		printf("Flow type %s not supported on port %d\n",
 				res->flow_type, res->port_id);
 		return;
@@ -11227,10 +11227,10 @@ cmd_get_hash_global_config_parsed(void *parsed_result,
 	}
 
 	for (i = 0; i < RTE_ETH_FLOW_MAX; i++) {
-		idx = i / UINT32_BIT;
-		offset = i % UINT32_BIT;
+		idx = i / UINT64_BIT;
+		offset = i % UINT64_BIT;
 		if (!(info.info.global_conf.valid_bit_mask[idx] &
-						(1UL << offset)))
+						(1ULL << offset)))
 			continue;
 		str = flowtype_to_str(i);
 		if (!str)
@@ -11238,7 +11238,7 @@ cmd_get_hash_global_config_parsed(void *parsed_result,
 		printf("Symmetric hash is %s globally for flow type %s "
 							"by port %d\n",
 			((info.info.global_conf.sym_hash_enable_mask[idx] &
-			(1UL << offset)) ? "enabled" : "disabled"), str,
+			(1ULL << offset)) ? "enabled" : "disabled"), str,
 							res->port_id);
 	}
 }
@@ -11299,12 +11299,12 @@ cmd_set_hash_global_config_parsed(void *parsed_result,
 			RTE_ETH_HASH_FUNCTION_DEFAULT;
 
 	ftype = str2flowtype(res->flow_type);
-	idx = ftype / (CHAR_BIT * sizeof(uint32_t));
-	offset = ftype % (CHAR_BIT * sizeof(uint32_t));
-	info.info.global_conf.valid_bit_mask[idx] |= (1UL << offset);
+	idx = ftype / UINT64_BIT;
+	offset = ftype % UINT64_BIT;
+	info.info.global_conf.valid_bit_mask[idx] |= (1ULL << offset);
 	if (!strcmp(res->enable, "enable"))
 		info.info.global_conf.sym_hash_enable_mask[idx] |=
-						(1UL << offset);
+						(1ULL << offset);
 	ret = rte_eth_dev_filter_ctrl(res->port_id, RTE_ETH_FILTER_HASH,
 					RTE_ETH_FILTER_SET, &info);
 	if (ret < 0)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7796e9e..d0473f0 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -8134,14 +8134,17 @@ i40e_get_hash_filter_global_config(struct i40e_hw *hw,
 		(reg & I40E_GLQF_CTL_HTOEP_MASK) ? "Toeplitz" : "Simple XOR");
 
 	/*
-	 * We work only with lowest 32 bits which is not correct, but to work
-	 * properly the valid_bit_mask size should be increased up to 64 bits
-	 * and this will brake ABI. This modification will be done in next
-	 * release
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
+	 * be checked.
 	 */
-	g_cfg->valid_bit_mask[0] = (uint32_t)adapter->flow_types_mask;
+	for (i = 1; i < RTE_SYM_HASH_MASK_ARRAY_SIZE; i++) {
+		g_cfg->valid_bit_mask[i] = 0ULL;
+		g_cfg->sym_hash_enable_mask[i] = 0ULL;
+	}
 
-	for (i = RTE_ETH_FLOW_UNKNOWN + 1; i < UINT32_BIT; i++) {
+	g_cfg->valid_bit_mask[0] = adapter->flow_types_mask;
+
+	for (i = RTE_ETH_FLOW_UNKNOWN + 1; i < UINT64_BIT; i++) {
 		if (!adapter->pctypes_tbl[i])
 			continue;
 		for (j = I40E_FILTER_PCTYPE_INVALID + 1;
@@ -8150,7 +8153,7 @@ i40e_get_hash_filter_global_config(struct i40e_hw *hw,
 				reg = i40e_read_rx_ctl(hw, I40E_GLQF_HSYM(j));
 				if (reg & I40E_GLQF_HSYM_SYMH_ENA_MASK) {
 					g_cfg->sym_hash_enable_mask[0] |=
-								(1UL << i);
+								(1ULL << i);
 				}
 			}
 		}
@@ -8164,7 +8167,7 @@ i40e_hash_global_config_check(const struct i40e_adapter *adapter,
 			      const struct rte_eth_hash_global_conf *g_cfg)
 {
 	uint32_t i;
-	uint32_t mask0, i40e_mask = adapter->flow_types_mask;
+	uint64_t mask0, i40e_mask = adapter->flow_types_mask;
 
 	if (g_cfg->hash_func != RTE_ETH_HASH_FUNCTION_TOEPLITZ &&
 		g_cfg->hash_func != RTE_ETH_HASH_FUNCTION_SIMPLE_XOR &&
@@ -8175,7 +8178,7 @@ i40e_hash_global_config_check(const struct i40e_adapter *adapter,
 	}
 
 	/*
-	 * As i40e supports less than 32 flow types, only first 32 bits need to
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
 	 * be checked.
 	 */
 	mask0 = g_cfg->valid_bit_mask[0];
@@ -8211,23 +8214,20 @@ i40e_set_hash_filter_global_config(struct i40e_hw *hw,
 	int ret;
 	uint16_t i, j;
 	uint32_t reg;
-	/*
-	 * We work only with lowest 32 bits which is not correct, but to work
-	 * properly the valid_bit_mask size should be increased up to 64 bits
-	 * and this will brake ABI. This modification will be done in next
-	 * release
-	 */
-	uint32_t mask0 = g_cfg->valid_bit_mask[0] &
-					(uint32_t)adapter->flow_types_mask;
+	uint64_t mask0 = g_cfg->valid_bit_mask[0] & adapter->flow_types_mask;
 
 	/* Check the input parameters */
 	ret = i40e_hash_global_config_check(adapter, g_cfg);
 	if (ret < 0)
 		return ret;
 
-	for (i = RTE_ETH_FLOW_UNKNOWN + 1; mask0 && i < UINT32_BIT; i++) {
+	/*
+	 * As i40e supports less than 64 flow types, only first 64 bits need to
+	 * be configured.
+	 */
+	for (i = RTE_ETH_FLOW_UNKNOWN + 1; mask0 && i < UINT64_BIT; i++) {
 		if (mask0 & (1UL << i)) {
-			reg = (g_cfg->sym_hash_enable_mask[0] & (1UL << i)) ?
+			reg = (g_cfg->sym_hash_enable_mask[0] & (1ULL << i)) ?
 					I40E_GLQF_HSYM_SYMH_ENA_MASK : 0;
 
 			for (j = I40E_FILTER_PCTYPE_INVALID + 1;
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 906c204..3a9e656 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -66,17 +66,17 @@
 #define I40E_COUNTER_INDEX_FDIR(pf_id)   (0 + (pf_id) * I40E_COUNTER_PF)
 
 #define I40E_FDIR_FLOWS ( \
-	(1 << RTE_ETH_FLOW_FRAG_IPV4) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
-	(1 << RTE_ETH_FLOW_FRAG_IPV6) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER) | \
-	(1 << RTE_ETH_FLOW_L2_PAYLOAD))
+	(1ULL << RTE_ETH_FLOW_FRAG_IPV4) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_FRAG_IPV6) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_L2_PAYLOAD))
 
 static int i40e_fdir_filter_programming(struct i40e_pf *pf,
 			enum i40e_filter_pctype pctype,
@@ -1999,6 +1999,7 @@ i40e_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir)
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	uint16_t num_flex_set = 0;
 	uint16_t num_flex_mask = 0;
+	uint16_t i;
 
 	if (dev->data->dev_conf.fdir_conf.mode == RTE_FDIR_MODE_PERFECT)
 		fdir->mode = RTE_FDIR_MODE_PERFECT;
@@ -2011,6 +2012,8 @@ i40e_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir)
 		(uint32_t)hw->func_caps.fd_filters_best_effort;
 	fdir->max_flexpayload = I40E_FDIR_MAX_FLEX_LEN;
 	fdir->flow_types_mask[0] = I40E_FDIR_FLOWS;
+	for (i = 1; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
+		fdir->flow_types_mask[i] = 0ULL;
 	fdir->flex_payload_unit = sizeof(uint16_t);
 	fdir->flex_bitmask_unit = sizeof(uint16_t);
 	fdir->max_flex_payload_segment_num = I40E_MAX_FLXPLD_FIED;
diff --git a/drivers/net/ixgbe/ixgbe_fdir.c b/drivers/net/ixgbe/ixgbe_fdir.c
index 551580c..236ab8c 100644
--- a/drivers/net/ixgbe/ixgbe_fdir.c
+++ b/drivers/net/ixgbe/ixgbe_fdir.c
@@ -41,14 +41,14 @@
 #define IXGBE_FDIRCMD_CMD_INTERVAL_US   10
 
 #define IXGBE_FDIR_FLOW_TYPES ( \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
-	(1 << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER))
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_UDP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_TCP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) | \
+	(1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER))
 
 #define IPV6_ADDR_TO_MASK(ipaddr, ipv6m) do { \
 	uint8_t ipv6_addr[16]; \
@@ -1407,7 +1407,7 @@ ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_hw_fdir_info *info =
 			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
-	uint32_t fdirctrl, max_num;
+	uint32_t fdirctrl, max_num, i;
 	uint8_t offset;
 
 	fdirctrl = IXGBE_READ_REG(hw, IXGBE_FDIRCTRL);
@@ -1439,9 +1439,11 @@ ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info
 
 	if (fdir_info->mode == RTE_FDIR_MODE_PERFECT_MAC_VLAN ||
 	    fdir_info->mode == RTE_FDIR_MODE_PERFECT_TUNNEL)
-		fdir_info->flow_types_mask[0] = 0;
+		fdir_info->flow_types_mask[0] = 0ULL;
 	else
 		fdir_info->flow_types_mask[0] = IXGBE_FDIR_FLOW_TYPES;
+	for (i = 1; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
+		fdir_info->flow_types_mask[i] = 0ULL;
 
 	fdir_info->flex_payload_unit = sizeof(uint16_t);
 	fdir_info->max_flex_payload_segment_num = 1;
diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 7991efa..668f59a 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -662,9 +662,9 @@ enum rte_fdir_mode {
 	RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode - tunnel. */
 };
 
-#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
+#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
 #define RTE_FLOW_MASK_ARRAY_SIZE \
-	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
+	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
 
 /**
  * A structure used to get the information of flow director filter.
@@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
 	uint32_t guarant_spc; /**< Guaranteed spaces.*/
 	uint32_t best_spc; /**< Best effort spaces.*/
 	/** Bit mask for every supported flow type. */
-	uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
+	uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
 	uint32_t max_flexpayload; /**< Total flex payload in bytes. */
 	/** Flexible payload unit in bytes. Size and alignments of all flex
 	    payload segments should be multiplies of this value. */
@@ -774,7 +774,7 @@ enum rte_eth_hash_function {
 };
 
 #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
-	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
+	(RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
 /**
  * A structure used to set or get global hash function configurations which
  * include symmetric hash enable per flow type and hash function type.
@@ -787,9 +787,9 @@ enum rte_eth_hash_function {
 struct rte_eth_hash_global_conf {
 	enum rte_eth_hash_function hash_func; /**< Hash function type */
 	/** Bit mask for symmetric hash enable per flow type */
-	uint32_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
+	uint64_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
 	/** Bit mask indicates if the corresponding bit is valid */
-	uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
+	uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
 };
 
 /**
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index b349599..9fb560e 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -34,6 +34,7 @@
 #include <rte_errno.h>
 #include <rte_spinlock.h>
 #include <rte_string_fns.h>
+#include <rte_compat.h>
 
 #include "rte_ether.h"
 #include "rte_ethdev.h"
@@ -3148,8 +3149,153 @@ rte_eth_dev_filter_supported(uint16_t port_id,
 }
 
 int
-rte_eth_dev_filter_ctrl(uint16_t port_id, enum rte_filter_type filter_type,
-		       enum rte_filter_op filter_op, void *arg)
+rte_eth_dev_filter_ctrl_v22(uint16_t port_id,
+			    enum rte_filter_type filter_type,
+			    enum rte_filter_op filter_op, void *arg);
+
+int
+rte_eth_dev_filter_ctrl_v22(uint16_t port_id,
+			    enum rte_filter_type filter_type,
+			    enum rte_filter_op filter_op, void *arg)
+{
+	struct rte_eth_fdir_info_v22 {
+		enum rte_fdir_mode mode;
+		struct rte_eth_fdir_masks mask;
+		struct rte_eth_fdir_flex_conf flex_conf;
+		uint32_t guarant_spc;
+		uint32_t best_spc;
+		uint32_t flow_types_mask[1];
+		uint32_t max_flexpayload;
+		uint32_t flex_payload_unit;
+		uint32_t max_flex_payload_segment_num;
+		uint16_t flex_payload_limit;
+		uint32_t flex_bitmask_unit;
+		uint32_t max_flex_bitmask_num;
+	};
+
+	struct rte_eth_hash_global_conf_v22 {
+		enum rte_eth_hash_function hash_func;
+		uint32_t sym_hash_enable_mask[1];
+		uint32_t valid_bit_mask[1];
+	};
+
+	struct rte_eth_hash_filter_info_v22 {
+		enum rte_eth_hash_filter_info_type info_type;
+		union {
+			uint8_t enable;
+			struct rte_eth_hash_global_conf_v22 global_conf;
+			struct rte_eth_input_set_conf input_set_conf;
+		} info;
+	};
+
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
+	if (filter_op == RTE_ETH_FILTER_INFO) {
+		int retval;
+		struct rte_eth_fdir_info_v22 *fdir_info_v22;
+		struct rte_eth_fdir_info fdir_info;
+
+		fdir_info_v22 = (struct rte_eth_fdir_info_v22 *)arg;
+
+		retval = (*dev->dev_ops->filter_ctrl)(dev, filter_type,
+			  filter_op, (void *)&fdir_info);
+		fdir_info_v22->mode = fdir_info.mode;
+		fdir_info_v22->mask = fdir_info.mask;
+		fdir_info_v22->flex_conf = fdir_info.flex_conf;
+		fdir_info_v22->guarant_spc = fdir_info.guarant_spc;
+		fdir_info_v22->best_spc = fdir_info.best_spc;
+		fdir_info_v22->flow_types_mask[0] =
+			(uint32_t)fdir_info.flow_types_mask[0];
+		fdir_info_v22->max_flexpayload = fdir_info.max_flexpayload;
+		fdir_info_v22->flex_payload_unit = fdir_info.flex_payload_unit;
+		fdir_info_v22->max_flex_payload_segment_num =
+			fdir_info.max_flex_payload_segment_num;
+		fdir_info_v22->flex_payload_limit =
+			fdir_info.flex_payload_limit;
+		fdir_info_v22->flex_bitmask_unit = fdir_info.flex_bitmask_unit;
+		fdir_info_v22->max_flex_bitmask_num =
+			fdir_info.max_flex_bitmask_num;
+		return retval;
+	} else if (filter_op == RTE_ETH_FILTER_GET) {
+		int retval;
+		struct rte_eth_hash_filter_info f_info;
+		struct rte_eth_hash_filter_info_v22 *f_info_v22 =
+			(struct rte_eth_hash_filter_info_v22 *)arg;
+
+		f_info.info_type = f_info_v22->info_type;
+		retval = (*dev->dev_ops->filter_ctrl)(dev, filter_type,
+			  filter_op, (void *)&f_info);
+
+		switch (f_info_v22->info_type) {
+		case RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT:
+			f_info_v22->info.enable = f_info.info.enable;
+			break;
+		case RTE_ETH_HASH_FILTER_GLOBAL_CONFIG:
+			f_info_v22->info.global_conf.hash_func =
+				f_info.info.global_conf.hash_func;
+			f_info_v22->info.global_conf.sym_hash_enable_mask[0] =
+				(uint32_t)
+				f_info.info.global_conf.sym_hash_enable_mask[0];
+			f_info_v22->info.global_conf.valid_bit_mask[0] =
+				(uint32_t)
+				f_info.info.global_conf.valid_bit_mask[0];
+			break;
+		case RTE_ETH_HASH_FILTER_INPUT_SET_SELECT:
+			f_info_v22->info.input_set_conf =
+				f_info.info.input_set_conf;
+			break;
+		default:
+			break;
+		}
+		return retval;
+	} else if (filter_op == RTE_ETH_FILTER_SET) {
+		struct rte_eth_hash_filter_info f_info;
+		struct rte_eth_hash_filter_info_v22 *f_v22 =
+			(struct rte_eth_hash_filter_info_v22 *)arg;
+
+		f_info.info_type = f_v22->info_type;
+		switch (f_v22->info_type) {
+		case RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT:
+			f_info.info.enable = f_v22->info.enable;
+			break;
+		case RTE_ETH_HASH_FILTER_GLOBAL_CONFIG:
+			f_info.info.global_conf.hash_func =
+				f_v22->info.global_conf.hash_func;
+			f_info.info.global_conf.sym_hash_enable_mask[0] =
+				(uint32_t)
+				f_v22->info.global_conf.sym_hash_enable_mask[0];
+			f_info.info.global_conf.valid_bit_mask[0] =
+				(uint32_t)
+				f_v22->info.global_conf.valid_bit_mask[0];
+			break;
+		case RTE_ETH_HASH_FILTER_INPUT_SET_SELECT:
+			f_info.info.input_set_conf =
+				f_v22->info.input_set_conf;
+			break;
+		default:
+			break;
+		}
+		return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op,
+						    (void *)&f_info);
+	} else
+		return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op,
+						    arg);
+}
+VERSION_SYMBOL(rte_eth_dev_filter_ctrl, _v22, 2.2);
+
+int
+rte_eth_dev_filter_ctrl_v1802(uint16_t port_id,
+			      enum rte_filter_type filter_type,
+			      enum rte_filter_op filter_op, void *arg);
+
+int
+rte_eth_dev_filter_ctrl_v1802(uint16_t port_id,
+			      enum rte_filter_type filter_type,
+			      enum rte_filter_op filter_op, void *arg)
 {
 	struct rte_eth_dev *dev;
 
@@ -3159,6 +3305,12 @@ rte_eth_dev_filter_ctrl(uint16_t port_id, enum rte_filter_type filter_type,
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
 	return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op, arg);
 }
+BIND_DEFAULT_SYMBOL(rte_eth_dev_filter_ctrl, _v1802, 18.02);
+MAP_STATIC_SYMBOL(int rte_eth_dev_filter_ctrl(uint16_t port_id,
+		  enum rte_filter_type filter_type,
+		  enum rte_filter_op filter_op, void *arg),
+		  rte_eth_dev_filter_ctrl_v1802);
+
 
 void *
 rte_eth_add_rx_callback(uint16_t port_id, uint16_t queue_id,
diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
index e9681ac..2906a18 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -198,6 +198,13 @@ DPDK_17.11 {
 
 } DPDK_17.08;
 
+DPDK_18.02 {
+	global:
+
+	rte_eth_dev_filter_ctrl;
+
+} DPDK_17.11;
+
 EXPERIMENTAL {
 	global:
 
-- 
2.5.5

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-15 17:33   ` [dpdk-dev] [PATCH v3] " Kirill Rybalchenko
@ 2018-01-15 21:27     ` Thomas Monjalon
  2018-01-16  9:44       ` Rybalchenko, Kirill
  2018-01-17 16:56     ` Ferruh Yigit
  1 sibling, 1 reply; 19+ messages in thread
From: Thomas Monjalon @ 2018-01-15 21:27 UTC (permalink / raw)
  To: Kirill Rybalchenko; +Cc: dev, andrey.chilikin, ferruh.yigit

15/01/2018 18:33, Kirill Rybalchenko:
> --- a/lib/librte_ether/rte_eth_ctrl.h
> +++ b/lib/librte_ether/rte_eth_ctrl.h
> @@ -662,9 +662,9 @@ enum rte_fdir_mode {
>         RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode - tunnel. */
>  };
>  
> -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
>  #define RTE_FLOW_MASK_ARRAY_SIZE \
> -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
>  
>  /**
>   * A structure used to get the information of flow director filter.
> @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
>         uint32_t guarant_spc; /**< Guaranteed spaces.*/
>         uint32_t best_spc; /**< Best effort spaces.*/
>         /** Bit mask for every supported flow type. */
> -       uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> +       uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
>         uint32_t max_flexpayload; /**< Total flex payload in bytes. */
>         /** Flexible payload unit in bytes. Size and alignments of all flex
>             payload segments should be multiplies of this value. */
> @@ -774,7 +774,7 @@ enum rte_eth_hash_function {
>  };
>  
>  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
>  /**
>   * A structure used to set or get global hash function configurations which
>   * include symmetric hash enable per flow type and hash function type.
> @@ -787,9 +787,9 @@ enum rte_eth_hash_function {
>  struct rte_eth_hash_global_conf {
>         enum rte_eth_hash_function hash_func; /**< Hash function type */
>         /** Bit mask for symmetric hash enable per flow type */
> -       uint32_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> +       uint64_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
>         /** Bit mask indicates if the corresponding bit is valid */
> -       uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> +       uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
>  };

This is still changing the ABI.
Am I missing something?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-15 21:27     ` Thomas Monjalon
@ 2018-01-16  9:44       ` Rybalchenko, Kirill
  2018-01-16  9:47         ` Thomas Monjalon
  0 siblings, 1 reply; 19+ messages in thread
From: Rybalchenko, Kirill @ 2018-01-16  9:44 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Chilikin, Andrey, Yigit, Ferruh

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday 15 January 2018 21:27
> To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> Cc: dev@dpdk.org; Chilikin, Andrey <andrey.chilikin@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [PATCH v3] ethdev: increase flow type limit from 32 to 64
> 
> 15/01/2018 18:33, Kirill Rybalchenko:
> > --- a/lib/librte_ether/rte_eth_ctrl.h
> > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > @@ -662,9 +662,9 @@ enum rte_fdir_mode {
> >         RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode -
> tunnel. */
> >  };
> >
> > -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> > +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
> >  #define RTE_FLOW_MASK_ARRAY_SIZE \
> > -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> >
> >  /**
> >   * A structure used to get the information of flow director filter.
> > @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
> >         uint32_t guarant_spc; /**< Guaranteed spaces.*/
> >         uint32_t best_spc; /**< Best effort spaces.*/
> >         /** Bit mask for every supported flow type. */
> > -       uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > +       uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> >         uint32_t max_flexpayload; /**< Total flex payload in bytes. */
> >         /** Flexible payload unit in bytes. Size and alignments of all flex
> >             payload segments should be multiplies of this value. */ @@
> > -774,7 +774,7 @@ enum rte_eth_hash_function {  };
> >
> >  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> > -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> >  /**
> >   * A structure used to set or get global hash function configurations which
> >   * include symmetric hash enable per flow type and hash function type.
> > @@ -787,9 +787,9 @@ enum rte_eth_hash_function {  struct
> > rte_eth_hash_global_conf {
> >         enum rte_eth_hash_function hash_func; /**< Hash function type */
> >         /** Bit mask for symmetric hash enable per flow type */
> > -       uint32_t
> sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > +       uint64_t
> sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> >         /** Bit mask indicates if the corresponding bit is valid */
> > -       uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > +       uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> >  };
> 
> This is still changing the ABI.
> Am I missing something?
> 
We change size of structures rte_eth_fdir_info and rte_eth_hash_filter_info.
Application can use these structures for DPDK library API call only in
rte_eth_dev_filter_ctrl() function call. In the patch this function is
modified in the way that it will be compatible with user binary applications
compiled with previous versions of DPDK library.

Thanks,
Kirill.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-16  9:44       ` Rybalchenko, Kirill
@ 2018-01-16  9:47         ` Thomas Monjalon
  2018-01-16 10:31           ` Rybalchenko, Kirill
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Monjalon @ 2018-01-16  9:47 UTC (permalink / raw)
  To: Rybalchenko, Kirill; +Cc: dev, Chilikin, Andrey, Yigit, Ferruh

16/01/2018 10:44, Rybalchenko, Kirill:
> Hi Thomas,
> 
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 15/01/2018 18:33, Kirill Rybalchenko:
> > > --- a/lib/librte_ether/rte_eth_ctrl.h
> > > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > > @@ -662,9 +662,9 @@ enum rte_fdir_mode {
> > >         RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode -
> > tunnel. */
> > >  };
> > >
> > > -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> > > +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
> > >  #define RTE_FLOW_MASK_ARRAY_SIZE \
> > > -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > >
> > >  /**
> > >   * A structure used to get the information of flow director filter.
> > > @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
> > >         uint32_t guarant_spc; /**< Guaranteed spaces.*/
> > >         uint32_t best_spc; /**< Best effort spaces.*/
> > >         /** Bit mask for every supported flow type. */
> > > -       uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > +       uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > >         uint32_t max_flexpayload; /**< Total flex payload in bytes. */
> > >         /** Flexible payload unit in bytes. Size and alignments of all flex
> > >             payload segments should be multiplies of this value. */ @@
> > > -774,7 +774,7 @@ enum rte_eth_hash_function {  };
> > >
> > >  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> > > -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > >  /**
> > >   * A structure used to set or get global hash function configurations which
> > >   * include symmetric hash enable per flow type and hash function type.
> > > @@ -787,9 +787,9 @@ enum rte_eth_hash_function {  struct
> > > rte_eth_hash_global_conf {
> > >         enum rte_eth_hash_function hash_func; /**< Hash function type */
> > >         /** Bit mask for symmetric hash enable per flow type */
> > > -       uint32_t
> > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > +       uint64_t
> > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > >         /** Bit mask indicates if the corresponding bit is valid */
> > > -       uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > +       uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > >  };
> > 
> > This is still changing the ABI.
> > Am I missing something?
> > 
> We change size of structures rte_eth_fdir_info and rte_eth_hash_filter_info.

Yes, and these structures are allocated and read by the application?
So it is an ABI break.

> Application can use these structures for DPDK library API call only in
> rte_eth_dev_filter_ctrl() function call. In the patch this function is
> modified in the way that it will be compatible with user binary applications
> compiled with previous versions of DPDK library.

Have you tried to use a patched DPDK with a binary compiled with DPDK 17.11?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-16  9:47         ` Thomas Monjalon
@ 2018-01-16 10:31           ` Rybalchenko, Kirill
  2018-01-16 10:38             ` Thomas Monjalon
  0 siblings, 1 reply; 19+ messages in thread
From: Rybalchenko, Kirill @ 2018-01-16 10:31 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Chilikin, Andrey, Yigit, Ferruh



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday 16 January 2018 09:48
> To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> Cc: dev@dpdk.org; Chilikin, Andrey <andrey.chilikin@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [PATCH v3] ethdev: increase flow type limit from 32 to 64
> 
> 16/01/2018 10:44, Rybalchenko, Kirill:
> > Hi Thomas,
> >
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 15/01/2018 18:33, Kirill Rybalchenko:
> > > > --- a/lib/librte_ether/rte_eth_ctrl.h
> > > > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > > > @@ -662,9 +662,9 @@ enum rte_fdir_mode {
> > > >         RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode
> -
> > > tunnel. */
> > > >  };
> > > >
> > > > -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> > > > +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
> > > >  #define RTE_FLOW_MASK_ARRAY_SIZE \
> > > > -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > > +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > > >
> > > >  /**
> > > >   * A structure used to get the information of flow director filter.
> > > > @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
> > > >         uint32_t guarant_spc; /**< Guaranteed spaces.*/
> > > >         uint32_t best_spc; /**< Best effort spaces.*/
> > > >         /** Bit mask for every supported flow type. */
> > > > -       uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > > +       uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > >         uint32_t max_flexpayload; /**< Total flex payload in bytes. */
> > > >         /** Flexible payload unit in bytes. Size and alignments of all flex
> > > >             payload segments should be multiplies of this value.
> > > > */ @@
> > > > -774,7 +774,7 @@ enum rte_eth_hash_function {  };
> > > >
> > > >  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> > > > -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > > +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > > >  /**
> > > >   * A structure used to set or get global hash function configurations
> which
> > > >   * include symmetric hash enable per flow type and hash function type.
> > > > @@ -787,9 +787,9 @@ enum rte_eth_hash_function {  struct
> > > > rte_eth_hash_global_conf {
> > > >         enum rte_eth_hash_function hash_func; /**< Hash function type
> */
> > > >         /** Bit mask for symmetric hash enable per flow type */
> > > > -       uint32_t
> > > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > +       uint64_t
> > > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > >         /** Bit mask indicates if the corresponding bit is valid */
> > > > -       uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > +       uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > >  };
> > >
> > > This is still changing the ABI.
> > > Am I missing something?
> > >
> > We change size of structures rte_eth_fdir_info and
> rte_eth_hash_filter_info.
> 
> Yes, and these structures are allocated and read by the application?
> So it is an ABI break.
If application binary was compiled with previous version of DPDK it makes
no difference if these structures were used internally there - it still will work.
If application binary was recompiled with new version of DPDK - again,
It will work. 
The only issue is if application binary was compiled with old version of DPDK
library, but used with new version of DPDK shared library and uses those
structures to call functions from this DPDK library. But it can be done
only by   rte_eth_dev_filter_ctrl() function, which handles this case properly.
 
> 
> > Application can use these structures for DPDK library API call only in
> > rte_eth_dev_filter_ctrl() function call. In the patch this function is
> > modified in the way that it will be compatible with user binary
> > applications compiled with previous versions of DPDK library.
> 
> Have you tried to use a patched DPDK with a binary compiled with DPDK
> 17.11?

Yes, actually, I did run testpmd from 17.08 with patched DPDK shared library. 
It works fine, as described.

Thanks,
Kirill.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-16 10:31           ` Rybalchenko, Kirill
@ 2018-01-16 10:38             ` Thomas Monjalon
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas Monjalon @ 2018-01-16 10:38 UTC (permalink / raw)
  To: Rybalchenko, Kirill; +Cc: dev, Chilikin, Andrey, Yigit, Ferruh

16/01/2018 11:31, Rybalchenko, Kirill:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 16/01/2018 10:44, Rybalchenko, Kirill:
> > > Hi Thomas,
> > >
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > 15/01/2018 18:33, Kirill Rybalchenko:
> > > > > --- a/lib/librte_ether/rte_eth_ctrl.h
> > > > > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > > > > @@ -662,9 +662,9 @@ enum rte_fdir_mode {
> > > > >         RTE_FDIR_MODE_PERFECT_TUNNEL,   /**< Enable FDIR filter mode
> > -
> > > > tunnel. */
> > > > >  };
> > > > >
> > > > > -#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> > > > > +#define UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
> > > > >  #define RTE_FLOW_MASK_ARRAY_SIZE \
> > > > > -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > > > +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > > > >
> > > > >  /**
> > > > >   * A structure used to get the information of flow director filter.
> > > > > @@ -681,7 +681,7 @@ struct rte_eth_fdir_info {
> > > > >         uint32_t guarant_spc; /**< Guaranteed spaces.*/
> > > > >         uint32_t best_spc; /**< Best effort spaces.*/
> > > > >         /** Bit mask for every supported flow type. */
> > > > > -       uint32_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > > > +       uint64_t flow_types_mask[RTE_FLOW_MASK_ARRAY_SIZE];
> > > > >         uint32_t max_flexpayload; /**< Total flex payload in bytes. */
> > > > >         /** Flexible payload unit in bytes. Size and alignments of all flex
> > > > >             payload segments should be multiplies of this value.
> > > > > */ @@
> > > > > -774,7 +774,7 @@ enum rte_eth_hash_function {  };
> > > > >
> > > > >  #define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> > > > > -       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT32_BIT)/UINT32_BIT)
> > > > > +       (RTE_ALIGN(RTE_ETH_FLOW_MAX, UINT64_BIT)/UINT64_BIT)
> > > > >  /**
> > > > >   * A structure used to set or get global hash function configurations
> > which
> > > > >   * include symmetric hash enable per flow type and hash function type.
> > > > > @@ -787,9 +787,9 @@ enum rte_eth_hash_function {  struct
> > > > > rte_eth_hash_global_conf {
> > > > >         enum rte_eth_hash_function hash_func; /**< Hash function type
> > */
> > > > >         /** Bit mask for symmetric hash enable per flow type */
> > > > > -       uint32_t
> > > > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > > +       uint64_t
> > > > sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > >         /** Bit mask indicates if the corresponding bit is valid */
> > > > > -       uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > > +       uint64_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> > > > >  };
> > > >
> > > > This is still changing the ABI.
> > > > Am I missing something?
> > > >
> > > We change size of structures rte_eth_fdir_info and
> > rte_eth_hash_filter_info.
> > 
> > Yes, and these structures are allocated and read by the application?
> > So it is an ABI break.
> If application binary was compiled with previous version of DPDK it makes
> no difference if these structures were used internally there - it still will work.
> If application binary was recompiled with new version of DPDK - again,
> It will work. 
> The only issue is if application binary was compiled with old version of DPDK
> library, but used with new version of DPDK shared library and uses those
> structures to call functions from this DPDK library. But it can be done
> only by   rte_eth_dev_filter_ctrl() function, which handles this case properly.

OK got it.
Thanks for your patience :)

> > > Application can use these structures for DPDK library API call only in
> > > rte_eth_dev_filter_ctrl() function call. In the patch this function is
> > > modified in the way that it will be compatible with user binary
> > > applications compiled with previous versions of DPDK library.
> > 
> > Have you tried to use a patched DPDK with a binary compiled with DPDK
> > 17.11?
> 
> Yes, actually, I did run testpmd from 17.08 with patched DPDK shared library. 
> It works fine, as described.
> 
> Thanks,
> Kirill.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
  2018-01-09 15:16   ` Rybalchenko, Kirill
  2018-01-10 13:50     ` Thomas Monjalon
@ 2018-01-16 11:13     ` Adrien Mazarguil
  2018-01-16 17:23       ` Rybalchenko, Kirill
  1 sibling, 1 reply; 19+ messages in thread
From: Adrien Mazarguil @ 2018-01-16 11:13 UTC (permalink / raw)
  To: Rybalchenko, Kirill
  Cc: dev, Wu, Jingjing, Xing, Beilei, johndale, neescoba,
	nelio.laranjeiro, yskoh, Lu, Wenzhuo, Ananyev, Konstantin,
	Chilikin, Andrey

On Tue, Jan 09, 2018 at 03:16:13PM +0000, Rybalchenko, Kirill wrote:
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Monday 4 December 2017 17:43
> > To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; johndale@cisco.com; neescoba@cisco.com;
> > nelio.laranjeiro@6wind.com; yskoh@mellanox.com; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Chilikin, Andrey
> > <andrey.chilikin@intel.com>
> > Subject: Re: [PATCH] ethdev: increase flow type limit from 32 to 64
> > 
> > Hi Kirill,
> > 
> > On Mon, Nov 27, 2017 at 12:29:47PM +0000, Kirill Rybalchenko wrote:
> > > Increase the internal limit for flow types from 32 to 64 to support
> > > future flow type extensions.
> > > Change type of variables from uint32_t[] to uint64_t[]:
> > >   rte_eth_fdir_info.flow_types_mask
> > >   rte_eth_hash_global_conf.sym_hash_enable_mask
> > >   rte_eth_hash_global_conf.valid_bit_mask
> > >
> > > This modification affects the following components:
> > >   net/i40e
> > >   net/enic
> > >   net/mlx5
> > >   net/ixgbe
> > >   app/testpmd
> > >
> > > Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> > 
> > Can you elaborate a bit on the need for these changes?
> > Have you considered implementing those future extensions through
> > rte_flow instead?
> 
> Hi Adrien, this is not a new feature but rather fix of existing limitation.
> In current implementation the symmetric hash mask and flow mask are
> represented by 32-bit variable, while hardware bitmask has 64 bits.
> Unfortunately, this modification changes ABI of the library as it changes size
> of rte_eth_fdir_info structure. All related PMDs (listed above) had to be modified
> accordingly.  

OK, no problem with this change. I assume you'll re-submit it since you sent
a deprecation notice, we'll review/ack subsequent mlx5 patches.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
  2018-01-16 11:13     ` Adrien Mazarguil
@ 2018-01-16 17:23       ` Rybalchenko, Kirill
  2018-01-16 18:03         ` Adrien Mazarguil
  0 siblings, 1 reply; 19+ messages in thread
From: Rybalchenko, Kirill @ 2018-01-16 17:23 UTC (permalink / raw)
  To: Adrien Mazarguil
  Cc: dev, Wu, Jingjing, Xing, Beilei, johndale, neescoba,
	nelio.laranjeiro, yskoh, Lu, Wenzhuo, Ananyev, Konstantin,
	Chilikin, Andrey

Hi Adrien, 
after some discussion we found that change I've done 
in Mellanox PMD is not really necessary: size of array
flow_types_mask[] is still 1 and the loop in patch 

for (i = 0; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
	info->flow_types_mask[i] = 0ULL;

will work exactly in the same way  as assignment

fdir_info->flow_types_mask[0] = 0;

in old version, though new version looks more proper
from programming style point of view.
So what do you think, shall I modify Mellanox PMD,
or better leave it as it is?

Thanks,
Kirill. 


> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Tuesday 16 January 2018 11:13
> To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; johndale@cisco.com; neescoba@cisco.com;
> nelio.laranjeiro@6wind.com; yskoh@mellanox.com; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Chilikin, Andrey
> <andrey.chilikin@intel.com>
> Subject: Re: [PATCH] ethdev: increase flow type limit from 32 to 64
> 
> On Tue, Jan 09, 2018 at 03:16:13PM +0000, Rybalchenko, Kirill wrote:
> > > -----Original Message-----
> > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > > Sent: Monday 4 December 2017 17:43
> > > To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> > > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>; johndale@cisco.com; neescoba@cisco.com;
> > > nelio.laranjeiro@6wind.com; yskoh@mellanox.com; Lu, Wenzhuo
> > > <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; Chilikin, Andrey
> > > <andrey.chilikin@intel.com>
> > > Subject: Re: [PATCH] ethdev: increase flow type limit from 32 to 64
> > >
> > > Hi Kirill,
> > >
> > > On Mon, Nov 27, 2017 at 12:29:47PM +0000, Kirill Rybalchenko wrote:
> > > > Increase the internal limit for flow types from 32 to 64 to
> > > > support future flow type extensions.
> > > > Change type of variables from uint32_t[] to uint64_t[]:
> > > >   rte_eth_fdir_info.flow_types_mask
> > > >   rte_eth_hash_global_conf.sym_hash_enable_mask
> > > >   rte_eth_hash_global_conf.valid_bit_mask
> > > >
> > > > This modification affects the following components:
> > > >   net/i40e
> > > >   net/enic
> > > >   net/mlx5
> > > >   net/ixgbe
> > > >   app/testpmd
> > > >
> > > > Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> > >
> > > Can you elaborate a bit on the need for these changes?
> > > Have you considered implementing those future extensions through
> > > rte_flow instead?
> >
> > Hi Adrien, this is not a new feature but rather fix of existing limitation.
> > In current implementation the symmetric hash mask and flow mask are
> > represented by 32-bit variable, while hardware bitmask has 64 bits.
> > Unfortunately, this modification changes ABI of the library as it
> > changes size of rte_eth_fdir_info structure. All related PMDs (listed
> > above) had to be modified accordingly.
> 
> OK, no problem with this change. I assume you'll re-submit it since you sent a
> deprecation notice, we'll review/ack subsequent mlx5 patches.
> 
> --
> Adrien Mazarguil
> 6WIND

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64
  2018-01-16 17:23       ` Rybalchenko, Kirill
@ 2018-01-16 18:03         ` Adrien Mazarguil
  0 siblings, 0 replies; 19+ messages in thread
From: Adrien Mazarguil @ 2018-01-16 18:03 UTC (permalink / raw)
  To: Rybalchenko, Kirill
  Cc: dev, Wu, Jingjing, Xing, Beilei, johndale, neescoba,
	nelio.laranjeiro, yskoh, Lu, Wenzhuo, Ananyev, Konstantin,
	Chilikin, Andrey

Hi Kirill,

On Tue, Jan 16, 2018 at 05:23:05PM +0000, Rybalchenko, Kirill wrote:
> Hi Adrien, 
> after some discussion we found that change I've done 
> in Mellanox PMD is not really necessary: size of array
> flow_types_mask[] is still 1 and the loop in patch 
> 
> for (i = 0; i < RTE_FLOW_MASK_ARRAY_SIZE; i++)
> 	info->flow_types_mask[i] = 0ULL;
> 
> will work exactly in the same way  as assignment
> 
> fdir_info->flow_types_mask[0] = 0;
> 
> in old version, though new version looks more proper
> from programming style point of view.
> So what do you think, shall I modify Mellanox PMD,
> or better leave it as it is?

If functionally the same, just drop the mlx change (involving fewer
reviewers is a good thing, right? :)

If it addressed a bug, it should have come as a separate "Fixes" patch
anyway.

> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Tuesday 16 January 2018 11:13
> > To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; johndale@cisco.com; neescoba@cisco.com;
> > nelio.laranjeiro@6wind.com; yskoh@mellanox.com; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Chilikin, Andrey
> > <andrey.chilikin@intel.com>
> > Subject: Re: [PATCH] ethdev: increase flow type limit from 32 to 64
> > 
> > On Tue, Jan 09, 2018 at 03:16:13PM +0000, Rybalchenko, Kirill wrote:
> > > > -----Original Message-----
> > > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > > > Sent: Monday 4 December 2017 17:43
> > > > To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> > > > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > > > <beilei.xing@intel.com>; johndale@cisco.com; neescoba@cisco.com;
> > > > nelio.laranjeiro@6wind.com; yskoh@mellanox.com; Lu, Wenzhuo
> > > > <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>; Chilikin, Andrey
> > > > <andrey.chilikin@intel.com>
> > > > Subject: Re: [PATCH] ethdev: increase flow type limit from 32 to 64
> > > >
> > > > Hi Kirill,
> > > >
> > > > On Mon, Nov 27, 2017 at 12:29:47PM +0000, Kirill Rybalchenko wrote:
> > > > > Increase the internal limit for flow types from 32 to 64 to
> > > > > support future flow type extensions.
> > > > > Change type of variables from uint32_t[] to uint64_t[]:
> > > > >   rte_eth_fdir_info.flow_types_mask
> > > > >   rte_eth_hash_global_conf.sym_hash_enable_mask
> > > > >   rte_eth_hash_global_conf.valid_bit_mask
> > > > >
> > > > > This modification affects the following components:
> > > > >   net/i40e
> > > > >   net/enic
> > > > >   net/mlx5
> > > > >   net/ixgbe
> > > > >   app/testpmd
> > > > >
> > > > > Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> > > >
> > > > Can you elaborate a bit on the need for these changes?
> > > > Have you considered implementing those future extensions through
> > > > rte_flow instead?
> > >
> > > Hi Adrien, this is not a new feature but rather fix of existing limitation.
> > > In current implementation the symmetric hash mask and flow mask are
> > > represented by 32-bit variable, while hardware bitmask has 64 bits.
> > > Unfortunately, this modification changes ABI of the library as it
> > > changes size of rte_eth_fdir_info structure. All related PMDs (listed
> > > above) had to be modified accordingly.
> > 
> > OK, no problem with this change. I assume you'll re-submit it since you sent a
> > deprecation notice, we'll review/ack subsequent mlx5 patches.
> > 
> > --
> > Adrien Mazarguil
> > 6WIND

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-15 17:33   ` [dpdk-dev] [PATCH v3] " Kirill Rybalchenko
  2018-01-15 21:27     ` Thomas Monjalon
@ 2018-01-17 16:56     ` Ferruh Yigit
  2018-01-18  9:24       ` Rybalchenko, Kirill
  2018-01-18 12:25       ` Ferruh Yigit
  1 sibling, 2 replies; 19+ messages in thread
From: Ferruh Yigit @ 2018-01-17 16:56 UTC (permalink / raw)
  To: Kirill Rybalchenko, dev; +Cc: andrey.chilikin, thomas

On 1/15/2018 5:33 PM, Kirill Rybalchenko wrote:
> Increase the internal limit for flow types from 32 to 64
> to support future flow type extensions.
> Change type of variables from uint32_t[] to uint64_t[]:
> rte_eth_fdir_info.flow_types_mask
> rte_eth_hash_global_conf.sym_hash_enable_mask
> rte_eth_hash_global_conf.valid_bit_mask
> 
> This modification affects the following components:
> net/i40e
> net/ixgbe
> app/testpmd
> 
> v2:
> implement versioning of rte_eth_dev_filter_ctrl function
> for ABI backward compatibility with version 17.11 and older
> 
> v3:
> fix code style warnings
> 
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>


I suggest keeping deprecation notice and clean versioning in next release, does
it make sense?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-17 16:56     ` Ferruh Yigit
@ 2018-01-18  9:24       ` Rybalchenko, Kirill
  2018-01-18 12:25       ` Ferruh Yigit
  1 sibling, 0 replies; 19+ messages in thread
From: Rybalchenko, Kirill @ 2018-01-18  9:24 UTC (permalink / raw)
  To: Yigit, Ferruh, dev; +Cc: Chilikin, Andrey, thomas



> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday 17 January 2018 16:57
> To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>; dev@dpdk.org
> Cc: Chilikin, Andrey <andrey.chilikin@intel.com>; thomas@monjalon.net
> Subject: Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32
> to 64
> 
> On 1/15/2018 5:33 PM, Kirill Rybalchenko wrote:
> > Increase the internal limit for flow types from 32 to 64 to support
> > future flow type extensions.
> > Change type of variables from uint32_t[] to uint64_t[]:
> > rte_eth_fdir_info.flow_types_mask
> > rte_eth_hash_global_conf.sym_hash_enable_mask
> > rte_eth_hash_global_conf.valid_bit_mask
> >
> > This modification affects the following components:
> > net/i40e
> > net/ixgbe
> > app/testpmd
> >
> > v2:
> > implement versioning of rte_eth_dev_filter_ctrl function for ABI
> > backward compatibility with version 17.11 and older
> >
> > v3:
> > fix code style warnings
> >
> > Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> 
> I suggest keeping deprecation notice and clean versioning in next release,
> does it make sense?

Yes, I think it should be done in this way, just to keep source codes  tidy.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3] ethdev: increase flow type limit from 32 to 64
  2018-01-17 16:56     ` Ferruh Yigit
  2018-01-18  9:24       ` Rybalchenko, Kirill
@ 2018-01-18 12:25       ` Ferruh Yigit
  1 sibling, 0 replies; 19+ messages in thread
From: Ferruh Yigit @ 2018-01-18 12:25 UTC (permalink / raw)
  To: Kirill Rybalchenko, dev; +Cc: andrey.chilikin, thomas

On 1/17/2018 4:56 PM, Ferruh Yigit wrote:
> On 1/15/2018 5:33 PM, Kirill Rybalchenko wrote:
>> Increase the internal limit for flow types from 32 to 64
>> to support future flow type extensions.
>> Change type of variables from uint32_t[] to uint64_t[]:
>> rte_eth_fdir_info.flow_types_mask
>> rte_eth_hash_global_conf.sym_hash_enable_mask
>> rte_eth_hash_global_conf.valid_bit_mask
>>
>> This modification affects the following components:
>> net/i40e
>> net/ixgbe
>> app/testpmd
>>
>> v2:
>> implement versioning of rte_eth_dev_filter_ctrl function
>> for ABI backward compatibility with version 17.11 and older
>>
>> v3:
>> fix code style warnings
>>
>> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-01-18 12:25 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27 12:29 [dpdk-dev] [PATCH] ethdev: increase flow type limit from 32 to 64 Kirill Rybalchenko
2017-12-04 17:43 ` Adrien Mazarguil
2018-01-09 15:16   ` Rybalchenko, Kirill
2018-01-10 13:50     ` Thomas Monjalon
2018-01-16 11:13     ` Adrien Mazarguil
2018-01-16 17:23       ` Rybalchenko, Kirill
2018-01-16 18:03         ` Adrien Mazarguil
2018-01-09 14:30 ` Zhang, Helin
2018-01-10  6:51 ` Xing, Beilei
2018-01-15 16:58 ` [dpdk-dev] [PATCH v2] " Kirill Rybalchenko
2018-01-15 17:33   ` [dpdk-dev] [PATCH v3] " Kirill Rybalchenko
2018-01-15 21:27     ` Thomas Monjalon
2018-01-16  9:44       ` Rybalchenko, Kirill
2018-01-16  9:47         ` Thomas Monjalon
2018-01-16 10:31           ` Rybalchenko, Kirill
2018-01-16 10:38             ` Thomas Monjalon
2018-01-17 16:56     ` Ferruh Yigit
2018-01-18  9:24       ` Rybalchenko, Kirill
2018-01-18 12:25       ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).