> Configuring flow control at ixgbe netdev-init is throwing error in port
> start.
> 
> For eg: without this fix, user cannot configure flow control on ixgbe dpdk
> port as below,
> 
> "
>     ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
>         options:dpdk-devargs=0000:05:00.1 options:rx-flow-ctrl=true "
> 
> Instead,  it must be configured as two different commands,
> 
> "
>     ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
>                options:dpdk-devargs=0000:05:00.1
>     ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true "
> 
> The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' fields
> before trying to configuring the dpdk ethdev. Hence OVS can no longer set
> the 'dont care' fields to just '0' as before. This commit make sure all
> the 'rte_eth_fc_conf' fields are populated with default values before the
> dev init.
> 

Thanks for the patch Sugesh, I think this is a better approach, I've validated 
with various SRIOV VFs as well as i350,ixgbe and i40e drivers, all were ok. 
Some minor comments below.

> Signed-off-by: Sugesh Chandran <[email protected]>
> ---
> V1 -> V2
> Read DPDK port flow-control parameters only when reconfiguration is
> required. This will avoid flow control read error on unsupported ports.

I think it's worth mentioning this in the commit message as rather than just in 
the version change.

> 
> ---
>  lib/netdev-dpdk.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index bb4d60f..11eebe3
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> 
>      mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp);
>      dev->buf_size = mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM;
> -
> -    /* Get the Flow control configuration for DPDK-ETH */
> -    diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf);
> -    if (diag) {
> -        VLOG_DBG("cannot get flow control parameters on port
> "DPDK_PORT_ID_FMT
> -                 ", err=%d", dev->port_id, diag);
> -    }
> -
>      return 0;
>  }
> 
> @@ -1776,6 +1768,12 @@ netdev_dpdk_set_config(struct netdev *netdev, const
> struct smap *args,
>      if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg)
> {
>          dev->fc_conf.mode = fc_mode;
>          dev->fc_conf.autoneg = autoneg;
> +        /* Get the Flow control configuration for DPDK-ETH */
> +        err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf);
> +        if (err) {
> +            VLOG_INFO("cannot get flow control parameters on port "
> +                DPDK_PORT_ID_FMT", err=%d", dev->port_id, err);

This should probably be an error or at least a warning instead of info as in 
this case someone has attempted to configure flow control but an error 
occurred. Also minor nit but the First word of the message should be 
capitalized. i.e. 'cannot' -> 'Cannot'.

Thanks
Ian
> +        }
>          dpdk_eth_flow_ctrl_setup(dev);
>      }
> 
> --
> 2.7.4

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to