On 7/20/2018 7:24 PM, Chandran, Sugesh wrote:
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 <sugesh.chand...@intel.com>; ovs-
d...@openvswitch.org
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.
Ok, but now we have a situation that when adding a VF port, even without
specifying the flow control option, it fails as now flow control
parameters are being passed on init irregardless of whether the user
intends to use it or not.
Now that I think of it I think this would probably break other port
types as well that don't support flow control (such as vdevs).
Adding a condition in OVS to check a VF port for similar parameters doesn’t
looks like
a clean approach? What do you think?
We've already had to do this for a few items (HW CRC enablement, LSC,
MTU support). It's not clean but until DPDK provides a way of checking
capabilities before init it's the best we can do.
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 think we can't fail completely based on an optional capability. And
this is not just for VF ports, conceivably you can have HW ports that
don't support a particular feature either supported by a netdev.
For the current patch you could warn a user that the feature is not
supported and continue the initialization process (similar to the set
mtu capability).
Ian
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 <sugesh.chand...@intel.com>
---
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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
