[ovs-dev] [PATCH v2] netdev-dpdk: Configure flow control only when necessary.

[Chandran, Sugesh]

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.maximets at samsung.com]
> Sent: Wednesday, September 21, 2016 11:39 AM
> To: dev at openvswitch.org; Daniele Di Proietto <diproiettod at vmware.com>
> Cc: Dyasly Sergey <s.dyasly at samsung.com>; Heetae Ahn
> <heetae82.ahn at samsung.com>; Chandran, Sugesh
> <sugesh.chandran at intel.com>; Bodireddy, Bhanuprakash
> <bhanuprakash.bodireddy at intel.com>; Loftus, Ciara
> <ciara.loftus at 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.maximets at 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);
> >
> >