netdev - RE: [net-next v5 05/15] devlink: Support get and set state of port function

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: <BY5PR12MB43225346806029AA31D63918DCC50@BY5PR12MB4322.namprd12.prod.outlook.com>
Date:   Wed, 16 Dec 2020 05:15:04 +0000
From:   Parav Pandit <parav@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>, Saeed Mahameed <saeed@...nel.org>
CC:     "David S. Miller" <davem@...emloft.net>,
        Jason Gunthorpe <jgg@...dia.com>,
        Leon Romanovsky <leonro@...dia.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        David Ahern <dsahern@...nel.org>,
        Jacob Keller <jacob.e.keller@...el.com>,
        Sridhar Samudrala <sridhar.samudrala@...el.com>,
        "david.m.ertman@...el.com" <david.m.ertman@...el.com>,
        "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
        "kiran.patil@...el.com" <kiran.patil@...el.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        Jiri Pirko <jiri@...dia.com>, Vu Pham <vuhuong@...dia.com>,
        Saeed Mahameed <saeedm@...dia.com>
Subject: RE: [net-next v5 05/15] devlink: Support get and set state of port
 function



> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Wednesday, December 16, 2020 6:08 AM
> 
> On Tue, 15 Dec 2020 01:03:48 -0800 Saeed Mahameed wrote:
> > From: Parav Pandit <parav@...dia.com>
> >
> > devlink port function can be in active or inactive state.
> > Allow users to get and set port function's state.
> >
> > When the port function it activated, its operational state may change
> > after a while when the device is created and driver binds to it.
> > Similarly on deactivation flow.
> 
> So what's the flow device should implement?
> 
> User requests deactivated, the device sends a notification to the driver
> bound to the device. What if the driver ignores it?
>
If driver ignores it, those devices are marked unusable for new allocation.
Device becomes usable only after it has act on the event.
 
> > $ devlink port function set pci/0000:06:00.0/32768 hw_addr
> > 00:00:00:00:88:88 state active
> 
> Is request to deactivate done by settings state to inactive?
>
Yes.
 
> > + * enum devlink_port_function_opstate - indicates operational state
> > + of port function
> > + * @DEVLINK_PORT_FUNCTION_OPSTATE_ATTACHED: Driver is attached
> to the
> > + function of port,
> 
> This name definitely needs to be shortened.
>
DEVLINK_PORT_FUNCTION_OPS_ATTACHED
Or
DEVLINK_PF_OPS_ATTACHED 

PF - port function
 
> > + *					    gracefufl tear down of the function,
> after
> 
> gracefufl
> 
> > + *					    inactivation of the port function,
> user should wait
> > + *					    for operational state to turn
> DETACHED.
> 
> Why do you indent the comment by 40 characters and then go over 80
> chars?
> 
Will fix it.

> > + * @DEVLINK_PORT_FUNCTION_OPSTATE_DETACHED: Driver is detached
> from the function of port; it is
> > + *					    safe to delete the port.
> > + */
> > +enum devlink_port_function_opstate {
> > +	DEVLINK_PORT_FUNCTION_OPSTATE_DETACHED,
> 
> The port function must be some Mellanox speak - for the second time - I
> have no idea what it means. Please use meaningful names.
>
It is not a Mellanox term.
Port function object is the one that represents function behind this port.
It is not a new term. Port function already exists in devlink whose operational state attribute is defined here.
 
> > devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct
> > devlink_port *por
> >
> >  	ops = devlink->ops;
> >  	err = devlink_port_function_hw_addr_fill(devlink, ops, port, msg,
> > extack, &msg_updated);
> 
> Wrap your code, please.
>
Sure, will do.