diff mbox

[dpdk-dev,v5,5/6] ixgbe: Config VF RSS

Message ID 1420612355-6666-6-git-send-email-changchun.ouyang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Ouyang Changchun Jan. 7, 2015, 6:32 a.m. UTC 
  It needs config RSS and IXGBE_MRQC and IXGBE_VFPSRTYPE to enable VF RSS.

The psrtype will determine how many queues the received packets will distribute to,
and the value of psrtype should depends on both facet: max VF rxq number which
has been negotiated with PF, and the number of rxq specified in config on guest.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>

Changes in v4:
 - the number of rxq from config should be power of 2 and should not bigger than
    max VF rxq number(negotiated between guest and host).

---
 lib/librte_pmd_ixgbe/ixgbe_pf.c   |  15 ++++++
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 103 +++++++++++++++++++++++++++++++++-----
 2 files changed, 106 insertions(+), 12 deletions(-)

Comments

Vladislav Zolotarov Jan. 8, 2015, 9:43 a.m. UTC | #1 
On 01/07/15 08:32, Ouyang Changchun wrote:
> It needs config RSS and IXGBE_MRQC and IXGBE_VFPSRTYPE to enable VF RSS.
>
> The psrtype will determine how many queues the received packets will distribute to,
> and the value of psrtype should depends on both facet: max VF rxq number which
> has been negotiated with PF, and the number of rxq specified in config on guest.
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
>
> Changes in v4:
>   - the number of rxq from config should be power of 2 and should not bigger than
>      max VF rxq number(negotiated between guest and host).
>
> ---
>   lib/librte_pmd_ixgbe/ixgbe_pf.c   |  15 ++++++
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 103 +++++++++++++++++++++++++++++++++-----
>   2 files changed, 106 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_pf.c b/lib/librte_pmd_ixgbe/ixgbe_pf.c
> index dbda9b5..93f6e43 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_pf.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_pf.c
> @@ -187,6 +187,21 @@ int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
>   	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_LO(hw->mac.num_rar_entries), 0);
>   	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_HI(hw->mac.num_rar_entries), 0);
>   
> +	/*
> +	 * VF RSS can support at most 4 queues for each VF, even if
> +	 * 8 queues are available for each VF, it need refine to 4
> +	 * queues here due to this limitation, otherwise no queue
> +	 * will receive any packet even RSS is enabled.
> +	 */
> +	if (eth_dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_RSS) {
> +		if (RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool == 8) {
> +			RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_32_POOLS;
> +			RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = 4;
> +			RTE_ETH_DEV_SRIOV(eth_dev).def_pool_q_idx =
> +				dev_num_vf(eth_dev) * 4;
> +		}
> +	}
> +
>   	/* set VMDq map to default PF pool */
>   	hw->mac.ops.set_vmdq(hw, 0, RTE_ETH_DEV_SRIOV(eth_dev).def_vmdq_idx);
>   
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index f69abda..e83a9ab 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -3327,6 +3327,68 @@ ixgbe_alloc_rx_queue_mbufs(struct igb_rx_queue *rxq)
>   }
>   
>   static int
> +ixgbe_config_vf_rss(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw;
> +	uint32_t mrqc;
> +
> +	ixgbe_rss_configure(dev);
> +
> +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +	/* MRQC: enable VF RSS */
> +	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> +	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
> +	switch (RTE_ETH_DEV_SRIOV(dev).active) {
> +	case ETH_64_POOLS:
> +		mrqc |= IXGBE_MRQC_VMDQRSS64EN;

> +		break;
> +
> +	case ETH_32_POOLS:
> +	case ETH_16_POOLS:

Isn't ETH_16_POOLS mode is invalid for VF RSS? It's what both spec 
states and what u handle in this patch in ixgbe_pf_host_configure(). 
IMHO it would be better to treat this mode value as an error here since 
if u get it here it indicates of a SW bug.

> +		mrqc |= IXGBE_MRQC_VMDQRSS32EN;
> +		break;
> +
> +	default:
> +		PMD_INIT_LOG(ERR, "Invalid pool number in IOV mode");
> +		return -EINVAL;
> +	}
> +
> +	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
> +
> +	return 0;
> +}
> +
> +static int
> +ixgbe_config_vf_default(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw =
> +		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +	switch (RTE_ETH_DEV_SRIOV(dev).active) {
> +	case ETH_64_POOLS:
> +		IXGBE_WRITE_REG(hw, IXGBE_MRQC,
> +			IXGBE_MRQC_VMDQEN);
> +		break;
> +
> +	case ETH_32_POOLS:
> +		IXGBE_WRITE_REG(hw, IXGBE_MRQC,
> +			IXGBE_MRQC_VMDQRT4TCEN);
> +		break;
> +
> +	case ETH_16_POOLS:
> +		IXGBE_WRITE_REG(hw, IXGBE_MRQC,
> +			IXGBE_MRQC_VMDQRT8TCEN);
> +		break;
> +	default:
> +		PMD_INIT_LOG(ERR,
> +			"invalid pool number in IOV mode");
> +		break;
> +	}
> +	return 0;
> +}
> +
> +static int
>   ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)
>   {
>   	struct ixgbe_hw *hw =
> @@ -3358,24 +3420,25 @@ ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)
>   			default: ixgbe_rss_disable(dev);
>   		}
>   	} else {
> -		switch (RTE_ETH_DEV_SRIOV(dev).active) {
>   		/*
>   		 * SRIOV active scheme
> -		 * FIXME if support DCB/RSS together with VMDq & SRIOV
> +		 * Support RSS together with VMDq & SRIOV
>   		 */
> -		case ETH_64_POOLS:
> -			IXGBE_WRITE_REG(hw, IXGBE_MRQC, IXGBE_MRQC_VMDQEN);
> -			break;
> -
> -		case ETH_32_POOLS:
> -			IXGBE_WRITE_REG(hw, IXGBE_MRQC, IXGBE_MRQC_VMDQRT4TCEN);
> +		switch (dev->data->dev_conf.rxmode.mq_mode) {
> +		case ETH_MQ_RX_RSS:
> +		case ETH_MQ_RX_VMDQ_RSS:
> +			ixgbe_config_vf_rss(dev);
>   			break;
>   
> -		case ETH_16_POOLS:
> -			IXGBE_WRITE_REG(hw, IXGBE_MRQC, IXGBE_MRQC_VMDQRT8TCEN);
> -			break;
> +		/* FIXME if support DCB/RSS together with VMDq & SRIOV */
> +		case ETH_MQ_RX_VMDQ_DCB:
> +		case ETH_MQ_RX_VMDQ_DCB_RSS:
> +			PMD_INIT_LOG(ERR,
> +				"Could not support DCB with VMDq & SRIOV");
> +			return -1;
>   		default:
> -			PMD_INIT_LOG(ERR, "invalid pool number in IOV mode");
> +			ixgbe_config_vf_default(dev);
> +			break;
>   		}
>   	}
>   
> @@ -3993,6 +4056,19 @@ ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
>   	PMD_INIT_FUNC_TRACE();
>   	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>   
> +	if (rte_is_power_of_2(dev->data->nb_rx_queues) == 0) {
> +		PMD_INIT_LOG(ERR, "The number of Rx queue invalid, "
> +			"it should be power of 2");
> +		return -1;
> +	}
> +
> +	if (dev->data->nb_rx_queues > hw->mac.max_rx_queues) {
> +		PMD_INIT_LOG(ERR, "The number of Rx queue invalid, "
> +			"it should be equal to or less than %d",
> +			hw->mac.max_rx_queues);
> +		return -1;
> +	}
> +
>   	/*
>   	 * When the VF driver issues a IXGBE_VF_RESET request, the PF driver
>   	 * disables the VF receipt of packets if the PF MTU is > 1500.
> @@ -4094,6 +4170,9 @@ ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
>   			IXGBE_PSRTYPE_IPV6HDR;
>   #endif
>   
> +	/* Set RQPL for VF RSS according to max Rx queue */
> +	psrtype |= (dev->data->nb_rx_queues >> 1) <<
> +		IXGBE_PSRTYPE_RQPL_SHIFT;
>   	IXGBE_WRITE_REG(hw, IXGBE_VFPSRTYPE, psrtype);
>   
>   	if (dev->data->dev_conf.rxmode.enable_scatter) {
Ouyang Changchun Jan. 9, 2015, 6:07 a.m. UTC | #2 
> -----Original Message-----
> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> Sent: Thursday, January 8, 2015 5:43 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 5/6] ixgbe: Config VF RSS
> 
> 
> On 01/07/15 08:32, Ouyang Changchun wrote:
> > It needs config RSS and IXGBE_MRQC and IXGBE_VFPSRTYPE to enable VF
> RSS.
> >
> > The psrtype will determine how many queues the received packets will
> > distribute to, and the value of psrtype should depends on both facet:
> > max VF rxq number which has been negotiated with PF, and the number of
> rxq specified in config on guest.
> >
> > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> >
> > Changes in v4:
> >   - the number of rxq from config should be power of 2 and should not
> bigger than
> >      max VF rxq number(negotiated between guest and host).
> >
> > ---
> >   lib/librte_pmd_ixgbe/ixgbe_pf.c   |  15 ++++++
> >   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 103
> +++++++++++++++++++++++++++++++++-----
> >   2 files changed, 106 insertions(+), 12 deletions(-)
> >
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_pf.c
> > b/lib/librte_pmd_ixgbe/ixgbe_pf.c index dbda9b5..93f6e43 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe_pf.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_pf.c
> > @@ -187,6 +187,21 @@ int ixgbe_pf_host_configure(struct rte_eth_dev
> *eth_dev)
> >   	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_LO(hw-
> >mac.num_rar_entries), 0);
> >   	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_HI(hw-
> >mac.num_rar_entries), 0);
> >
> > +	/*
> > +	 * VF RSS can support at most 4 queues for each VF, even if
> > +	 * 8 queues are available for each VF, it need refine to 4
> > +	 * queues here due to this limitation, otherwise no queue
> > +	 * will receive any packet even RSS is enabled.
> > +	 */
> > +	if (eth_dev->data->dev_conf.rxmode.mq_mode ==
> ETH_MQ_RX_VMDQ_RSS) {
> > +		if (RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool == 8) {
> > +			RTE_ETH_DEV_SRIOV(eth_dev).active =
> ETH_32_POOLS;
> > +			RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = 4;
> > +			RTE_ETH_DEV_SRIOV(eth_dev).def_pool_q_idx =
> > +				dev_num_vf(eth_dev) * 4;
> > +		}
> > +	}
> > +
> >   	/* set VMDq map to default PF pool */
> >   	hw->mac.ops.set_vmdq(hw, 0,
> > RTE_ETH_DEV_SRIOV(eth_dev).def_vmdq_idx);
> >
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > index f69abda..e83a9ab 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > @@ -3327,6 +3327,68 @@ ixgbe_alloc_rx_queue_mbufs(struct
> igb_rx_queue *rxq)
> >   }
> >
> >   static int
> > +ixgbe_config_vf_rss(struct rte_eth_dev *dev) {
> > +	struct ixgbe_hw *hw;
> > +	uint32_t mrqc;
> > +
> > +	ixgbe_rss_configure(dev);
> > +
> > +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > +
> > +	/* MRQC: enable VF RSS */
> > +	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> > +	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
> > +	switch (RTE_ETH_DEV_SRIOV(dev).active) {
> > +	case ETH_64_POOLS:
> > +		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
> 
> > +		break;
> > +
> > +	case ETH_32_POOLS:
> > +	case ETH_16_POOLS:
> 
> Isn't ETH_16_POOLS mode is invalid for VF RSS? It's what both spec states
> and what u handle in this patch in ixgbe_pf_host_configure().
> IMHO it would be better to treat this mode value as an error here since if u
> get it here it indicates of a SW bug.

I think we discussed it before already,  return err here will break here in the case of max vf number is less than 16.
If doing that, This make the library seems can't support vf rss in the case of max vf num less than 16.
So we obviously don't hope it break here.
Vladislav Zolotarov Jan. 9, 2015, 2:01 p.m. UTC | #3 
On 01/09/15 08:07, Ouyang, Changchun wrote:
>
>> -----Original Message-----
>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>> Sent: Thursday, January 8, 2015 5:43 PM
>> To: Ouyang, Changchun; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v5 5/6] ixgbe: Config VF RSS
>>
>>
>> On 01/07/15 08:32, Ouyang Changchun wrote:
>>> It needs config RSS and IXGBE_MRQC and IXGBE_VFPSRTYPE to enable VF
>> RSS.
>>> The psrtype will determine how many queues the received packets will
>>> distribute to, and the value of psrtype should depends on both facet:
>>> max VF rxq number which has been negotiated with PF, and the number of
>> rxq specified in config on guest.
>>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
>>>
>>> Changes in v4:
>>>    - the number of rxq from config should be power of 2 and should not
>> bigger than
>>>       max VF rxq number(negotiated between guest and host).
>>>
>>> ---
>>>    lib/librte_pmd_ixgbe/ixgbe_pf.c   |  15 ++++++
>>>    lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 103
>> +++++++++++++++++++++++++++++++++-----
>>>    2 files changed, 106 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_pf.c
>>> b/lib/librte_pmd_ixgbe/ixgbe_pf.c index dbda9b5..93f6e43 100644
>>> --- a/lib/librte_pmd_ixgbe/ixgbe_pf.c
>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_pf.c
>>> @@ -187,6 +187,21 @@ int ixgbe_pf_host_configure(struct rte_eth_dev
>> *eth_dev)
>>>    	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_LO(hw-
>>> mac.num_rar_entries), 0);
>>>    	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_HI(hw-
>>> mac.num_rar_entries), 0);
>>>
>>> +	/*
>>> +	 * VF RSS can support at most 4 queues for each VF, even if
>>> +	 * 8 queues are available for each VF, it need refine to 4
>>> +	 * queues here due to this limitation, otherwise no queue
>>> +	 * will receive any packet even RSS is enabled.
>>> +	 */
>>> +	if (eth_dev->data->dev_conf.rxmode.mq_mode ==
>> ETH_MQ_RX_VMDQ_RSS) {
>>> +		if (RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool == 8) {
>>> +			RTE_ETH_DEV_SRIOV(eth_dev).active =
>> ETH_32_POOLS;
>>> +			RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = 4;
>>> +			RTE_ETH_DEV_SRIOV(eth_dev).def_pool_q_idx =
>>> +				dev_num_vf(eth_dev) * 4;
>>> +		}
>>> +	}
>>> +
>>>    	/* set VMDq map to default PF pool */
>>>    	hw->mac.ops.set_vmdq(hw, 0,
>>> RTE_ETH_DEV_SRIOV(eth_dev).def_vmdq_idx);
>>>
>>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> index f69abda..e83a9ab 100644
>>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> @@ -3327,6 +3327,68 @@ ixgbe_alloc_rx_queue_mbufs(struct
>> igb_rx_queue *rxq)
>>>    }
>>>
>>>    static int
>>> +ixgbe_config_vf_rss(struct rte_eth_dev *dev) {
>>> +	struct ixgbe_hw *hw;
>>> +	uint32_t mrqc;
>>> +
>>> +	ixgbe_rss_configure(dev);
>>> +
>>> +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>>> +
>>> +	/* MRQC: enable VF RSS */
>>> +	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
>>> +	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
>>> +	switch (RTE_ETH_DEV_SRIOV(dev).active) {
>>> +	case ETH_64_POOLS:
>>> +		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
>>> +		break;
>>> +
>>> +	case ETH_32_POOLS:
>>> +	case ETH_16_POOLS:
>> Isn't ETH_16_POOLS mode is invalid for VF RSS? It's what both spec states
>> and what u handle in this patch in ixgbe_pf_host_configure().
>> IMHO it would be better to treat this mode value as an error here since if u
>> get it here it indicates of a SW bug.
> I think we discussed it before already,  return err here will break here in the case of max vf number is less than 16.
> If doing that, This make the library seems can't support vf rss in the case of max vf num less than 16.
> So we obviously don't hope it break here.

I don't remember we were discussing these specific lines. However I do 
remember we talked about the previous section of this patch.
I'm afraid u are missing my point here: ixgbe_pf_host_configure() is 
called before ixgbe_config_vf_rss() in the ixgbe_dev_start() flow. This 
means that
RTE_ETH_DEV_SRIOV(dev).active will already be adjusted by your (!!!) 
code in the ixgbe_pf_host_configure() when u get to 
ixgbe_config_vf_rss() and it should not be equal ETH_16_POOLS unless 
there is a bug in your code.

So, unless I've missed something here, don't u think an assert() would 
be appropriate if RTE_ETH_DEV_SRIOV(dev).active equals ETH_16_POOLS?

thanks,
vlad

>
>
Ouyang Changchun Jan. 12, 2015, 5:11 a.m. UTC | #4 
> -----Original Message-----
> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> Sent: Friday, January 9, 2015 10:02 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 5/6] ixgbe: Config VF RSS
> 
> 
> On 01/09/15 08:07, Ouyang, Changchun wrote:
> >
> >> -----Original Message-----
> >> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >> Sent: Thursday, January 8, 2015 5:43 PM
> >> To: Ouyang, Changchun; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH v5 5/6] ixgbe: Config VF RSS
> >>
> >>
> >> On 01/07/15 08:32, Ouyang Changchun wrote:
> >>> It needs config RSS and IXGBE_MRQC and IXGBE_VFPSRTYPE to enable
> VF
> >> RSS.
> >>> The psrtype will determine how many queues the received packets will
> >>> distribute to, and the value of psrtype should depends on both facet:
> >>> max VF rxq number which has been negotiated with PF, and the number
> >>> of
> >> rxq specified in config on guest.
> >>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> >>>
> >>> Changes in v4:
> >>>    - the number of rxq from config should be power of 2 and should
> >>> not
> >> bigger than
> >>>       max VF rxq number(negotiated between guest and host).
> >>>
> >>> ---
> >>>    lib/librte_pmd_ixgbe/ixgbe_pf.c   |  15 ++++++
> >>>    lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 103
> >> +++++++++++++++++++++++++++++++++-----
> >>>    2 files changed, 106 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_pf.c
> >>> b/lib/librte_pmd_ixgbe/ixgbe_pf.c index dbda9b5..93f6e43 100644
> >>> --- a/lib/librte_pmd_ixgbe/ixgbe_pf.c
> >>> +++ b/lib/librte_pmd_ixgbe/ixgbe_pf.c
> >>> @@ -187,6 +187,21 @@ int ixgbe_pf_host_configure(struct rte_eth_dev
> >> *eth_dev)
> >>>    	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_LO(hw-
> mac.num_rar_entries), 0);
> >>>    	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_HI(hw-
> mac.num_rar_entries), 0);
> >>>
> >>> +	/*
> >>> +	 * VF RSS can support at most 4 queues for each VF, even if
> >>> +	 * 8 queues are available for each VF, it need refine to 4
> >>> +	 * queues here due to this limitation, otherwise no queue
> >>> +	 * will receive any packet even RSS is enabled.
> >>> +	 */
> >>> +	if (eth_dev->data->dev_conf.rxmode.mq_mode ==
> >> ETH_MQ_RX_VMDQ_RSS) {
> >>> +		if (RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool == 8) {
> >>> +			RTE_ETH_DEV_SRIOV(eth_dev).active =
> >> ETH_32_POOLS;
> >>> +			RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = 4;
> >>> +			RTE_ETH_DEV_SRIOV(eth_dev).def_pool_q_idx =
> >>> +				dev_num_vf(eth_dev) * 4;
> >>> +		}
> >>> +	}
> >>> +
> >>>    	/* set VMDq map to default PF pool */
> >>>    	hw->mac.ops.set_vmdq(hw, 0,
> >>> RTE_ETH_DEV_SRIOV(eth_dev).def_vmdq_idx);
> >>>
> >>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >>> index f69abda..e83a9ab 100644
> >>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >>> @@ -3327,6 +3327,68 @@ ixgbe_alloc_rx_queue_mbufs(struct
> >> igb_rx_queue *rxq)
> >>>    }
> >>>
> >>>    static int
> >>> +ixgbe_config_vf_rss(struct rte_eth_dev *dev) {
> >>> +	struct ixgbe_hw *hw;
> >>> +	uint32_t mrqc;
> >>> +
> >>> +	ixgbe_rss_configure(dev);
> >>> +
> >>> +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> >>> +
> >>> +	/* MRQC: enable VF RSS */
> >>> +	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> >>> +	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
> >>> +	switch (RTE_ETH_DEV_SRIOV(dev).active) {
> >>> +	case ETH_64_POOLS:
> >>> +		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
> >>> +		break;
> >>> +
> >>> +	case ETH_32_POOLS:
> >>> +	case ETH_16_POOLS:
> >> Isn't ETH_16_POOLS mode is invalid for VF RSS? It's what both spec
> >> states and what u handle in this patch in ixgbe_pf_host_configure().
> >> IMHO it would be better to treat this mode value as an error here
> >> since if u get it here it indicates of a SW bug.
> > I think we discussed it before already,  return err here will break here in the
> case of max vf number is less than 16.
> > If doing that, This make the library seems can't support vf rss in the case of
> max vf num less than 16.
> > So we obviously don't hope it break here.
> 
> I don't remember we were discussing these specific lines. However I do
> remember we talked about the previous section of this patch.
> I'm afraid u are missing my point here: ixgbe_pf_host_configure() is called
> before ixgbe_config_vf_rss() in the ixgbe_dev_start() flow. This means that
> RTE_ETH_DEV_SRIOV(dev).active will already be adjusted by your (!!!) code
> in the ixgbe_pf_host_configure() when u get to
> ixgbe_config_vf_rss() and it should not be equal ETH_16_POOLS unless there
> is a bug in your code.
> 
> So, unless I've missed something here, don't u think an assert() would be
> appropriate if RTE_ETH_DEV_SRIOV(dev).active equals ETH_16_POOLS?

Ooh, thanks for identifying this. Here ETH_16_POOLS branch not necessary, as you said, I 
Have resolved it in function ixgbe_pf_host_configure.
Then I will fix it in v6.
Thanks again
Changchun
diff mbox

Patch

diff --git a/lib/librte_pmd_ixgbe/ixgbe_pf.c b/lib/librte_pmd_ixgbe/ixgbe_pf.c
index dbda9b5..93f6e43 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_pf.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_pf.c
@@ -187,6 +187,21 @@  int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
 	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_LO(hw->mac.num_rar_entries), 0);
 	IXGBE_WRITE_REG(hw, IXGBE_MPSAR_HI(hw->mac.num_rar_entries), 0);
 
+	/*
+	 * VF RSS can support at most 4 queues for each VF, even if
+	 * 8 queues are available for each VF, it need refine to 4
+	 * queues here due to this limitation, otherwise no queue
+	 * will receive any packet even RSS is enabled.
+	 */
+	if (eth_dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_RSS) {
+		if (RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool == 8) {
+			RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_32_POOLS;
+			RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = 4;
+			RTE_ETH_DEV_SRIOV(eth_dev).def_pool_q_idx =
+				dev_num_vf(eth_dev) * 4;
+		}
+	}
+
 	/* set VMDq map to default PF pool */
 	hw->mac.ops.set_vmdq(hw, 0, RTE_ETH_DEV_SRIOV(eth_dev).def_vmdq_idx);
 
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index f69abda..e83a9ab 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -3327,6 +3327,68 @@  ixgbe_alloc_rx_queue_mbufs(struct igb_rx_queue *rxq)
 }
 
 static int
+ixgbe_config_vf_rss(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw;
+	uint32_t mrqc;
+
+	ixgbe_rss_configure(dev);
+
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* MRQC: enable VF RSS */
+	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
+	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
+	switch (RTE_ETH_DEV_SRIOV(dev).active) {
+	case ETH_64_POOLS:
+		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
+		break;
+
+	case ETH_32_POOLS:
+	case ETH_16_POOLS:
+		mrqc |= IXGBE_MRQC_VMDQRSS32EN;
+		break;
+
+	default:
+		PMD_INIT_LOG(ERR, "Invalid pool number in IOV mode");
+		return -EINVAL;
+	}
+
+	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
+
+	return 0;
+}
+
+static int
+ixgbe_config_vf_default(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	switch (RTE_ETH_DEV_SRIOV(dev).active) {
+	case ETH_64_POOLS:
+		IXGBE_WRITE_REG(hw, IXGBE_MRQC,
+			IXGBE_MRQC_VMDQEN);
+		break;
+
+	case ETH_32_POOLS:
+		IXGBE_WRITE_REG(hw, IXGBE_MRQC,
+			IXGBE_MRQC_VMDQRT4TCEN);
+		break;
+
+	case ETH_16_POOLS:
+		IXGBE_WRITE_REG(hw, IXGBE_MRQC,
+			IXGBE_MRQC_VMDQRT8TCEN);
+		break;
+	default:
+		PMD_INIT_LOG(ERR,
+			"invalid pool number in IOV mode");
+		break;
+	}
+	return 0;
+}
+
+static int
 ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw =
@@ -3358,24 +3420,25 @@  ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)
 			default: ixgbe_rss_disable(dev);
 		}
 	} else {
-		switch (RTE_ETH_DEV_SRIOV(dev).active) {
 		/*
 		 * SRIOV active scheme
-		 * FIXME if support DCB/RSS together with VMDq & SRIOV
+		 * Support RSS together with VMDq & SRIOV
 		 */
-		case ETH_64_POOLS:
-			IXGBE_WRITE_REG(hw, IXGBE_MRQC, IXGBE_MRQC_VMDQEN);
-			break;
-
-		case ETH_32_POOLS:
-			IXGBE_WRITE_REG(hw, IXGBE_MRQC, IXGBE_MRQC_VMDQRT4TCEN);
+		switch (dev->data->dev_conf.rxmode.mq_mode) {
+		case ETH_MQ_RX_RSS:
+		case ETH_MQ_RX_VMDQ_RSS:
+			ixgbe_config_vf_rss(dev);
 			break;
 
-		case ETH_16_POOLS:
-			IXGBE_WRITE_REG(hw, IXGBE_MRQC, IXGBE_MRQC_VMDQRT8TCEN);
-			break;
+		/* FIXME if support DCB/RSS together with VMDq & SRIOV */
+		case ETH_MQ_RX_VMDQ_DCB:
+		case ETH_MQ_RX_VMDQ_DCB_RSS:
+			PMD_INIT_LOG(ERR,
+				"Could not support DCB with VMDq & SRIOV");
+			return -1;
 		default:
-			PMD_INIT_LOG(ERR, "invalid pool number in IOV mode");
+			ixgbe_config_vf_default(dev);
+			break;
 		}
 	}
 
@@ -3993,6 +4056,19 @@  ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
+	if (rte_is_power_of_2(dev->data->nb_rx_queues) == 0) {
+		PMD_INIT_LOG(ERR, "The number of Rx queue invalid, "
+			"it should be power of 2");
+		return -1;
+	}
+
+	if (dev->data->nb_rx_queues > hw->mac.max_rx_queues) {
+		PMD_INIT_LOG(ERR, "The number of Rx queue invalid, "
+			"it should be equal to or less than %d",
+			hw->mac.max_rx_queues);
+		return -1;
+	}
+
 	/*
 	 * When the VF driver issues a IXGBE_VF_RESET request, the PF driver
 	 * disables the VF receipt of packets if the PF MTU is > 1500.
@@ -4094,6 +4170,9 @@  ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
 			IXGBE_PSRTYPE_IPV6HDR;
 #endif
 
+	/* Set RQPL for VF RSS according to max Rx queue */
+	psrtype |= (dev->data->nb_rx_queues >> 1) <<
+		IXGBE_PSRTYPE_RQPL_SHIFT;
 	IXGBE_WRITE_REG(hw, IXGBE_VFPSRTYPE, psrtype);
 
 	if (dev->data->dev_conf.rxmode.enable_scatter) {