[ovs-dev] [PATCH 03/12] odp-util: Bump up maximum number of ODP actions.
Jesse Gross
jesse at nicira.com
Tue Dec 7 20:59:01 UTC 2010
On Tue, Dec 7, 2010 at 12:17 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Tue, Dec 07, 2010 at 11:34:38AM -0800, Jesse Gross wrote:
>> On Tue, Dec 7, 2010 at 11:00 AM, Ben Pfaff <blp at nicira.com> wrote:
>> > The kernel supports more than a single page of actions now, so userspace
>> > should be able to take advantage of this.
>> >
>> > Upcoming commits will completely replace this data structure but this
>> > commit makes the bug fix clear and is suitable for cherry-picking to
>> > long-term support branches.
>>
>> MAX_ODP_ACTIONS isn't just a theoretical maximum - it's an array size
>> in "struct odp_actions". So every time that we use it, we end up
>> putting 16 pages on the stack. That doesn't seem like a great idea to
>> me. It's also overkill: the kernel only allows a maximum of 2 * the
>> maximum number of ports. At 1024 ports and 8 bytes per action, that's
>> a maximum of 16k that we could usefully use.
>
> I don't see why we should care about 64 kB on the stack. It's just a
> single "subl $65536, %esp" instruction. The stack is both paged and
> growable. And we never have more than one of these things on the stack
> at once, and we only have one thread.
Sure, it's OK on Linux. I was thinking about less flexible platforms
though, like an embedded system on a switch.
>
> But I'll happily reduce the 64 kB to 16 kB. For now I've applied that
> change.
>
> The upcoming patches change the actions to be dynamically allocated
> using struct ofpbuf.
OK, that's fine. In the future, we won't use this and we don't
currently run on any platform except for Linux.
>
>> I know that we had to bump up the number of actions in the kernel to
>> handle flooding on large numbers of ports. So in reality, it looks
>> like it was just wrapping around the array and overwriting some of the
>> earlier ports?
>
> Yes. This didn't go up to the kernel, though: the overwrites get
> detected in xlate_actions(), recorded, and the flow setup is aborted.
> So those packets would get dropped, I guess, until the MAC address was
> learned.
Hmm, that does appear to be what's happening. However, I thought that
the kernel changes fixed at least some errors, so some flows must have
made it.
More information about the dev
mailing list