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

Message ID 20180710132337.18211-1-sugesh.chandran@intel.com
State New
Headers show
Series
  • [ovs-dev] netdev-dpdk: Fix failure to configure flow control at netdev-init.
Related show

Commit Message

Sugesh Chandran July 10, 2018, 1:23 p.m.
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>
---
 lib/netdev-dpdk.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Ian Stokes July 13, 2018, 5:13 p.m. | #1
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?

Ian
> Signed-off-by: Sugesh Chandran <sugesh.chandran@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;
>
Sugesh Chandran July 16, 2018, 12:19 p.m. | #2
Hi Ian,

Regards
_Sugesh

> -----Original Message-----
...
> > 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?
> 
[Sugesh]Yes, that’s right. It need to backported to 2.8 and 2.7 too.
Should I send separate patch for those branches as well?

> Ian
> > Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com>
...
Ian Stokes July 16, 2018, 12:51 p.m. | #3
> Hi Ian,
> 
> Regards
> _Sugesh
> 
> > -----Original Message-----
> ...
> > > 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?
> >
> [Sugesh]Yes, that’s right. It need to backported to 2.8 and 2.7 too.
> Should I send separate patch for those branches as well?

Yes, it doesn't apply cleanly for 2.8/2.7 so a patch for those branches would be needed.

Thanks
Ian
> 
> > Ian
> > > Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com>
> ...
Sugesh Chandran July 16, 2018, 1:07 p.m. | #4
Regards
_Sugesh


> -----Original Message-----
> From: Stokes, Ian
> Sent: Monday, July 16, 2018 1:52 PM
> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-
> dev@openvswitch.org
> Subject: RE: [ovs-dev] [PATCH] netdev-dpdk: Fix failure to configure flow control
> at netdev-init.
> 
> > Hi Ian,
> >
> > Regards
> > _Sugesh
> >
> > > -----Original Message-----
> > ...
> > > > 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?
> > >
> > [Sugesh]Yes, that’s right. It need to backported to 2.8 and 2.7 too.
> > Should I send separate patch for those branches as well?
> 
> Yes, it doesn't apply cleanly for 2.8/2.7 so a patch for those branches would be
> needed.
[Sugesh] Sure, I will send patches for those versions.

> 
> Thanks
> Ian
> >
> > > Ian
> > > > Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com>
> > ...

Patch

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;