[dpdk-dev] examples/l3fwd: force CRC stripping for i40evf
Checks
Commit Message
Commit 1bbcc5d21129 ("i40evf: report error for unsupported CRC
stripping config") broke l3fwd, since it was forcing that CRC was
kept. Now, if i40evf is running, CRC stripping will be enabled.
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
examples/l3fwd/main.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
Comments
2016-11-09 09:23, Björn Töpel:
> Commit 1bbcc5d21129 ("i40evf: report error for unsupported CRC
> stripping config") broke l3fwd, since it was forcing that CRC was
> kept. Now, if i40evf is running, CRC stripping will be enabled.
[...]
> + rte_eth_dev_info_get(portid, &dev_info);
> + if (dev_info.driver_name &&
> + strcmp(dev_info.driver_name, "net_i40e_vf") == 0) {
> + /* i40evf require that CRC stripping is enabled. */
> + port_conf.rxmode.hw_strip_crc = 1;
> + } else {
> + port_conf.rxmode.hw_strip_crc = 0;
> + }
Thanks for raising the issue.
It is completely defeating the generic ethdev API.
We must not have different behaviours depending of the driver.
Why it cannot be fixed in the driver?
> Thanks for raising the issue. It is completely defeating the generic
> ethdev API. We must not have different behaviours depending of the
> driver. Why it cannot be fixed in the driver?
I should probably refer to the thread, where the concern was raised:
http://dpdk.org/ml/archives/dev/2016-July/044555.html
So, the issue is that i40evf *only support* CRC stripping for some
setups (i40e Linux driver for PF, i40evf DPDK driver VF).
The l3fwd application disables CRC stripping for all ports, which leads
to i40evf_dev_configure() failure -- which from my POV is correct.
Mis-configuring a port shouldn't be allowed.
I'm open to suggestions here. What would be a better way to solve this?
Maybe just adding a command-line option to the l3fwd application is a
better way around?
Björn
Hi,
>
> Commit 1bbcc5d21129 ("i40evf: report error for unsupported CRC
> stripping config") broke l3fwd, since it was forcing that CRC was
> kept. Now, if i40evf is running, CRC stripping will be enabled.
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> ---
> examples/l3fwd/main.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 7223e773107e..b60278794135 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -906,6 +906,14 @@ main(int argc, char **argv)
> n_tx_queue = MAX_TX_QUEUE_PER_PORT;
> printf("Creating queues: nb_rxq=%d nb_txq=%u... ",
> nb_rx_queue, (unsigned)n_tx_queue );
> + rte_eth_dev_info_get(portid, &dev_info);
> + if (dev_info.driver_name &&
> + strcmp(dev_info.driver_name, "net_i40e_vf") == 0) {
> + /* i40evf require that CRC stripping is enabled. */
> + port_conf.rxmode.hw_strip_crc = 1;
> + } else {
> + port_conf.rxmode.hw_strip_crc = 0;
> + }
That's a common problem across Intel VF devices.
Bothe igb and ixgbe overcomes that problem just by forcing ' rxmode.hw_strip_crc = 1;'
at PMD itself:
static int
ixgbevf_dev_configure(struct rte_eth_dev *dev)
{
...
/*
* VF has no ability to enable/disable HW CRC
* Keep the persistent behavior the same as Host PF
*/
#ifndef RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC
if (!conf->rxmode.hw_strip_crc) {
PMD_INIT_LOG(NOTICE, "VF can't disable HW CRC Strip");
conf->rxmode.hw_strip_crc = 1;
}
#else
if (conf->rxmode.hw_strip_crc) {
PMD_INIT_LOG(NOTICE, "VF can't enable HW CRC Strip");
conf->rxmode.hw_strip_crc = 0;
}
#endif
Wonder, can't i40e VF do the same?
BTW, all other examples would experience same problem too, right?
Konstantin
> ret = rte_eth_dev_configure(portid, nb_rx_queue,
> (uint16_t)n_tx_queue, &port_conf);
> if (ret < 0)
> @@ -946,7 +954,6 @@ main(int argc, char **argv)
> printf("txq=%u,%d,%d ", lcore_id, queueid, socketid);
> fflush(stdout);
>
> - rte_eth_dev_info_get(portid, &dev_info);
> txconf = &dev_info.default_txconf;
> if (port_conf.rxmode.jumbo_frame)
> txconf->txq_flags = 0;
> --
> 2.9.3
Adding Helin to the conversation.
> That's a common problem across Intel VF devices. Bothe igb and ixgbe
> overcomes that problem just by forcing ' rxmode.hw_strip_crc = 1;' at
> PMD itself:
[...]
> Wonder, can't i40e VF do the same?
Right, however, and this (silent failure) approach was rejected by the
maintainers. Please, refer to this thread:
http://dpdk.org/ml/archives/dev/2016-April/037523.html
> BTW, all other examples would experience same problem too, right?
Correct, so the broader question would be "what is the correct behavior
for an example application, when a port configuration isn't supported by
the hardware?".
My stand, FWIW, is that igb and ixgbe should have the same semantics as
i40e currently has, i.e. return an error to the user if the port is
mis-configured, NOT changing the setting behind the users back.
Björn
2016-11-09 11:05, Björn Töpel:
> > BTW, all other examples would experience same problem too, right?
>
> Correct, so the broader question would be "what is the correct behavior
> for an example application, when a port configuration isn't supported by
> the hardware?".
>
> My stand, FWIW, is that igb and ixgbe should have the same semantics as
> i40e currently has, i.e. return an error to the user if the port is
> mis-configured, NOT changing the setting behind the users back.
Yes it sounds sane.
>
> Adding Helin to the conversation.
>
> > That's a common problem across Intel VF devices. Bothe igb and ixgbe
> > overcomes that problem just by forcing ' rxmode.hw_strip_crc = 1;' at
> > PMD itself:
>
> [...]
>
> > Wonder, can't i40e VF do the same?
>
> Right, however, and this (silent failure) approach was rejected by the
> maintainers. Please, refer to this thread:
> http://dpdk.org/ml/archives/dev/2016-April/037523.html
>
> > BTW, all other examples would experience same problem too, right?
>
> Correct, so the broader question would be "what is the correct behavior
> for an example application, when a port configuration isn't supported by
> the hardware?".
>
> My stand, FWIW, is that igb and ixgbe should have the same semantics as
> i40e currently has, i.e. return an error to the user if the port is
> mis-configured, NOT changing the setting behind the users back.
>
Fine by me, but then it means that the fix haw to include changes for all apps
plus ixgbe and igb PMDs, correct? :)
Konstantin
>> Correct, so the broader question would be "what is the correct
>> behavior for an example application, when a port configuration
>> isn't supported by the hardware?".
>>
>> My stand, FWIW, is that igb and ixgbe should have the same
>> semantics as i40e currently has, i.e. return an error to the user
>> if the port is mis-configured, NOT changing the setting behind the
>> users back.
>>
>
> Fine by me, but then it means that the fix haw to include changes
> for all apps plus ixgbe and igb PMDs, correct? :)
Ugh. Correct, I guess. :-)
As for ixgbe and igb - they need a patch changing from silent ignore to
explicit error. Regarding the apps, I guess all the apps that rely on
that disabling CRC stripping always work, need some work. Or should all
the example applications have CRC stripping *enabled* by default? I'd
assume that all DPDK supported NICs has support for CRC stripping and I
guess this is the rational for having it on by default for Intel VFs.
In general, for the example applications, if an application relies on a
property for a port, that the hardware doesn't support -- what would be
the desired behavior? Or is it implied that the example applications
only use a common, minimal set of features that are know to be supported
by all DPDK supported hardware?
Isn't it perfectly valid that some example applications wont run for all
hardware?
Finally, why doesn't l3fwd have the CRC stripped?
Björn
> -----Original Message-----
> From: Topel, Bjorn
> Sent: Wednesday, November 9, 2016 11:28 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> Cc: Xu, Qian Q <qian.q.xu@intel.com>; Yao, Lei A <lei.a.yao@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> thomas.monjalon@6wind.com
> Subject: Re: [dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf
>
> >> Correct, so the broader question would be "what is the correct
> >> behavior for an example application, when a port configuration
> >> isn't supported by the hardware?".
> >>
> >> My stand, FWIW, is that igb and ixgbe should have the same
> >> semantics as i40e currently has, i.e. return an error to the user
> >> if the port is mis-configured, NOT changing the setting behind the
> >> users back.
> >>
> >
> > Fine by me, but then it means that the fix haw to include changes
> > for all apps plus ixgbe and igb PMDs, correct? :)
>
> Ugh. Correct, I guess. :-)
>
> As for ixgbe and igb - they need a patch changing from silent ignore to
> explicit error. Regarding the apps, I guess all the apps that rely on
> that disabling CRC stripping always work, need some work. Or should all
> the example applications have CRC stripping *enabled* by default? I'd
> assume that all DPDK supported NICs has support for CRC stripping
From the sources, it seems that only nfp doesn't support HW CRC stripping.
In fact, as I can see some non-Intel PMDs just ignore hw_stip_crc value
and assume that CRC strip is always on.
> and I
> guess this is the rational for having it on by default for Intel VFs.
>
> In general, for the example applications, if an application relies on a
> property for a port, that the hardware doesn't support -- what would be
> the desired behavior? Or is it implied that the example applications
> only use a common, minimal set of features that are know to be supported
> by all DPDK supported hardware?
>
> Isn't it perfectly valid that some example applications wont run for all
> hardware?
>
> Finally, why doesn't l3fwd have the CRC stripped?
I don’t know any good reason for that for l3fwd or any other sample app.
I think it is just a 'historical' reason.
Konstantin
>
>
> Björn
> -----Original Message-----
> From: Topel, Bjorn
> Sent: Wednesday, November 9, 2016 7:28 PM
> To: Ananyev, Konstantin; dev@dpdk.org; Zhang, Helin
> Cc: Xu, Qian Q; Yao, Lei A; Wu, Jingjing; thomas.monjalon@6wind.com
> Subject: Re: [dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for
> i40evf
>
> >> Correct, so the broader question would be "what is the correct
> >> behavior for an example application, when a port configuration isn't
> >> supported by the hardware?".
> >>
> >> My stand, FWIW, is that igb and ixgbe should have the same semantics
> >> as i40e currently has, i.e. return an error to the user if the port
> >> is mis-configured, NOT changing the setting behind the users back.
> >>
> >
> > Fine by me, but then it means that the fix haw to include changes for
> > all apps plus ixgbe and igb PMDs, correct? :)
>
> Ugh. Correct, I guess. :-)
>
> As for ixgbe and igb - they need a patch changing from silent ignore to
> explicit error. Regarding the apps, I guess all the apps that rely on that
> disabling CRC stripping always work, need some work. Or should all the
> example applications have CRC stripping *enabled* by default? I'd assume
> that all DPDK supported NICs has support for CRC stripping and I guess this is
> the rational for having it on by default for Intel VFs.
>
> In general, for the example applications, if an application relies on a property
> for a port, that the hardware doesn't support -- what would be the desired
> behavior? Or is it implied that the example applications only use a common,
> minimal set of features that are know to be supported by all DPDK supported
> hardware?
>
> Isn't it perfectly valid that some example applications wont run for all
> hardware?
>
> Finally, why doesn't l3fwd have the CRC stripped?
>
>
> Björn
Yes, i40e driver changed a little bit on that according to the review comments
during implementation, comparing to igb and ixgbe.
I'd suggest to re-invesitgate if we can do the similar thing in igb and ixgbe driver.
Any critical issue now? Or just an improvement comments?
Thanks,
Helin
Björn/Konstantin wrote:
>> Finally, why doesn't l3fwd have the CRC stripped?
>
> I don’t know any good reason for that for l3fwd or any other sample
> app. I think it is just a 'historical' reason.
Ok! Then I'd suggest changing the l3fwd default to actually *strip* CRC
instead of not doing it. Lei, any comments?
Helin wrote:
> Yes, i40e driver changed a little bit on that according to the
> review comments during implementation, comparing to igb and ixgbe.
> I'd suggest to re-invesitgate if we can do the similar thing in igb
> and ixgbe driver.
Good. Let's do that!
> Any critical issue now? Or just an improvement comments?
Not from my perspective. The issue is that Lei needs some kind of
work-around for l3fwd with i40evf, so I'll let Lei comment on how
critical it is.
Björn
I'm testing some DPDK sample under VMware. During the testing work, I find l3fwd+ ixgbe vf can work ,but L3fwd + i40evf can't work. So I reported this issue to Bjorn. From my perspective, if can add new parameter in l3fwd sample like what have already don’t in testpmd "----crc-strip enable" is a better way to resolve this issue.
Lei
-----Original Message-----
From: Topel, Bjorn
Sent: Wednesday, November 9, 2016 9:10 PM
To: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
Cc: Xu, Qian Q <qian.q.xu@intel.com>; Yao, Lei A <lei.a.yao@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; thomas.monjalon@6wind.com
Subject: Re: [dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf
Björn/Konstantin wrote:
>> Finally, why doesn't l3fwd have the CRC stripped?
>
> I don’t know any good reason for that for l3fwd or any other sample
> app. I think it is just a 'historical' reason.
Ok! Then I'd suggest changing the l3fwd default to actually *strip* CRC instead of not doing it. Lei, any comments?
Helin wrote:
> Yes, i40e driver changed a little bit on that according to the review
> comments during implementation, comparing to igb and ixgbe.
> I'd suggest to re-invesitgate if we can do the similar thing in igb
> and ixgbe driver.
Good. Let's do that!
> Any critical issue now? Or just an improvement comments?
Not from my perspective. The issue is that Lei needs some kind of work-around for l3fwd with i40evf, so I'll let Lei comment on how critical it is.
Björn
Lei wrote:
> I'm testing some DPDK sample under VMware. During the testing work, I
> find l3fwd+ ixgbe vf can work ,but L3fwd + i40evf can't work. So I
> reported this issue to Bjorn. From my perspective, if can add new
> parameter in l3fwd sample like what have already don’t in testpmd
> "----crc-strip enable" is a better way to resolve this issue.
(Please don't top-post.)
As discussed in the thread, it might be better to just change the
default in l3fwd from .hw_strip_crc = 0 to 1.
I'll be looking into changing igbvf and ixgbevf to match the semantics
of i40evf.
Björn
2016-11-10 07:17, Björn Töpel:
> As discussed in the thread, it might be better to just change the
> default in l3fwd from .hw_strip_crc = 0 to 1.
>
> I'll be looking into changing igbvf and ixgbevf to match the semantics
> of i40evf.
Just to make it sure, you mean returning an error in the driver when
a configuration cannot be applied, right?
Thomas wrote:
> Just to make it sure, you mean returning an error in the driver when
> a configuration cannot be applied, right?
Yes, as in 1bbcc5d21129 ("i40evf: report error for unsupported CRC
stripping config"), where -EINVAL is returned.
Björn
@@ -906,6 +906,14 @@ main(int argc, char **argv)
n_tx_queue = MAX_TX_QUEUE_PER_PORT;
printf("Creating queues: nb_rxq=%d nb_txq=%u... ",
nb_rx_queue, (unsigned)n_tx_queue );
+ rte_eth_dev_info_get(portid, &dev_info);
+ if (dev_info.driver_name &&
+ strcmp(dev_info.driver_name, "net_i40e_vf") == 0) {
+ /* i40evf require that CRC stripping is enabled. */
+ port_conf.rxmode.hw_strip_crc = 1;
+ } else {
+ port_conf.rxmode.hw_strip_crc = 0;
+ }
ret = rte_eth_dev_configure(portid, nb_rx_queue,
(uint16_t)n_tx_queue, &port_conf);
if (ret < 0)
@@ -946,7 +954,6 @@ main(int argc, char **argv)
printf("txq=%u,%d,%d ", lcore_id, queueid, socketid);
fflush(stdout);
- rte_eth_dev_info_get(portid, &dev_info);
txconf = &dev_info.default_txconf;
if (port_conf.rxmode.jumbo_frame)
txconf->txq_flags = 0;