On Wed, Nov 17, 2021 at 08:49:29PM -0800, Jakub Kicinski wrote:
> On Wed, 17 Nov 2021 20:26:20 +0200 Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxx>
> >
> > devlink_port_register() is in-kernel API and as such can't really fail
> > as long as driver author didn't make a mistake by providing already existing
> > port index. Instead of relying on various error prints from the driver,
> > convert the existence check to be WARN_ON(), so such a mistake will be
> > caught easier.
> >
> > As an outcome of this conversion, it was made clear that this function
> > should be void and devlink->lock was intended to protect addition to
> > port_list.
>
> Leave this error checking in please.
Are you referring to error checks in the drivers or the below section
from devlink_port_register()?
mutex_lock(&devlink->lock);
if (devlink_port_index_exists(devlink, port_index)) {
mutex_unlock(&devlink->lock);
return -EEXIST;
}
Because if it is latter, any driver (I didn't find any) that will rely
on this -EEXIST field should have some sort of locking in top level.
Otherwise nothing will prevent from doing port unregister right
before "return --EXEEXIST".
So change to WARN_ON() will be much more effective in finding wrong
drivers, because they manage port_index and not devlink.
And because this function can't fail, the drivers have a plenty of dead
code.
Thanks
