On Wed, Jul 11, 2012 at 12:37:41AM -0700, Justin Pettit wrote:
> On Jul 9, 2012, at 2:21 PM, Ben Pfaff wrote:
>
> > + if (!VLOG_DROP_WARN(&rl)) {
> > + struct ds s;
> > +
> > + ds_init(&s);
> > + ds_put_hex_dump(&s, in, n_in * sizeof *in, 0, false);
> > + VLOG_WARN("bad action format at offset %#x:\n%s",
> > + (n_in - left) * sizeof *a, ds_cstr(&s));
>
> Very minor, but there are a couple of inconsistencies. First, you're
> taking sizeof "*in" in one spot and "*a" later. It seems like "sizeof
> *a" is more common in the function with two calls. Second, the offset
> is printed in decimal in the bad action case and hex in the bad action
> format case.
OK, I changed sizeof *in to sizeof *a. Fair enough.
I actually prefer decimal over hexadecimal most of the time (I only have
10 fingers) but referring to output of ds_put_hex_dump() I want to use
hexadecimal because that's the base that ds_put_hex_dump() uses for the
offsets it prints along the left margin. So it's inconsistent within
the function but I didn't just pick it randomly at least.
If you want to switch to hex for the other use in this function for
consistency, I guess I'd be OK with that.
I'm going to push the following in a minute or two unless I get an
objection back.
Thanks,
Ben.
--8<--------------------------cut here-------------------------->8--
From: Ben Pfaff <[email protected]>
Date: Mon, 9 Jul 2012 14:21:00 -0700
Subject: [PATCH] ofp-actions: Add hex dump of bad actions to log message on
error.
This should make debugging easier in such cases.
Bug #12460.
Reported-by: Natasha Gude <[email protected]>
Reported-by: James Schmidt <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
---
lib/ofp-actions.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 2254f53..93f6bf7 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -450,8 +450,15 @@ ofpacts_from_openflow10(const union ofp_action *in, size_t
n_in,
}
}
if (left) {
- VLOG_WARN_RL(&rl, "bad action format at offset %zu",
- (n_in - left) * sizeof *a);
+ if (!VLOG_DROP_WARN(&rl)) {
+ struct ds s;
+
+ ds_init(&s);
+ ds_put_hex_dump(&s, in, n_in * sizeof *a, 0, false);
+ VLOG_WARN("bad action format at offset %#x:\n%s",
+ (n_in - left) * sizeof *a, ds_cstr(&s));
+ ds_destroy(&s);
+ }
return OFPERR_OFPBAC_BAD_LEN;
}
--
1.7.2.5
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
