lkml.org 
[lkml]   [2022]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 841/846] sch_api: Dont skip qdisc attach on ingress
    Date
    From: Maxim Mikityanskiy <maximmi@nvidia.com>

    commit de2d807b294d3d2ce5e59043ae2634016765d076 upstream.

    The attach callback of struct Qdisc_ops is used by only a few qdiscs:
    mq, mqprio and htb. qdisc_graft() contains the following logic
    (pseudocode):

    if (!qdisc->ops->attach) {
    if (ingress)
    do ingress stuff;
    else
    do egress stuff;
    }
    if (!ingress) {
    ...
    if (qdisc->ops->attach)
    qdisc->ops->attach(qdisc);
    } else {
    ...
    }

    As we see, the attach callback is not called if the qdisc is being
    attached to ingress (TC_H_INGRESS). That wasn't a problem for mq and
    mqprio, since they contain a check that they are attached to TC_H_ROOT,
    and they can't be attached to TC_H_INGRESS anyway.

    However, the commit cited below added the attach callback to htb. It is
    needed for the hardware offload, but in the non-offload mode it
    simulates the "do egress stuff" part of the pseudocode above. The
    problem is that when htb is attached to ingress, neither "do ingress
    stuff" nor attach() is called. It results in an inconsistency, and the
    following message is printed to dmesg:

    unregister_netdevice: waiting for lo to become free. Usage count = 2

    This commit addresses the issue by running "do ingress stuff" in the
    ingress flow even in the attach callback is present, which is fine,
    because attach isn't going to be called afterwards.

    The bug was found by syzbot and reported by Eric.

    Fixes: d03b195b5aa0 ("sch_htb: Hierarchical QoS hardware offload")
    Signed-off-by: Maxim Mikityanskiy <maximmi@nvidia.com>
    Reported-by: Eric Dumazet <edumazet@google.com>
    Reviewed-by: Eric Dumazet <edumazet@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/sched/sch_api.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    --- a/net/sched/sch_api.c
    +++ b/net/sched/sch_api.c
    @@ -1062,7 +1062,7 @@ static int qdisc_graft(struct net_device

    qdisc_offload_graft_root(dev, new, old, extack);

    - if (new && new->ops->attach)
    + if (new && new->ops->attach && !ingress)
    goto skip;

    for (i = 0; i < num_q; i++) {

    \
     
     \ /
      Last update: 2022-01-24 23:11    [W:4.453 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site