> -----Original Message-----
> From: Zhang, AlvinX <alvinx.zh...@intel.com>
> Sent: Thursday, April 22, 2021 1:08 PM
> To: Wu, Jingjing <jingjing...@intel.com>; Xing, Beilei <beilei.x...@intel.com>
> Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zh...@intel.com>; sta...@dpdk.org
> Subject: [PATCH v5] net/iavf: fix hash configuration on i40e VF
>
> In i40evf PMD, the VF directly accesses the hash enable registers to enable or
> disable hashing on ingress packets. When binding i40e VF to iavf, because the
> PF doesn't support VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF capability.
> Therefore, the VF hashing cannot be enabled.
Better to descript what's the issue first and then give the root cause.
>
> This patch adds support of hash configuration for i40e VF by sending
> VIRTCHNL_OP_SET_RSS_HENA message to the PF after checking that the PF
> does not support VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF capability.
>
> Fixes: c678299594a8 ("net/iavf: fix default RSS configuration")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Alvin Zhang <alvinx.zh...@intel.com>
> ---
>
> v2: Update commit log.
> v3: Increase compatibility with old drivers.
> v4: Fix compile issues.
> v5: Rebase to 1b593b9c832e9b284cc59665fe662242a3fc1daf.
> ---
> drivers/net/iavf/iavf.h | 1 +
> drivers/net/iavf/iavf_ethdev.c | 111
> ++++++++++++++++++++++++++++++++++++++++-
> drivers/net/iavf/iavf_vchnl.c | 45 ++++++++++++++---
> 3 files changed, 150 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> d1ae5a3..1e73f01 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -336,6 +336,7 @@ 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_get_hena_caps(struct iavf_adapter *adapter, uint64_t *caps);
> 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, diff --git
> a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c index
> d523a06..97a2dc7 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -246,6 +246,107 @@ struct rte_iavf_xstats_name_off { }
>
> static int
> +iavf_config_rss_hf(struct iavf_adapter *adapter, uint64_t rss_hf) {
> + static const uint64_t map_hena_rss[] = {
> + /* IPv4 */
> + [IAVF_FILTER_PCTYPE_NONF_UNICAST_IPV4_UDP] =
> + ETH_RSS_NONFRAG_IPV4_UDP,
> + [IAVF_FILTER_PCTYPE_NONF_MULTICAST_IPV4_UDP] =
> + ETH_RSS_NONFRAG_IPV4_UDP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV4_UDP] =
> + ETH_RSS_NONFRAG_IPV4_UDP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV4_TCP_SYN_NO_ACK] =
> + ETH_RSS_NONFRAG_IPV4_TCP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV4_TCP] =
> + ETH_RSS_NONFRAG_IPV4_TCP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV4_SCTP] =
> + ETH_RSS_NONFRAG_IPV4_SCTP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV4_OTHER] =
> + ETH_RSS_NONFRAG_IPV4_OTHER,
> + [IAVF_FILTER_PCTYPE_FRAG_IPV4] = ETH_RSS_FRAG_IPV4,
> +
> + /* IPv6 */
> + [IAVF_FILTER_PCTYPE_NONF_UNICAST_IPV6_UDP] =
> + ETH_RSS_NONFRAG_IPV6_UDP,
> + [IAVF_FILTER_PCTYPE_NONF_MULTICAST_IPV6_UDP] =
> + ETH_RSS_NONFRAG_IPV6_UDP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV6_UDP] =
> + ETH_RSS_NONFRAG_IPV6_UDP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV6_TCP_SYN_NO_ACK] =
> + ETH_RSS_NONFRAG_IPV6_TCP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV6_TCP] =
> + ETH_RSS_NONFRAG_IPV6_TCP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV6_SCTP] =
> + ETH_RSS_NONFRAG_IPV6_SCTP,
> + [IAVF_FILTER_PCTYPE_NONF_IPV6_OTHER] =
> + ETH_RSS_NONFRAG_IPV6_OTHER,
> + [IAVF_FILTER_PCTYPE_FRAG_IPV6] = ETH_RSS_FRAG_IPV6,
> +
> + /* L2 Payload */
> + [IAVF_FILTER_PCTYPE_L2_PAYLOAD] = ETH_RSS_L2_PAYLOAD
> + };
> +
> + const uint64_t ipv4_rss = ETH_RSS_NONFRAG_IPV4_UDP |
> + ETH_RSS_NONFRAG_IPV4_TCP |
> + ETH_RSS_NONFRAG_IPV4_SCTP |
> + ETH_RSS_NONFRAG_IPV4_OTHER |
> + ETH_RSS_FRAG_IPV4;
> +
> + const uint64_t ipv6_rss = ETH_RSS_NONFRAG_IPV6_UDP |
> + ETH_RSS_NONFRAG_IPV6_TCP |
> + ETH_RSS_NONFRAG_IPV6_SCTP |
> + ETH_RSS_NONFRAG_IPV6_OTHER |
> + ETH_RSS_FRAG_IPV6;
> +
> + struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> + uint64_t caps = 0, hena = 0, valid_rss_hf = 0;
> + uint32_t i;
> + int ret;
> +
> + ret = iavf_get_hena_caps(adapter, &caps);
> + if (ret)
> + return ret;
> + /**
> + * ETH_RSS_IPV4 and ETH_RSS_IPV6 can be considered as 2
> + * generalizations of all other IPv4 and IPv6 RSS types.
> + */
> + if (rss_hf & ETH_RSS_IPV4)
> + rss_hf |= ipv4_rss;
> +
> + if (rss_hf & ETH_RSS_IPV6)
> + rss_hf |= ipv6_rss;
> +
> + RTE_BUILD_BUG_ON(RTE_DIM(map_hena_rss) > sizeof(uint64_t) *
> CHAR_BIT);
> +
> + for (i = 0; i < RTE_DIM(map_hena_rss); i++) {
> + uint64_t bit = BIT_ULL(i);
> +
> + if ((caps & bit) && (map_hena_rss[i] & rss_hf)) {
> + valid_rss_hf |= map_hena_rss[i];
> + hena |= bit;
> + }
> + }
> +
> + ret = iavf_set_hena(adapter, hena);
> + if (ret)
> + return ret;
> +
> + if (valid_rss_hf & ipv4_rss)
> + valid_rss_hf |= rss_hf & ETH_RSS_IPV4;
> +
> + if (valid_rss_hf & ipv6_rss)
> + valid_rss_hf |= rss_hf & ETH_RSS_IPV6;
> +
> + if (rss_hf & ~valid_rss_hf)
> + PMD_DRV_LOG(WARNING, "Unsupported rss_hf 0x%" PRIx64,
> + rss_hf & ~valid_rss_hf);
> +
> + vf->rss_hf = valid_rss_hf;
> + return 0;
> +}
> +
> +static int
> iavf_init_rss(struct iavf_adapter *adapter) {
> struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter); @@ -301,6
> +402,10 @@ struct rte_iavf_xstats_name_off {
> PMD_DRV_LOG(ERR, "fail to set default RSS");
> return ret;
> }
> + } else {
> + ret = iavf_config_rss_hf(adapter, rss_conf->rss_hf);
> + if (ret != -ENOTSUP)
> + return ret;
> }
>
> return 0;
> @@ -1282,6 +1387,10 @@ static int iavf_config_rx_queues_irqs(struct
> rte_eth_dev *dev,
> PMD_DRV_LOG(ERR, "fail to set new RSS");
> return ret;
> }
> + } else {
> + ret = iavf_config_rss_hf(adapter, rss_conf->rss_hf);
> + if (ret != -ENOTSUP)
> + return ret;
> }
>
> return 0;
> @@ -2096,7 +2205,7 @@ static int iavf_parse_devargs(struct rte_eth_dev
> *dev)
> struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> int ret = 0;
>
> - if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF) {
> + if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
> /* Set hena = 0 to ask PF to cleanup all existing RSS. */
> ret = iavf_set_hena(adapter, 0);
> if (ret)
> diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
> index
> 45096cb..8f2df0a 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -9,6 +9,7 @@
> #include <unistd.h>
> #include <stdarg.h>
> #include <inttypes.h>
> +#include <assert.h>
> #include <rte_byteorder.h>
> #include <rte_common.h>
>
> @@ -228,13 +229,19 @@
> rte_delay_ms(ASQ_DELAY_MS);
> /* If don't read msg or read sys event, continue */
> } while (i++ < MAX_TRY_TIMES);
> - /* If there's no response is received, clear command */
> - if (i >= MAX_TRY_TIMES ||
> - vf->cmd_retval != VIRTCHNL_STATUS_SUCCESS) {
> - err = -1;
> - PMD_DRV_LOG(ERR, "No response or return failure
> (%d)"
> - " for cmd %d", vf->cmd_retval, args->ops);
> +
> + if (i >= MAX_TRY_TIMES) {
> + PMD_DRV_LOG(ERR, "No response for cmd %d", args-
> >ops);
> _clear_cmd(vf);
> + err = -EIO;
> + } else if (vf->cmd_retval ==
> + (uint32_t)VIRTCHNL_STATUS_ERR_NOT_SUPPORTED)
> {
> + PMD_DRV_LOG(ERR, "Cmd %d not supported", args-
> >ops);
> + err = -ENOTSUP;
> + } else if (vf->cmd_retval != VIRTCHNL_STATUS_SUCCESS) {
> + PMD_DRV_LOG(ERR, "Return failure %d for cmd %d",
> + vf->cmd_retval, args->ops);
> + err = -EINVAL;
Seems it's not related to the issue, better to split them.
> }
> break;
> }
> @@ -1496,6 +1503,32 @@
> }
>
> int
> +iavf_get_hena_caps(struct iavf_adapter *adapter, uint64_t *caps) {
> + struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> + struct iavf_cmd_info args;
> + int err;
> +
> + assert(caps);
Why need assert here? Can we remove it?
> +
> + args.ops = VIRTCHNL_OP_GET_RSS_HENA_CAPS;
> + args.in_args = NULL;
> + args.in_args_size = 0;
> + 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_GET_RSS_HENA_CAPS");
> + return err;
> + }
> +
> + *caps = ((struct virtchnl_rss_hena *)args.out_buffer)->hena;
> + return 0;
> +}
> +
> +int
> iavf_set_hena(struct iavf_adapter *adapter, uint64_t hena) {
> struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> --
> 1.8.3.1
