Re: [ovs-dev] [PATCH] netdev-dpdk: fix check for "net_nfp" driver
On 17/05/18 17:45, Timothy Redaelli wrote:
> Currently the check of "net_nfp" driver while enabling scatter compares
> only the first 6 bytes, but "net_nfp" is 7 bytes long.
>
> This change fixes the check by comparing the first 7 bytes.
>
> CC: Pablo Cascón <pablo.cas...@netronome.com>
> CC: Simon Horman <simon.hor...@netronome.com>
> Fixes: 65a87968f4cf ("netdev-dpdk: don't enable scatter for jumbo RX support
> for nfp")
> Signed-off-by: Timothy Redaelli <tredae...@redhat.com>
> ---
> lib/netdev-dpdk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 87152a7b9..cee8961c0 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -785,7 +785,7 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int
> n_rxq, int n_txq)
> * than highlighting the one known not to need scatter */
> if (dev->mtu > ETHER_MTU) {
> rte_eth_dev_info_get(dev->port_id, );
> -if (strncmp(info.driver_name, "net_nfp", 6)) {
> +if (strncmp(info.driver_name, "net_nfp", 7)) {
> conf.rxmode.enable_scatter = 1;
> }
> }
Good catch, sorry for the embarrassing bug :( Luckily as of now there is no
other PMD that starts with "net_nf". Still a bug, thanks for the fix!
Acked-by: Pablo Cascón <pablo.cas...@netronome.com>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 1/1] netdev-dpdk: don't enable scatter for jumbo RX support for nfp
On 01/05/18 11:34, Stokes, Ian wrote:
On 04/27/2018 05:40 PM, Pablo Cascón wrote:
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo RX support. This change fixes
the issue by not enabling scatter only for the PMD/NIC known not to
need it to support jumbo RX.
Acked-by: Kevin Traynor <ktray...@redhat.com>
Thanks all, I'll apply this to DPDK_MERGE and backport to the previous
releases, it will be part of this week's pull request.
Thanks
Ian
Thanks!
Note: this change is temporary and not needed for later releases
OVS/DPDK
Reported-by: Louis Peens <louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón <pablo.cas...@netronome.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
ee39cbe..fdc8f66 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
int n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
-/* For some NICs (e.g. Niantic), scatter_rx mode needs to be
explicitly
- * enabled. */
+/* As of DPDK 17.11.1 a few PMDs require to explicitly enable
+ * scatter to support jumbo RX. Checking the offload capabilities
+ * is not an option as PMDs are not required yet to report
+ * them. The only reliable info is the driver name and knowledge
+ * (testing or code review). Listing all such PMDs feels harder
+ * than highlighting the one known not to need scatter */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (strncmp(info.driver_name, "net_nfp", 6)) {
+conf.rxmode.enable_scatter = 1;
+}
}
conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 1/1] netdev-dpdk: don't enable scatter for jumbo RX support for nfp
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo RX support. This change fixes
the issue by not enabling scatter only for the PMD/NIC known not to
need it to support jumbo RX.
Note: this change is temporary and not needed for later releases OVS/DPDK
Reported-by: Louis Peens <louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón <pablo.cas...@netronome.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ee39cbe..fdc8f66 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
-/* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
- * enabled. */
+/* As of DPDK 17.11.1 a few PMDs require to explicitly enable
+ * scatter to support jumbo RX. Checking the offload capabilities
+ * is not an option as PMDs are not required yet to report
+ * them. The only reliable info is the driver name and knowledge
+ * (testing or code review). Listing all such PMDs feels harder
+ * than highlighting the one known not to need scatter */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (strncmp(info.driver_name, "net_nfp", 6)) {
+conf.rxmode.enable_scatter = 1;
+}
}
conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 1/1] netdev-dpdk: remove enabling scatter for jumbo RX support
On 26/04/18 19:41, Stokes, Ian wrote:
On 04/26/2018 04:26 PM, Stokes, Ian wrote:
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo RX support. This change
fixes the issue by only enabling scatter for NICs known to need it to
support jumbo RX. Add a quirk for "igb" while the PMD is fixed.
Thanks for the v3 Pablo, comments inline.
Reported-by: Louis Peens <louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón <pablo.cas...@netronome.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
---
Changelog:
v3->v2:
- only check for driver_name
- tested with "nfp" and not with "igb"
lib/netdev-dpdk.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
ee39cbe..02ed85b
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,17 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
*dev, int n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
-/* For some NICs (e.g. Niantic), scatter_rx mode needs to be
explicitly
- * enabled. */
+/* Quirk: as of DPDK 17.11.1 igb's PMD requires explicitly
+ * enabling scatter to support jumbo RX. Note: PMDs are not
+ * required to set the offload capabilities and so is not reliable
+ * info, only the driver_name is after testing the PMD/NIC */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (!strcmp(info.driver_name, "igb")) {
For igb devices condition above will never hit, the driver name in info
will be reported as 'net_e1000_igb'.
I'm seeing a bit of a list growing for other drivers that support the
current behavior/requirement for scatter with Jumbo frames.
Through testing today I saw that Scatter must be set for ixgbe SRIOV
functions also, "net_ixgbe_vf".
From code inspection em devices "net_e1000_em" would have to be included
also.
I would think that 'net_e1000_igb_vf' will be required also when set_mtu
support is introduced in the future (quite likely), but for the moment
that’s not relevant.
I'm wondering if there's an alternative solution to checking driver
names as we could end up with an extensive list. Would have to look at
this.
Did I hear someone say "alternative solution"? ;-)
---8<---
That's why I think it's better to make the exception for "nfp" at present.
It works for nfp, it works for Intel and it works for any other NICs that
currently work.
When OVS is updated to a future DPDK release where all the PMDs support
setting/not setting this flag, then I agree any workaround should be made
for NICs that are not properly reporting their status, or have quirks.
---8<---
https://mail.openvswitch.org/pipermail/ovs-dev/2018-April/346256.html
+1
I do think this is a simpler solution until we have the offload API fully
supported in DPDK for the relevant devices.
OK fine, reluctantly agree. Let me share a tested v4 that makes the
exception for "nfp".
@Pablo: Do you want the solution backported to previous OVS versions also? The
"nfp" approach would be ok I think but as flagged in previous mail, listing
igb, ixgbe etc would be a different story as the behavior of these changed between DPDK
releases.
Yes, backport would be good please. The behavior of "nfp" has not
changed (scatter is not needed for jumbo across releases)
Just something we should be aware of.
Ian
Kevin.
Ian
+conf.rxmode.enable_scatter = 1;
+}
}
conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 1/1] netdev-dpdk: remove enabling scatter for jumbo RX support
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo RX support. This change fixes
the issue by only enabling scatter for NICs known to need it to
support jumbo RX. Add a quirk for "igb" while the PMD is fixed.
Reported-by: Louis Peens <louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón <pablo.cas...@netronome.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
---
Changelog:
v3->v2:
- only check for driver_name
- tested with "nfp" and not with "igb"
lib/netdev-dpdk.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ee39cbe..02ed85b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,17 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
-/* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
- * enabled. */
+/* Quirk: as of DPDK 17.11.1 igb's PMD requires explicitly
+ * enabling scatter to support jumbo RX. Note: PMDs are not
+ * required to set the offload capabilities and so is not reliable
+ * info, only the driver_name is after testing the PMD/NIC */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (!strcmp(info.driver_name, "igb")) {
+conf.rxmode.enable_scatter = 1;
+}
}
conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 1/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter
On 19/04/18 19:25, Kevin Traynor wrote:
On 04/19/2018 03:32 PM, Pablo Cascón wrote:
On 18/04/18 18:35, Kevin Traynor wrote:
On 04/18/2018 03:41 PM, Pablo Cascón wrote:
On 13/04/18 19:45, Kevin Traynor wrote:
On 04/13/2018 04:20 PM, Stokes, Ian wrote:
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo support on RX. This change
fixes
the issue by only enabling scatter for NICs supporting it. Add a
quirk for
"igb" while the PMD is fixed to advertise scatter.
Thanks for the v2 Pablo.
Adding Eelco and Kevin as they had some comments on the v1.
FYI I'm investigating on the DPDK side to see how/when the flag
should be set and used for igb and ixgbe as well as other drivers.
https://dpdk.org/ml/archives/dev/2018-April/097056.html
Reported-by: Louis Peens<louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón<pablo.cas...@netronome.com>
Reviewed-by: Simon Horman<simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
ee39cbe..8f6a0a3
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
*dev,
int n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
/* For some NICs (e.g. Niantic), scatter_rx mode needs to be
explicitly
* enabled. */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER) {
+conf.rxmode.enable_scatter = 1;
+} else if (!strcmp(info.driver_name, "igb")) {
+/* Quirk: as of DPDK 17.11.1 igb's PMD requires
explicitly
+ enabling scatter but fails to advertise it. */
Can you check name for "nfp" and don't set enable_scatter? I don't
think
most of the PMDs used these new offload defines in DPDK17.11.
(sorry for the delay replying, was on PTO)
While it is technically possible to do that for the "nfp" it is not the
preferred solution IMHO. The "nfp" PMD is doing the right thing (TM) by
not advertising it supports scatter (it doesn't) and not requiring it
for jumbo traffic. If we're to add a quirk it should be for the
to-be-fixed PMD(s).
I will agree with you...but only in the future :-)
You are considering that the other drivers are to-be-fixed but OVS
supports DPDK 17.11 LTS and it is not required for PMDs to support the
new offloads API (of which this define is associated with). So, Ian can
correct me if I'm wrong, but I don't think that other PMDs need to or
will set this flag in any 17.11.X stable release.
I see, was assuming that PMDs were required to use the capabilities from
the time they were added. If the PMDs are not required to then for "DPDK
17.11.x stable release" the capability bit is not something to reliably
check. Then it could make sense to rely on other information such as the
driver name.
(note that the table at
https://dpdk.org/doc/guides/nics/overview.html#id1 does not match the
code at least for "igb" on master or tag 17.11, so might not be a
reliable way to know either)
That's why I think it's better to make the exception for "nfp" at
present. It works for nfp, it works for Intel and it works for any other
NICs that currently work.
There's some logic to it, something like "only add code for the device
driver we're supporting in a better way or fixing as to avoid to
potentially breaking others". The counter argument is that jumbo does
not mean scatter and this patch is removing that link.
One way to look at it is that there are 2 different parts of the issue:
1) jumbo RX does not need scatter
2) there's no trivial way, without testing, to tell which NICs a)
require scatter (and support it) even if it is not advertised and b)
support jumbo with or without scatter
IMHO we should fix 1 with this patch as current code is wrongly linking
the jumbo and scatter. And for 2 let NIC maintainers test it while the
review process (or after) and add a quirk if need be (only for PMDs that
won't RX jumbo otherwise, regardless of what they advertise). "igb" can
be covered once tested and others will come if needed.
I'm not totally against that approach. I'm just a little concerned that
the default is changing and other NIC vendors might not notice or test
for a long time, but you are right that it is technically the more
correct way to do things.
Yeah there is a small risk that there is another NIC other than igb that
also requires scatter for jumbo RX, reckon it is small: would have to be
a NIC/PMD being fine before the "jumbo equals scatter" was added
(67fe6d63 Aug 2017) and after.
All right, will post a v3
Re: [ovs-dev] [PATCH v2 1/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter
On 18/04/18 18:35, Kevin Traynor wrote:
On 04/18/2018 03:41 PM, Pablo Cascón wrote:
On 13/04/18 19:45, Kevin Traynor wrote:
On 04/13/2018 04:20 PM, Stokes, Ian wrote:
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo support on RX. This change
fixes
the issue by only enabling scatter for NICs supporting it. Add a
quirk for
"igb" while the PMD is fixed to advertise scatter.
Thanks for the v2 Pablo.
Adding Eelco and Kevin as they had some comments on the v1.
FYI I'm investigating on the DPDK side to see how/when the flag
should be set and used for igb and ixgbe as well as other drivers.
https://dpdk.org/ml/archives/dev/2018-April/097056.html
Reported-by: Louis Peens<louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón<pablo.cas...@netronome.com>
Reviewed-by: Simon Horman<simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
ee39cbe..8f6a0a3
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
int n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
/* For some NICs (e.g. Niantic), scatter_rx mode needs to be
explicitly
* enabled. */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER) {
+conf.rxmode.enable_scatter = 1;
+} else if (!strcmp(info.driver_name, "igb")) {
+/* Quirk: as of DPDK 17.11.1 igb's PMD requires explicitly
+ enabling scatter but fails to advertise it. */
Can you check name for "nfp" and don't set enable_scatter? I don't think
most of the PMDs used these new offload defines in DPDK17.11.
(sorry for the delay replying, was on PTO)
While it is technically possible to do that for the "nfp" it is not the
preferred solution IMHO. The "nfp" PMD is doing the right thing (TM) by
not advertising it supports scatter (it doesn't) and not requiring it
for jumbo traffic. If we're to add a quirk it should be for the
to-be-fixed PMD(s).
I will agree with you...but only in the future :-)
You are considering that the other drivers are to-be-fixed but OVS
supports DPDK 17.11 LTS and it is not required for PMDs to support the
new offloads API (of which this define is associated with). So, Ian can
correct me if I'm wrong, but I don't think that other PMDs need to or
will set this flag in any 17.11.X stable release.
I see, was assuming that PMDs were required to use the capabilities from
the time they were added. If the PMDs are not required to then for "DPDK
17.11.x stable release" the capability bit is not something to reliably
check. Then it could make sense to rely on other information such as the
driver name.
(note that the table at
https://dpdk.org/doc/guides/nics/overview.html#id1 does not match the
code at least for "igb" on master or tag 17.11, so might not be a
reliable way to know either)
That's why I think it's better to make the exception for "nfp" at
present. It works for nfp, it works for Intel and it works for any other
NICs that currently work.
There's some logic to it, something like "only add code for the device
driver we're supporting in a better way or fixing as to avoid to
potentially breaking others". The counter argument is that jumbo does
not mean scatter and this patch is removing that link.
One way to look at it is that there are 2 different parts of the issue:
1) jumbo RX does not need scatter
2) there's no trivial way, without testing, to tell which NICs a)
require scatter (and support it) even if it is not advertised and b)
support jumbo with or without scatter
IMHO we should fix 1 with this patch as current code is wrongly linking
the jumbo and scatter. And for 2 let NIC maintainers test it while the
review process (or after) and add a quirk if need be (only for PMDs that
won't RX jumbo otherwise, regardless of what they advertise). "igb" can
be covered once tested and others will come if needed.
When OVS is updated to a future DPDK release where all the PMDs support
setting/not setting this flag, then I agree any workaround should be
made for NICs that are not properly reporting their status, or have quirks.
Agree with that, once PMDs are required to report their caps for those
features not working only because of the mis reporting then a per NIC
workaround will be required.
On a different note, and it may be irrelevant depending on the outcome
of the discussion, but this is mixing using defines introduced as part
of the new API and bitfields from the old API. It will wo
Re: [ovs-dev] [PATCH v2 1/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter
On 13/04/18 19:45, Kevin Traynor wrote:
On 04/13/2018 04:20 PM, Stokes, Ian wrote:
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo support on RX. This change fixes
the issue by only enabling scatter for NICs supporting it. Add a quirk for
"igb" while the PMD is fixed to advertise scatter.
Thanks for the v2 Pablo.
Adding Eelco and Kevin as they had some comments on the v1.
FYI I'm investigating on the DPDK side to see how/when the flag should be set
and used for igb and ixgbe as well as other drivers.
https://dpdk.org/ml/archives/dev/2018-April/097056.html
Reported-by: Louis Peens<louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón<pablo.cas...@netronome.com>
Reviewed-by: Simon Horman<simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ee39cbe..8f6a0a3
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
int n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
/* For some NICs (e.g. Niantic), scatter_rx mode needs to be
explicitly
* enabled. */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER) {
+conf.rxmode.enable_scatter = 1;
+} else if (!strcmp(info.driver_name, "igb")) {
+/* Quirk: as of DPDK 17.11.1 igb's PMD requires explicitly
+ enabling scatter but fails to advertise it. */
Can you check name for "nfp" and don't set enable_scatter? I don't think
most of the PMDs used these new offload defines in DPDK17.11.
(sorry for the delay replying, was on PTO)
While it is technically possible to do that for the "nfp" it is not the
preferred solution IMHO. The "nfp" PMD is doing the right thing (TM) by
not advertising it supports scatter (it doesn't) and not requiring it
for jumbo traffic. If we're to add a quirk it should be for the
to-be-fixed PMD(s).
I'm not sure this is acceptable. I'm worried it sets a precedent for code for
specific devices and could lead to further instances of this in the future.
It could be argued the scatter approach up to now was specific to Niantic but
it also worked for igb and i40e. I40e devices don’t require scatter but can
handle it without issue if it is set.
In the past this type of solution has been rejected as the preferred approach
was to keep netdev-dpdk code generic as possible.
The principle makes sense in general to me. I think this should be a
temporary exception.
That’s why I suggest deferring the patch in OVS until the required changes are
made in DPDK to satisfy all cases. 17.11.2 is targeted for May 19th. We could
have a solution in place for then.
That would be fine when it comes to releases, not so fine for
(potential) backports in distros and upstream consumers (interested on
commits on between releases)
I'm not trying to obstruct this but these cases do arise so interested to hear
what others think?
+1
Ian
+conf.rxmode.enable_scatter = 1;
+}
}
conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 1/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo support on RX. This change
fixes the issue by only enabling scatter for NICs supporting it. Add a
quirk for "igb" while the PMD is fixed to advertise scatter.
Reported-by: Louis Peens <louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón <pablo.cas...@netronome.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ee39cbe..8f6a0a3 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
/* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
* enabled. */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER) {
+conf.rxmode.enable_scatter = 1;
+} else if (!strcmp(info.driver_name, "igb")) {
+/* Quirk: as of DPDK 17.11.1 igb's PMD requires explicitly
+ enabling scatter but fails to advertise it. */
+conf.rxmode.enable_scatter = 1;
+}
}
conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 0/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter
Hello, here is a v2, this cover letter is to highlight that there is a quirk being added and that it has not been tested with "igb" (don't have an available system with igb at the moment). Some test with "igb" would be greatly appreciated, along with any feedback. Thanks, Pablo Pablo Cascón (1): netdev-dpdk: fix RX jumbo for NICs not supporting scatter lib/netdev-dpdk.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter
On 12/04/18 14:05, Stokes, Ian wrote:
On 10/04/18 21:08, Stokes, Ian wrote:
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo support on RX. This change
fixes the issue by only enabling scatter for NICs supporting it.
Reported-by: Louis Peens <louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón <pablo.cas...@netronome.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
ee39cbe..28b20b5
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,14 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
*dev, int n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
/* For some NICs (e.g. Niantic), scatter_rx mode needs to be
explicitly
* enabled. */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER)
+conf.rxmode.enable_scatter = 1;
Thanks for this, quick note:
conf.rxmode.enable_scatter = 1; should be enclosed in braces as per OVS
coding style.
With an ixgbe device (a Niantic in this case), it never hits the offload
condition above so that it can set scatter.
Currently DEV_RX_OFFLOAD_SCATTER is not set for ixgbe or igb devices as part of
info.rx_offload_capa.
Surprisingly MTU still worked for the ixgbe device.
Digging a little deeper it seems the need to set scatter explicitly for ixgbe
was modified by commit
net/ixgbe: remove MTU setting limitation (c009c6b1)
"This patch allows setting this special MTU when device is stopped, because
scattered_rx will be re-configured during next port start and driver may enable scattered
receive according new MTU value."
In the ixgbe case it's ok because the device is stopped at the time we call set
mtu.
However the patch breaks existing functionality for igb devices as they do not
have a flag set for DEV_RX_OFFLOAD_SCATTER either and still explicitly require
scatter to be set regardless of being stopped. Otherwise they fail to
reconfigure and remain in a down state.
I think a patch is needed for DPDK to set the DEV_RX_OFFLOAD_SCATTER flag for
ixgbe and igb devices. I can look into that.
Thanks for the testing and investigation. Sorry for unearthing this bug :)
If setting the DEV_RX_OFFLOAD_SCATTER flag in the igb and ixgbe PMD
fixes the issue (when this patch is applied to OVS) please warm other
PMDs in the DPDK's mailing list
In an effort to avoid breaking existing functionality in OVS I suggest we defer
this patch until that support is in place for DPDK 17.11.2.
Unfortunately deferring this patch would hurt Netronome's use case, this
jumbo/scatter bug needs to be fixed. Will post a v2 including your style
feedback and a check for driver_name being "igb" to set scatter
regardless of the capability. That will make OVS-DPDK to work with "igb"
before and after your fix. We can comment in that v2 if other extra
checks are needed
We could combine the current patch with the incremental below to fix the style
issue and comment.
Thoughts?
-/* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
- * enabled. */
+/* For some NICs scatter_rx mode needs to be explicitly enabled. */
if (dev->mtu > ETHER_MTU) {
rte_eth_dev_info_get(dev->port_id, );
-if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER)
+if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER) {
conf.rxmode.enable_scatter = 1;
+}
}
Thanks
Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter
On 10/04/18 21:08, Stokes, Ian wrote:
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo support on RX. This change fixes
the issue by only enabling scatter for NICs supporting it.
Reported-by: Louis Peens <louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón <pablo.cas...@netronome.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ee39cbe..28b20b5
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,14 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
int n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
/* For some NICs (e.g. Niantic), scatter_rx mode needs to be
explicitly
* enabled. */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER)
+conf.rxmode.enable_scatter = 1;
Thanks for this, quick note:
conf.rxmode.enable_scatter = 1; should be enclosed in braces as per OVS coding
style.
Thanks for the feedback, sorry about the lack of braces. Let me know if
a v2 is needed
I'll have some time to test this tomorrow, I take it this should be backported
to OVS2.9 and OVS 2.8 also?
Yes please, the change is a welcome one for both 2.8 and 2.9. Also it
applies nicely on both. Let me know if a per branch patch is needed.
Thanks
Pablo
Ian
}
conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter
Currently to RX jumbo packets fails for NICs not supporting scatter.
Scatter is not strictly needed for jumbo support on RX. This change
fixes the issue by only enabling scatter for NICs supporting it.
Reported-by: Louis Peens <louis.pe...@netronome.com>
Signed-off-by: Pablo Cascón <pablo.cas...@netronome.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
---
lib/netdev-dpdk.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ee39cbe..28b20b5 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -694,11 +694,14 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
n_rxq, int n_txq)
int diag = 0;
int i;
struct rte_eth_conf conf = port_conf;
+struct rte_eth_dev_info info;
/* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
* enabled. */
if (dev->mtu > ETHER_MTU) {
-conf.rxmode.enable_scatter = 1;
+rte_eth_dev_info_get(dev->port_id, );
+if (info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER)
+conf.rxmode.enable_scatter = 1;
}
conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
