[ovs-dev] [ofp-util logging 3/3] ofp-util: Improve log messages for bad Nicira extension actions.

Justin Pettit jpettit at nicira.com
Fri Dec 24 00:31:45 UTC 2010


Looks good.

--Justin


On Dec 23, 2010, at 10:41 AM, Ben Pfaff wrote:

> check_action_exact_len() will always report that a Nicira extension action
> has type 65535 (OFPAT_VENDOR), which isn't very helpful for debugging.
> This introduces a new function that reports the subtype.
> 
> Also, log the subtype of unknown Nicira vendor actions.
> ---
> lib/ofp-util.c |   35 ++++++++++++++++++++++++++---------
> 1 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 1891467..9210413 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -1645,6 +1645,20 @@ check_action_exact_len(const union ofp_action *a, unsigned int len,
>     return 0;
> }
> 
> +static int
> +check_nx_action_exact_len(const struct nx_action_header *a,
> +                          unsigned int len, unsigned int required_len)
> +{
> +    if (len != required_len) {
> +        VLOG_WARN_RL(&bad_ofmsg_rl,
> +                     "Nicira action %"PRIu16" has invalid length %"PRIu16" "
> +                     "(must be %u)\n",
> +                     ntohs(a->subtype), ntohs(a->len), required_len);
> +        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
> +    }
> +    return 0;
> +}
> +
> /* Checks that 'port' is a valid output port for the OFPAT_OUTPUT action, given
>  * that the switch will never have more than 'max_ports' ports.  Returns 0 if
>  * 'port' is valid, otherwise an ofp_mkerr() return code. */
> @@ -1723,19 +1737,19 @@ check_nicira_action(const union ofp_action *a, unsigned int len,
>     case NXAST_DROP_SPOOFED_ARP:
>     case NXAST_SET_QUEUE:
>     case NXAST_POP_QUEUE:
> -        return check_action_exact_len(a, len, 16);
> +        return check_nx_action_exact_len(nah, len, 16);
> 
>     case NXAST_REG_MOVE:
> -        error = check_action_exact_len(a, len,
> -                                       sizeof(struct nx_action_reg_move));
> +        error = check_nx_action_exact_len(nah, len,
> +                                          sizeof(struct nx_action_reg_move));
>         if (error) {
>             return error;
>         }
>         return nxm_check_reg_move((const struct nx_action_reg_move *) a, flow);
> 
>     case NXAST_REG_LOAD:
> -        error = check_action_exact_len(a, len,
> -                                       sizeof(struct nx_action_reg_load));
> +        error = check_nx_action_exact_len(nah, len,
> +                                          sizeof(struct nx_action_reg_load));
>         if (error) {
>             return error;
>         }
> @@ -1745,12 +1759,12 @@ check_nicira_action(const union ofp_action *a, unsigned int len,
>         return 0;
> 
>     case NXAST_SET_TUNNEL64:
> -        return check_action_exact_len(a, len,
> -                                      sizeof(struct nx_action_set_tunnel64));
> +        return check_nx_action_exact_len(
> +            nah, len, sizeof(struct nx_action_set_tunnel64));
> 
>     case NXAST_MULTIPATH:
> -        error = check_action_exact_len(a, len,
> -                                       sizeof(struct nx_action_multipath));
> +        error = check_nx_action_exact_len(
> +            nah, len, sizeof(struct nx_action_multipath));
>         if (error) {
>             return error;
>         }
> @@ -1758,6 +1772,9 @@ check_nicira_action(const union ofp_action *a, unsigned int len,
> 
>     case NXAST_SNAT__OBSOLETE:
>     default:
> +        VLOG_WARN_RL(&bad_ofmsg_rl,
> +                     "unknown Nicira vendor action subtype %"PRIu16,
> +                     ntohs(nah->subtype));
>         return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR_TYPE);
>     }
> }
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list