linux-kernel - Re: [RFC PATCH net-next v5 04/11] net/smc: implement some unsupported operations of loopback-ism

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: Thu, 04 Apr 2024 13:42:10 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Wen Gu <guwen@...ux.alibaba.com>, Gerd Bayer <gbayer@...ux.ibm.com>,
        wintera@...ux.ibm.com, twinkler@...ux.ibm.com, hca@...ux.ibm.com,
        gor@...ux.ibm.com, agordeev@...ux.ibm.com, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        wenjia@...ux.ibm.com, jaka@...ux.ibm.com
Cc: borntraeger@...ux.ibm.com, svens@...ux.ibm.com, alibuda@...ux.alibaba.com,
        tonylu@...ux.alibaba.com, linux-kernel@...r.kernel.org,
        linux-s390@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next v5 04/11] net/smc: implement some
 unsupported operations of loopback-ism

On Thu, 2024-04-04 at 17:32 +0800, Wen Gu wrote:
> 
> On 2024/4/4 00:25, Gerd Bayer wrote:
> > On Sun, 2024-03-24 at 21:55 +0800, Wen Gu wrote:
> > > This implements some operations that loopback-ism does not support
> > > currently:
> > >   - vlan operations, since there is no strong use-case for it.
> > >   - signal_event operations, since there is no event to be processed
> > > by the loopback-ism device.
> > 
> > Hi Wen,
> > 
> > I wonder if the these operations that are not supported by loopback-ism
> > should rather be marked "optional" in the struct smcd_ops, and the
> > calling code should call these only when they are implemented.
> > 
> > Of course this would mean more changes to net/smc/smc_core.c - but
> > loopback-ism could omit these "boiler-plate" functions.
> > 
> 
> Hi Gerd.
> 
> Thank you for the thoughts! I agree that checks like 'if(smcd->ops->xxx)'
> can avoid the device driver from implementing unsupported operations. But I
> am afraid that which operations need to be defined as 'optional' may differ
> from different device perspectives (e.g. for loopback-ism they are vlan-related
> opts and signal_event). So I perfer to simply let the smc protocol assume
> that all operations have been implemented, and let drivers to decide which
> ones are unsupported in implementation. What do you think?
> 
> Thanks!
> 

I agree with Gerd, in my opinion it is better to document ops as
optional and then allow their function pointers to be NULL and check
for that. Acting like they are supported and then they turn out to be
nops to me seems to contradict the principle of least surprises. I also
think we can find a subset of mandatory ops without which SMC-D is
impossible and then everything else should be optional.

As a first guess I think the following options may be mandatory:

* query_remote_gid()
* register_dmb()/unregister_dmb()
* move_data()
  For this one could argue that either move_data() or
  attach_dmb()/detach_dmb() is required though personally I would
  prefer to always have move_data() as a fallback and simple API
* supports_v2()
* get_local_gid()
* get_chid()
* get_dev()
> >