Hi Ian, Thank you for testing it on VF. Please find my comments below,
Regards _Sugesh > -----Original Message----- > From: Stokes, Ian > Sent: Friday, July 20, 2018 6:30 PM > To: Chandran, Sugesh <[email protected]>; ovs- > [email protected] > Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Fix failure to configure flow > control > at netdev-init. > > On 7/13/2018 6:13 PM, Ian Stokes wrote: > > On 7/10/2018 2:23 PM, Sugesh Chandran wrote: > >> 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've applied to dpdk_merge and it will be > > part of this weeks pull request. > > > > I assume it should be backported to OVS 2.9 at least, do you know if > > it should go to 2.8/2.7 also? > > Hi Sugesh, > > during further testing I found this breaks functionality for Virtual > functions with > OVS (specifically tested with igb_vf, i40e_vf and ixgbe_vf). > > The port itself will fail to initialize with the following error: > > netdev_dpdk|INFO|cannot get flow control parameters on port 2, err=-95 > > And is not added. as such I think flow control should only be optional as > there is > no guarantee it will be available for a given dpdk device and if unavailable > it > should not stop the port from being initialized. [Sugesh] I am not sure if we need to add this condition in OVS. As a virtual switch, OVS should able to use these APIs irrespective of NIC/port types. Actually we are masking this error , without the patch. It is possible to hit this issue, when we configure the flow control on a VF port. Adding a condition in OVS to check a VF port for similar parameters doesn’t looks like a clean approach? What do you think? This leads to another question , whats the impact of modifying other netdev parameters on a VF port. Does it error out/silently fail/work as normal netdev ports? > > I've pulled this patch from the merge request for master, I think the branch > patches will have to be reworked also. > > Ian > > > > Ian > >> Signed-off-by: Sugesh Chandran <[email protected]> > >> --- > >> lib/netdev-dpdk.c | 15 +++++++-------- > >> 1 file changed, 7 insertions(+), 8 deletions(-) > >> > >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > >> bb4d60f26..023b80d3e 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; > >> } > >> @@ -1773,6 +1765,13 @@ netdev_dpdk_set_config(struct netdev *netdev, > >> const struct smap *args, > >> autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); > >> fc_mode = fc_mode_set[tx_fc_en][rx_fc_en]; > >> + /* 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); > >> + } > >> + > >> if (dev->fc_conf.mode != fc_mode || autoneg != > >> dev->fc_conf.autoneg) { > >> dev->fc_conf.mode = fc_mode; > >> dev->fc_conf.autoneg = autoneg; > >> > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
