[ovs-dev] [PATCHv2] netdev-dpdk: Fix failure to configure flow control at netdev-init.

Chandran, Sugesh sugesh.chandran at intel.com
Tue Jul 31 07:56:41 UTC 2018


Hi Ian,

Regards
_Sugesh

> -----Original Message-----
> From: Stokes, Ian
> Sent: Monday, July 30, 2018 7:53 PM
> To: Chandran, Sugesh <sugesh.chandran at intel.com>; ovs-
> dev at openvswitch.org
> Subject: RE: [PATCHv2] netdev-dpdk: Fix failure to configure flow control at
> netdev-init.
> 
> > 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.
[Sugesh] Thank you Ian for validating it on different hardwares.
> 
> > Signed-off-by: Sugesh Chandran <sugesh.chandran at intel.com>
> > ---
> > 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.
[Sugesh] Sure. Will send out next version with this 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'.
[Sugesh] Make sense, will change it accordingly.
> 
> Thanks
> Ian
> > +        }
> >          dpdk_eth_flow_ctrl_setup(dev);
> >      }
> >
> > --
> > 2.7.4



More information about the dev mailing list