[dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF hosts

[dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF hosts

[Björn Töpel]

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 ignores, and warns, 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..f88eb79 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. */
+			PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip.");
+			conf->rxmode.hw_strip_crc = 1;
+		}
+	}
+
 	return i40evf_init_vlan(dev);
 }
 
-- 
2.7.4

----------------------------------------------------------------------
Intel Sweden AB
Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
Registration Number: 556189-6027

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF hosts

[David Marchand]

CC maintainers.

On Thu, Apr 14, 2016 at 4:58 PM, Björn Töpel <bjorn.topel@intel.com> wrote:
> 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 ignores, and warns, 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..f88eb79 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. */
> +                       PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip.");
> +                       conf->rxmode.hw_strip_crc = 1;
> +               }
> +       }
> +
>         return i40evf_init_vlan(dev);
>  }

Not sure this is the right way to handle it.
The driver should return an error rather than silently discard what
the application asked.

>
> --
> 2.7.4
>
> ----------------------------------------------------------------------
> Intel Sweden AB
> Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
> Registration Number: 556189-6027
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.

Please, remove this.


-- 
David Marchand

Re: [dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF hosts

[Zhang, Helin]

> -----Original Message-----
> From: David Marchand [mailto:david.marchand@6wind.com]
> Sent: Friday, April 15, 2016 3:41 PM
> To: Topel, Bjorn <bjorn.topel@intel.com>
> Cc: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF
> hosts
> 
> CC maintainers.
> 
> On Thu, Apr 14, 2016 at 4:58 PM, Björn Töpel <bjorn.topel@intel.com> wrote:
> > 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 ignores, and warns, 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..f88eb79 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. */
> > +                       PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC
> Strip.");
> > +                       conf->rxmode.hw_strip_crc = 1;
> > +               }
> > +       }
> > +
> >         return i40evf_init_vlan(dev);
> >  }
> 
> Not sure this is the right way to handle it.
> The driver should return an error rather than silently discard what the application
> asked.
I also think it should return an error with checking if the host is kernel driver, and crc strip is disabled in VF.
Thank you David!

Regards,
Helin

> 
> >
> > --
> > 2.7.4
> >
> > ----------------------------------------------------------------------
> > Intel Sweden AB
> > Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
> > Registration Number: 556189-6027
> >
> > This e-mail and any attachments may contain confidential material for
> > the sole use of the intended recipient(s). Any review or distribution
> > by others is strictly prohibited. If you are not the intended
> > recipient, please contact the sender and delete all copies.
> 
> Please, remove this.
> 
> 
> --
> David Marchand

Re: [dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF hosts

[Topel, Bjorn]

>> Not sure this is the right way to handle it.  The driver should
>> return an error rather than silently discard what the
>> application asked.
>
> I also think it should return an error with checking if the
> host is kernel driver, and crc strip is disabled in VF.  Thank
> you David!

Thanks for reviewing my patch, Helin and David.

I agree that it's subtle to ignore the error, and just log the
error. This is how ixgbevf behaves (refer to
ixgbevf_dev_configure), so I figured that i40evf should behave
analogous.

I'll submit a v2 of the patch that returns an EINVAL and logs the
failure.

Would it make sense to change the ixgbevf_dev_configure as well,
in a separate patch?


>> ----------------------------------------------------------------------
>> Intel Sweden AB
>> Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
>> Registration Number: 556189-6027
>>
>> This e-mail and any attachments may contain confidential material for
>> the sole use of the intended recipient(s). Any review or distribution
>> by others is strictly prohibited. If you are not the intended
>> recipient, please contact the sender and delete all copies.
>
> Please, remove this.

Noted. Will make sure to fix that for future revisions. Thanks!


Björn
----------------------------------------------------------------------
Intel Sweden AB
Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
Registration Number: 556189-6027

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF hosts

[Zhang, Helin]

> -----Original Message-----
> From: Topel, Bjorn
> Sent: Tuesday, April 19, 2016 2:47 AM
> To: Zhang, Helin <helin.zhang@intel.com>; David Marchand
> <david.marchand@6wind.com>
> Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF
> hosts
> 
> >> Not sure this is the right way to handle it.  The driver should
> >> return an error rather than silently discard what the application
> >> asked.
> >
> > I also think it should return an error with checking if the host is
> > kernel driver, and crc strip is disabled in VF.  Thank you David!
> 
> Thanks for reviewing my patch, Helin and David.
> 
> I agree that it's subtle to ignore the error, and just log the error. This is how
> ixgbevf behaves (refer to ixgbevf_dev_configure), so I figured that i40evf should
> behave analogous.
> 
> I'll submit a v2 of the patch that returns an EINVAL and logs the failure.
> 
> Would it make sense to change the ixgbevf_dev_configure as well, in a separate
> patch?
Yes, I agree with you that ixgbe and i40e should be consistent. Thank you!

/Helin

> 
> 
> >> ---------------------------------------------------------------------
> >> -
> >> Intel Sweden AB
> >> Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm,
> >> Sweden Registration Number: 556189-6027
> >>
> >> This e-mail and any attachments may contain confidential material for
> >> the sole use of the intended recipient(s). Any review or distribution
> >> by others is strictly prohibited. If you are not the intended
> >> recipient, please contact the sender and delete all copies.
> >
> > Please, remove this.
> 
> Noted. Will make sure to fix that for future revisions. Thanks!
> 
> 
> Björn

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

[Björn Töpel]

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. */
+			PMD_INIT_LOG(ERR, "VF can't disable HW CRC Strip");
+			return -EINVAL;
+		}
+	}
+
 	return i40evf_init_vlan(dev);
 }
 
-- 
2.7.4

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

[Zhang, Helin]

> -----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

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

[Topel, Bjorn]

>> +     /* 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

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

[Zhang, Helin]

> -----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

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

[Topel, Bjorn]

>> >> +     /* 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

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

[Zhang, Helin]

> -----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

[dpdk-dev] [PATCH v3] i40evf: Report error if HW CRC strip is disabled for non-DPDK PF hosts

[Björn Töpel]

On hosts running a non-DPDK PF driver, 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 non-DPDK 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..25f1cea 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 non-DPDK PF drivers, 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 running non-DPDK PF driver. */
+			PMD_INIT_LOG(ERR, "VF can't disable HW CRC Strip");
+			return -EINVAL;
+		}
+	}
+
 	return i40evf_init_vlan(dev);
 }
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3] i40evf: Report error if HW CRC strip is disabled for non-DPDK PF hosts

[Zhang, Helin]

> -----Original Message-----
> From: Topel, Bjorn
> Sent: Friday, April 22, 2016 1:39 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin; Wu, Jingjing; Topel, Bjorn
> Subject: [PATCH v3] i40evf: Report error if HW CRC strip is disabled for non-
> DPDK PF hosts
> 
> On hosts running a non-DPDK PF driver, 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 non-DPDK PF kernel driver, and
> returns an error, if HW CRC stripping was disabled.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

Re: [dpdk-dev] [PATCH v3] i40evf: Report error if HW CRC strip is disabled for non-DPDK PF hosts

[Bruce Richardson]

On Fri, Apr 29, 2016 at 01:42:33PM +0000, Zhang, Helin wrote:
> 
> 
> > -----Original Message-----
> > From: Topel, Bjorn
> > Sent: Friday, April 22, 2016 1:39 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Helin; Wu, Jingjing; Topel, Bjorn
> > Subject: [PATCH v3] i40evf: Report error if HW CRC strip is disabled for non-
> > DPDK PF hosts
> > 
> > On hosts running a non-DPDK PF driver, 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 non-DPDK PF kernel driver, and
> > returns an error, if HW CRC stripping was disabled.
> > 
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> Acked-by: Helin Zhang <helin.zhang@intel.com>

Applied to dpdk-next-net/rel_16_07

/Bruce