[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CALnjE+p3wHtcn2YpXKhHXB7E-qviap=Oe+iV-vdjCbD5k5_TUQ@mail.gmail.com>
Date: Mon, 21 Jul 2014 18:52:45 -0700
From: Pravin Shelar <pshelar@...ira.com>
To: David Miller <davem@...emloft.net>
Cc: David Laight <David.Laight@...lab.com>,
netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v6 04/11] net: Export xmit_recursion
On Mon, Jul 21, 2014 at 2:30 PM, David Miller <davem@...emloft.net> wrote:
> 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.
>
OK, since recirc needs more work, I will post the series without recirc patches.
Thanks,
Pravin.
> 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
Powered by blists - more mailing lists