On Oct 23, 2013, at 9:46 AM, Ben Pfaff <[email protected]> wrote:
> On Tue, Oct 22, 2013 at 05:20:39PM -0700, Jarno Rajahalme wrote:
>> Make union ofp_action more generic by allowing it to be used for
>> all actions. OFP_ACTION_ALIGN (which is 8, as the sizeof(union
>> ofp_action) used to be) is now used where sizeof union ofp_action
>> was used before. On the same note, variable name 'n_actions' is
>> changed to 'max_actions' as the number of actions cannot be simply
>> divided when actions have various sizes.
>>
>> Signed-off-by: Jarno Rajahalme <[email protected]>
>
> I think that this pointer arithmetic in ofpacts_from_openflow() is now
> wrong:
> log_bad_action(in, n_in, a - in, error);
Oops, would you be happy with this incremental:
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 777677a..df88ce9 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -602,8 +602,8 @@ action_is_valid(const union ofp_action *a, size_t
max_actions)
(ITER) = action_next(ITER)))
static void
-log_bad_action(const union ofp_action *actions, size_t max_actions, size_t ofs,
- enum ofperr error)
+log_bad_action(const union ofp_action *actions, size_t max_actions,
+ const union ofp_action *bad_action, enum ofperr error)
{
if (!VLOG_DROP_WARN(&rl)) {
struct ds s;
@@ -611,7 +611,8 @@ log_bad_action(const union ofp_action *actions, size_t
max_actions, size_t ofs,
ds_init(&s);
ds_put_hex_dump(&s, actions, max_actions * OFP_ACTION_ALIGN, 0, false);
VLOG_WARN("bad action at offset %#zx (%s):\n%s",
- ofs * OFP_ACTION_ALIGN, ofperr_get_name(error), ds_cstr(&s));
+ (char *)bad_action - (char *)actions,
+ ofperr_get_name(error), ds_cstr(&s));
ds_destroy(&s);
}
}
@@ -628,13 +629,13 @@ ofpacts_from_openflow(const union ofp_action *in, size_t
n_in,
ACTION_FOR_EACH (a, left, in, n_in) {
enum ofperr error = ofpact_from_openflow(a, out);
if (error) {
- log_bad_action(in, n_in, a - in, error);
+ log_bad_action(in, n_in, a, error);
return error;
}
}
if (left) {
enum ofperr error = OFPERR_OFPBAC_BAD_LEN;
- log_bad_action(in, n_in, n_in - left, error);
+ log_bad_action(in, n_in, a, error);
return error;
}
Jarno
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
