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

[Chandran, Sugesh]

Regards
_Sugesh


> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Wednesday, September 21, 2016 10:50 AM
> To: Chandran, Sugesh ;
> dev@openvswitch.org; Daniele Di Proietto 
> Cc: Dyasly Sergey ; Heetae Ahn
> ; Bodireddy, Bhanuprakash
> ; Loftus, Ciara
> 
> Subject: Re: [PATCH] netdev-dpdk: Configure flow control only when
> necessary.
> 
> On 21.09.2016 11:26, Chandran, Sugesh wrote:
> >
> >
> > Regards
> > _Sugesh
> >
> >> -Original Message-
> >> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> >> Sent: Tuesday, September 20, 2016 10:52 AM
> >> To: Chandran, Sugesh ;
> >> dev@openvswitch.org; Daniele Di Proietto 
> >> Cc: Dyasly Sergey ; Heetae Ahn
> >> ; Bodireddy, Bhanuprakash
> >> ; Loftus, Ciara
> >> 
> >> Subject: Re: [PATCH] netdev-dpdk: Configure flow control only when
> >> necessary.
> >>
> >> Hi, Sugesh,
> >> Thanks for review. My comments inline.
> >>
> >> Bets regards, Ilya Maximets.
> >>
> >> On 20.09.2016 11:41, Chandran, Sugesh wrote:
> >>> Hi Ilya,
> >>> Thank you for sending out the patch.
> >>> I verified that the patch is working fine.
> >>> Please find some comments below.
> >>>
> >>> Regards
> >>> _Sugesh
> >>>
> >>>
>  -Original Message-
>  From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>  Sent: Monday, September 19, 2016 2:34 PM
>  To: dev@openvswitch.org; Daniele Di Proietto
> >> 
>  Cc: Dyasly Sergey ; Heetae Ahn
>  ; Chandran, Sugesh
>  ; Bodireddy, Bhanuprakash
>  ; Loftus, Ciara
>  ; Ilya Maximets 
>  Subject: [PATCH] netdev-dpdk: Configure flow control only when
> >> necessary.
> 
>  It is not necessary to touch the physical device each time, if the
>  configuration has not been changed. Also, few style issues fixed.
> 
>  Signed-off-by: Ilya Maximets 
>  ---
>   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
>  6d334db..1632b97 100644
>  --- a/lib/netdev-dpdk.c
>  +++ b/lib/netdev-dpdk.c
>  @@ -1057,6 +1057,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)
> >>> [Sugesh] I assume this mutex is needed irrelevant of flow director
> >> configuration.
> >>> Can you mention this as well in the commit message.
> >>
> >> You right. I just thought that this annotation can be treated as a style 
> >> fix.
> >> If you disagree, I can mention it in a commit-message.
> > [Sugesh] I would prefer to specify it explicitly.
> 
> OK.
> 
> >>
>   {
>   int new_n_rxq;
> 
>  @@ -1071,24 +1072,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(>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;
> >>> [Sugesh] smap_get_bool itself returns 1, 0 based on the input. I
> >>> wouldn't
> >> use the conditional operator to check the result.
> >>> Same comment for the following smap_get* as well.
> >>
> >> 'smap_get_bool()' returns 'bool'.
> >> And 

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

[Ilya Maximets]

On 21.09.2016 11:26, Chandran, Sugesh wrote:
> 
> 
> Regards
> _Sugesh
> 
>> -Original Message-
>> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>> Sent: Tuesday, September 20, 2016 10:52 AM
>> To: Chandran, Sugesh ;
>> dev@openvswitch.org; Daniele Di Proietto 
>> Cc: Dyasly Sergey ; Heetae Ahn
>> ; Bodireddy, Bhanuprakash
>> ; Loftus, Ciara
>> 
>> Subject: Re: [PATCH] netdev-dpdk: Configure flow control only when
>> necessary.
>>
>> Hi, Sugesh,
>> Thanks for review. My comments inline.
>>
>> Bets regards, Ilya Maximets.
>>
>> On 20.09.2016 11:41, Chandran, Sugesh wrote:
>>> Hi Ilya,
>>> Thank you for sending out the patch.
>>> I verified that the patch is working fine.
>>> Please find some comments below.
>>>
>>> Regards
>>> _Sugesh
>>>
>>>
 -Original Message-
 From: Ilya Maximets [mailto:i.maxim...@samsung.com]
 Sent: Monday, September 19, 2016 2:34 PM
 To: dev@openvswitch.org; Daniele Di Proietto
>> 
 Cc: Dyasly Sergey ; Heetae Ahn
 ; Chandran, Sugesh
 ; Bodireddy, Bhanuprakash
 ; Loftus, Ciara
 ; Ilya Maximets 
 Subject: [PATCH] netdev-dpdk: Configure flow control only when
>> necessary.

 It is not necessary to touch the physical device each time, if the
 configuration has not been changed. Also, few style issues fixed.

 Signed-off-by: Ilya Maximets 
 ---
  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
 6d334db..1632b97 100644
 --- a/lib/netdev-dpdk.c
 +++ b/lib/netdev-dpdk.c
 @@ -1057,6 +1057,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)
>>> [Sugesh] I assume this mutex is needed irrelevant of flow director
>> configuration.
>>> Can you mention this as well in the commit message.
>>
>> You right. I just thought that this annotation can be treated as a style fix.
>> If you disagree, I can mention it in a commit-message.
> [Sugesh] I would prefer to specify it explicitly.

OK.

>>
  {
  int new_n_rxq;

 @@ -1071,24 +1072,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(>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;
>>> [Sugesh] smap_get_bool itself returns 1, 0 based on the input. I wouldn't
>> use the conditional operator to check the result.
>>> Same comment for the following smap_get* as well.
>>
>> 'smap_get_bool()' returns 'bool'.
>> And according to "C DIALECT" section of CodingStyle.md:
>> "
>>   Most C99 features are OK because they are widely implemented:
>>
>>  * bool and , but don't assume that bool or _Bool can
>>only take on the values 0 or 1, because this behavior can't be
>>simulated on C89 compilers.
>> "
>>
>> This means that 'bool' must be directly converted to 0 or 1 before using as 
>> an
>> index.
> [Sugesh] Agreed, if that's the case, it would nice to set default value '0' 
> for any other value 
> than '1'. What do you think? In this case you are setting the flow director 
> for all the values except '0'.  
> Do you think that's the expected behavior than setting default for any 
> invalid inputs?

'smap_get_bool' returns boolean. 'true' will be converted to '1' and 'false' to 
0.

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

[Chandran, Sugesh]

Regards
_Sugesh

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Tuesday, September 20, 2016 10:52 AM
> To: Chandran, Sugesh ;
> dev@openvswitch.org; Daniele Di Proietto 
> Cc: Dyasly Sergey ; Heetae Ahn
> ; Bodireddy, Bhanuprakash
> ; Loftus, Ciara
> 
> Subject: Re: [PATCH] netdev-dpdk: Configure flow control only when
> necessary.
> 
> Hi, Sugesh,
> Thanks for review. My comments inline.
> 
> Bets regards, Ilya Maximets.
> 
> On 20.09.2016 11:41, Chandran, Sugesh wrote:
> > Hi Ilya,
> > Thank you for sending out the patch.
> > I verified that the patch is working fine.
> > Please find some comments below.
> >
> > Regards
> > _Sugesh
> >
> >
> >> -Original Message-
> >> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> >> Sent: Monday, September 19, 2016 2:34 PM
> >> To: dev@openvswitch.org; Daniele Di Proietto
> 
> >> Cc: Dyasly Sergey ; Heetae Ahn
> >> ; Chandran, Sugesh
> >> ; Bodireddy, Bhanuprakash
> >> ; Loftus, Ciara
> >> ; Ilya Maximets 
> >> Subject: [PATCH] netdev-dpdk: Configure flow control only when
> necessary.
> >>
> >> It is not necessary to touch the physical device each time, if the
> >> configuration has not been changed. Also, few style issues fixed.
> >>
> >> Signed-off-by: Ilya Maximets 
> >> ---
> >>  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
> >> 6d334db..1632b97 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -1057,6 +1057,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)
> > [Sugesh] I assume this mutex is needed irrelevant of flow director
> configuration.
> > Can you mention this as well in the commit message.
> 
> You right. I just thought that this annotation can be treated as a style fix.
> If you disagree, I can mention it in a commit-message.
[Sugesh] I would prefer to specify it explicitly.
> 
> >>  {
> >>  int new_n_rxq;
> >>
> >> @@ -1071,24 +1072,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(>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;
> > [Sugesh] smap_get_bool itself returns 1, 0 based on the input. I wouldn't
> use the conditional operator to check the result.
> > Same comment for the following smap_get* as well.
> 
> 'smap_get_bool()' returns 'bool'.
> And according to "C DIALECT" section of CodingStyle.md:
> "
>   Most C99 features are OK because they are widely implemented:
> 
>   * bool and , but don't assume that bool or _Bool can
> only take on the values 0 or 1, because this behavior can't be
> simulated on C89 compilers.
> "
> 
> This means that 'bool' must be directly converted to 0 or 1 before using as an
> index.
[Sugesh] Agreed, if that's the case, it would nice to set default value '0' for 
any other value 
than '1'. What do you think? In this case you are setting the flow director for 
all the values except '0'.  
Do you think that's the expected behavior than setting default for any invalid 
inputs?

> 
> >> +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 = 

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

[Ilya Maximets]

Hi, Sugesh,
Thanks for review. My comments inline.

Bets regards, Ilya Maximets.

On 20.09.2016 11:41, Chandran, Sugesh wrote:
> Hi Ilya,
> Thank you for sending out the patch.
> I verified that the patch is working fine.
> Please find some comments below.
> 
> Regards
> _Sugesh
> 
> 
>> -Original Message-
>> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>> Sent: Monday, September 19, 2016 2:34 PM
>> To: dev@openvswitch.org; Daniele Di Proietto 
>> Cc: Dyasly Sergey ; Heetae Ahn
>> ; Chandran, Sugesh
>> ; Bodireddy, Bhanuprakash
>> ; Loftus, Ciara
>> ; Ilya Maximets 
>> Subject: [PATCH] netdev-dpdk: Configure flow control only when necessary.
>>
>> It is not necessary to touch the physical device each time, if the
>> configuration has not been changed. Also, few style issues fixed.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  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 6d334db..1632b97 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1057,6 +1057,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)
> [Sugesh] I assume this mutex is needed irrelevant of flow director 
> configuration.
> Can you mention this as well in the commit message.

You right. I just thought that this annotation can be treated as a style fix.
If you disagree, I can mention it in a commit-message.

>>  {
>>  int new_n_rxq;
>>
>> @@ -1071,24 +1072,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(>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;
> [Sugesh] smap_get_bool itself returns 1, 0 based on the input. I wouldn't use 
> the conditional operator to check the result.
> Same comment for the following smap_get* as well.

'smap_get_bool()' returns 'bool'.
And according to "C DIALECT" section of CodingStyle.md:
"
  Most C99 features are OK because they are widely implemented:

* bool and , but don't assume that bool or _Bool can
  only take on the values 0 or 1, because this behavior can't be
  simulated on C89 compilers.
"

This means that 'bool' must be directly converted to 0 or 1 before
using as an index.

>> +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(>mutex);
>>
>> --
>> 2.7.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Chandran, Sugesh]

Hi Ilya,
Thank you for sending out the patch.
I verified that the patch is working fine.
Please find some comments below.

Regards
_Sugesh


> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Monday, September 19, 2016 2:34 PM
> To: dev@openvswitch.org; Daniele Di Proietto 
> Cc: Dyasly Sergey ; Heetae Ahn
> ; Chandran, Sugesh
> ; Bodireddy, Bhanuprakash
> ; Loftus, Ciara
> ; Ilya Maximets 
> Subject: [PATCH] netdev-dpdk: Configure flow control only when necessary.
> 
> It is not necessary to touch the physical device each time, if the
> configuration has not been changed. Also, few style issues fixed.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  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 6d334db..1632b97 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1057,6 +1057,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)
[Sugesh] I assume this mutex is needed irrelevant of flow director 
configuration. Can you mention this as well in the commit message.
>  {
>  int new_n_rxq;
> 
> @@ -1071,24 +1072,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(>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;
[Sugesh] smap_get_bool itself returns 1, 0 based on the input. I wouldn't use 
the conditional operator to check the result.
Same comment for the following smap_get* as well.
> +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(>mutex);
> 
> --
> 2.7.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev