netdev - Re: [PATCH net-next v6 04/11] net: Export xmit

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: <20140721.143040.505863988764762548.davem@davemloft.net>
Date:	Mon, 21 Jul 2014 14:30:40 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	pshelar@...ira.com
Cc:	David.Laight@...lab.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v6 04/11] net: Export xmit_recursion

From: Pravin Shelar <pshelar@...ira.com>
Date: Mon, 21 Jul 2014 10:01:52 -0700

> On Mon, Jul 21, 2014 at 1:52 AM, David Laight <David.Laight@...lab.com> wrote:
>> From: Pravin B Shelar
>>> Next patch adds re-circulation action to OVS. This is recursive
>>> action needs to be limited to safe number recursions to
>>> avoid overflowing kernel stack space.  This patch export
>>> xmit_recursion so that OVS can increment and check the value
>>> before executing recirc action.
>> ...
>>> +#define NET_RECURSION_LIMIT 10
>>
>> I'm not entirely sure that any amount of recursion is a good idea
>> inside the kernel.
>> If the nested call frame is small enough so that your 10 nestings
>> are valid it must also be reasonably easy to arrange for the code
>> to be converted to a loop.
>>
>> Remember that the highest stack use is likely to be inside some
>> error path (probably inside printk) so any experiments that seem
>> to imply that that stack use is ok are doomed.
>>
>> Static analysis is possible (indirect calls make it tricky) provided
>> there is no recursion.
>> I did some static stack use analysis for an embedded system many
>> years ago. It showed that we needed much larger stacks that we'd
>> allowed for.
> 
> I am using same recursion counter as networking stack does. Recursion
> in OVS has lesser cost compared to recursion in networking stack, So
> it is conservative to use same counter to track recursion in OVS as
> networking stack recursion.

The networking core uses it is to prevent untenable configurations.

I was going to bring this issue up the other day but I've been
to busy to do so.  I really want you to seriously look into using
an iterative algorithm, it can't be so hard inside of openvswitch
itself.

Any time you go into openvswitch from "somewhere else" you initialize
a per-cpu structure that keeps track of work to do, if you need to pop
an MPLS header or whatever and recurse, record it in that structure
and just return.

This both eliminates recursion, and is faster, since you'll be reusing
the same stack frame that's fresh and hot in the caches already.

I want to do this in the core networking at some point too.
--
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