netdev - RE: [PATCH net-next v2 4/8] 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]

Date:   Fri, 18 Sep 2020 03:30:45 +0000
From:   Parav Pandit <parav@...dia.com>
To:     David Ahern <dsahern@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     Jiri Pirko <jiri@...dia.com>
Subject: RE: [PATCH net-next v2 4/8] devlink: Support get and set state of
 port function



> From: David Ahern <dsahern@...il.com>
> Sent: Friday, September 18, 2020 1:54 AM
> 
> On 9/17/20 11:20 AM, Parav Pandit wrote:
> > diff --git a/net/core/devlink.c b/net/core/devlink.c index
> > d152489e48da..c82098cb75da 100644
> > --- a/net/core/devlink.c
> > +++ b/net/core/devlink.c
> > @@ -595,6 +598,40 @@ devlink_port_function_hw_addr_fill(struct devlink
> *devlink, const struct devlink
> >  	return 0;
> >  }
> >
> > +static bool devlink_port_function_state_valid(u8 state)
> 
> you have a named enum so why not 'enum devlink_port_function_state state'?
>
Right. I should. I missed it.
Will do.
 
> 
> > +{
> > +	return state == DEVLINK_PORT_FUNCTION_STATE_INACTIVE ||
> > +	       state == DEVLINK_PORT_FUNCTION_STATE_ACTIVE;
> > +}
> > +
> > +static int devlink_port_function_state_fill(struct devlink *devlink, const struct
> devlink_ops *ops,
> > +					    struct devlink_port *port, struct
> sk_buff *msg,
> > +					    struct netlink_ext_ack *extack, bool
> *msg_updated) {
> > +	enum devlink_port_function_state state;
> > +	int err;
> > +
> > +	if (!ops->port_function_state_get)
> > +		return 0;
> > +
> > +	err = ops->port_function_state_get(devlink, port, &state, extack);
> > +	if (err) {
> > +		if (err == -EOPNOTSUPP)
> > +			return 0;
> > +		return err;
> > +	}
> > +	if (!devlink_port_function_state_valid(state)) {
> > +		WARN_ON(1);
> 
> WARN_ON_ONCE at most.
> 
Yep. I will change to WARN_ON_ONCE.