On Mon, Dec 08, 2014 at 02:01:31PM -0800, Jarno Rajahalme wrote:
> Reject bundle add messages containing messages that should not be bundled.
>
> Signed-off-by: Jarno Rajahalme <[email protected]>
Thanks for the patch!
In ofputil_is_bundlable(), there's a "case" that falls through with an
empty statement:
> + case OFPTYPE_TABLE_FEATURES_STATS_REPLY:
> + case OFPTYPE_ROLE_STATUS:
> + ;
> + }
> +
> + return false;
I'd tend to write "break;" instead of ";".
Here, because of the confusion that OFPERR_OFPBFC_MSG_BAD_XID in place
of 'error' could cause, it might be worth logging 'error' (with a rate
limit) if it's nonzero so that the logs at least give a hint:
> @@ -8513,6 +8601,12 @@ ofputil_decode_bundle_add(const struct ofp_header *oh,
> return OFPERR_OFPBFC_MSG_BAD_XID;
> }
>
> + /* Reject unbundlable messages. */
> + error = ofptype_decode(&type, msg->msg);
> + if (error || !ofputil_is_bundlable(type)) {
> + return OFPERR_OFPBFC_MSG_UNSUP; /* 'error' could be confusing. */
> + }
> +
> return 0;
> }
Acked-by: Ben Pfaff <[email protected]>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
