Re: [PATCH net] net_sched: prio: properly report out of memory errors

[Eric Dumazet]

On Sun, 2016-06-12 at 20:45 -0700, Cong Wang wrote:
> On Sun, Jun 12, 2016 at 4:21 PM, Eric Dumazet  wrote:
> > +   struct Qdisc *child;
> > +
> > +   if (q->queues[i] != _qdisc)
> > +   continue;
> > +
> > +   child = qdisc_create_dflt(sch->dev_queue, _qdisc_ops,
> > + TC_H_MAKE(sch->handle, i + 1));
> > +   if (!child)
> > +   return -ENOMEM;
> 
> Since this is inside a loop, shouldn't we kfree the previous child
> creations when we fail?

You're right.

prio_init() needs to do the cleanup, as prio_destroy() wont be called
from qdisc_create()

I am testing a fix with fault injection.

Thanks.

Re: [PATCH net] net_sched: prio: properly report out of memory errors

[Cong Wang]

On Sun, Jun 12, 2016 at 4:21 PM, Eric Dumazet  wrote:
> +   struct Qdisc *child;
> +
> +   if (q->queues[i] != _qdisc)
> +   continue;
> +
> +   child = qdisc_create_dflt(sch->dev_queue, _qdisc_ops,
> + TC_H_MAKE(sch->handle, i + 1));
> +   if (!child)
> +   return -ENOMEM;

Since this is inside a loop, shouldn't we kfree the previous child
creations when we fail?



> +   sch_tree_lock(sch);
> +   q->queues[i] = child;
> +   sch_tree_unlock(sch);
> }
> return 0;
>  }
>
>

Re: [PATCH net] net_sched: prio: properly report out of memory errors

[David Miller]

From: Eric Dumazet 
Date: Sun, 12 Jun 2016 16:21:47 -0700

> From: Eric Dumazet 
> 
> At Qdisc creation or change time, prio_tune() creates missing
> pfifo qdiscs but does not return an error code if one
> qdisc could not be allocated.
> 
> Leaving a qdisc in non operational state without telling user
> anything about this problem is not good.
> 
> Also, testing if we replace something different than noop_qdisc
> a second time makes no sense so I removed useless code.
> 
> Signed-off-by: Eric Dumazet 

Applied.