netdev - Re: [PATCH RFC v4 net-next 11/13] pds

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]

Message-ID: <398bef7a-795b-d105-d8e5-57ef1c39049c@amd.com>
Date:   Tue, 14 Mar 2023 09:53:25 -0700
From:   Shannon Nelson <shannon.nelson@....com>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     brett.creeley@....com, davem@...emloft.net, netdev@...r.kernel.org,
        kuba@...nel.org, drivers@...sando.io, jiri@...nulli.us
Subject: Re: [PATCH RFC v4 net-next 11/13] pds_core: add the aux client API

On 3/14/23 5:14 AM, Leon Romanovsky wrote:
> On Tue, Mar 07, 2023 at 09:13:08PM -0800, Shannon Nelson wrote:
>> Add the client API operations for registering, unregistering,
>> and running adminq commands.  We expect to add additional
>> operations for other clients, including requesting additional
>> private adminqs and IRQs, but don't have the need yet,
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
>> ---
>>   drivers/net/ethernet/amd/pds_core/auxbus.c | 134 ++++++++++++++++++++-
>>   include/linux/pds/pds_auxbus.h             |  42 +++++++
>>   2 files changed, 174 insertions(+), 2 deletions(-)
> 
> <...>
> 
>> +static struct pds_core_ops pds_core_ops = {
>> +     .register_client = pds_client_register,
>> +     .unregister_client = pds_client_unregister,
>> +     .adminq_cmd = pds_client_adminq_cmd,
>> +};
> 
> <...>
> 
>> +/*
>> + *   ptrs to functions to be used by the client for core services
>> + */
>> +struct pds_core_ops {
>> +     /* .register() - register the client with the device
>> +      * padev:  ptr to the client device info
>> +      * Register the client with the core and with the DSC.  The core
>> +      * will fill in the client padrv->client_id for use in calls
>> +      * to the DSC AdminQ
>> +      */
>> +     int (*register_client)(struct pds_auxiliary_dev *padev);
>> +
>> +     /* .unregister() - disconnect the client from the device
>> +      * padev:  ptr to the client device info
>> +      * Disconnect the client from the core and with the DSC.
>> +      */
>> +     int (*unregister_client)(struct pds_auxiliary_dev *padev);
>> +
>> +     /* .adminq_cmd() - process an adminq request for the client
>> +      * padev:  ptr to the client device
>> +      * req:     ptr to buffer with request
>> +      * req_len: length of actual struct used for request
>> +      * resp:    ptr to buffer where answer is to be copied
>> +      * flags:   optional flags defined by enum pds_core_adminq_flags
>> +      *          and used for more flexible adminq behvior
>> +      *
>> +      * returns 0 on success, or
>> +      *         negative for error
>> +      * Client sends pointers to request and response buffers
>> +      * Core copies request data into pds_core_client_request_cmd
>> +      * Core sets other fields as needed
>> +      * Core posts to AdminQ
>> +      * Core copies completion data into response buffer
>> +      */
>> +     int (*adminq_cmd)(struct pds_auxiliary_dev *padev,
>> +                       union pds_core_adminq_cmd *req,
>> +                       size_t req_len,
>> +                       union pds_core_adminq_comp *resp,
>> +                       u64 flags);
>> +};
> 
> I don't expect to see any register/unregister AUX client code at all.
> 
> All clients are registered and unregistered through
> auxiliary_driver_register()/auxiliary_driver_unregister() calls and
> perform as standalone drivers.
> 
> Maybe client, register and unregister words means something else in this
> series..

Yeah, I'm not thrilled with the overlap in nomenclature either.  In this 
case we're talking about the logic in the pds_vdpa module connecting to 
the services needed in the device FW, and getting a client_id from the 
FW that is used for tracking client context in the FW.  Maybe these 
names can change to something like "fw_client_reg" and "fw_client_unreg" 
- would that make it more clear?

sln


> 
> Thanks
> 
>>   #endif /* _PDSC_AUXBUS_H_ */
>> --
>> 2.17.1
>>