Hi Ian,
Regards
_Sugesh
> -----Original Message-----
> From: Stokes, Ian
> Sent: Monday, July 30, 2018 7:53 PM
> To: Chandran, Sugesh <sugesh.chand...@intel.com>; ovs-
> d...@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.chand...@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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
