subject:"Re\: \[ovs\-dev\] \[PATCH\] netdev\-dpdk\: Add Flow Control support."

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

2016-07-28 Thread Chandran, Sugesh

Hi Bhanu and Daniele,
Thank you for your time to review the patch,

Please find my answers inline prefix with [Sugesh]

Regards
_Sugesh

From: Daniele Di Proietto [mailto:diproiet...@ovn.org]
Sent: Thursday, July 28, 2016 1:38 AM
To: Chandran, Sugesh <sugesh.chand...@intel.com>; Bodireddy, Bhanuprakash 
<bhanuprakash.bodire...@intel.com>
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Add Flow Control support.

Thanks for the patch.
I agree, I'd be nice to document this in 
INSTALL.DPDK-ADVANCED.md<http://INSTALL.DPDK-ADVANCED.md>
[Sugesh] Agreed, Will add that.
We should also document the new fields in vswitchd/vswitch.xml.
[Sugesh] Will do
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().
[Sugesh] Will update it.
I assume it's ok to call rte_eth_dev_flow_ctrl_get()/_set() while the device is 
transmitting/receiving.
[Sugesh], yes, it’s a run time parameter.
Maybe it would be better to cache fc_conf in struct netdev_dpdk and call _set() 
only if we have to make a change?
[Sugesh] : Will add it in the ‘netdev_dpdk’.

2016-07-22 6:18 GMT-07:00 Sugesh Chandran 
<sugesh.chand...@intel.com<mailto:sugesh.chand...@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.chand...@intel.com<mailto:sugesh.chand...@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.
[Sugesh] Sorry, Will correct it.

+{
+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()?
[Sugesh] : Either I can change it to rte_strerror(abs(ret)), or print the error 
code as is.
All the existing ‘flow_ctrl_get’ driver implementation doesn’t return any error 
code in DPDK. It returns ‘0’ always. So I think its OK to just print the error 
code as it is. What do you think?

+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) {
+

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

2016-07-27 Thread Daniele Di Proietto

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 :

> 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 
> ---
>  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;
>

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

2016-07-22 Thread Bodireddy, Bhanuprakash

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-boun...@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 
>---
> 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, _conf);
>+dpdk_eth_flow_ctrl_config(dev, _conf);
>+}
> ovs_mutex_unlock(>mutex);
>
> return 0;
>--
>2.5.0
>
>___
>dev mailing list
>dev@openvswitch.org
>http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

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

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

3 matches

Site Navigation

Mail list logo

Footer information