Re: [PATCH net-next 2/4] net: dsa: Link aggregation support

[Andrew Lunn <andrew@lunn.ch>]

> If you go with the static array, you theoretically can not get the
> equivalent of an ENOMEM. Practically though you have to iterate through
> the array and look for a free entry, but you still have to put a return
> statement at the bottom of that function, right? Or panic I suppose. My
> guess is you end up with:
> 
>     struct dsa_lag *dsa_lag_get(dst)
>     {
>         for (lag in dst->lag_array) {
>             if (lag->dev == NULL)
>                 return lag;
>         }
> 
>         return NULL;
>     }

I would put a WARN() in here, and return the NULL pointer. That will
then likely opps soon afterwards and kill of the user space tool
configuring the LAG, maybe leaving rtnl locked, and so all network
configuration dies. But that is all fine, since you cannot have more
LAGs than ports. This can never happen. If it does happen, something
is badly wrong and we want to know about it. And so a very obvious
explosion is good.

> So now we have just traded dealing with an ENOMEM for a NULL pointer;
> pretty much the same thing.

ENOMEM you have to handle correctly, unwind everything and leaving the
stack in the same state as before. Being out of memory is not a reason
to explode. Have you tested this? It is the sort of thing which does
not happen very often, so i expect is broken.

   Andrew