diff mbox

[ovs-dev] netdev-dpdk: Add Flow Control support.

Message ID 1469193494-20609-1-git-send-email-sugesh.chandran@intel.com
State Changes Requested
Delegated to: Daniele Di Proietto
Headers show

Commit Message

Chandran, Sugesh July 22, 2016, 1:18 p.m. UTC
Add support for flow-control(mac control frame) to DPDK enabled physical
port types. By default, the flow-control is OFF on both rx and tx side.
The flow control can be enabled/disabled either when adding a port to OVS
or at run time.

For eg:
To enable flow control support at tx side while adding a port, add the
'tx-flow-ctrl' option to the 'ovs-vsctl add-port' command-line as below.

 'ovs-vsctl add-port br0 dpdk0 -- \
  set Interface dpdk0 type=dpdk options:tx-flow-ctrl=on'

Similarly to enable rx flow control,
 'ovs-vsctl add-port br0 dpdk0 -- \
  set Interface dpdk0 type=dpdk options:rx-flow-ctrl=on'

And to enable the flow control auto-negotiation,
 'ovs-vsctl add-port br0 dpdk0 -- \
  set Interface dpdk0 type=dpdk options:flow-ctrl-autoneg=on'

To turn ON the tx flow control at run time(After the port is being added
to OVS), the command-line input will be,
 'ovs-vsctl set Interface dpdk0 options:tx-flow-ctrl=on'

The flow control parameters can be turned off by setting 'off' to the
respective parameter. To turn off the flow control at tx side,
 'ovs-vsctl set Interface dpdk0 options:tx-flow-ctrl=off'

Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com>
---
 lib/netdev-dpdk.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

Comments

Bodireddy, Bhanuprakash July 22, 2016, 2:34 p.m. UTC | #1
Thanks for the patch, can you also add the flow control options to the INSTALL-ADVANCED.md?

Regards,
Bhanu Prakash.

>-----Original Message-----

>From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Sugesh

>Chandran

>Sent: Friday, July 22, 2016 2:18 PM

>To: dev@openvswitch.org

>Subject: [ovs-dev] [PATCH] netdev-dpdk: Add Flow Control support.

>

>Add support for flow-control(mac control frame) to DPDK enabled physical

>port types. By default, the flow-control is OFF on both rx and tx side.

>The flow control can be enabled/disabled either when adding a port to OVS or

>at run time.

>

>For eg:

>To enable flow control support at tx side while adding a port, add the 'tx-flow-

>ctrl' option to the 'ovs-vsctl add-port' command-line as below.

>

> 'ovs-vsctl add-port br0 dpdk0 -- \

>  set Interface dpdk0 type=dpdk options:tx-flow-ctrl=on'

>

>Similarly to enable rx flow control,

> 'ovs-vsctl add-port br0 dpdk0 -- \

>  set Interface dpdk0 type=dpdk options:rx-flow-ctrl=on'

>

>And to enable the flow control auto-negotiation,  'ovs-vsctl add-port br0

>dpdk0 -- \

>  set Interface dpdk0 type=dpdk options:flow-ctrl-autoneg=on'

>

>To turn ON the tx flow control at run time(After the port is being added to

>OVS), the command-line input will be,  'ovs-vsctl set Interface dpdk0

>options:tx-flow-ctrl=on'

>

>The flow control parameters can be turned off by setting 'off' to the

>respective parameter. To turn off the flow control at tx side,  'ovs-vsctl set

>Interface dpdk0 options:tx-flow-ctrl=off'

>

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

>---

> lib/netdev-dpdk.c | 68

>+++++++++++++++++++++++++++++++++++++++++++++++++++++++

> 1 file changed, 68 insertions(+)

>

>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 85b18fd..74efd25

>100644

>--- a/lib/netdev-dpdk.c

>+++ b/lib/netdev-dpdk.c

>@@ -634,6 +634,67 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk

>*dev, int n_rxq, int n_txq)

>     return diag;

> }

>

>+static void

>+dpdk_eth_parse_flow_ctrl(struct netdev_dpdk *dev,

>+                         const struct smap *args,

>+                         struct rte_eth_fc_conf *fc_conf)

>+                         OVS_REQUIRES(dev->mutex) {

>+    int ret = 0;

>+    int rx_fc_en = 0;

>+    int tx_fc_en = 0;

>+    const char *rx_flow_mode;

>+    const char *tx_flow_mode;

>+    const char *flow_autoneg;

>+    enum rte_eth_fc_mode fc_mode_set[2][2] = {{RTE_FC_NONE,

>RTE_FC_TX_PAUSE},

>+                                              {RTE_FC_RX_PAUSE, RTE_FC_FULL}

>+                                             };

>+

>+    ret = rte_eth_dev_flow_ctrl_get(dev->port_id, fc_conf);

>+    if (ret != 0) {

>+        VLOG_DBG("cannot get flow control parameters on port=%d, err=%s",

>+                 dev->port_id, rte_strerror(ret));

>+        return;

>+    }

>+    rx_flow_mode = smap_get(args, "rx-flow-ctrl");

>+    tx_flow_mode = smap_get(args, "tx-flow-ctrl");

>+    flow_autoneg = smap_get(args, "flow-ctrl-autoneg");

>+    if (rx_flow_mode) {

>+        if (!strcmp(rx_flow_mode, "on")) {

>+            rx_fc_en = 1;

>+        }

>+        else if (!strcmp(rx_flow_mode, "off")) {

>+            rx_fc_en = 0;

>+        }

>+    }

>+    if (tx_flow_mode) {

>+        if (!strcmp(tx_flow_mode, "on")) {

>+            tx_fc_en =1;

>+        }

>+        else if (!strcmp(tx_flow_mode, "off")) {

>+            tx_fc_en =0;

>+        }

>+    }

>+    if (flow_autoneg) {

>+        if (!strcmp(flow_autoneg, "on")) {

>+            fc_conf->autoneg = 1;

>+        }

>+        else if (!strcmp(flow_autoneg, "off")) {

>+            fc_conf->autoneg = 0;

>+        }

>+    }

>+    fc_conf->mode = fc_mode_set[tx_fc_en][rx_fc_en]; }

>+

>+static void

>+dpdk_eth_flow_ctrl_config(struct netdev_dpdk *dev,

>+                          struct rte_eth_fc_conf *fc_conf)

>+                          OVS_REQUIRES(dev->mutex) {

>+    if (rte_eth_dev_flow_ctrl_set(dev->port_id, fc_conf) != 0) {

>+        VLOG_ERR("Failed to enable flow control on device %d", dev->port_id);

>+    }

>+}

>

> static int

> dpdk_eth_dev_init(struct netdev_dpdk *dev) OVS_REQUIRES(dpdk_mutex)

>@@ -991,6 +1052,13 @@ netdev_dpdk_set_config(struct netdev *netdev,

>const struct smap *args)

>         dev->requested_n_rxq = new_n_rxq;

>         netdev_request_reconfigure(netdev);

>     }

>+

>+    /* Flow control configuration for DPDK Ethernet ports. */

>+    if (dev->type == DPDK_DEV_ETH) {

>+        struct rte_eth_fc_conf fc_conf = {0};

>+        dpdk_eth_parse_flow_ctrl(dev, args, &fc_conf);

>+        dpdk_eth_flow_ctrl_config(dev, &fc_conf);

>+    }

>     ovs_mutex_unlock(&dev->mutex);

>

>     return 0;

>--

>2.5.0

>

>_______________________________________________

>dev mailing list

>dev@openvswitch.org

>http://openvswitch.org/mailman/listinfo/dev
Daniele Di Proietto July 28, 2016, 12:37 a.m. UTC | #2
Thanks for the patch.

I agree, I'd be nice to document this in INSTALL.DPDK-ADVANCED.md

We should also document the new fields in vswitchd/vswitch.xml.

Probably it's better to use "true" and "false" rather that "on" and "off",
for consistency with other configuration options and so that we can use
smap_get_bool().

I assume it's ok to call rte_eth_dev_flow_ctrl_get()/_set() while the
device is transmitting/receiving.

Maybe it would be better to cache fc_conf in struct netdev_dpdk and call
_set() only if we have to make a change?

2016-07-22 6:18 GMT-07:00 Sugesh Chandran <sugesh.chandran@intel.com>:

> Add support for flow-control(mac control frame) to DPDK enabled physical
> port types. By default, the flow-control is OFF on both rx and tx side.
> The flow control can be enabled/disabled either when adding a port to OVS
> or at run time.
>
> For eg:
> To enable flow control support at tx side while adding a port, add the
> 'tx-flow-ctrl' option to the 'ovs-vsctl add-port' command-line as below.
>
>  'ovs-vsctl add-port br0 dpdk0 -- \
>   set Interface dpdk0 type=dpdk options:tx-flow-ctrl=on'
>
> Similarly to enable rx flow control,
>  'ovs-vsctl add-port br0 dpdk0 -- \
>   set Interface dpdk0 type=dpdk options:rx-flow-ctrl=on'
>
> And to enable the flow control auto-negotiation,
>  'ovs-vsctl add-port br0 dpdk0 -- \
>   set Interface dpdk0 type=dpdk options:flow-ctrl-autoneg=on'
>
> To turn ON the tx flow control at run time(After the port is being added
> to OVS), the command-line input will be,
>  'ovs-vsctl set Interface dpdk0 options:tx-flow-ctrl=on'
>
> The flow control parameters can be turned off by setting 'off' to the
> respective parameter. To turn off the flow control at tx side,
>  'ovs-vsctl set Interface dpdk0 options:tx-flow-ctrl=off'
>
> Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com>
> ---
>  lib/netdev-dpdk.c | 68
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 85b18fd..74efd25 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -634,6 +634,67 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
> n_rxq, int n_txq)
>      return diag;
>  }
>
> +static void
> +dpdk_eth_parse_flow_ctrl(struct netdev_dpdk *dev,
> +                         const struct smap *args,
> +                         struct rte_eth_fc_conf *fc_conf)
> +                         OVS_REQUIRES(dev->mutex)
>

Minor: the other thread safety annotations are not aligned with the
parameters, but are just indented four spaces.


> +{
> +    int ret = 0;
> +    int rx_fc_en = 0;
> +    int tx_fc_en = 0;
> +    const char *rx_flow_mode;
> +    const char *tx_flow_mode;
> +    const char *flow_autoneg;
> +    enum rte_eth_fc_mode fc_mode_set[2][2] = {{RTE_FC_NONE,
> RTE_FC_TX_PAUSE},
> +                                              {RTE_FC_RX_PAUSE,
> RTE_FC_FULL}
> +                                             };
> +
> +    ret = rte_eth_dev_flow_ctrl_get(dev->port_id, fc_conf);
> +    if (ret != 0) {
> +        VLOG_DBG("cannot get flow control parameters on port=%d, err=%s",
> +                 dev->port_id, rte_strerror(ret));
>

I'm not sure, do we need to change 'ret' sign before passing it to
rte_strerror()?


> +        return;
> +    }
> +    rx_flow_mode = smap_get(args, "rx-flow-ctrl");
> +    tx_flow_mode = smap_get(args, "tx-flow-ctrl");
> +    flow_autoneg = smap_get(args, "flow-ctrl-autoneg");
> +    if (rx_flow_mode) {
> +        if (!strcmp(rx_flow_mode, "on")) {
> +            rx_fc_en = 1;
> +        }
> +        else if (!strcmp(rx_flow_mode, "off")) {
> +            rx_fc_en = 0;
> +        }
> +    }
> +    if (tx_flow_mode) {
> +        if (!strcmp(tx_flow_mode, "on")) {
> +            tx_fc_en =1;
> +        }
> +        else if (!strcmp(tx_flow_mode, "off")) {
> +            tx_fc_en =0;
> +        }
> +    }
> +    if (flow_autoneg) {
> +        if (!strcmp(flow_autoneg, "on")) {
> +            fc_conf->autoneg = 1;
> +        }
> +        else if (!strcmp(flow_autoneg, "off")) {
> +            fc_conf->autoneg = 0;
> +        }
> +    }
> +    fc_conf->mode = fc_mode_set[tx_fc_en][rx_fc_en];
> +}
> +
> +static void
> +dpdk_eth_flow_ctrl_config(struct netdev_dpdk *dev,
> +                          struct rte_eth_fc_conf *fc_conf)
> +                          OVS_REQUIRES(dev->mutex)
>

Minor: the other thread safety annotations are not aligned with the
parameters, but are just indented four spaces.


> +{
> +    if (rte_eth_dev_flow_ctrl_set(dev->port_id, fc_conf) != 0) {
>

I'd drop the != 0


> +        VLOG_ERR("Failed to enable flow control on device %d",
> dev->port_id);
>

VLOG_WARN is probably enough


> +    }
> +}
>
>  static int
>  dpdk_eth_dev_init(struct netdev_dpdk *dev) OVS_REQUIRES(dpdk_mutex)
> @@ -991,6 +1052,13 @@ netdev_dpdk_set_config(struct netdev *netdev, const
> struct smap *args)
>          dev->requested_n_rxq = new_n_rxq;
>          netdev_request_reconfigure(netdev);
>      }
> +
> +    /* Flow control configuration for DPDK Ethernet ports. */
> +    if (dev->type == DPDK_DEV_ETH) {
> +        struct rte_eth_fc_conf fc_conf = {0};
> +        dpdk_eth_parse_flow_ctrl(dev, args, &fc_conf);
>

I think it'd be better to extract the fields from 'args' here and pass them
to dpdk_eth_parse_flow_ctrl(), which would probably need a new name, but
that's just my preference.


> +        dpdk_eth_flow_ctrl_config(dev, &fc_conf);
> +    }
>      ovs_mutex_unlock(&dev->mutex);
>
>      return 0;
> --
> 2.5.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
diff mbox

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 85b18fd..74efd25 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -634,6 +634,67 @@  dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
     return diag;
 }
 
+static void
+dpdk_eth_parse_flow_ctrl(struct netdev_dpdk *dev,
+                         const struct smap *args,
+                         struct rte_eth_fc_conf *fc_conf)
+                         OVS_REQUIRES(dev->mutex)
+{
+    int ret = 0;
+    int rx_fc_en = 0;
+    int tx_fc_en = 0;
+    const char *rx_flow_mode;
+    const char *tx_flow_mode;
+    const char *flow_autoneg;
+    enum rte_eth_fc_mode fc_mode_set[2][2] = {{RTE_FC_NONE, RTE_FC_TX_PAUSE},
+                                              {RTE_FC_RX_PAUSE, RTE_FC_FULL}
+                                             };
+
+    ret = rte_eth_dev_flow_ctrl_get(dev->port_id, fc_conf);
+    if (ret != 0) {
+        VLOG_DBG("cannot get flow control parameters on port=%d, err=%s",
+                 dev->port_id, rte_strerror(ret));
+        return;
+    }
+    rx_flow_mode = smap_get(args, "rx-flow-ctrl");
+    tx_flow_mode = smap_get(args, "tx-flow-ctrl");
+    flow_autoneg = smap_get(args, "flow-ctrl-autoneg");
+    if (rx_flow_mode) {
+        if (!strcmp(rx_flow_mode, "on")) {
+            rx_fc_en = 1;
+        }
+        else if (!strcmp(rx_flow_mode, "off")) {
+            rx_fc_en = 0;
+        }
+    }
+    if (tx_flow_mode) {
+        if (!strcmp(tx_flow_mode, "on")) {
+            tx_fc_en =1;
+        }
+        else if (!strcmp(tx_flow_mode, "off")) {
+            tx_fc_en =0;
+        }
+    }
+    if (flow_autoneg) {
+        if (!strcmp(flow_autoneg, "on")) {
+            fc_conf->autoneg = 1;
+        }
+        else if (!strcmp(flow_autoneg, "off")) {
+            fc_conf->autoneg = 0;
+        }
+    }
+    fc_conf->mode = fc_mode_set[tx_fc_en][rx_fc_en];
+}
+
+static void
+dpdk_eth_flow_ctrl_config(struct netdev_dpdk *dev,
+                          struct rte_eth_fc_conf *fc_conf)
+                          OVS_REQUIRES(dev->mutex)
+{
+    if (rte_eth_dev_flow_ctrl_set(dev->port_id, fc_conf) != 0) {
+        VLOG_ERR("Failed to enable flow control on device %d", dev->port_id);
+    }
+}
 
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev) OVS_REQUIRES(dpdk_mutex)
@@ -991,6 +1052,13 @@  netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args)
         dev->requested_n_rxq = new_n_rxq;
         netdev_request_reconfigure(netdev);
     }
+
+    /* Flow control configuration for DPDK Ethernet ports. */
+    if (dev->type == DPDK_DEV_ETH) {
+        struct rte_eth_fc_conf fc_conf = {0};
+        dpdk_eth_parse_flow_ctrl(dev, args, &fc_conf);
+        dpdk_eth_flow_ctrl_config(dev, &fc_conf);
+    }
     ovs_mutex_unlock(&dev->mutex);
 
     return 0;