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.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> > --- > 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 > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev