Why are the variables uint8_t instead of bool? I think we shouldn't assume that converting to bool always returns 0 or 1, but the return value of smap_get_bool() is always 0 or 1 (as we always go through ! or !=). I would remove the ternary operator
Thanks, Daniele 2016-09-21 7:06 GMT-07:00 Chandran, Sugesh <sugesh.chand...@intel.com>: > Hi Ilya, > Thank you for the patch. It looks fine for me. > Also I verified that the flow control is working fine with the > modification. > > > > Regards > _Sugesh > > > > -----Original Message----- > > From: Ilya Maximets [mailto:i.maxim...@samsung.com] > > Sent: Wednesday, September 21, 2016 11:39 AM > > To: dev@openvswitch.org; Daniele Di Proietto <diproiet...@vmware.com> > > Cc: Dyasly Sergey <s.dya...@samsung.com>; Heetae Ahn > > <heetae82....@samsung.com>; Chandran, Sugesh > > <sugesh.chand...@intel.com>; Bodireddy, Bhanuprakash > > <bhanuprakash.bodire...@intel.com>; Loftus, Ciara > > <ciara.lof...@intel.com> > > Subject: Re: [PATCH v2] netdev-dpdk: Configure flow control only when > > necessary. > > > > On 21.09.2016 13:35, Ilya Maximets wrote: > > > It is not necessary to touch the physical device each time, if the > > > configuration has not been changed. Also, few style issues fixed. > > > > > > Thread-safety annotation added to 'dpdk_set_rxq_config()'. It was > > > missed while previous refactoring of the flow control configuration. > > > > > > Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> > > > --- > > > > Sorry, forget about the changelog: > > > > Version 2: > > > > * Only commit-message updated. (thread-safety annotation) > > > > > lib/netdev-dpdk.c | 30 +++++++++++++++++------------- > > > 1 file changed, 17 insertions(+), 13 deletions(-) > > > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > > > 89bdc4d..602217f 100644 > > > --- a/lib/netdev-dpdk.c > > > +++ b/lib/netdev-dpdk.c > > > @@ -1059,6 +1059,7 @@ netdev_dpdk_get_config(const struct netdev > > > *netdev, struct smap *args) > > > > > > static void > > > dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args) > > > + OVS_REQUIRES(dev->mutex) > > > { > > > int new_n_rxq; > > > > > > @@ -1073,24 +1074,27 @@ static int > > > netdev_dpdk_set_config(struct netdev *netdev, const struct smap > > > *args) { > > > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > > > + uint8_t rx_fc_en, tx_fc_en, autoneg; > > > + enum rte_eth_fc_mode fc_mode; > > > + static const enum rte_eth_fc_mode fc_mode_set[2][2] = { > > > + {RTE_FC_NONE, RTE_FC_TX_PAUSE}, > > > + {RTE_FC_RX_PAUSE, RTE_FC_FULL } > > > + }; > > > > > > ovs_mutex_lock(&dev->mutex); > > > > > > dpdk_set_rxq_config(dev, args); > > > > > > - /* Flow control support is only available for DPDK Ethernet > ports. */ > > > - bool rx_fc_en = false; > > > - bool tx_fc_en = false; > > > - enum rte_eth_fc_mode fc_mode_set[2][2] = > > > - {{RTE_FC_NONE, > RTE_FC_TX_PAUSE}, > > > - {RTE_FC_RX_PAUSE, RTE_FC_FULL} > > > - }; > > > - rx_fc_en = smap_get_bool(args, "rx-flow-ctrl", false); > > > - tx_fc_en = smap_get_bool(args, "tx-flow-ctrl", false); > > > - dev->fc_conf.autoneg = smap_get_bool(args, "flow-ctrl-autoneg", > > false); > > > - dev->fc_conf.mode = fc_mode_set[tx_fc_en][rx_fc_en]; > > > - > > > - dpdk_eth_flow_ctrl_setup(dev); > > > + rx_fc_en = smap_get_bool(args, "rx-flow-ctrl", false) ? 1 : 0; > > > + tx_fc_en = smap_get_bool(args, "tx-flow-ctrl", false) ? 1 : 0; > > > + autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false) ? 1 : > > > + 0; > > > + > > > + fc_mode = fc_mode_set[tx_fc_en][rx_fc_en]; > > > + if (dev->fc_conf.mode != fc_mode || autoneg != dev- > > >fc_conf.autoneg) { > > > + dev->fc_conf.mode = fc_mode; > > > + dev->fc_conf.autoneg = autoneg; > > > + dpdk_eth_flow_ctrl_setup(dev); > > > + } > > > > > > ovs_mutex_unlock(&dev->mutex); > > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
