On Thu, Sep 06, 2018 at 12:37:17AM +0300, Or Gerlitz wrote:

On Wed, Sep 5, 2018 at 9:11 PM, Leon Romanovsky  wrote:

quoted

On Wed, Sep 05, 2018 at 10:38:00AM -0600, Jason Gunthorpe wrote:

quoted

On Wed, Sep 05, 2018 at 08:10:25AM +0300, Leon Romanovsky wrote:

quoted

quoted

quoted

-       int en_encap_decap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN);
+       int en_encap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN_ENCAP);
+       int en_decap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN_DECAP);

Yuk, please don't use !!.

  bool en_decap = flags & MLX5_FLOW_TABLE_TUNNEL_EN_DECAP;

We need to provide en_encap and en_decap as an input to MLX5_SET(...)
which is passed to FW as 0 or 1.

Boolean type is declared in C as int and treated as zero for false
and any other value for true,

No, that isn't right, the kernel uses C99's _Bool intrinsic type, which
is guaranteed to only hold 0 or 1 by the compiler.

See types.h:

typedef _Bool                   bool;

Exciting, it took me a while to find C99 standard and relevant 6.3.1.2.
Anyway, this patch didn't change previous functionality, which used "!!"
convention.

so? if we didn't do things properly prior to the patch, why not fixing it along
with the patch? lets fix

Or,

What exactly "to fix"? Both code lines:
1. Have correct syntax
2. Implement proper C99
3. Give same compiler code
4. Have same readability

There is nothing to fix.

And this patch is already merged, so if you truly care about this,
please go ahead and prepare patch for whole driver, or better for
whole kernel.

 kernel git:(rdma-next) git grep "\!\!" |wc -l
8125

Thanks