[dpdk-dev,v2] i40evf: Report error if HW CRC strip is disabled for Linux PF hosts

  On Linux PF hosts, the VF has no means of changing the HW CRC strip
setting for a RX queue. It's implicitly enabled.

This patch checks if the host is running a Linux PF kernel driver, and
returns an error, if HW CRC stripping was disabled.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
  

> -----Original Message-----

> From: Topel, Bjorn

> Sent: Thursday, April 21, 2016 11:03 PM

> To: dev@dpdk.org

> Cc: david.marchand@6wind.com; Zhang, Helin <helin.zhang@intel.com>; Wu,

> Jingjing <jingjing.wu@intel.com>; Topel, Bjorn <bjorn.topel@intel.com>

> Subject: [PATCH v2] i40evf: Report error if HW CRC strip is disabled for Linux PF

> hosts

> 

> On Linux PF hosts, the VF has no means of changing the HW CRC strip setting for

> a RX queue. It's implicitly enabled.

> 

> This patch checks if the host is running a Linux PF kernel driver, and returns an

> error, if HW CRC stripping was disabled.

> 

> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>

> ---

>  drivers/net/i40e/i40e_ethdev_vf.c | 15 +++++++++++++++

>  1 file changed, 15 insertions(+)

> 

> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c

> b/drivers/net/i40e/i40e_ethdev_vf.c

> index 2bce69b..0057ed6 100644

> --- a/drivers/net/i40e/i40e_ethdev_vf.c

> +++ b/drivers/net/i40e/i40e_ethdev_vf.c

> @@ -1567,6 +1567,8 @@ i40evf_dev_configure(struct rte_eth_dev *dev)  {

>  	struct i40e_adapter *ad =

>  		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);

> +	struct rte_eth_conf *conf = &dev->data->dev_conf;

> +	struct i40e_vf *vf;

> 

>  	/* Initialize to TRUE. If any of Rx queues doesn't meet the bulk

>  	 * allocation or vector Rx preconditions we will reset it.

> @@ -1576,6 +1578,19 @@ i40evf_dev_configure(struct rte_eth_dev *dev)

>  	ad->tx_simple_allowed = true;

>  	ad->tx_vec_allowed = true;

> 

> +	/* For Linux PF hosts, VF has no ability to disable HW CRC strip,

> +	 * and is implicitly enabled by the PF.

> +	 */

> +	if (!conf->rxmode.hw_strip_crc) {

> +		vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);

> +		if ((vf->version_major == I40E_VIRTCHNL_VERSION_MAJOR) &&

> +		    (vf->version_minor <= I40E_VIRTCHNL_VERSION_MINOR)) {

> +			/* Peer is Linux PF host. */

Can you reword above comments?
It just means the host is not DPDK PF host driver, it could be Linux driver,
and possible others (e.g. FreeBSD, VMWARE?).

Thanks,
Helin

> +			PMD_INIT_LOG(ERR, "VF can't disable HW CRC Strip");

> +			return -EINVAL;

> +		}

> +	}

> +

>  	return i40evf_init_vlan(dev);

>  }

> 

> --

> 2.7.4
  

>> +     /* For Linux PF hosts, VF has no ability to disable HW CRC strip,
>> +      * and is implicitly enabled by the PF.
>> +      */
>> +     if (!conf->rxmode.hw_strip_crc) {
>> +             vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>> +             if ((vf->version_major == I40E_VIRTCHNL_VERSION_MAJOR) &&
>> +                 (vf->version_minor <= I40E_VIRTCHNL_VERSION_MINOR)) {
>> +                     /* Peer is Linux PF host. */
> Can you reword above comments?
> It just means the host is not DPDK PF host driver, it could be Linux driver,
> and possible others (e.g. FreeBSD, VMWARE?).

Sure, I'll reword it! The broader question, however, is this correct for non-Linux/non-DPDK PF drivers? 
For FreeBSD I'll dig into the code, but for VMWARE (and I'd assume Microsoft Windows) it'll be harder.

Do you have any insights on the behavior for the non-open i40e PF drivers?

From the documentation [1], it's unclear whether non-Linux/non-DPDK PF drivers are supported. My
interpretation was that only DPDK and Linux PF hosts are supported for Fortville NICs.


Björn


[1] http://dpdk.org/doc/guides/nics/intel_vf.html
  

> -----Original Message-----
> From: Topel, Bjorn
> Sent: Friday, April 22, 2016 12:55 PM
> To: Zhang, Helin; dev@dpdk.org
> Cc: david.marchand@6wind.com; Wu, Jingjing
> Subject: RE: [PATCH v2] i40evf: Report error if HW CRC strip is disabled for
> Linux PF hosts
> 
> >> +     /* For Linux PF hosts, VF has no ability to disable HW CRC strip,
> >> +      * and is implicitly enabled by the PF.
> >> +      */
> >> +     if (!conf->rxmode.hw_strip_crc) {
> >> +             vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> >> +             if ((vf->version_major == I40E_VIRTCHNL_VERSION_MAJOR) &&
> >> +                 (vf->version_minor <= I40E_VIRTCHNL_VERSION_MINOR)) {
> >> +                     /* Peer is Linux PF host. */
> > Can you reword above comments?
> > It just means the host is not DPDK PF host driver, it could be Linux
> > driver, and possible others (e.g. FreeBSD, VMWARE?).
> 
> Sure, I'll reword it! The broader question, however, is this correct for non-
> Linux/non-DPDK PF drivers?
> For FreeBSD I'll dig into the code, but for VMWARE (and I'd assume Microsoft
> Windows) it'll be harder.
> 
> Do you have any insights on the behavior for the non-open i40e PF drivers?
> 
> From the documentation [1], it's unclear whether non-Linux/non-DPDK PF
> drivers are supported. My interpretation was that only DPDK and Linux PF
> hosts are supported for Fortville NICs.
I guess only DPDK is different, though I am not sure.
As all other NIC drivers were developped by the same organization.
Even assuming that FreeBSD supports both configuration, it will not be a problem,
as DPDK just doesn't support, and nothing wrong.

Thanks,
Helin

> 
> 
> Björn
> 
> 
> [1] http://dpdk.org/doc/guides/nics/intel_vf.html
  

>> >> +     /* For Linux PF hosts, VF has no ability to disable HW CRC strip,
>> >> +      * and is implicitly enabled by the PF.
>> >> +      */
>> >> +     if (!conf->rxmode.hw_strip_crc) {
>> >> +             vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>> >> +             if ((vf->version_major == I40E_VIRTCHNL_VERSION_MAJOR) &&
>> >> +                 (vf->version_minor <= I40E_VIRTCHNL_VERSION_MINOR)) {
>> >> +                     /* Peer is Linux PF host. */
>> > Can you reword above comments?
>> > It just means the host is not DPDK PF host driver, it could be Linux
>> > driver, and possible others (e.g. FreeBSD, VMWARE?).
>>
>> Sure, I'll reword it! The broader question, however, is this correct for non-
>> Linux/non-DPDK PF drivers?
>> For FreeBSD I'll dig into the code, but for VMWARE (and I'd assume Microsoft
>> Windows) it'll be harder.
>>
>> Do you have any insights on the behavior for the non-open i40e PF drivers?
>>
>> From the documentation [1], it's unclear whether non-Linux/non-DPDK PF
>> drivers are supported. My interpretation was that only DPDK and Linux PF
>> hosts are supported for Fortville NICs.
> I guess only DPDK is different, though I am not sure.
> As all other NIC drivers were developped by the same organization.
> Even assuming that FreeBSD supports both configuration, it will not be a problem,
> as DPDK just doesn't support, and nothing wrong.

I verified against the FreeBSD ixl-1.4.27 driver, and it
behaves (in terms of rxq crcstrip) the same way.

It would be a problem if the non-Linux/non-DPDK drivers had
it (rx crcstrip) *disabled* by default. (Further, being able to
actually change the setting from a VF would be nice as well. :-))
This doesn't seem to be case, though.

So, I'll change the wording from "Linux PF hosts" to "non-DPDK PF
host". Would that be OK?


Björn
  

> -----Original Message-----
> From: Topel, Bjorn
> Sent: Friday, April 22, 2016 1:17 PM
> To: Zhang, Helin <helin.zhang@intel.com>; dev@dpdk.org
> Cc: david.marchand@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: RE: [PATCH v2] i40evf: Report error if HW CRC strip is disabled for Linux
> PF hosts
> 
> >> >> +     /* For Linux PF hosts, VF has no ability to disable HW CRC strip,
> >> >> +      * and is implicitly enabled by the PF.
> >> >> +      */
> >> >> +     if (!conf->rxmode.hw_strip_crc) {
> >> >> +             vf =
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> >> >> +             if ((vf->version_major ==
> I40E_VIRTCHNL_VERSION_MAJOR) &&
> >> >> +                 (vf->version_minor <=
> I40E_VIRTCHNL_VERSION_MINOR)) {
> >> >> +                     /* Peer is Linux PF host. */
> >> > Can you reword above comments?
> >> > It just means the host is not DPDK PF host driver, it could be
> >> > Linux driver, and possible others (e.g. FreeBSD, VMWARE?).
> >>
> >> Sure, I'll reword it! The broader question, however, is this correct
> >> for non- Linux/non-DPDK PF drivers?
> >> For FreeBSD I'll dig into the code, but for VMWARE (and I'd assume
> >> Microsoft
> >> Windows) it'll be harder.
> >>
> >> Do you have any insights on the behavior for the non-open i40e PF drivers?
> >>
> >> From the documentation [1], it's unclear whether non-Linux/non-DPDK
> >> PF drivers are supported. My interpretation was that only DPDK and
> >> Linux PF hosts are supported for Fortville NICs.
> > I guess only DPDK is different, though I am not sure.
> > As all other NIC drivers were developped by the same organization.
> > Even assuming that FreeBSD supports both configuration, it will not be
> > a problem, as DPDK just doesn't support, and nothing wrong.
> 
> I verified against the FreeBSD ixl-1.4.27 driver, and it behaves (in terms of rxq
> crcstrip) the same way.
> 
> It would be a problem if the non-Linux/non-DPDK drivers had it (rx crcstrip)
> *disabled* by default. (Further, being able to actually change the setting from a
> VF would be nice as well. :-)) This doesn't seem to be case, though.
> 
> So, I'll change the wording from "Linux PF hosts" to "non-DPDK PF host". Would
> that be OK?
I would agree with you. :)
Thank you!

Helin

> 
> 
> Björn