All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Saeed Mahameed <saeedm@nvidia.com>
Cc: Paolo Abeni <pabeni@redhat.com>, Jakub Kicinski <kuba@kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	Jason Gunthorpe <jgg@nvidia.com>,
	linux-netdev <netdev@vger.kernel.org>,
	RDMA mailing list <linux-rdma@vger.kernel.org>,
	Raed Salem <raeds@nvidia.com>
Subject: Re: [PATCH mlx5-next 01/17] net/mlx5: Simplify IPsec flow steering init/cleanup functions
Date: Mon, 11 Apr 2022 09:37:50 +0300	[thread overview]
Message-ID: <YlPMvuDXHFsZReLt@unreal> (raw)
In-Reply-To: <20220410215813.sxqmvmm5wkeguj6y@sx1>

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@nvidia.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

> 
> 

  reply	other threads:[~2022-04-11  6:38 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-10  8:28 [PATCH mlx5-next 00/17] Extra IPsec cleanup Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 01/17] net/mlx5: Simplify IPsec flow steering init/cleanup functions Leon Romanovsky
2022-04-10 16:46   ` Saeed Mahameed
2022-04-10 17:21     ` Leon Romanovsky
2022-04-10 21:58       ` Saeed Mahameed
2022-04-11  6:37         ` Leon Romanovsky [this message]
2022-04-10  8:28 ` [PATCH mlx5-next 02/17] net/mlx5: Check IPsec TX flow steering namespace in advance Leon Romanovsky
2022-04-10 23:46   ` Saeed Mahameed
2022-04-11  6:21     ` Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 03/17] net/mlx5: Don't hide fallback to software IPsec in FS code Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 04/17] net/mlx5: Reduce useless indirection in IPsec FS add/delete flows Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 05/17] net/mlx5: Store IPsec ESN update work in XFRM state Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 06/17] net/mlx5: Remove useless validity check Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 07/17] net/mlx5: Merge various control path IPsec headers into one file Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 08/17] net/mlx5: Remove accel notations and indirections from esp functions Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 09/17] net/mlx5: Simplify HW context interfaces by using SA entry Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 10/17] net/mlx5: Clean IPsec FS add/delete rules Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 11/17] net/mlx5: Make sure that no dangling IPsec FS pointers exist Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 12/17] net/mlx5: Don't advertise IPsec netdev support for non-IPsec device Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 13/17] net/mlx5: Simplify IPsec capabilities logic Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 14/17] net/mlx5: Remove not-supported ICV length Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 15/17] net/mlx5: Cleanup XFRM attributes struct Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 16/17] net/mlx5: Allow future addition of IPsec object modifiers Leon Romanovsky
2022-04-10  8:28 ` [PATCH mlx5-next 17/17] net/mlx5: Don't perform lookup after already known sec_path Leon Romanovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YlPMvuDXHFsZReLt@unreal \
    --to=leon@kernel.org \
    --cc=davem@davemloft.net \
    --cc=jgg@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=raeds@nvidia.com \
    --cc=saeedm@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.