netdev - Re: [PATCH] r8169: revert "add byte queue limit support".

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 24 Jul 2012 07:09:16 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Francois Romieu <romieu@...zoreil.com>
Cc:	David Miller <davem@...emloft.net>, hayeswang@...ltek.com,
	netdev@...r.kernel.org,
	Alex Villacís Lasso 
	<a_villacis@...osanto.com>, Josh Boyer <jwboyer@...hat.com>,
	Tom Herbert <therbert@...gle.com>
Subject: Re: [PATCH] r8169: revert "add byte queue limit support".

On Tue, 2012-07-24 at 07:06 +0200, Eric Dumazet wrote:
> On Mon, 2012-07-23 at 22:55 +0200, Francois Romieu wrote:
> > This reverts commit 036dafa28da1e2565a8529de2ae663c37b7a0060.
> > 
> > First it appears in bisection, then reverting it solves the usual
> > netdev watchdog problem for different people. I don't have a proper
> > fix yet so get rid of it.
> > 
> > Bisected-and-reported-by: Alex Villacís Lasso <a_villacis@...osanto.com>
> > Signed-off-by: Francois Romieu <romieu@...zoreil.com>
> > Cc: Josh Boyer <jwboyer@...hat.com>
> > Cc: Hayes Wang <hayeswang@...ltek.com>
> > ---
> 
> bisection is not always the right way to qualify a problem.
> 
> BQL in itself had some fixes coming _after_ commit 036dafa28da1e2565
> 
> Is there an easy way to reproduce the problem ?
> 
> Thanks
> 

BQL fixes are :

commit 914bec1011a25f65cdc94988a6f974bfb9a3c10d
Author: Hiroaki SHIMODA <shimoda.hiroaki@...il.com>
Date:   Wed May 30 12:25:37 2012 +0000

    bql: Avoid possible inconsistent calculation.
    
    dql->num_queued could change while processing dql_completed().
    To provide consistent calculation, added an on stack variable.
    
    Signed-off-by: Hiroaki SHIMODA <shimoda.hiroaki@...il.com>
    Cc: Tom Herbert <therbert@...gle.com>
    Cc: Eric Dumazet <eric.dumazet@...il.com>
    Cc: Denys Fedoryshchenko <denys@...p.net.lb>
    Signed-off-by: Eric Dumazet <edumazet@...gle.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>

commit 25426b794efdc70dde7fd3134dc56fac3e7d562d
Author: Hiroaki SHIMODA <shimoda.hiroaki@...il.com>
Date:   Wed May 30 12:25:19 2012 +0000

    bql: Avoid unneeded limit decrement.
    
    When below pattern is observed,
    
                                                   TIME
           dql_queued()         dql_completed()     |
          a) initial state                          |
                                                    |
          b) X bytes queued                         V
    
          c) Y bytes queued
                               d) X bytes completed
          e) Z bytes queued
                               f) Y bytes completed
    
    a) dql->limit has already some value and there is no in-flight
packet.
    b) X bytes queued.
    c) Y bytes queued and excess limit.
    d) X bytes completed and dql->prev_ovlimit is set and also
       dql->prev_num_queued is set Y.
    e) Z bytes queued.
    f) Y bytes completed. inprogress and prev_inprogress are true.
    
    At f), according to the comment, all_prev_completed becomes
    true and limit should be increased. But POSDIFF() ignores
    (completed == dql->prev_num_queued) case, so limit is decreased.
    
    Signed-off-by: Hiroaki SHIMODA <shimoda.hiroaki@...il.com>
    Cc: Tom Herbert <therbert@...gle.com>
    Cc: Eric Dumazet <eric.dumazet@...il.com>
    Cc: Denys Fedoryshchenko <denys@...p.net.lb>
    Acked-by: Eric Dumazet <edumazet@...gle.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>

commit 0cfd32b736ae0c36b42697584811042726c07cba
Author: Hiroaki SHIMODA <shimoda.hiroaki@...il.com>
Date:   Wed May 30 12:24:39 2012 +0000

    bql: Fix POSDIFF() to integer overflow aware.
    
    POSDIFF() fails to take into account integer overflow case.
    
    Signed-off-by: Hiroaki SHIMODA <shimoda.hiroaki@...il.com>
    Cc: Tom Herbert <therbert@...gle.com>
    Cc: Eric Dumazet <eric.dumazet@...il.com>
    Cc: Denys Fedoryshchenko <denys@...p.net.lb>
    Acked-by: Eric Dumazet <edumazet@...gle.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html