[ovs-dev] [PATCH 1/1] netdev-dpdk: fix RX jumbo for NICs not supporting scatter

Pablo Cascón pablo.cascon at netronome.com
Fri Apr 13 09:51:35 UTC 2018


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.peens at netronome.com>
>>>> Signed-off-by: Pablo Cascón <pablo.cascon at netronome.com>
>>>> Reviewed-by: Simon Horman <simon.horman at 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.

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



More information about the dev mailing list