This looks good in principle.
Test 706 (ofproto-dpif - too many output actions) fails on this, as it
expects the old VLOG_ERR in the logs.
There is also an error from ofproto_dpif_xlate in this case,
complaining about resubmits yielding >64kB of actions. I assume this
is intended to remain as a hint in the logs that something's a bit
strange, even though we should now handle this case.
On Wed, Oct 9, 2013 at 6:09 PM, Ben Pfaff <b...@nicira.com> wrote:
>>
>> If the datapath actions exceed the maximum size of a Netlink attribute
>> (about 64 kB), then previously we would assert-fail (before commit
>> 542024c4c3d36 "ofproto-dpif-xlate: Suppress oversize datapath actions.")
>> or just drop all of them (after that commit). This commit makes OVS cope
>> by slow-pathing the flow and executing all of its actions in userspace.
>>
>> Signed-off-by: Ben Pfaff <b...@nicira.com>
>> ---
>> lib/dpif.c | 4 +++-
>> ofproto/ofproto-dpif-xlate.c | 11 ++++++-----
>> 2 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/dpif.c b/lib/dpif.c
>> index b284e13..ed84f5f 100644
>> --- a/lib/dpif.c
>> +++ b/lib/dpif.c
>> @@ -1183,6 +1183,8 @@ dpif_execute__(struct dpif *dpif, const struct
>> dpif_execute *execute)
>> * OVS_ACTION_ATTR_USERSPACE actions it passes the packet through to the
>> dpif
>> * implementation.
>> *
>> + * This works even if 'actions_len' is too long for a Netlink attribute.
>> + *
>> * Returns 0 if successful, otherwise a positive errno value. */
>> int
>> dpif_execute(struct dpif *dpif,
>> @@ -1198,7 +1200,7 @@ dpif_execute(struct dpif *dpif,
>> execute.actions = actions;
>> execute.actions_len = actions_len;
>> execute.packet = buf;
>> - execute.needs_help = needs_help;
>> + execute.needs_help = needs_help || nl_attr_oversized(actions_len);
>> return dpif_execute__(dpif, &execute);
>> }
>>
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index 4fb0d5e..ec83db7 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -49,6 +49,7 @@
>> #include "vlog.h"
>>
>> COVERAGE_DEFINE(xlate_actions);
>> +COVERAGE_DEFINE(xlate_actions_oversize);
>>
>> VLOG_DEFINE_THIS_MODULE(ofproto_dpif_xlate);
>>
>> @@ -2832,11 +2833,11 @@ xlate_actions__(struct xlate_in *xin, struct
>> xlate_out *xout)
>>
>> if (nl_attr_oversized(ctx.xout->odp_actions.size)) {
>> /* These datapath actions are too big for a Netlink attribute, so
>> we
>> - * can't execute them. */
>> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
>> -
>> - VLOG_ERR_RL(&rl, "discarding oversize datapath actions");
>> - ofpbuf_clear(&ctx.xout->odp_actions);
>> + * can't hand them to the kernel directly. dpif_execute() can
>> execute
>> + * them one by one with help, so just mark the result as
>> SLOW_ACTION to
>> + * prevent the flow from being installed. */
>> + COVERAGE_INC(xlate_actions_oversize);
>> + ctx.xout->slow |= SLOW_ACTION;
>> }
>>
>> ofpbuf_uninit(&ctx.stack);
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
