netdev - Re: [PATCHv2] ipv4: Do not cache routing failures due to disabled forwarding.

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]

Message-ID: <541A7EFA.8070506@green-communications.fr>
Date:	Thu, 18 Sep 2014 08:43:06 +0200
From:	Nicolas Cavallari <nicolas.cavallari@...en-communications.fr>
To:	Julian Anastasov <ja@....bg>
CC:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Alexey Kuznetsov <kuznet@....inr.ac.ru>,
	James Morris <jmorris@...ei.org>,
	Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [PATCHv2] ipv4: Do not cache routing failures due to disabled
 forwarding.

On 18/09/2014 07:17, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 15 Sep 2014, Nicolas Cavallari wrote:
> 
>> If we cache them, the kernel will reuse them, independently of
>> whether forwarding is enabled or not.  Which means that if forwarding is
>> disabled on the input interface where the first routing request comes
>> from, then that unreachable result will be cached and reused for
>> other interfaces, even if forwarding is enabled on them.
>>
>> This can be verified with two interfaces A and B and an output interface
>> C, where B has forwarding enabled, but not A and trying
>> ip route get $dst iif A from $src && ip route get $dst iif B from $src
>>
>> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@...en-communications.fr>
> 
> 	Looks good to me,
> 
> Reviewed-by: Julian Anastasov <ja@....bg>
> 
> 	Still, I fail to see how the compiler optimizes
> the jump, 'goto local_input' still jumps to res.fi check.
> I tried even likely() after 'local_input:" checks for
> res.fi and !itag but the 'if (res.fi) {' block is still
> moved below and reached with jnz. I expected likely() to
> prefer the res.fi != NULL path.

Different compiler/arch/options perhaps ?  I'm using Debian's GCC
4.9.1-11 on amd64 with a defconfig + CONFIG_DEBUG_INFO. It does this:

.LBE2727:
        .loc 1 1800 0
        movb    $7, -102(%rbp)  #, res.type
        .loc 1 1801 0
        movq    $0, -96(%rbp)   #, res.fi
        xorl    %ecx, %ecx      # D.60248
        .loc 1 1653 0
        xorl    %r12d, %r12d    # flags
.LVL737:
        .loc 1 1749 0
        xorl    %r15d, %r15d    # do_cache
.LVL738:
        jmp     .L784   #

where .L784 is :
        .loc 1 1762 0
        movl    324(%r11), %edi # in_dev_74->cnf.data,
        xorl    %esi, %esi      # D.60250
.LVL601:
        movq    %r10, -128(%rbp)        # skb, %sfp
        testl   %edi, %edi      #
        movq    264(%r14), %rdi # _75->loopback_dev, _75->loopback_dev
        setne   %sil    #, D.60250
        xorl    %edx, %edx      #
.LVL602:
        call    rt_dst_alloc    #

As for the res.fi and itag check in local_input, it jumps only when the
test fails.
--
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