> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Monday, September 28, 2015 8:53 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH 3/4] ixgbe: VF RSS hash query and update
>
> This patch implements the VF RSS hash query and update function on 10G
> NICs. But the update function is only provided for x550. Because the
> other NICs don't have the separate registers for VF, we don't want to
> let a VF NIC change the shared RSS hash registers. It may cause PF and
> other VF NICs' behavior change without being noticed.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 2 +
> drivers/net/ixgbe/ixgbe_ethdev.h | 6 +++
> drivers/net/ixgbe/ixgbe_rxtx.c | 95
> ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 103 insertions(+)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index a1ef26f..5e50ee6 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -497,6 +497,8 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
> .mac_addr_set = ixgbevf_set_default_mac_addr,
> .get_reg_length = ixgbevf_get_reg_length,
> .get_reg = ixgbevf_get_regs,
> + .rss_hash_update = ixgbevf_dev_rss_hash_update,
> + .rss_hash_conf_get = ixgbevf_dev_rss_hash_conf_get,
> };
>
> /* store statistics names and its offset in stats structure */
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> index c3d4f4f..30952b6 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -377,6 +377,12 @@ int ixgbe_dev_rss_hash_update(struct rte_eth_dev *dev,
> int ixgbe_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
> struct rte_eth_rss_conf *rss_conf);
>
> +int ixgbevf_dev_rss_hash_update(struct rte_eth_dev *dev,
> + struct rte_eth_rss_conf *rss_conf);
> +
> +int ixgbevf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
> + struct rte_eth_rss_conf *rss_conf);
> +
> /*
> * Flow director function prototypes
> */
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 4a2d24a..5b64ece 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2895,6 +2895,101 @@ ixgbevf_hw_rss_hash_set_x550(struct ixgbe_hw *hw,
> IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
> }
>
> +int
> +ixgbevf_dev_rss_hash_update(struct rte_eth_dev *dev,
> + struct rte_eth_rss_conf *rss_conf)
> +{
> + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + uint32_t vfmrqc;
> + uint64_t rss_hf;
> +
> + if (hw->mac.type != ixgbe_mac_X550_vf &&
> + hw->mac.type != ixgbe_mac_X550EM_x_vf) {
> + PMD_DRV_LOG(ERR, "RSS hash update is not supported on this "
> + "VF NIC.");
> + return -ENOTSUP;
> + }
> +
> + /*
> + * There's hardware limitation:
> + * "RSS enabling cannot be done dynamically while it must be
> + * preceded by a software reset"
> + * Before changing anything, first check that the update RSS operation
> + * does not attempt to disable RSS, if RSS was enabled at
> + * initialization time, or does not attempt to enable RSS, if RSS was
> + * disabled at initialization time.
> + */
> + rss_hf = rss_conf->rss_hf & IXGBE_RSS_OFFLOAD_ALL;
> + vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
> + if (!(vfmrqc & IXGBE_MRQC_RSSEN)) { /* RSS disabled */
> + if (rss_hf != 0) /* Enable RSS */
> + return -(EINVAL);
> + return 0; /* Nothing to do */
> + }
> + /* RSS enabled */
> + if (rss_hf == 0) /* Disable RSS */
> + return -(EINVAL);
> + ixgbevf_hw_rss_hash_set_x550(hw, rss_conf);
> + return 0;
> +}
Same comment as before: this function is just copy & paste
ixgbe_dev_rss_hash_update(),
the only difference is the HW register: VFMRQC vs MRQC.
Pls unify - create a common static function that would be called by both
ixgbe_dev_rss_hash_update()
and ixgbevf_dev_rss_hash_update(), or something.
> +
> +int
> +ixgbevf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
> + struct rte_eth_rss_conf *rss_conf)
> +{
> + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + uint8_t *hash_key;
> + uint32_t vfmrqc;
> + uint32_t rss_key;
> + uint64_t rss_hf;
> + uint16_t i;
> +
> + if (hw->mac.type != ixgbe_mac_X550_vf &&
> + hw->mac.type != ixgbe_mac_X550EM_x_vf) {
> + return ixgbe_dev_rss_hash_conf_get(dev, rss_conf);
> + }
> +
> + hash_key = rss_conf->rss_key;
> + if (hash_key != NULL) {
> + /* Return RSS hash key */
> + for (i = 0; i < 10; i++) {
> + rss_key = IXGBE_READ_REG_ARRAY(hw, IXGBE_VFRSSRK(0), i);
> + hash_key[(i * 4)] = rss_key & 0x000000FF;
> + hash_key[(i * 4) + 1] = (rss_key >> 8) & 0x000000FF;
> + hash_key[(i * 4) + 2] = (rss_key >> 16) & 0x000000FF;
> + hash_key[(i * 4) + 3] = (rss_key >> 24) & 0x000000FF;
> + }
> + }
> +
> + /* Get RSS functions configured in VFMRQC register */
> + vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
> + if ((vfmrqc & IXGBE_MRQC_RSSEN) == 0) { /* RSS is disabled */
> + rss_conf->rss_hf = 0;
> + return 0;
> + }
> + rss_hf = 0;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4)
> + rss_hf |= ETH_RSS_IPV4;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4_TCP)
> + rss_hf |= ETH_RSS_NONFRAG_IPV4_TCP;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6)
> + rss_hf |= ETH_RSS_IPV6;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX)
> + rss_hf |= ETH_RSS_IPV6_EX;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_TCP)
> + rss_hf |= ETH_RSS_NONFRAG_IPV6_TCP;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX_TCP)
> + rss_hf |= ETH_RSS_IPV6_TCP_EX;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4_UDP)
> + rss_hf |= ETH_RSS_NONFRAG_IPV4_UDP;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_UDP)
> + rss_hf |= ETH_RSS_NONFRAG_IPV6_UDP;
> + if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX_UDP)
> + rss_hf |= ETH_RSS_IPV6_UDP_EX;
> + rss_conf->rss_hf = rss_hf;
> + return 0;
> +}
Still the same: this is a clone of ixgbe_dev_rss_hash_conf_get() except the HW
regsiters names.
No need to create another copy of identical copy.
Pls unify.
Konstantin
> +
> static void
> ixgbevf_rss_configure_x550(struct rte_eth_dev *dev)
> {
> --
> 1.9.3