netdev - Re: [patch net-next v2 14/15] devlink: move port_del() to devlink_port

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] [thread-next>] [day] [month] [year] [list]

Date: Sun, 28 May 2023 23:33:34 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiri Pirko <jiri@...nulli.us>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, davem@...emloft.net,
 edumazet@...gle.com, leon@...nel.org, saeedm@...dia.com, moshe@...dia.com,
 jesse.brandeburg@...el.com, anthony.l.nguyen@...el.com, tariqt@...dia.com,
 idosch@...dia.com, petrm@...dia.com, simon.horman@...igine.com,
 ecree.xilinx@...il.com, habetsm.xilinx@...il.com,
 michal.wilczynski@...el.com, jacob.e.keller@...el.com
Subject: Re: [patch net-next v2 14/15] devlink: move port_del() to
 devlink_port_ops

On Sat, 27 May 2023 09:42:45 +0200 Jiri Pirko wrote:
> >I didn't think this thru last time, I thought port_new will move 
> >in another patch, but that's impossible (obviously?).
> >
> >Isn't it kinda weird that the new callback is in one place and del
> >callback is in another? Asymmetric ?  
> 
> Yeah, I don't know how to do it differently. port_new() has to be
> devlink op, as it operates not on the port but on the device. However,
> port_del() operates on device. I was thinking about changing the name of
> port_del() to port_destructor() or something like that which would make
> the symmetricity issue bit less visible. IDK, up to you. One way or
> another, I think this could be easily done as a follow-up (I have 15
> patches now already anyway).

One could argue logically removing a port is also an operation of 
the parent (i.e. the devlink instance). The fact that the port gets
destroyed in the process is secondary. Ergo maybe we should skip 
this patch?