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 <[email protected]>
Signed-off-by: Pablo Cascón <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
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, &info);
+ 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, &info);
- 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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
