[ovs-dev] [ofp-print 16/18] Make compiler complain about unhandled OpenFlow and Nicira action types.

Justin Pettit jpettit at nicira.com
Thu Dec 9 07:21:16 UTC 2010


On Dec 8, 2010, at 4:27 PM, Ben Pfaff wrote:

> static void
> ofp_print_nx_action(struct ds *string, const struct nx_action_header *nah)
> {
> +    enum nx_action_subtype subtype = ntohs(nah->subtype);
> +    switch (subtype) {
> +    case NXAST_SNAT__OBSOLETE:
> +        break;

Do you think we should print a warning for obsoleted actions?

>     default:
> -        ds_put_format(string, "***unknown Nicira action:%d***",
> +        ds_put_format(string, "***unknown Nicira action:%"PRIu16"***",
>                       ntohs(nah->subtype));

You can just use "subtype".

> #include "ofpbuf.h"
> #include "packets.h"
> #include "random.h"
> +#include "type-props.h"

Props to the type, yo.

> @@ -1680,6 +1681,7 @@ check_nicira_action(const union ofp_action *a, unsigned int len,
>                     const struct flow *flow)
> {
>     const struct nx_action_header *nah;
> +    uint16_t subtype;

What's the reason for not treating this as a enum "nx_action_subtype" immediately?

> +    subtype = ntohs(nah->subtype);
> +    if (subtype > TYPE_MAXIMUM(enum nx_action_subtype)) {
> +        /* This is necessary because enum nx_action_subtype is probably an
> +         * 8-bit type, so the cast below throws away the top 8 bits. */
> +        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR_TYPE);
> +    }

Why is nx_action_subtype likely a 8-bit type?  

--Justin






More information about the dev mailing list