[ovs-dev] [PATCH 1/2] ofp-util: Add OFP_ACTION_ALIGN macro to header.
Ben Pfaff
blp at nicira.com
Sat Nov 13 00:27:04 UTC 2010
I don't want to diverge more from OpenFlow upstream than we already
have.
On Fri, Nov 12, 2010 at 12:41:50PM -0800, Justin Pettit wrote:
> Looks good. I don't see a reason not to define OFP_ACTION_ALIGN in
> openflow.h. I don't think our openflow.h is identical to the existing
> reference implementations, anyway.
>
> --Justin
>
>
> On Nov 10, 2010, at 4:28 PM, Ben Pfaff wrote:
>
> > It seems that this should really be in openflow.h but so far it isn't.
> > ---
> > lib/ofp-print.c | 7 ++++---
> > lib/ofp-util.c | 17 +++++++----------
> > lib/ofp-util.h | 3 +++
> > 3 files changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> > index bc77756..87ae185 100644
> > --- a/lib/ofp-print.c
> > +++ b/lib/ofp-print.c
> > @@ -30,6 +30,7 @@
> > #include "compiler.h"
> > #include "dynamic-string.h"
> > #include "flow.h"
> > +#include "ofp-util.h"
> > #include "ofpbuf.h"
> > #include "openflow/openflow.h"
> > #include "openflow/nicira-ext.h"
> > @@ -294,10 +295,10 @@ ofp_print_action(struct ds *string, const struct ofp_action_header *ah,
> > return -1;
> > }
> >
> > - if ((len % 8) != 0) {
> > + if ((len % OFP_ACTION_ALIGN) != 0) {
> > ds_put_format(string,
> > - "***action %"PRIu16" length not a multiple of 8***\n",
> > - type);
> > + "***action %"PRIu16" length not a multiple of %d***\n",
> > + type, OFP_ACTION_ALIGN);
> > return -1;
> > }
> >
> > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> > index 838617c..74e2a8a 100644
> > --- a/lib/ofp-util.c
> > +++ b/lib/ofp-util.c
> > @@ -460,9 +460,6 @@ flow_stats_next(struct flow_stats_iterator *iter)
> > return fs;
> > }
> >
> > -/* Alignment of ofp_actions. */
> > -#define ACTION_ALIGNMENT 8
> > -
> > static int
> > check_action_exact_len(const union ofp_action *a, unsigned int len,
> > unsigned int required_len)
> > @@ -619,7 +616,7 @@ validate_actions(const union ofp_action *actions, size_t n_actions,
> > for (i = 0; i < n_actions; ) {
> > const union ofp_action *a = &actions[i];
> > unsigned int len = ntohs(a->header.len);
> > - unsigned int n_slots = len / ACTION_ALIGNMENT;
> > + unsigned int n_slots = len / OFP_ACTION_ALIGN;
> > unsigned int slots_left = &actions[n_actions] - a;
> > int error;
> >
> > @@ -631,9 +628,9 @@ validate_actions(const union ofp_action *actions, size_t n_actions,
> > } else if (!len) {
> > VLOG_DBG_RL(&bad_ofmsg_rl, "action has invalid length 0");
> > return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
> > - } else if (len % ACTION_ALIGNMENT) {
> > + } else if (len % OFP_ACTION_ALIGN) {
> > VLOG_DBG_RL(&bad_ofmsg_rl, "action length %u is not a multiple "
> > - "of %d", len, ACTION_ALIGNMENT);
> > + "of %d", len, OFP_ACTION_ALIGN);
> > return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
> > }
> >
> > @@ -678,7 +675,7 @@ actions_next(struct actions_iterator *iter)
> > if (iter->pos != iter->end) {
> > const union ofp_action *a = iter->pos;
> > unsigned int len = ntohs(a->header.len);
> > - iter->pos += len / ACTION_ALIGNMENT;
> > + iter->pos += len / OFP_ACTION_ALIGN;
> > return a;
> > } else {
> > return NULL;
> > @@ -908,9 +905,9 @@ int
> > ofputil_pull_actions(struct ofpbuf *b, unsigned int actions_len,
> > union ofp_action **actionsp, size_t *n_actionsp)
> > {
> > - if (actions_len % ACTION_ALIGNMENT != 0) {
> > + if (actions_len % OFP_ACTION_ALIGN != 0) {
> > VLOG_DBG_RL(&bad_ofmsg_rl, "OpenFlow message actions length %u "
> > - "is not a multiple of %d", actions_len, ACTION_ALIGNMENT);
> > + "is not a multiple of %d", actions_len, OFP_ACTION_ALIGN);
> > goto error;
> > }
> >
> > @@ -922,7 +919,7 @@ ofputil_pull_actions(struct ofpbuf *b, unsigned int actions_len,
> > goto error;
> > }
> >
> > - *n_actionsp = actions_len / ACTION_ALIGNMENT;
> > + *n_actionsp = actions_len / OFP_ACTION_ALIGN;
> > return 0;
> >
> > error:
> > diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> > index 6f8c259..3467366 100644
> > --- a/lib/ofp-util.h
> > +++ b/lib/ofp-util.h
> > @@ -26,6 +26,9 @@
> > struct ofpbuf;
> > struct ofp_action_header;
> >
> > +/* Alignment of ofp_actions. */
> > +#define OFP_ACTION_ALIGN 8
> > +
> > /* OpenFlow protocol utility functions. */
> > void *make_openflow(size_t openflow_len, uint8_t type, struct ofpbuf **);
> > void *make_nxmsg(size_t openflow_len, uint32_t subtype, struct ofpbuf **);
> > --
> > 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