RE: [PATCH net-next v2 4/8] devlink: Support get and set state of port function

[Parav Pandit]

> From: David Ahern <dsah...@gmail.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.