| Message ID | 1532362856-49844-1-git-send-email-sugesh.chandran@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,PATCHv2] netdev-dpdk: Fix failure to configure flow control at netdev-init. | expand |
> 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 <sugesh.chandran@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. > > --- > 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
Hi Ian, Regards _Sugesh > -----Original Message----- > From: Stokes, Ian > Sent: Monday, July 30, 2018 7:53 PM > To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs- > dev@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@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
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); + } dpdk_eth_flow_ctrl_setup(dev); }
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. Signed-off-by: Sugesh Chandran <sugesh.chandran@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. --- lib/netdev-dpdk.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)