lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 11 Apr 2022 09:37:50 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Saeed Mahameed <saeedm@...dia.com>
Cc:     Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jason Gunthorpe <jgg@...dia.com>,
        linux-netdev <netdev@...r.kernel.org>,
        RDMA mailing list <linux-rdma@...r.kernel.org>,
        Raed Salem <raeds@...dia.com>
Subject: Re: [PATCH mlx5-next 01/17] net/mlx5: Simplify IPsec flow steering
 init/cleanup functions

On Sun, Apr 10, 2022 at 02:58:13PM -0700, Saeed Mahameed wrote:
> On 10 Apr 20:21, Leon Romanovsky wrote:
> > On Sun, Apr 10, 2022 at 09:46:20AM -0700, Saeed Mahameed wrote:
> > > On 10 Apr 11:28, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro@...dia.com>
> > > >
> > > > Cleanup IPsec FS initialization and cleanup functions.
> > > 
> > > Can you be more clear about what are you cleaning up ?
> > > 
> > > unfolding/joining static functions shouldn't be considered as cleanup.
> > 
> > And how would you describe extensive usage of one time called functions
> > that have no use as standalone ones?
> > 
> 
> Functional programming.

The separation between various modules and their functions is function
programming, but wrapper in .c file over basic kernel primitive (kzalloc)
is obfuscation.

> 
> > This patch makes sure that all flow steering initialized and cleaned at
> > one place and allows me to present coherent picture of what is needed
> > for IPsec FS.
> > 
> 
> This is already the case before this patch.

With two main differences: 
First, it is is less code to achieve the same and second, it is easy
to read (I read this code a lot lately).

 3 files changed, 27 insertions(+), 54 deletions(-)

> 
> > You should focus on the end result of this series rather on single patch.
> > 15 files changed, 320 insertions(+), 839 deletions(-)
> 
> Overall the series is fine, this patch in particular is unnecessary cancelation of
> others previous decisions, which i personally like and might as well have
> suggested myself, so let's avoid such clutter.

Sorry, but I disagree that removal of useless indirection that hurts
readability is clutter. It is refactoring.

Please focus on end goal of this series.

Thanks

> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ