DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/iavf: improve default RSS
@ 2020-12-03  3:26 Xuan Ding
  2020-12-23 10:14 ` Zhang, Qi Z
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Xuan Ding @ 2020-12-03  3:26 UTC (permalink / raw)
  To: qi.z.zhang, jingjing.wu, beilei.xing; +Cc: dev, Xuan Ding

This patch adds support to actively configure RSS. Any kernel PF enabled
default RSS will be disabled during initialization. Currently supported
default rss_type: ipv4[6], ipv4[6]_udp, ipv4[6]_tcp, ipv4[6]_sctp.

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
---
 drivers/net/iavf/iavf.h        | 12 ++++-
 drivers/net/iavf/iavf_ethdev.c | 76 ++++++++++++++++++++++++-------
 drivers/net/iavf/iavf_hash.c   | 83 ++++++++++++++++++++++++----------
 drivers/net/iavf/iavf_vchnl.c  | 23 ++++++++++
 4 files changed, 153 insertions(+), 41 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 6d5912d8c..9754273b2 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -46,11 +46,18 @@
 	VIRTCHNL_VF_OFFLOAD_RX_POLLING)
 
 #define IAVF_RSS_OFFLOAD_ALL ( \
+	ETH_RSS_IPV4 | \
 	ETH_RSS_FRAG_IPV4 |         \
 	ETH_RSS_NONFRAG_IPV4_TCP |  \
 	ETH_RSS_NONFRAG_IPV4_UDP |  \
 	ETH_RSS_NONFRAG_IPV4_SCTP | \
-	ETH_RSS_NONFRAG_IPV4_OTHER)
+	ETH_RSS_NONFRAG_IPV4_OTHER | \
+	ETH_RSS_IPV6 | \
+	ETH_RSS_FRAG_IPV6 | \
+	ETH_RSS_NONFRAG_IPV6_TCP | \
+	ETH_RSS_NONFRAG_IPV6_UDP | \
+	ETH_RSS_NONFRAG_IPV6_SCTP | \
+	ETH_RSS_NONFRAG_IPV6_OTHER)
 
 #define IAVF_MISC_VEC_ID                RTE_INTR_VEC_ZERO_OFFSET
 #define IAVF_RX_VEC_START               RTE_INTR_VEC_RXTX_OFFSET
@@ -153,6 +160,7 @@ struct iavf_info {
 
 	uint8_t *rss_lut;
 	uint8_t *rss_key;
+	uint64_t rss_hf;
 	uint16_t nb_msix;   /* number of MSI-X interrupts on Rx */
 	uint16_t msix_base; /* msix vector base from */
 	uint16_t max_rss_qregion; /* max RSS queue region supported by PF */
@@ -321,6 +329,8 @@ int iavf_fdir_check(struct iavf_adapter *adapter,
 		struct iavf_fdir_conf *filter);
 int iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
 			 struct virtchnl_rss_cfg *rss_cfg, bool add);
+int iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena);
+int iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add);
 int iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 			struct rte_ether_addr *mc_addrs,
 			uint32_t mc_addrs_num, bool add);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 7e3c26a94..d2fa16825 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -267,10 +267,6 @@ iavf_init_rss(struct iavf_adapter *adapter)
 		return ret;
 	}
 
-	/* In IAVF, RSS enablement is set by PF driver. It is not supported
-	 * to set based on rss_conf->rss_hf.
-	 */
-
 	/* configure RSS key */
 	if (!rss_conf->rss_key) {
 		/* Calculate the default hash key */
@@ -295,6 +291,13 @@ iavf_init_rss(struct iavf_adapter *adapter)
 	if (ret)
 		return ret;
 
+	/* Set RSS hash configuration based on rss_conf->rss_hf. */
+	ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to set default RSS");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -1102,33 +1105,66 @@ iavf_dev_rss_reta_query(struct rte_eth_dev *dev,
 }
 
 static int
-iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
-			struct rte_eth_rss_conf *rss_conf)
+iavf_set_rss_key(struct iavf_adapter *adapter, uint8_t *key, uint8_t key_len)
 {
-	struct iavf_adapter *adapter =
-		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
 
-	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
-		return -ENOTSUP;
-
 	/* HENA setting, it is enabled by default, no change */
-	if (!rss_conf->rss_key || rss_conf->rss_key_len == 0) {
+	if (!key || key_len == 0) {
 		PMD_DRV_LOG(DEBUG, "No key to be configured");
 		return 0;
-	} else if (rss_conf->rss_key_len != vf->vf_res->rss_key_size) {
+	} else if (key_len != vf->vf_res->rss_key_size) {
 		PMD_DRV_LOG(ERR, "The size of hash key configured "
 			"(%d) doesn't match the size of hardware can "
-			"support (%d)", rss_conf->rss_key_len,
+			"support (%d)", key_len,
 			vf->vf_res->rss_key_size);
 		return -EINVAL;
 	}
 
-	rte_memcpy(vf->rss_key, rss_conf->rss_key, rss_conf->rss_key_len);
+	rte_memcpy(vf->rss_key, key, key_len);
 
 	return iavf_configure_rss_key(adapter);
 }
 
+static int
+iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
+			struct rte_eth_rss_conf *rss_conf)
+{
+	struct iavf_adapter *adapter =
+		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
+	int ret;
+
+	adapter->eth_dev->data->dev_conf.rx_adv_conf.rss_conf = *rss_conf;
+
+	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
+		return -ENOTSUP;
+
+	/* Set hash key. */
+	ret = iavf_set_rss_key(adapter, rss_conf->rss_key,
+			       rss_conf->rss_key_len);
+	if (ret)
+		return ret;
+
+	if (rss_conf->rss_hf == 0)
+		return 0;
+
+	/* Overwritten default RSS. */
+	ret = iavf_set_hena(adapter, 0);
+	if (ret)
+		PMD_DRV_LOG(ERR, "%s Remove rss vsi fail %d",
+			    __func__, ret);
+
+	/* Set new RSS configuration. */
+	ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to set new RSS");
+		return ret;
+	}
+
+	return 0;
+}
+
 static int
 iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 			  struct rte_eth_rss_conf *rss_conf)
@@ -1140,8 +1176,7 @@ iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
 		return -ENOTSUP;
 
-	 /* Just set it to default value now. */
-	rss_conf->rss_hf = IAVF_RSS_OFFLOAD_ALL;
+	rss_conf->rss_hf = vf->rss_hf;
 
 	if (!rss_conf->rss_key)
 		return 0;
@@ -2029,6 +2064,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 		return ret;
 	}
 
+	/* Set hena = 0 to ask PF to cleanup all existing RSS. */
+	ret = iavf_set_hena(adapter, 0);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to disable default PF RSS");
+		return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index c4c73e664..1f3238dab 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -429,17 +429,6 @@ static struct iavf_pattern_match_item iavf_hash_pattern_list[] = {
 	{iavf_pattern_eth_ipv6_gtpc,			ETH_RSS_IPV6,			&ipv6_udp_gtpc_tmplt},
 };
 
-struct virtchnl_proto_hdrs *iavf_hash_default_hdrs[] = {
-	&inner_ipv4_tmplt,
-	&inner_ipv4_udp_tmplt,
-	&inner_ipv4_tcp_tmplt,
-	&inner_ipv4_sctp_tmplt,
-	&inner_ipv6_tmplt,
-	&inner_ipv6_udp_tmplt,
-	&inner_ipv6_tcp_tmplt,
-	&inner_ipv6_sctp_tmplt,
-};
-
 static struct iavf_flow_engine iavf_hash_engine = {
 	.init = iavf_hash_init,
 	.create = iavf_hash_create,
@@ -458,24 +447,76 @@ static struct iavf_flow_parser iavf_hash_parser = {
 	.stage = IAVF_FLOW_STAGE_RSS,
 };
 
-static int
-iavf_hash_default_set(struct iavf_adapter *ad, bool add)
+int
+iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add)
 {
+	struct iavf_info *vf =  IAVF_DEV_PRIVATE_TO_VF(ad);
 	struct virtchnl_rss_cfg *rss_cfg;
-	uint16_t i;
+
+#define IAVF_RSS_HF_ALL ( \
+	ETH_RSS_IPV4 | \
+	ETH_RSS_IPV6 | \
+	ETH_RSS_NONFRAG_IPV4_UDP | \
+	ETH_RSS_NONFRAG_IPV6_UDP | \
+	ETH_RSS_NONFRAG_IPV4_TCP | \
+	ETH_RSS_NONFRAG_IPV6_TCP | \
+	ETH_RSS_NONFRAG_IPV4_SCTP | \
+	ETH_RSS_NONFRAG_IPV6_SCTP)
 
 	rss_cfg = rte_zmalloc("iavf rss rule",
 			      sizeof(struct virtchnl_rss_cfg), 0);
 	if (!rss_cfg)
 		return -ENOMEM;
 
-	for (i = 0; i < RTE_DIM(iavf_hash_default_hdrs); i++) {
-		rss_cfg->proto_hdrs = *iavf_hash_default_hdrs[i];
+	if (rss_hf & ETH_RSS_IPV4) {
+		rss_cfg->proto_hdrs = inner_ipv4_tmplt;
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP) {
+		rss_cfg->proto_hdrs = inner_ipv4_udp_tmplt;
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP) {
+		rss_cfg->proto_hdrs = inner_ipv4_tcp_tmplt;
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_SCTP) {
+		rss_cfg->proto_hdrs = inner_ipv4_sctp_tmplt;
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_IPV6) {
+		rss_cfg->proto_hdrs = inner_ipv6_tmplt;
 		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
 
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP) {
+		rss_cfg->proto_hdrs = inner_ipv6_udp_tmplt;
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
 		iavf_add_del_rss_cfg(ad, rss_cfg, add);
 	}
 
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP) {
+		rss_cfg->proto_hdrs = inner_ipv6_tcp_tmplt;
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_SCTP) {
+		rss_cfg->proto_hdrs = inner_ipv6_sctp_tmplt;
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	vf->rss_hf = rss_hf & IAVF_RSS_HF_ALL;
 	return 0;
 }
 
@@ -510,12 +551,6 @@ iavf_hash_init(struct iavf_adapter *ad)
 		return ret;
 	}
 
-	ret = iavf_hash_default_set(ad, true);
-	if (ret) {
-		PMD_DRV_LOG(ERR, "fail to set default RSS");
-		iavf_unregister_parser(parser, ad);
-	}
-
 	return ret;
 }
 
@@ -1089,6 +1124,7 @@ static void
 iavf_hash_uninit(struct iavf_adapter *ad)
 {
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
+	struct rte_eth_rss_conf *rss_conf;
 
 	if (vf->vf_reset)
 		return;
@@ -1099,7 +1135,8 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF))
 		return;
 
-	if (iavf_hash_default_set(ad, false))
+	rss_conf = &ad->eth_dev->data->dev_conf.rx_adv_conf.rss_conf;
+	if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
 		PMD_DRV_LOG(ERR, "fail to delete default RSS");
 
 	iavf_unregister_parser(&iavf_hash_parser, ad);
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 33d03af65..c718a75c4 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1341,6 +1341,29 @@ iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
 	return err;
 }
 
+int
+iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena)
+{
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
+	struct virtchnl_rss_hena vrh;
+	struct iavf_cmd_info args;
+	int err;
+
+	vrh.hena = hena;
+	args.ops = VIRTCHNL_OP_SET_RSS_HENA;
+	args.in_args = (u8 *)&vrh;
+	args.in_args_size = sizeof(vrh);
+	args.out_buffer = vf->aq_resp;
+	args.out_size = IAVF_AQ_BUF_SZ;
+
+	err = iavf_execute_vf_cmd(adapter, &args);
+	if (err)
+		PMD_DRV_LOG(ERR,
+			    "Failed to execute command of OP_SET_RSS_HENA");
+
+	return err;
+}
+
 int
 iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 			struct rte_ether_addr *mc_addrs,
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH] net/iavf: improve default RSS
  2020-12-03  3:26 [dpdk-dev] [PATCH] net/iavf: improve default RSS Xuan Ding
@ 2020-12-23 10:14 ` Zhang, Qi Z
  2020-12-23 11:52   ` Ding, Xuan
  2020-12-23 12:30 ` [dpdk-dev] [PATCH v2] " Xuan Ding
  2020-12-23 12:52 ` [dpdk-dev] [PATCH v3] " Xuan Ding
  2 siblings, 1 reply; 7+ messages in thread
From: Zhang, Qi Z @ 2020-12-23 10:14 UTC (permalink / raw)
  To: Ding, Xuan, Wu, Jingjing, Xing, Beilei; +Cc: dev



> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Thursday, December 3, 2020 11:26 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>
> Subject: [PATCH] net/iavf: improve default RSS
> 
> This patch adds support to actively configure RSS. Any kernel PF enabled
> default RSS will be disabled during initialization. Currently supported default
> rss_type: ipv4[6], ipv4[6]_udp, ipv4[6]_tcp, ipv4[6]_sctp.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>  drivers/net/iavf/iavf.h        | 12 ++++-
>  drivers/net/iavf/iavf_ethdev.c | 76 ++++++++++++++++++++++++-------
>  drivers/net/iavf/iavf_hash.c   | 83 ++++++++++++++++++++++++----------
>  drivers/net/iavf/iavf_vchnl.c  | 23 ++++++++++
>  4 files changed, 153 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> 6d5912d8c..9754273b2 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -46,11 +46,18 @@
>  	VIRTCHNL_VF_OFFLOAD_RX_POLLING)
> 
>  #define IAVF_RSS_OFFLOAD_ALL ( \
> +	ETH_RSS_IPV4 | \
>  	ETH_RSS_FRAG_IPV4 |         \
>  	ETH_RSS_NONFRAG_IPV4_TCP |  \
>  	ETH_RSS_NONFRAG_IPV4_UDP |  \
>  	ETH_RSS_NONFRAG_IPV4_SCTP | \
> -	ETH_RSS_NONFRAG_IPV4_OTHER)
> +	ETH_RSS_NONFRAG_IPV4_OTHER | \
> +	ETH_RSS_IPV6 | \
> +	ETH_RSS_FRAG_IPV6 | \
> +	ETH_RSS_NONFRAG_IPV6_TCP | \
> +	ETH_RSS_NONFRAG_IPV6_UDP | \
> +	ETH_RSS_NONFRAG_IPV6_SCTP | \
> +	ETH_RSS_NONFRAG_IPV6_OTHER)
> 
>  #define IAVF_MISC_VEC_ID                RTE_INTR_VEC_ZERO_OFFSET
>  #define IAVF_RX_VEC_START               RTE_INTR_VEC_RXTX_OFFSET
> @@ -153,6 +160,7 @@ struct iavf_info {
> 
>  	uint8_t *rss_lut;
>  	uint8_t *rss_key;
> +	uint64_t rss_hf;
>  	uint16_t nb_msix;   /* number of MSI-X interrupts on Rx */
>  	uint16_t msix_base; /* msix vector base from */
>  	uint16_t max_rss_qregion; /* max RSS queue region supported by PF */
> @@ -321,6 +329,8 @@ int iavf_fdir_check(struct iavf_adapter *adapter,
>  		struct iavf_fdir_conf *filter);
>  int iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
>  			 struct virtchnl_rss_cfg *rss_cfg, bool add);
> +int iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena); int
> +iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add);
>  int iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
>  			struct rte_ether_addr *mc_addrs,
>  			uint32_t mc_addrs_num, bool add);
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c index
> 7e3c26a94..d2fa16825 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -267,10 +267,6 @@ iavf_init_rss(struct iavf_adapter *adapter)
>  		return ret;
>  	}
> 
> -	/* In IAVF, RSS enablement is set by PF driver. It is not supported
> -	 * to set based on rss_conf->rss_hf.
> -	 */
> -
>  	/* configure RSS key */
>  	if (!rss_conf->rss_key) {
>  		/* Calculate the default hash key */
> @@ -295,6 +291,13 @@ iavf_init_rss(struct iavf_adapter *adapter)
>  	if (ret)
>  		return ret;
> 
> +	/* Set RSS hash configuration based on rss_conf->rss_hf. */
> +	ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "fail to set default RSS");
> +		return ret;
> +	}
> +
>  	return 0;
>  }
> 
> @@ -1102,33 +1105,66 @@ iavf_dev_rss_reta_query(struct rte_eth_dev
> *dev,  }
> 
>  static int
> -iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
> -			struct rte_eth_rss_conf *rss_conf)
> +iavf_set_rss_key(struct iavf_adapter *adapter, uint8_t *key, uint8_t
> +key_len)
>  {
> -	struct iavf_adapter *adapter =
> -		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
>  	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> 
> -	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
> -		return -ENOTSUP;
> -
>  	/* HENA setting, it is enabled by default, no change */
> -	if (!rss_conf->rss_key || rss_conf->rss_key_len == 0) {
> +	if (!key || key_len == 0) {
>  		PMD_DRV_LOG(DEBUG, "No key to be configured");
>  		return 0;
> -	} else if (rss_conf->rss_key_len != vf->vf_res->rss_key_size) {
> +	} else if (key_len != vf->vf_res->rss_key_size) {
>  		PMD_DRV_LOG(ERR, "The size of hash key configured "
>  			"(%d) doesn't match the size of hardware can "
> -			"support (%d)", rss_conf->rss_key_len,
> +			"support (%d)", key_len,
>  			vf->vf_res->rss_key_size);
>  		return -EINVAL;
>  	}
> 
> -	rte_memcpy(vf->rss_key, rss_conf->rss_key, rss_conf->rss_key_len);
> +	rte_memcpy(vf->rss_key, key, key_len);
> 
>  	return iavf_configure_rss_key(adapter);  }
> 
> +static int
> +iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
> +			struct rte_eth_rss_conf *rss_conf)
> +{
> +	struct iavf_adapter *adapter =
> +		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> +	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> +	int ret;
> +
> +	adapter->eth_dev->data->dev_conf.rx_adv_conf.rss_conf = *rss_conf;
> +
> +	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
> +		return -ENOTSUP;
> +
> +	/* Set hash key. */
> +	ret = iavf_set_rss_key(adapter, rss_conf->rss_key,
> +			       rss_conf->rss_key_len);
> +	if (ret)
> +		return ret;
> +
> +	if (rss_conf->rss_hf == 0)
> +		return 0;
> +
> +	/* Overwritten default RSS. */
> +	ret = iavf_set_hena(adapter, 0);
> +	if (ret)
> +		PMD_DRV_LOG(ERR, "%s Remove rss vsi fail %d",
> +			    __func__, ret);
> +
> +	/* Set new RSS configuration. */
> +	ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "fail to set new RSS");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
>  			  struct rte_eth_rss_conf *rss_conf) @@ -1140,8 +1176,7 @@
> iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
>  	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
>  		return -ENOTSUP;
> 
> -	 /* Just set it to default value now. */
> -	rss_conf->rss_hf = IAVF_RSS_OFFLOAD_ALL;
> +	rss_conf->rss_hf = vf->rss_hf;
> 
>  	if (!rss_conf->rss_key)
>  		return 0;
> @@ -2029,6 +2064,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
>  		return ret;
>  	}
> 
> +	/* Set hena = 0 to ask PF to cleanup all existing RSS. */
> +	ret = iavf_set_hena(adapter, 0);
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "fail to disable default PF RSS");
> +		return ret;
> +	}
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c index
> c4c73e664..1f3238dab 100644
> --- a/drivers/net/iavf/iavf_hash.c
> +++ b/drivers/net/iavf/iavf_hash.c
> @@ -429,17 +429,6 @@ static struct iavf_pattern_match_item
> iavf_hash_pattern_list[] = {
>  	{iavf_pattern_eth_ipv6_gtpc,			ETH_RSS_IPV6,
> 	&ipv6_udp_gtpc_tmplt},
>  };
> 
> -struct virtchnl_proto_hdrs *iavf_hash_default_hdrs[] = {
> -	&inner_ipv4_tmplt,
> -	&inner_ipv4_udp_tmplt,
> -	&inner_ipv4_tcp_tmplt,
> -	&inner_ipv4_sctp_tmplt,
> -	&inner_ipv6_tmplt,
> -	&inner_ipv6_udp_tmplt,
> -	&inner_ipv6_tcp_tmplt,
> -	&inner_ipv6_sctp_tmplt,
> -};
> -
>  static struct iavf_flow_engine iavf_hash_engine = {
>  	.init = iavf_hash_init,
>  	.create = iavf_hash_create,
> @@ -458,24 +447,76 @@ static struct iavf_flow_parser iavf_hash_parser = {
>  	.stage = IAVF_FLOW_STAGE_RSS,
>  };
> 
> -static int
> -iavf_hash_default_set(struct iavf_adapter *ad, bool add)
> +int
> +iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add)
>  {
> +	struct iavf_info *vf =  IAVF_DEV_PRIVATE_TO_VF(ad);
>  	struct virtchnl_rss_cfg *rss_cfg;
> -	uint16_t i;
> +
> +#define IAVF_RSS_HF_ALL ( \
> +	ETH_RSS_IPV4 | \
> +	ETH_RSS_IPV6 | \
> +	ETH_RSS_NONFRAG_IPV4_UDP | \
> +	ETH_RSS_NONFRAG_IPV6_UDP | \
> +	ETH_RSS_NONFRAG_IPV4_TCP | \
> +	ETH_RSS_NONFRAG_IPV6_TCP | \
> +	ETH_RSS_NONFRAG_IPV4_SCTP | \
> +	ETH_RSS_NONFRAG_IPV6_SCTP)
> 
>  	rss_cfg = rte_zmalloc("iavf rss rule",
>  			      sizeof(struct virtchnl_rss_cfg), 0);

I didn't see the rss_cfg has been freed somewhere, can you check if we need to fix it in v2, though it is not brought by this patch.
I think no need to malloc , "struct virtchnl_rss_cfg rss_cfg" should be fine.

>  	if (!rss_cfg)
>  		return -ENOMEM;
> 
> -	for (i = 0; i < RTE_DIM(iavf_hash_default_hdrs); i++) {
> -		rss_cfg->proto_hdrs = *iavf_hash_default_hdrs[i];
> +	if (rss_hf & ETH_RSS_IPV4) {
> +		rss_cfg->proto_hdrs = inner_ipv4_tmplt;
> +		rss_cfg->rss_algorithm =
> VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;

Better to assign rss_cfg->rss_algorithm before the if branch, so we can avoid duplicate code in each if branch.

> +		iavf_add_del_rss_cfg(ad, rss_cfg, add);
> +	}
> +
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP) {
> +		rss_cfg->proto_hdrs = inner_ipv4_udp_tmplt;
> +		rss_cfg->rss_algorithm =
> VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> +		iavf_add_del_rss_cfg(ad, rss_cfg, add);
> +	}
> +
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP) {
> +		rss_cfg->proto_hdrs = inner_ipv4_tcp_tmplt;
> +		rss_cfg->rss_algorithm =
> VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> +		iavf_add_del_rss_cfg(ad, rss_cfg, add);
> +	}
> +
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_SCTP) {
> +		rss_cfg->proto_hdrs = inner_ipv4_sctp_tmplt;
> +		rss_cfg->rss_algorithm =
> VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> +		iavf_add_del_rss_cfg(ad, rss_cfg, add);
> +	}
> +
> +	if (rss_hf & ETH_RSS_IPV6) {
> +		rss_cfg->proto_hdrs = inner_ipv6_tmplt;
>  		rss_cfg->rss_algorithm =
> VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> +		iavf_add_del_rss_cfg(ad, rss_cfg, add);
> +	}
> 
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP) {
> +		rss_cfg->proto_hdrs = inner_ipv6_udp_tmplt;
> +		rss_cfg->rss_algorithm =
> VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
>  		iavf_add_del_rss_cfg(ad, rss_cfg, add);
>  	}
> 
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP) {
> +		rss_cfg->proto_hdrs = inner_ipv6_tcp_tmplt;
> +		rss_cfg->rss_algorithm =
> VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> +		iavf_add_del_rss_cfg(ad, rss_cfg, add);
> +	}
> +
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_SCTP) {
> +		rss_cfg->proto_hdrs = inner_ipv6_sctp_tmplt;
> +		rss_cfg->rss_algorithm =
> VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> +		iavf_add_del_rss_cfg(ad, rss_cfg, add);
> +	}
> +
> +	vf->rss_hf = rss_hf & IAVF_RSS_HF_ALL;
>  	return 0;
>  }
> 
> @@ -510,12 +551,6 @@ iavf_hash_init(struct iavf_adapter *ad)
>  		return ret;
>  	}
> 
> -	ret = iavf_hash_default_set(ad, true);
> -	if (ret) {
> -		PMD_DRV_LOG(ERR, "fail to set default RSS");
> -		iavf_unregister_parser(parser, ad);
> -	}
> -
>  	return ret;
>  }
> 
> @@ -1089,6 +1124,7 @@ static void
>  iavf_hash_uninit(struct iavf_adapter *ad)  {
>  	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
> +	struct rte_eth_rss_conf *rss_conf;
> 
>  	if (vf->vf_reset)
>  		return;
> @@ -1099,7 +1135,8 @@ iavf_hash_uninit(struct iavf_adapter *ad)
>  	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF))
>  		return;
> 
> -	if (iavf_hash_default_set(ad, false))
> +	rss_conf = &ad->eth_dev->data->dev_conf.rx_adv_conf.rss_conf;
> +	if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
>  		PMD_DRV_LOG(ERR, "fail to delete default RSS");
> 
>  	iavf_unregister_parser(&iavf_hash_parser, ad); diff --git
> a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> 33d03af65..c718a75c4 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -1341,6 +1341,29 @@ iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
>  	return err;
>  }
> 
> +int
> +iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena) {
> +	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> +	struct virtchnl_rss_hena vrh;
> +	struct iavf_cmd_info args;
> +	int err;
> +
> +	vrh.hena = hena;
> +	args.ops = VIRTCHNL_OP_SET_RSS_HENA;
> +	args.in_args = (u8 *)&vrh;
> +	args.in_args_size = sizeof(vrh);
> +	args.out_buffer = vf->aq_resp;
> +	args.out_size = IAVF_AQ_BUF_SZ;
> +
> +	err = iavf_execute_vf_cmd(adapter, &args);
> +	if (err)
> +		PMD_DRV_LOG(ERR,
> +			    "Failed to execute command of OP_SET_RSS_HENA");
> +
> +	return err;
> +}
> +
>  int
>  iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
>  			struct rte_ether_addr *mc_addrs,
> --
> 2.17.1


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

* Re: [dpdk-dev] [PATCH] net/iavf: improve default RSS
  2020-12-23 10:14 ` Zhang, Qi Z
@ 2020-12-23 11:52   ` Ding, Xuan
  0 siblings, 0 replies; 7+ messages in thread
From: Ding, Xuan @ 2020-12-23 11:52 UTC (permalink / raw)
  To: Zhang, Qi Z, Wu, Jingjing, Xing, Beilei; +Cc: dev

Hi Qi,

Thank you for your comments, replies in line.

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Wednesday, December 23, 2020 6:15 PM
> To: Ding, Xuan <xuan.ding@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/iavf: improve default RSS
> 
> 
> 
> > -----Original Message-----
> > From: Ding, Xuan <xuan.ding@intel.com>
> > Sent: Thursday, December 3, 2020 11:26 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>;
> > Xing, Beilei <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>
> > Subject: [PATCH] net/iavf: improve default RSS
> >
> > This patch adds support to actively configure RSS. Any kernel PF enabled
> > default RSS will be disabled during initialization. Currently supported
> default
> > rss_type: ipv4[6], ipv4[6]_udp, ipv4[6]_tcp, ipv4[6]_sctp.
> >
> > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > ---
> >  drivers/net/iavf/iavf.h        | 12 ++++-
> >  drivers/net/iavf/iavf_ethdev.c | 76 ++++++++++++++++++++++++-------
> >  drivers/net/iavf/iavf_hash.c   | 83 ++++++++++++++++++++++++----------
> >  drivers/net/iavf/iavf_vchnl.c  | 23 ++++++++++
> >  4 files changed, 153 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> > 6d5912d8c..9754273b2 100644
> > --- a/drivers/net/iavf/iavf.h
> > +++ b/drivers/net/iavf/iavf.h
> > @@ -46,11 +46,18 @@
> >  VIRTCHNL_VF_OFFLOAD_RX_POLLING)
> >
> >  #define IAVF_RSS_OFFLOAD_ALL ( \
> > +ETH_RSS_IPV4 | \
> >  ETH_RSS_FRAG_IPV4 |         \
> >  ETH_RSS_NONFRAG_IPV4_TCP |  \
> >  ETH_RSS_NONFRAG_IPV4_UDP |  \
> >  ETH_RSS_NONFRAG_IPV4_SCTP | \
> > -ETH_RSS_NONFRAG_IPV4_OTHER)
> > +ETH_RSS_NONFRAG_IPV4_OTHER | \
> > +ETH_RSS_IPV6 | \
> > +ETH_RSS_FRAG_IPV6 | \
> > +ETH_RSS_NONFRAG_IPV6_TCP | \
> > +ETH_RSS_NONFRAG_IPV6_UDP | \
> > +ETH_RSS_NONFRAG_IPV6_SCTP | \
> > +ETH_RSS_NONFRAG_IPV6_OTHER)
> >
> >  #define IAVF_MISC_VEC_ID                RTE_INTR_VEC_ZERO_OFFSET
> >  #define IAVF_RX_VEC_START               RTE_INTR_VEC_RXTX_OFFSET
> > @@ -153,6 +160,7 @@ struct iavf_info {
> >
> >  uint8_t *rss_lut;
> >  uint8_t *rss_key;
> > +uint64_t rss_hf;
> >  uint16_t nb_msix;   /* number of MSI-X interrupts on Rx */
> >  uint16_t msix_base; /* msix vector base from */
> >  uint16_t max_rss_qregion; /* max RSS queue region supported by PF */
> > @@ -321,6 +329,8 @@ int iavf_fdir_check(struct iavf_adapter *adapter,
> >  struct iavf_fdir_conf *filter);
> >  int iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
> >   struct virtchnl_rss_cfg *rss_cfg, bool add);
> > +int iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena); int
> > +iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add);
> >  int iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
> >  struct rte_ether_addr *mc_addrs,
> >  uint32_t mc_addrs_num, bool add);
> > diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index
> > 7e3c26a94..d2fa16825 100644
> > --- a/drivers/net/iavf/iavf_ethdev.c
> > +++ b/drivers/net/iavf/iavf_ethdev.c
> > @@ -267,10 +267,6 @@ iavf_init_rss(struct iavf_adapter *adapter)
> >  return ret;
> >  }
> >
> > -/* In IAVF, RSS enablement is set by PF driver. It is not supported
> > - * to set based on rss_conf->rss_hf.
> > - */
> > -
> >  /* configure RSS key */
> >  if (!rss_conf->rss_key) {
> >  /* Calculate the default hash key */
> > @@ -295,6 +291,13 @@ iavf_init_rss(struct iavf_adapter *adapter)
> >  if (ret)
> >  return ret;
> >
> > +/* Set RSS hash configuration based on rss_conf->rss_hf. */
> > +ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
> > +if (ret) {
> > +PMD_DRV_LOG(ERR, "fail to set default RSS");
> > +return ret;
> > +}
> > +
> >  return 0;
> >  }
> >
> > @@ -1102,33 +1105,66 @@ iavf_dev_rss_reta_query(struct rte_eth_dev
> > *dev,  }
> >
> >  static int
> > -iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
> > -struct rte_eth_rss_conf *rss_conf)
> > +iavf_set_rss_key(struct iavf_adapter *adapter, uint8_t *key, uint8_t
> > +key_len)
> >  {
> > -struct iavf_adapter *adapter =
> > -IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> >  struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> >
> > -if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
> > -return -ENOTSUP;
> > -
> >  /* HENA setting, it is enabled by default, no change */
> > -if (!rss_conf->rss_key || rss_conf->rss_key_len == 0) {
> > +if (!key || key_len == 0) {
> >  PMD_DRV_LOG(DEBUG, "No key to be configured");
> >  return 0;
> > -} else if (rss_conf->rss_key_len != vf->vf_res->rss_key_size) {
> > +} else if (key_len != vf->vf_res->rss_key_size) {
> >  PMD_DRV_LOG(ERR, "The size of hash key configured "
> >  "(%d) doesn't match the size of hardware can "
> > -"support (%d)", rss_conf->rss_key_len,
> > +"support (%d)", key_len,
> >  vf->vf_res->rss_key_size);
> >  return -EINVAL;
> >  }
> >
> > -rte_memcpy(vf->rss_key, rss_conf->rss_key, rss_conf->rss_key_len);
> > +rte_memcpy(vf->rss_key, key, key_len);
> >
> >  return iavf_configure_rss_key(adapter);  }
> >
> > +static int
> > +iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
> > +struct rte_eth_rss_conf *rss_conf)
> > +{
> > +struct iavf_adapter *adapter =
> > +IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > +struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> > +int ret;
> > +
> > +adapter->eth_dev->data->dev_conf.rx_adv_conf.rss_conf = *rss_conf;
> > +
> > +if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
> > +return -ENOTSUP;
> > +
> > +/* Set hash key. */
> > +ret = iavf_set_rss_key(adapter, rss_conf->rss_key,
> > +       rss_conf->rss_key_len);
> > +if (ret)
> > +return ret;
> > +
> > +if (rss_conf->rss_hf == 0)
> > +return 0;
> > +
> > +/* Overwritten default RSS. */
> > +ret = iavf_set_hena(adapter, 0);
> > +if (ret)
> > +PMD_DRV_LOG(ERR, "%s Remove rss vsi fail %d",
> > +    __func__, ret);
> > +
> > +/* Set new RSS configuration. */
> > +ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
> > +if (ret) {
> > +PMD_DRV_LOG(ERR, "fail to set new RSS");
> > +return ret;
> > +}
> > +
> > +return 0;
> > +}
> > +
> >  static int
> >  iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
> >    struct rte_eth_rss_conf *rss_conf) @@ -1140,8 +1176,7 @@
> > iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
> >  if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
> >  return -ENOTSUP;
> >
> > - /* Just set it to default value now. */
> > -rss_conf->rss_hf = IAVF_RSS_OFFLOAD_ALL;
> > +rss_conf->rss_hf = vf->rss_hf;
> >
> >  if (!rss_conf->rss_key)
> >  return 0;
> > @@ -2029,6 +2064,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
> >  return ret;
> >  }
> >
> > +/* Set hena = 0 to ask PF to cleanup all existing RSS. */
> > +ret = iavf_set_hena(adapter, 0);
> > +if (ret) {
> > +PMD_DRV_LOG(ERR, "fail to disable default PF RSS");
> > +return ret;
> > +}
> > +
> >  return 0;
> >  }
> >
> > diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
> index
> > c4c73e664..1f3238dab 100644
> > --- a/drivers/net/iavf/iavf_hash.c
> > +++ b/drivers/net/iavf/iavf_hash.c
> > @@ -429,17 +429,6 @@ static struct iavf_pattern_match_item
> > iavf_hash_pattern_list[] = {
> >  {iavf_pattern_eth_ipv6_gtpc,ETH_RSS_IPV6,
> > &ipv6_udp_gtpc_tmplt},
> >  };
> >
> > -struct virtchnl_proto_hdrs *iavf_hash_default_hdrs[] = {
> > -&inner_ipv4_tmplt,
> > -&inner_ipv4_udp_tmplt,
> > -&inner_ipv4_tcp_tmplt,
> > -&inner_ipv4_sctp_tmplt,
> > -&inner_ipv6_tmplt,
> > -&inner_ipv6_udp_tmplt,
> > -&inner_ipv6_tcp_tmplt,
> > -&inner_ipv6_sctp_tmplt,
> > -};
> > -
> >  static struct iavf_flow_engine iavf_hash_engine = {
> >  .init = iavf_hash_init,
> >  .create = iavf_hash_create,
> > @@ -458,24 +447,76 @@ static struct iavf_flow_parser iavf_hash_parser =
> {
> >  .stage = IAVF_FLOW_STAGE_RSS,
> >  };
> >
> > -static int
> > -iavf_hash_default_set(struct iavf_adapter *ad, bool add)
> > +int
> > +iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add)
> >  {
> > +struct iavf_info *vf =  IAVF_DEV_PRIVATE_TO_VF(ad);
> >  struct virtchnl_rss_cfg *rss_cfg;
> > -uint16_t i;
> > +
> > +#define IAVF_RSS_HF_ALL ( \
> > +ETH_RSS_IPV4 | \
> > +ETH_RSS_IPV6 | \
> > +ETH_RSS_NONFRAG_IPV4_UDP | \
> > +ETH_RSS_NONFRAG_IPV6_UDP | \
> > +ETH_RSS_NONFRAG_IPV4_TCP | \
> > +ETH_RSS_NONFRAG_IPV6_TCP | \
> > +ETH_RSS_NONFRAG_IPV4_SCTP | \
> > +ETH_RSS_NONFRAG_IPV6_SCTP)
> >
> >  rss_cfg = rte_zmalloc("iavf rss rule",
> >        sizeof(struct virtchnl_rss_cfg), 0);
> 
> I didn’t see the rss_cfg has been freed somewhere, can you check if we
> need to fix it in v2, though it is not brought by this patch.
> I think no need to malloc , "struct virtchnl_rss_cfg rss_cfg" should be fine.

You are right, the rss_cfg is better only as a local variable here, will refine it in v2.

> 
> >  if (!rss_cfg)
> >  return -ENOMEM;
> >
> > -for (i = 0; i < RTE_DIM(iavf_hash_default_hdrs); i++) {
> > -rss_cfg->proto_hdrs = *iavf_hash_default_hdrs[i];
> > +if (rss_hf & ETH_RSS_IPV4) {
> > +rss_cfg->proto_hdrs = inner_ipv4_tmplt;
> > +rss_cfg->rss_algorithm =
> > VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> 
> Better to assign rss_cfg->rss_algorithm before the if branch, so we can avoid
> duplicate code in each if branch.

Yes, will refine it in v2, thanks.

> 
> > +iavf_add_del_rss_cfg(ad, rss_cfg, add);
> > +}
> > +
> > +if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP) {
> > +rss_cfg->proto_hdrs = inner_ipv4_udp_tmplt;
> > +rss_cfg->rss_algorithm =
> > VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> > +iavf_add_del_rss_cfg(ad, rss_cfg, add);
> > +}
> > +
> > +if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP) {
> > +rss_cfg->proto_hdrs = inner_ipv4_tcp_tmplt;
> > +rss_cfg->rss_algorithm =
> > VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> > +iavf_add_del_rss_cfg(ad, rss_cfg, add);
> > +}
> > +
> > +if (rss_hf & ETH_RSS_NONFRAG_IPV4_SCTP) {
> > +rss_cfg->proto_hdrs = inner_ipv4_sctp_tmplt;
> > +rss_cfg->rss_algorithm =
> > VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> > +iavf_add_del_rss_cfg(ad, rss_cfg, add);
> > +}
> > +
> > +if (rss_hf & ETH_RSS_IPV6) {
> > +rss_cfg->proto_hdrs = inner_ipv6_tmplt;
> >  rss_cfg->rss_algorithm =
> > VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> > +iavf_add_del_rss_cfg(ad, rss_cfg, add);
> > +}
> >
> > +if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP) {
> > +rss_cfg->proto_hdrs = inner_ipv6_udp_tmplt;
> > +rss_cfg->rss_algorithm =
> > VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> >  iavf_add_del_rss_cfg(ad, rss_cfg, add);
> >  }
> >
> > +if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP) {
> > +rss_cfg->proto_hdrs = inner_ipv6_tcp_tmplt;
> > +rss_cfg->rss_algorithm =
> > VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> > +iavf_add_del_rss_cfg(ad, rss_cfg, add);
> > +}
> > +
> > +if (rss_hf & ETH_RSS_NONFRAG_IPV6_SCTP) {
> > +rss_cfg->proto_hdrs = inner_ipv6_sctp_tmplt;
> > +rss_cfg->rss_algorithm =
> > VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> > +iavf_add_del_rss_cfg(ad, rss_cfg, add);
> > +}
> > +
> > +vf->rss_hf = rss_hf & IAVF_RSS_HF_ALL;
> >  return 0;
> >  }
> >
> > @@ -510,12 +551,6 @@ iavf_hash_init(struct iavf_adapter *ad)
> >  return ret;
> >  }
> >
> > -ret = iavf_hash_default_set(ad, true);
> > -if (ret) {
> > -PMD_DRV_LOG(ERR, "fail to set default RSS");
> > -iavf_unregister_parser(parser, ad);
> > -}
> > -
> >  return ret;
> >  }
> >
> > @@ -1089,6 +1124,7 @@ static void
> >  iavf_hash_uninit(struct iavf_adapter *ad)  {
> >  struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
> > +struct rte_eth_rss_conf *rss_conf;
> >
> >  if (vf->vf_reset)
> >  return;
> > @@ -1099,7 +1135,8 @@ iavf_hash_uninit(struct iavf_adapter *ad)
> >  if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF))
> >  return;
> >
> > -if (iavf_hash_default_set(ad, false))
> > +rss_conf = &ad->eth_dev->data->dev_conf.rx_adv_conf.rss_conf;
> > +if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
> >  PMD_DRV_LOG(ERR, "fail to delete default RSS");
> >
> >  iavf_unregister_parser(&iavf_hash_parser, ad); diff --git
> > a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> > 33d03af65..c718a75c4 100644
> > --- a/drivers/net/iavf/iavf_vchnl.c
> > +++ b/drivers/net/iavf/iavf_vchnl.c
> > @@ -1341,6 +1341,29 @@ iavf_add_del_rss_cfg(struct iavf_adapter
> *adapter,
> >  return err;
> >  }
> >
> > +int
> > +iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena) {
> > +struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> > +struct virtchnl_rss_hena vrh;
> > +struct iavf_cmd_info args;
> > +int err;
> > +
> > +vrh.hena = hena;
> > +args.ops = VIRTCHNL_OP_SET_RSS_HENA;
> > +args.in_args = (u8 *)&vrh;
> > +args.in_args_size = sizeof(vrh);
> > +args.out_buffer = vf->aq_resp;
> > +args.out_size = IAVF_AQ_BUF_SZ;
> > +
> > +err = iavf_execute_vf_cmd(adapter, &args);
> > +if (err)
> > +PMD_DRV_LOG(ERR,
> > +    "Failed to execute command of OP_SET_RSS_HENA");
> > +
> > +return err;
> > +}
> > +
> >  int
> >  iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
> >  struct rte_ether_addr *mc_addrs,
> > --
> > 2.17.1
> 


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

* [dpdk-dev] [PATCH v2] net/iavf: improve default RSS
  2020-12-03  3:26 [dpdk-dev] [PATCH] net/iavf: improve default RSS Xuan Ding
  2020-12-23 10:14 ` Zhang, Qi Z
@ 2020-12-23 12:30 ` Xuan Ding
  2020-12-23 12:54   ` Zhang, Qi Z
  2020-12-23 12:52 ` [dpdk-dev] [PATCH v3] " Xuan Ding
  2 siblings, 1 reply; 7+ messages in thread
From: Xuan Ding @ 2020-12-23 12:30 UTC (permalink / raw)
  To: qi.z.zhang, jingjing.wu, beilei.xing; +Cc: dev, Xuan Ding

This patch adds support to actively configure RSS through port config.
Any kernel PF enabled default RSS will be disabled during initialization.
Besides, default RSS will be configured based on rte_eth_rss_conf->rss_hf.
Currently supported default rss_type: ipv4[6], ipv4[6]_udp, ipv4[6]_tcp,
ipv4[6]_sctp.

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
---

v2:
* Revised the commit log.
* Fixed a bug to cause memory leak.
* Optimized the code to avoid duplication.
---
 drivers/net/iavf/iavf.h        | 12 ++++-
 drivers/net/iavf/iavf_ethdev.c | 76 ++++++++++++++++++++++++-------
 drivers/net/iavf/iavf_hash.c   | 81 ++++++++++++++++++++++------------
 drivers/net/iavf/iavf_vchnl.c  | 23 ++++++++++
 4 files changed, 146 insertions(+), 46 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 6d5912d8c..9754273b2 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -46,11 +46,18 @@
 	VIRTCHNL_VF_OFFLOAD_RX_POLLING)
 
 #define IAVF_RSS_OFFLOAD_ALL ( \
+	ETH_RSS_IPV4 | \
 	ETH_RSS_FRAG_IPV4 |         \
 	ETH_RSS_NONFRAG_IPV4_TCP |  \
 	ETH_RSS_NONFRAG_IPV4_UDP |  \
 	ETH_RSS_NONFRAG_IPV4_SCTP | \
-	ETH_RSS_NONFRAG_IPV4_OTHER)
+	ETH_RSS_NONFRAG_IPV4_OTHER | \
+	ETH_RSS_IPV6 | \
+	ETH_RSS_FRAG_IPV6 | \
+	ETH_RSS_NONFRAG_IPV6_TCP | \
+	ETH_RSS_NONFRAG_IPV6_UDP | \
+	ETH_RSS_NONFRAG_IPV6_SCTP | \
+	ETH_RSS_NONFRAG_IPV6_OTHER)
 
 #define IAVF_MISC_VEC_ID                RTE_INTR_VEC_ZERO_OFFSET
 #define IAVF_RX_VEC_START               RTE_INTR_VEC_RXTX_OFFSET
@@ -153,6 +160,7 @@ struct iavf_info {
 
 	uint8_t *rss_lut;
 	uint8_t *rss_key;
+	uint64_t rss_hf;
 	uint16_t nb_msix;   /* number of MSI-X interrupts on Rx */
 	uint16_t msix_base; /* msix vector base from */
 	uint16_t max_rss_qregion; /* max RSS queue region supported by PF */
@@ -321,6 +329,8 @@ int iavf_fdir_check(struct iavf_adapter *adapter,
 		struct iavf_fdir_conf *filter);
 int iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
 			 struct virtchnl_rss_cfg *rss_cfg, bool add);
+int iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena);
+int iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add);
 int iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 			struct rte_ether_addr *mc_addrs,
 			uint32_t mc_addrs_num, bool add);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 7e3c26a94..d2fa16825 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -267,10 +267,6 @@ iavf_init_rss(struct iavf_adapter *adapter)
 		return ret;
 	}
 
-	/* In IAVF, RSS enablement is set by PF driver. It is not supported
-	 * to set based on rss_conf->rss_hf.
-	 */
-
 	/* configure RSS key */
 	if (!rss_conf->rss_key) {
 		/* Calculate the default hash key */
@@ -295,6 +291,13 @@ iavf_init_rss(struct iavf_adapter *adapter)
 	if (ret)
 		return ret;
 
+	/* Set RSS hash configuration based on rss_conf->rss_hf. */
+	ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to set default RSS");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -1102,33 +1105,66 @@ iavf_dev_rss_reta_query(struct rte_eth_dev *dev,
 }
 
 static int
-iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
-			struct rte_eth_rss_conf *rss_conf)
+iavf_set_rss_key(struct iavf_adapter *adapter, uint8_t *key, uint8_t key_len)
 {
-	struct iavf_adapter *adapter =
-		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
 
-	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
-		return -ENOTSUP;
-
 	/* HENA setting, it is enabled by default, no change */
-	if (!rss_conf->rss_key || rss_conf->rss_key_len == 0) {
+	if (!key || key_len == 0) {
 		PMD_DRV_LOG(DEBUG, "No key to be configured");
 		return 0;
-	} else if (rss_conf->rss_key_len != vf->vf_res->rss_key_size) {
+	} else if (key_len != vf->vf_res->rss_key_size) {
 		PMD_DRV_LOG(ERR, "The size of hash key configured "
 			"(%d) doesn't match the size of hardware can "
-			"support (%d)", rss_conf->rss_key_len,
+			"support (%d)", key_len,
 			vf->vf_res->rss_key_size);
 		return -EINVAL;
 	}
 
-	rte_memcpy(vf->rss_key, rss_conf->rss_key, rss_conf->rss_key_len);
+	rte_memcpy(vf->rss_key, key, key_len);
 
 	return iavf_configure_rss_key(adapter);
 }
 
+static int
+iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
+			struct rte_eth_rss_conf *rss_conf)
+{
+	struct iavf_adapter *adapter =
+		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
+	int ret;
+
+	adapter->eth_dev->data->dev_conf.rx_adv_conf.rss_conf = *rss_conf;
+
+	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
+		return -ENOTSUP;
+
+	/* Set hash key. */
+	ret = iavf_set_rss_key(adapter, rss_conf->rss_key,
+			       rss_conf->rss_key_len);
+	if (ret)
+		return ret;
+
+	if (rss_conf->rss_hf == 0)
+		return 0;
+
+	/* Overwritten default RSS. */
+	ret = iavf_set_hena(adapter, 0);
+	if (ret)
+		PMD_DRV_LOG(ERR, "%s Remove rss vsi fail %d",
+			    __func__, ret);
+
+	/* Set new RSS configuration. */
+	ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to set new RSS");
+		return ret;
+	}
+
+	return 0;
+}
+
 static int
 iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 			  struct rte_eth_rss_conf *rss_conf)
@@ -1140,8 +1176,7 @@ iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
 		return -ENOTSUP;
 
-	 /* Just set it to default value now. */
-	rss_conf->rss_hf = IAVF_RSS_OFFLOAD_ALL;
+	rss_conf->rss_hf = vf->rss_hf;
 
 	if (!rss_conf->rss_key)
 		return 0;
@@ -2029,6 +2064,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 		return ret;
 	}
 
+	/* Set hena = 0 to ask PF to cleanup all existing RSS. */
+	ret = iavf_set_hena(adapter, 0);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to disable default PF RSS");
+		return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index c4c73e664..8fda6d0cb 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -429,17 +429,6 @@ static struct iavf_pattern_match_item iavf_hash_pattern_list[] = {
 	{iavf_pattern_eth_ipv6_gtpc,			ETH_RSS_IPV6,			&ipv6_udp_gtpc_tmplt},
 };
 
-struct virtchnl_proto_hdrs *iavf_hash_default_hdrs[] = {
-	&inner_ipv4_tmplt,
-	&inner_ipv4_udp_tmplt,
-	&inner_ipv4_tcp_tmplt,
-	&inner_ipv4_sctp_tmplt,
-	&inner_ipv6_tmplt,
-	&inner_ipv6_udp_tmplt,
-	&inner_ipv6_tcp_tmplt,
-	&inner_ipv6_sctp_tmplt,
-};
-
 static struct iavf_flow_engine iavf_hash_engine = {
 	.init = iavf_hash_init,
 	.create = iavf_hash_create,
@@ -458,24 +447,64 @@ static struct iavf_flow_parser iavf_hash_parser = {
 	.stage = IAVF_FLOW_STAGE_RSS,
 };
 
-static int
-iavf_hash_default_set(struct iavf_adapter *ad, bool add)
+int
+iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add)
 {
+	struct iavf_info *vf =  IAVF_DEV_PRIVATE_TO_VF(ad);
 	struct virtchnl_rss_cfg *rss_cfg;
-	uint16_t i;
 
-	rss_cfg = rte_zmalloc("iavf rss rule",
-			      sizeof(struct virtchnl_rss_cfg), 0);
-	if (!rss_cfg)
-		return -ENOMEM;
+#define IAVF_RSS_HF_ALL ( \
+	ETH_RSS_IPV4 | \
+	ETH_RSS_IPV6 | \
+	ETH_RSS_NONFRAG_IPV4_UDP | \
+	ETH_RSS_NONFRAG_IPV6_UDP | \
+	ETH_RSS_NONFRAG_IPV4_TCP | \
+	ETH_RSS_NONFRAG_IPV6_TCP | \
+	ETH_RSS_NONFRAG_IPV4_SCTP | \
+	ETH_RSS_NONFRAG_IPV6_SCTP)
+
+	rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+	if (rss_hf & ETH_RSS_IPV4) {
+		rss_cfg->proto_hdrs = inner_ipv4_tmplt;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP) {
+		rss_cfg->proto_hdrs = inner_ipv4_udp_tmplt;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP) {
+		rss_cfg->proto_hdrs = inner_ipv4_tcp_tmplt;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
 
-	for (i = 0; i < RTE_DIM(iavf_hash_default_hdrs); i++) {
-		rss_cfg->proto_hdrs = *iavf_hash_default_hdrs[i];
-		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_SCTP) {
+		rss_cfg->proto_hdrs = inner_ipv4_sctp_tmplt;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_IPV6) {
+		rss_cfg->proto_hdrs = inner_ipv6_tmplt;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP) {
+		rss_cfg->proto_hdrs = inner_ipv6_udp_tmplt;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP) {
+		rss_cfg->proto_hdrs = inner_ipv6_tcp_tmplt;
+		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	}
 
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_SCTP) {
+		rss_cfg->proto_hdrs = inner_ipv6_sctp_tmplt;
 		iavf_add_del_rss_cfg(ad, rss_cfg, add);
 	}
 
+	vf->rss_hf = rss_hf & IAVF_RSS_HF_ALL;
 	return 0;
 }
 
@@ -510,12 +539,6 @@ iavf_hash_init(struct iavf_adapter *ad)
 		return ret;
 	}
 
-	ret = iavf_hash_default_set(ad, true);
-	if (ret) {
-		PMD_DRV_LOG(ERR, "fail to set default RSS");
-		iavf_unregister_parser(parser, ad);
-	}
-
 	return ret;
 }
 
@@ -1089,6 +1112,7 @@ static void
 iavf_hash_uninit(struct iavf_adapter *ad)
 {
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
+	struct rte_eth_rss_conf *rss_conf;
 
 	if (vf->vf_reset)
 		return;
@@ -1099,7 +1123,8 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF))
 		return;
 
-	if (iavf_hash_default_set(ad, false))
+	rss_conf = &ad->eth_dev->data->dev_conf.rx_adv_conf.rss_conf;
+	if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
 		PMD_DRV_LOG(ERR, "fail to delete default RSS");
 
 	iavf_unregister_parser(&iavf_hash_parser, ad);
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 33d03af65..c718a75c4 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1341,6 +1341,29 @@ iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
 	return err;
 }
 
+int
+iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena)
+{
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
+	struct virtchnl_rss_hena vrh;
+	struct iavf_cmd_info args;
+	int err;
+
+	vrh.hena = hena;
+	args.ops = VIRTCHNL_OP_SET_RSS_HENA;
+	args.in_args = (u8 *)&vrh;
+	args.in_args_size = sizeof(vrh);
+	args.out_buffer = vf->aq_resp;
+	args.out_size = IAVF_AQ_BUF_SZ;
+
+	err = iavf_execute_vf_cmd(adapter, &args);
+	if (err)
+		PMD_DRV_LOG(ERR,
+			    "Failed to execute command of OP_SET_RSS_HENA");
+
+	return err;
+}
+
 int
 iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 			struct rte_ether_addr *mc_addrs,
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3] net/iavf: improve default RSS
  2020-12-03  3:26 [dpdk-dev] [PATCH] net/iavf: improve default RSS Xuan Ding
  2020-12-23 10:14 ` Zhang, Qi Z
  2020-12-23 12:30 ` [dpdk-dev] [PATCH v2] " Xuan Ding
@ 2020-12-23 12:52 ` Xuan Ding
  2020-12-23 12:59   ` Zhang, Qi Z
  2 siblings, 1 reply; 7+ messages in thread
From: Xuan Ding @ 2020-12-23 12:52 UTC (permalink / raw)
  To: qi.z.zhang, jingjing.wu, beilei.xing; +Cc: dev, Xuan Ding

This patch adds support to actively configure RSS through port config.
Any kernel PF enabled default RSS will be disabled during initialization.
Besides, default RSS will be configured based on rte_eth_rss_conf->rss_hf.
Currently supported default rss_type: ipv4[6], ipv4[6]_udp, ipv4[6]_tcp,
ipv4[6]_sctp.

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
---

v3:
* Fixed one compilation error.

v2:
* Revised the commit log.
* Fixed a bug to cause memory leak.
* Optimized the code to avoid duplication.
---
 drivers/net/iavf/iavf.h        | 12 ++++-
 drivers/net/iavf/iavf_ethdev.c | 76 +++++++++++++++++++++++-------
 drivers/net/iavf/iavf_hash.c   | 85 ++++++++++++++++++++++------------
 drivers/net/iavf/iavf_vchnl.c  | 23 +++++++++
 4 files changed, 148 insertions(+), 48 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 6d5912d8c..9754273b2 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -46,11 +46,18 @@
 	VIRTCHNL_VF_OFFLOAD_RX_POLLING)
 
 #define IAVF_RSS_OFFLOAD_ALL ( \
+	ETH_RSS_IPV4 | \
 	ETH_RSS_FRAG_IPV4 |         \
 	ETH_RSS_NONFRAG_IPV4_TCP |  \
 	ETH_RSS_NONFRAG_IPV4_UDP |  \
 	ETH_RSS_NONFRAG_IPV4_SCTP | \
-	ETH_RSS_NONFRAG_IPV4_OTHER)
+	ETH_RSS_NONFRAG_IPV4_OTHER | \
+	ETH_RSS_IPV6 | \
+	ETH_RSS_FRAG_IPV6 | \
+	ETH_RSS_NONFRAG_IPV6_TCP | \
+	ETH_RSS_NONFRAG_IPV6_UDP | \
+	ETH_RSS_NONFRAG_IPV6_SCTP | \
+	ETH_RSS_NONFRAG_IPV6_OTHER)
 
 #define IAVF_MISC_VEC_ID                RTE_INTR_VEC_ZERO_OFFSET
 #define IAVF_RX_VEC_START               RTE_INTR_VEC_RXTX_OFFSET
@@ -153,6 +160,7 @@ struct iavf_info {
 
 	uint8_t *rss_lut;
 	uint8_t *rss_key;
+	uint64_t rss_hf;
 	uint16_t nb_msix;   /* number of MSI-X interrupts on Rx */
 	uint16_t msix_base; /* msix vector base from */
 	uint16_t max_rss_qregion; /* max RSS queue region supported by PF */
@@ -321,6 +329,8 @@ int iavf_fdir_check(struct iavf_adapter *adapter,
 		struct iavf_fdir_conf *filter);
 int iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
 			 struct virtchnl_rss_cfg *rss_cfg, bool add);
+int iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena);
+int iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add);
 int iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 			struct rte_ether_addr *mc_addrs,
 			uint32_t mc_addrs_num, bool add);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 7e3c26a94..d2fa16825 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -267,10 +267,6 @@ iavf_init_rss(struct iavf_adapter *adapter)
 		return ret;
 	}
 
-	/* In IAVF, RSS enablement is set by PF driver. It is not supported
-	 * to set based on rss_conf->rss_hf.
-	 */
-
 	/* configure RSS key */
 	if (!rss_conf->rss_key) {
 		/* Calculate the default hash key */
@@ -295,6 +291,13 @@ iavf_init_rss(struct iavf_adapter *adapter)
 	if (ret)
 		return ret;
 
+	/* Set RSS hash configuration based on rss_conf->rss_hf. */
+	ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to set default RSS");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -1102,33 +1105,66 @@ iavf_dev_rss_reta_query(struct rte_eth_dev *dev,
 }
 
 static int
-iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
-			struct rte_eth_rss_conf *rss_conf)
+iavf_set_rss_key(struct iavf_adapter *adapter, uint8_t *key, uint8_t key_len)
 {
-	struct iavf_adapter *adapter =
-		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
 
-	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
-		return -ENOTSUP;
-
 	/* HENA setting, it is enabled by default, no change */
-	if (!rss_conf->rss_key || rss_conf->rss_key_len == 0) {
+	if (!key || key_len == 0) {
 		PMD_DRV_LOG(DEBUG, "No key to be configured");
 		return 0;
-	} else if (rss_conf->rss_key_len != vf->vf_res->rss_key_size) {
+	} else if (key_len != vf->vf_res->rss_key_size) {
 		PMD_DRV_LOG(ERR, "The size of hash key configured "
 			"(%d) doesn't match the size of hardware can "
-			"support (%d)", rss_conf->rss_key_len,
+			"support (%d)", key_len,
 			vf->vf_res->rss_key_size);
 		return -EINVAL;
 	}
 
-	rte_memcpy(vf->rss_key, rss_conf->rss_key, rss_conf->rss_key_len);
+	rte_memcpy(vf->rss_key, key, key_len);
 
 	return iavf_configure_rss_key(adapter);
 }
 
+static int
+iavf_dev_rss_hash_update(struct rte_eth_dev *dev,
+			struct rte_eth_rss_conf *rss_conf)
+{
+	struct iavf_adapter *adapter =
+		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
+	int ret;
+
+	adapter->eth_dev->data->dev_conf.rx_adv_conf.rss_conf = *rss_conf;
+
+	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
+		return -ENOTSUP;
+
+	/* Set hash key. */
+	ret = iavf_set_rss_key(adapter, rss_conf->rss_key,
+			       rss_conf->rss_key_len);
+	if (ret)
+		return ret;
+
+	if (rss_conf->rss_hf == 0)
+		return 0;
+
+	/* Overwritten default RSS. */
+	ret = iavf_set_hena(adapter, 0);
+	if (ret)
+		PMD_DRV_LOG(ERR, "%s Remove rss vsi fail %d",
+			    __func__, ret);
+
+	/* Set new RSS configuration. */
+	ret = iavf_rss_hash_set(adapter, rss_conf->rss_hf, true);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to set new RSS");
+		return ret;
+	}
+
+	return 0;
+}
+
 static int
 iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 			  struct rte_eth_rss_conf *rss_conf)
@@ -1140,8 +1176,7 @@ iavf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF))
 		return -ENOTSUP;
 
-	 /* Just set it to default value now. */
-	rss_conf->rss_hf = IAVF_RSS_OFFLOAD_ALL;
+	rss_conf->rss_hf = vf->rss_hf;
 
 	if (!rss_conf->rss_key)
 		return 0;
@@ -2029,6 +2064,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 		return ret;
 	}
 
+	/* Set hena = 0 to ask PF to cleanup all existing RSS. */
+	ret = iavf_set_hena(adapter, 0);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to disable default PF RSS");
+		return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index c4c73e664..3d63b7eb6 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -429,17 +429,6 @@ static struct iavf_pattern_match_item iavf_hash_pattern_list[] = {
 	{iavf_pattern_eth_ipv6_gtpc,			ETH_RSS_IPV6,			&ipv6_udp_gtpc_tmplt},
 };
 
-struct virtchnl_proto_hdrs *iavf_hash_default_hdrs[] = {
-	&inner_ipv4_tmplt,
-	&inner_ipv4_udp_tmplt,
-	&inner_ipv4_tcp_tmplt,
-	&inner_ipv4_sctp_tmplt,
-	&inner_ipv6_tmplt,
-	&inner_ipv6_udp_tmplt,
-	&inner_ipv6_tcp_tmplt,
-	&inner_ipv6_sctp_tmplt,
-};
-
 static struct iavf_flow_engine iavf_hash_engine = {
 	.init = iavf_hash_init,
 	.create = iavf_hash_create,
@@ -458,24 +447,64 @@ static struct iavf_flow_parser iavf_hash_parser = {
 	.stage = IAVF_FLOW_STAGE_RSS,
 };
 
-static int
-iavf_hash_default_set(struct iavf_adapter *ad, bool add)
+int
+iavf_rss_hash_set(struct iavf_adapter *ad, uint64_t rss_hf, bool add)
 {
-	struct virtchnl_rss_cfg *rss_cfg;
-	uint16_t i;
+	struct iavf_info *vf =  IAVF_DEV_PRIVATE_TO_VF(ad);
+	struct virtchnl_rss_cfg rss_cfg;
+
+#define IAVF_RSS_HF_ALL ( \
+	ETH_RSS_IPV4 | \
+	ETH_RSS_IPV6 | \
+	ETH_RSS_NONFRAG_IPV4_UDP | \
+	ETH_RSS_NONFRAG_IPV6_UDP | \
+	ETH_RSS_NONFRAG_IPV4_TCP | \
+	ETH_RSS_NONFRAG_IPV6_TCP | \
+	ETH_RSS_NONFRAG_IPV4_SCTP | \
+	ETH_RSS_NONFRAG_IPV6_SCTP)
+
+	rss_cfg.rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+	if (rss_hf & ETH_RSS_IPV4) {
+		rss_cfg.proto_hdrs = inner_ipv4_tmplt;
+		iavf_add_del_rss_cfg(ad, &rss_cfg, add);
+	}
 
-	rss_cfg = rte_zmalloc("iavf rss rule",
-			      sizeof(struct virtchnl_rss_cfg), 0);
-	if (!rss_cfg)
-		return -ENOMEM;
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP) {
+		rss_cfg.proto_hdrs = inner_ipv4_udp_tmplt;
+		iavf_add_del_rss_cfg(ad, &rss_cfg, add);
+	}
 
-	for (i = 0; i < RTE_DIM(iavf_hash_default_hdrs); i++) {
-		rss_cfg->proto_hdrs = *iavf_hash_default_hdrs[i];
-		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP) {
+		rss_cfg.proto_hdrs = inner_ipv4_tcp_tmplt;
+		iavf_add_del_rss_cfg(ad, &rss_cfg, add);
+	}
 
-		iavf_add_del_rss_cfg(ad, rss_cfg, add);
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_SCTP) {
+		rss_cfg.proto_hdrs = inner_ipv4_sctp_tmplt;
+		iavf_add_del_rss_cfg(ad, &rss_cfg, add);
 	}
 
+	if (rss_hf & ETH_RSS_IPV6) {
+		rss_cfg.proto_hdrs = inner_ipv6_tmplt;
+		iavf_add_del_rss_cfg(ad, &rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP) {
+		rss_cfg.proto_hdrs = inner_ipv6_udp_tmplt;
+		iavf_add_del_rss_cfg(ad, &rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP) {
+		rss_cfg.proto_hdrs = inner_ipv6_tcp_tmplt;
+		iavf_add_del_rss_cfg(ad, &rss_cfg, add);
+	}
+
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_SCTP) {
+		rss_cfg.proto_hdrs = inner_ipv6_sctp_tmplt;
+		iavf_add_del_rss_cfg(ad, &rss_cfg, add);
+	}
+
+	vf->rss_hf = rss_hf & IAVF_RSS_HF_ALL;
 	return 0;
 }
 
@@ -510,12 +539,6 @@ iavf_hash_init(struct iavf_adapter *ad)
 		return ret;
 	}
 
-	ret = iavf_hash_default_set(ad, true);
-	if (ret) {
-		PMD_DRV_LOG(ERR, "fail to set default RSS");
-		iavf_unregister_parser(parser, ad);
-	}
-
 	return ret;
 }
 
@@ -1089,6 +1112,7 @@ static void
 iavf_hash_uninit(struct iavf_adapter *ad)
 {
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
+	struct rte_eth_rss_conf *rss_conf;
 
 	if (vf->vf_reset)
 		return;
@@ -1099,7 +1123,8 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF))
 		return;
 
-	if (iavf_hash_default_set(ad, false))
+	rss_conf = &ad->eth_dev->data->dev_conf.rx_adv_conf.rss_conf;
+	if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
 		PMD_DRV_LOG(ERR, "fail to delete default RSS");
 
 	iavf_unregister_parser(&iavf_hash_parser, ad);
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 33d03af65..c718a75c4 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1341,6 +1341,29 @@ iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
 	return err;
 }
 
+int
+iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena)
+{
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
+	struct virtchnl_rss_hena vrh;
+	struct iavf_cmd_info args;
+	int err;
+
+	vrh.hena = hena;
+	args.ops = VIRTCHNL_OP_SET_RSS_HENA;
+	args.in_args = (u8 *)&vrh;
+	args.in_args_size = sizeof(vrh);
+	args.out_buffer = vf->aq_resp;
+	args.out_size = IAVF_AQ_BUF_SZ;
+
+	err = iavf_execute_vf_cmd(adapter, &args);
+	if (err)
+		PMD_DRV_LOG(ERR,
+			    "Failed to execute command of OP_SET_RSS_HENA");
+
+	return err;
+}
+
 int
 iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 			struct rte_ether_addr *mc_addrs,
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v2] net/iavf: improve default RSS
  2020-12-23 12:30 ` [dpdk-dev] [PATCH v2] " Xuan Ding
@ 2020-12-23 12:54   ` Zhang, Qi Z
  0 siblings, 0 replies; 7+ messages in thread
From: Zhang, Qi Z @ 2020-12-23 12:54 UTC (permalink / raw)
  To: Ding, Xuan, Wu, Jingjing, Xing, Beilei; +Cc: dev



> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Wednesday, December 23, 2020 8:31 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>
> Subject: [PATCH v2] net/iavf: improve default RSS
> 
> This patch adds support to actively configure RSS through port config.
> Any kernel PF enabled default RSS will be disabled during initialization.
> Besides, default RSS will be configured based on rte_eth_rss_conf->rss_hf.
> Currently supported default rss_type: ipv4[6], ipv4[6]_udp, ipv4[6]_tcp,
> ipv4[6]_sctp.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

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

* Re: [dpdk-dev] [PATCH v3] net/iavf: improve default RSS
  2020-12-23 12:52 ` [dpdk-dev] [PATCH v3] " Xuan Ding
@ 2020-12-23 12:59   ` Zhang, Qi Z
  0 siblings, 0 replies; 7+ messages in thread
From: Zhang, Qi Z @ 2020-12-23 12:59 UTC (permalink / raw)
  To: Ding, Xuan, Wu, Jingjing, Xing, Beilei; +Cc: dev



> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Wednesday, December 23, 2020 8:52 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>
> Subject: [PATCH v3] net/iavf: improve default RSS
> 
> This patch adds support to actively configure RSS through port config.
> Any kernel PF enabled default RSS will be disabled during initialization.
> Besides, default RSS will be configured based on rte_eth_rss_conf->rss_hf.
> Currently supported default rss_type: ipv4[6], ipv4[6]_udp, ipv4[6]_tcp,
> ipv4[6]_sctp.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

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

end of thread, other threads:[~2020-12-23 12:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03  3:26 [dpdk-dev] [PATCH] net/iavf: improve default RSS Xuan Ding
2020-12-23 10:14 ` Zhang, Qi Z
2020-12-23 11:52   ` Ding, Xuan
2020-12-23 12:30 ` [dpdk-dev] [PATCH v2] " Xuan Ding
2020-12-23 12:54   ` Zhang, Qi Z
2020-12-23 12:52 ` [dpdk-dev] [PATCH v3] " Xuan Ding
2020-12-23 12:59   ` Zhang, Qi Z

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git