> 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, &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.

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.

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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to