[ovs-dev] [PATCH] netdev-dpdk: Remove unnecessary 'if' statement

[Loftus, Ciara]

> 
> Hi Ciara,
> Thank you for fixing this.
> Changes are looks fine for me.
> A minor comment as below.
> Acked!
> 
> 
> Regards
> _Sugesh
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Ciara
> Loftus
> > Sent: Friday, August 12, 2016 5:17 PM
> > To: dev at openvswitch.org
> > Subject: [ovs-dev] [PATCH] netdev-dpdk: Remove unnecessary 'if'
> > statement
> >
> > Only devices of type "DPDK_DEV_ETH" use the netdev_dpdk_set_config
> > function, so no need to check for the device type within the function.
> >
> > Fixes: 9fd39370c12c ("netdev-dpdk: Add Flow Control support.")
> > Signed-off-by: Ciara Loftus <ciara.loftus at intel.com>
> > ---
> >  lib/netdev-dpdk.c | 27 +++++++++++++--------------
> >  1 file changed, 13 insertions(+), 14 deletions(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 9a1f7cd..f772998
> > 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1024,20 +1024,19 @@ netdev_dpdk_set_config(struct netdev
> *netdev,
> > const struct smap *args)
> >      }
> >
> >      /* Flow control configuration for DPDK Ethernet ports. */
> > -    if (dev->type == DPDK_DEV_ETH) {
> > -        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);
> > -    }
> [Sugesh] I would add a comment to say that the flow control is supported
> only on
> Eth/physical ports for better readability.

Thanks for the review Sugesh, I'll improve the comment for the v2.
Out of interest, do 'dpdkr' ivshm ports support flow ctrl? At the moment (with and without this patch) we attempt to initialise flow ctrl for these ports. If that's not intended behaviour I'll fix that in v2 as well.

Thanks,
Ciara

> > +    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);
> > +
> >      ovs_mutex_unlock(&dev->mutex);
> >
> >      return 0;
> > --
> > 2.4.3
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev