Hi Ben,

Thanks for the review. Yes, I find this approach better where data is
decoded only once.

I have a doubt regarding the following structure:

struct ofputil_requestforward {
    ovs_be32 xid;
    enum ofp14_requestforward_reason reason;
    union {
        /* reason == OFPRFR_METER_MOD. */
        struct ofputil_meter_mod *meter_mod;

        /* reason == OFPRFR_GROUP_MOD. */
        struct {
            struct ofputil_group_mod *group_mod;
            struct ofpbuf bands;
        };
    };
};

Here, 'bands' are included as a part of group_mod, but as per the code and
specification, bands are part of meter_mod. Please help me understand why
'bands' have been included with group_mod. As per my understanding, the
structure should be in this manner:

struct ofputil_requestforward {
    ovs_be32 xid;
    enum ofp14_requestforward_reason reason;
    union {
        /* reason == OFPRFR_METER_MOD. */
        struct {
            struct ofputil_meter_mod *meter_mod;
            struct ofpbuf bands;
        };

        /* reason == OFPRFR_GROUP_MOD. */
        struct ofputil_group_mod *group_mod;
    };
};

Kindly validate my understanding.

Thanks & Regards
Niti Rohilla
Tata Consultancy Services

On Thu, Sep 10, 2015 at 1:48 AM, Ben Pfaff <b...@nicira.com> wrote:

> On Wed, Sep 09, 2015 at 05:33:42PM +0530, niti1...@gmail.com wrote:
> > From: Niti Rohilla <niti.rohi...@tcs.com>
> >
> > This patch adds support for Openflow1.4 Group & meter change notification
> > messages. In a multi controller environment, when a controller modifies
> the
> > state of group and meter table, the request that successfully modifies
> this
> > state is forwarded to other controllers. Other controllers are informed
> with
> > the OFPT_REQUESTFORWARD message. Request forwarding is enabled on a per
> > controller channel basis using the Set Asynchronous Configuration
> Message.
> >
> > Signed-off-by: Niti Rohilla <niti.rohi...@tcs.com>
> > ---
> > Difference between v7->v8:
> > - Memory leak issue in ofputil_re_encode_requestforward() has been
> resolved.
> > - Modified the test case so that ofputil_re_encode_requestforward() get
> >   excercised in the testsuite to re-encode the request.
> > - Rebased with the latest master.
>
> After I looked at this for a while, I realized that there was more
> decoding than necessary.  It shouldn't be necessary to decode a message
> once to execute it, then decode it again to reencode it inside
> ofputil_re_encode_requestforward().  That even introduces a possibility
> for failure the second time (though it shouldn't happen).
>
> I decided that it's better, therefore, to make ofputil_requestforward
> include the decoded form of the inner message instead of the raw form,
> like this:
>
> struct ofputil_requestforward {
>     ovs_be32 xid;
>     enum ofp14_requestforward_reason reason;
>     union {
>         /* reason == OFPRFR_METER_MOD. */
>         struct ofputil_meter_mod *meter_mod;
>
>         /* reason == OFPRFR_GROUP_MOD. */
>         struct {
>             struct ofputil_group_mod *group_mod;
>             struct ofpbuf bands;
>         };
>     };
> };
>
> This required further changes elsewhere, but they were straightforward.
>
> Anyway, I folded in the following incremental and applied this to
> master.  Thanks!
>
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index b2d3b4f..d0c94ce 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1185,21 +1185,9 @@ ofp_print_meter_config(struct ds *s, const struct
> ofputil_meter_config *mc)
>  }
>
>  static void
> -ofp_print_meter_mod(struct ds *s, const struct ofp_header *oh)
> +ofp_print_meter_mod__(struct ds *s, const struct ofputil_meter_mod *mm)
>  {
> -    struct ofputil_meter_mod mm;
> -    struct ofpbuf bands;
> -    enum ofperr error;
> -
> -    ofpbuf_init(&bands, 64);
> -    error = ofputil_decode_meter_mod(oh, &mm, &bands);
> -    if (error) {
> -        ofpbuf_uninit(&bands);
> -        ofp_print_error(s, error);
> -        return;
> -    }
> -
> -    switch (mm.command) {
> +    switch (mm->command) {
>      case OFPMC13_ADD:
>          ds_put_cstr(s, " ADD ");
>          break;
> @@ -1210,10 +1198,26 @@ ofp_print_meter_mod(struct ds *s, const struct
> ofp_header *oh)
>          ds_put_cstr(s, " DEL ");
>          break;
>      default:
> -        ds_put_format(s, " cmd:%d ", mm.command);
> +        ds_put_format(s, " cmd:%d ", mm->command);
>      }
>
> -    ofp_print_meter_config(s, &mm.meter);
> +    ofp_print_meter_config(s, &mm->meter);
> +}
> +
> +static void
> +ofp_print_meter_mod(struct ds *s, const struct ofp_header *oh)
> +{
> +    struct ofputil_meter_mod mm;
> +    struct ofpbuf bands;
> +    enum ofperr error;
> +
> +    ofpbuf_init(&bands, 64);
> +    error = ofputil_decode_meter_mod(oh, &mm, &bands);
> +    if (error) {
> +        ofp_print_error(s, error);
> +    } else {
> +        ofp_print_meter_mod__(s, &mm);
> +    }
>      ofpbuf_uninit(&bands);
>  }
>
> @@ -2333,7 +2337,8 @@ ofp_print_bucket_id(struct ds *s, const char *label,
> uint32_t bucket_id,
>
>  static void
>  ofp_print_group(struct ds *s, uint32_t group_id, uint8_t type,
> -                struct ovs_list *p_buckets, struct ofputil_group_props
> *props,
> +                const struct ovs_list *p_buckets,
> +                const struct ofputil_group_props *props,
>                  enum ofp_version ofp_version, bool suppress_type)
>  {
>      struct ofputil_bucket *bucket;
> @@ -2529,22 +2534,15 @@ ofp_print_group_features(struct ds *string, const
> struct ofp_header *oh)
>  }
>
>  static void
> -ofp_print_group_mod(struct ds *s, const struct ofp_header *oh)
> +ofp_print_group_mod__(struct ds *s, enum ofp_version ofp_version,
> +                      const struct ofputil_group_mod *gm)
>  {
> -    struct ofputil_group_mod gm;
> -    int error;
>      bool bucket_command = false;
>
> -    error = ofputil_decode_group_mod(oh, &gm);
> -    if (error) {
> -        ofp_print_error(s, error);
> -        return;
> -    }
> -
>      ds_put_char(s, '\n');
>
>      ds_put_char(s, ' ');
> -    switch (gm.command) {
> +    switch (gm->command) {
>      case OFPGC11_ADD:
>          ds_put_cstr(s, "ADD");
>          break;
> @@ -2568,17 +2566,31 @@ ofp_print_group_mod(struct ds *s, const struct
> ofp_header *oh)
>          break;
>
>      default:
> -        ds_put_format(s, "cmd:%"PRIu16"", gm.command);
> +        ds_put_format(s, "cmd:%"PRIu16"", gm->command);
>      }
>      ds_put_char(s, ' ');
>
>      if (bucket_command) {
>          ofp_print_bucket_id(s, "command_bucket_id:",
> -                            gm.command_bucket_id, oh->version);
> +                            gm->command_bucket_id, ofp_version);
>      }
>
> -    ofp_print_group(s, gm.group_id, gm.type, &gm.buckets, &gm.props,
> -                    oh->version, bucket_command);
> +    ofp_print_group(s, gm->group_id, gm->type, &gm->buckets, &gm->props,
> +                    ofp_version, bucket_command);
> +}
> +
> +static void
> +ofp_print_group_mod(struct ds *s, const struct ofp_header *oh)
> +{
> +    struct ofputil_group_mod gm;
> +    int error;
> +
> +    error = ofputil_decode_group_mod(oh, &gm);
> +    if (error) {
> +        ofp_print_error(s, error);
> +        return;
> +    }
> +    ofp_print_group_mod__(s, oh->version, &gm);
>      ofputil_bucket_list_destroy(&gm.buckets);
>  }
>
> @@ -3057,7 +3069,6 @@ ofp_print_requestforward(struct ds *string, const
> struct ofp_header *oh)
>  {
>      struct ofputil_requestforward rf;
>      enum ofperr error;
> -    enum ofptype type;
>
>      error = ofputil_decode_requestforward(oh, &rf);
>      if (error) {
> @@ -3065,21 +3076,20 @@ ofp_print_requestforward(struct ds *string, const
> struct ofp_header *oh)
>          return;
>      }
>
> -    error = ofptype_decode(&type, rf.request);
> -    if (error) {
> -        ofp_print_error(string, error);
> -        return;
> -    }
> -
>      ds_put_cstr(string, " reason=");
>
> -    if (type == OFPTYPE_GROUP_MOD) {
> +    switch (rf.reason) {
> +    case OFPRFR_GROUP_MOD:
>          ds_put_cstr(string, "group_mod");
> -        ofp_print_group_mod(string, rf.request);
> -    } else if (type == OFPTYPE_METER_MOD) {
> +        ofp_print_group_mod__(string, oh->version, rf.group_mod);
> +        break;
> +
> +    case OFPRFR_METER_MOD:
>          ds_put_cstr(string, "meter_mod");
> -        ofp_print_meter_mod(string, rf.request);
> +        ofp_print_meter_mod__(string, rf.meter_mod);
> +        break;
>      }
> +    ofputil_destroy_requestforward(&rf);
>  }
>
>  static void
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 05ee6bb..d90cca8 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -5376,106 +5376,125 @@ ofputil_decode_role_status(const struct
> ofp_header *oh,
>      return 0;
>  }
>
> -/* Re-encodes the "GROUP_MOD" or "METER_MOD" request header for the
> - * 'protocol' in the ofconn. */
> -enum ofperr
> -ofputil_re_encode_requestforward(uint8_t reason,
> -                                 const struct ofp_header *request,
> -                                 struct ofp_header **requestp,
> -                                 enum ofputil_protocol protocol)
> +/* Encodes 'rf' according to 'protocol', and returns the encoded message.
> + * 'protocol' must be for OpenFlow 1.4 or later. */
> +struct ofpbuf *
> +ofputil_encode_requestforward(const struct ofputil_requestforward *rf,
> +                              enum ofputil_protocol protocol)
>  {
> -    enum ofp_version version;
> -    struct ofp_header *rf;
> -    struct ofpbuf *buf;
> -    enum ofperr error;
> -
> -    version = ofputil_protocol_to_ofp_version(protocol);
> -    switch (reason) {
> -    case OFPRFR_GROUP_MOD: {
> -        struct ofputil_group_mod gm;
> +    enum ofp_version ofp_version =
> ofputil_protocol_to_ofp_version(protocol);
> +    struct ofpbuf *inner;
>
> -        error = ofputil_decode_group_mod(request, &gm);
> -        if (error) {
> -            return error;
> -        }
> -        buf = ofputil_encode_group_mod(version, &gm);
> -        ofputil_bucket_list_destroy(&gm.buckets);
> +    switch (rf->reason) {
> +    case OFPRFR_GROUP_MOD:
> +        inner = ofputil_encode_group_mod(ofp_version, rf->group_mod);
>          break;
> -    }
> -    case OFPRFR_METER_MOD: {
> -        struct ofputil_meter_mod mm;
> -        struct ofpbuf bands;
>
> -        ofpbuf_init(&bands, 64);
> -        error = ofputil_decode_meter_mod(request, &mm, &bands);
> -        if (error) {
> -            ofpbuf_uninit(&bands);
> -            return error;
> -        }
> -        buf = ofputil_encode_meter_mod(version, &mm);
> -        ofpbuf_uninit(&bands);
> +    case OFPRFR_METER_MOD:
> +        inner = ofputil_encode_meter_mod(ofp_version, rf->meter_mod);
>          break;
> -    }
> +
>      default:
>          OVS_NOT_REACHED();
>      }
> -    rf = buf->data;
> -    rf->length = htons(buf->size);
> -    rf->xid = request->xid;
> -    *requestp = rf;
> -    return 0;
> -}
>
> -/* Encodes "request forward" message encapsulating "GROUP_MOD" or
> "METER_MOD"
> - * request in 'rf'. The OpenFlow version of OFPT_REQUESTFORWARD message
> and
> - * 'rf->request' should be same.
> - * 'rf->request' is not converted from host to network byte order because
> - * "GROUP_MOD" or "METER_MOD" request header in 'rf' is directly
> encapsulated
> - * in request forward message. */
> -struct ofpbuf *
> -ofputil_encode_requestforward(const struct ofputil_requestforward *rf)
> -{
> -    struct ofpbuf *buf;
> +    struct ofp_header *inner_oh = inner->data;
> +    inner_oh->xid = rf->xid;
> +    inner_oh->length = htons(inner->size);
>
> -    buf = ofpraw_alloc_xid(OFPRAW_OFPT14_REQUESTFORWARD,
> rf->request->version,
> -                           htonl(0), ntohs(rf->request->length));
> -    ofpbuf_put(buf, rf->request, ntohs(rf->request->length));
> -    return buf;
> +    struct ofpbuf *outer = ofpraw_alloc_xid(OFPRAW_OFPT14_REQUESTFORWARD,
> +                                            ofp_version, htonl(0),
> +                                            inner->size);
> +    ofpbuf_put(outer, inner->data, inner->size);
> +    ofpbuf_delete(inner);
> +
> +    return outer;
>  }
>
> -/* Converts OpenFlow request forward  message 'oh' into an abstract
> request
> - * forward in 'rf'. Returns 0 if successful, otherwise an OpenFlow error
> - * code. */
> +/* Decodes OFPT_REQUESTFORWARD message 'outer'.  On success, puts the
> decoded
> + * form into '*rf' and returns 0, and the caller is later responsible for
> + * freeing the content of 'rf', with ofputil_destroy_requestforward(rf).
> On
> + * failure, returns an ofperr and '*rf' is indeterminate. */
>  enum ofperr
> -ofputil_decode_requestforward(const struct ofp_header *oh,
> +ofputil_decode_requestforward(const struct ofp_header *outer,
>                                struct ofputil_requestforward *rf)
>  {
>      struct ofpbuf b;
> -    enum ofpraw raw;
>      enum ofperr error;
> -    size_t inner_len;
>
> -    ofpbuf_use_const(&b, oh, ntohs(oh->length));
> -    error = ofpraw_pull(&raw, &b);
> -    if (error) {
> -        return error;
> -    }
> -
> -    ovs_assert(raw == OFPRAW_OFPT14_REQUESTFORWARD);
> +    ofpbuf_use_const(&b, outer, ntohs(outer->length));
>
> -    rf->request = b.data;
> +    /* Skip past outer message. */
> +    enum ofpraw outer_raw = ofpraw_pull_assert(&b);
> +    ovs_assert(outer_raw == OFPRAW_OFPT14_REQUESTFORWARD);
>
> -    if (rf->request->version != oh->version) {
> -        return OFPERR_OFPBRC_BAD_VERSION;
> +    /* Validate inner message. */
> +    if (b.size < sizeof(struct ofp_header)) {
> +        return OFPERR_OFPBFC_MSG_BAD_LEN;
>      }
> -    inner_len = ntohs(rf->request->length);
> +    const struct ofp_header *inner = b.data;
> +    unsigned int inner_len = ntohs(inner->length);
>      if (inner_len < sizeof(struct ofp_header) || inner_len > b.size) {
>          return OFPERR_OFPBFC_MSG_BAD_LEN;
>      }
> +    if (inner->version != outer->version) {
> +        return OFPERR_OFPBRC_BAD_VERSION;
> +    }
> +
> +    /* Parse inner message. */
> +    enum ofptype type;
> +    error = ofptype_decode(&type, inner);
> +    if (error) {
> +        return error;
> +    }
> +
> +    rf->xid = inner->xid;
> +    if (type == OFPTYPE_GROUP_MOD) {
> +        rf->reason = OFPRFR_GROUP_MOD;
> +        rf->group_mod = xmalloc(sizeof *rf->group_mod);
> +        error = ofputil_decode_group_mod(inner, rf->group_mod);
> +        if (error) {
> +            free(rf->group_mod);
> +            return error;
> +        }
> +    } else if (type == OFPTYPE_METER_MOD) {
> +        rf->reason = OFPRFR_METER_MOD;
> +        rf->meter_mod = xmalloc(sizeof *rf->meter_mod);
> +        ofpbuf_init(&rf->bands, 64);
> +        error = ofputil_decode_meter_mod(inner, rf->meter_mod,
> &rf->bands);
> +        if (error) {
> +            free(rf->meter_mod);
> +            ofpbuf_uninit(&rf->bands);
> +            return error;
> +        }
> +    } else {
> +        return OFPERR_OFPBFC_MSG_UNSUP;
> +    }
>
>      return 0;
>  }
>
> +/* Frees the content of 'rf', which should have been initialized through a
> + * successful call to ofputil_decode_requestforward(). */
> +void
> +ofputil_destroy_requestforward(struct ofputil_requestforward *rf)
> +{
> +    if (!rf) {
> +        return;
> +    }
> +
> +    switch (rf->reason) {
> +    case OFPRFR_GROUP_MOD:
> +        ofputil_uninit_group_mod(rf->group_mod);
> +        free(rf->group_mod);
> +        break;
> +
> +    case OFPRFR_METER_MOD:
> +        ofpbuf_uninit(&rf->bands);
> +        free(rf->meter_mod);
> +    }
> +}
> +
>  /* Table stats. */
>
>  /* OpenFlow 1.0 and 1.1 don't distinguish between a field that cannot be
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index 7589f51..cbd6175 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -861,22 +861,6 @@ struct ofpbuf *ofputil_encode_role_status(
>  enum ofperr ofputil_decode_role_status(const struct ofp_header *oh,
>                                         struct ofputil_role_status *rs);
>
> -struct ofputil_requestforward {
> -    const struct ofp_header *request;
> -};
> -
> -enum ofperr
> -ofputil_re_encode_requestforward(uint8_t reason,
> -                                 const struct ofp_header *request,
> -                                 struct ofp_header **requestp,
> -                                 enum ofputil_protocol protocol);
> -struct ofpbuf *
> -ofputil_encode_requestforward(const struct ofputil_requestforward *rf);
> -
> -enum ofperr
> -ofputil_decode_requestforward(const struct ofp_header *oh,
> -                              struct ofputil_requestforward *rf);
> -
>  /* Abstract table stats.
>   *
>   * This corresponds to the OpenFlow 1.3 table statistics structure, which
> only
> @@ -1255,4 +1239,25 @@ struct ofpbuf
> *ofputil_encode_get_async_config(const struct ofp_header *,
>                                                 uint32_t
> master[OAM_N_TYPES],
>                                                 uint32_t
> slave[OAM_N_TYPES]);
>
> +struct ofputil_requestforward {
> +    ovs_be32 xid;
> +    enum ofp14_requestforward_reason reason;
> +    union {
> +        /* reason == OFPRFR_METER_MOD. */
> +        struct ofputil_meter_mod *meter_mod;
> +
> +        /* reason == OFPRFR_GROUP_MOD. */
> +        struct {
> +            struct ofputil_group_mod *group_mod;
> +            struct ofpbuf bands;
> +        };
> +    };
> +};
> +
> +struct ofpbuf *ofputil_encode_requestforward(
> +    const struct ofputil_requestforward *, enum ofputil_protocol);
> +enum ofperr ofputil_decode_requestforward(const struct ofp_header *,
> +                                          struct ofputil_requestforward
> *);
> +void ofputil_destroy_requestforward(struct ofputil_requestforward *);
> +
>  #endif /* ofp-util.h */
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index 60f3d9a..ef2c06f 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -1714,46 +1714,21 @@ connmgr_send_port_status(struct connmgr *mgr,
> struct ofconn *source,
>   * controller OFPT_GROUP_MOD and OFPT_METER_MOD, specify 'source' as the
>   * controller connection that sent the request; otherwise, specify
> 'source'
>   * as NULL. */
> -enum ofperr
> -connmgr_send_requestforward(struct connmgr *mgr, struct ofconn *source,
> -                            uint8_t reason, const struct ofp_header *oh)
> +void
> +connmgr_send_requestforward(struct connmgr *mgr, const struct ofconn
> *source,
> +                            const struct ofputil_requestforward *rf)
>  {
> -    struct ofputil_requestforward rf;
>      struct ofconn *ofconn;
> -    struct ofp_header *request = NULL;
> -    enum ofperr error;
>
>      LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
> -        if (ofconn_receives_async_msg(ofconn, OAM_REQUESTFORWARD,
> reason)) {
> -            struct ofpbuf *msg;
> -
> -            rf.request = oh;
> -            /* For OpenFlow 1.4 and later, it generates
> OFPT_REQUESTFORWARD
> -             * for OFPT_GROUP_MOD and OFPT_METER_MOD, but not back to the
> -             * originating controller.  In a single-controller
> environment, in
> -             * particular, this means that it will never generate
> -             * OFPT_REQUESTFORWARD for OFPT_GROUP_MOD and OFPT_METER_MOD
> at
> -             * all. */
> -            if (rconn_get_version(ofconn->rconn) < OFP14_VERSION
> -                || ofconn == source) {
> -                continue;
> -            }
> -            /* When the version of GROUP_MOD or METER_MOD request is not
> same
> -             * as that of ofconn, then re-encode the GROUP_MOD or
> METER_MOD
> -             * request for the protocol in use. */
> -            if (oh->version != rconn_get_version(ofconn->rconn)) {
> -                error = ofputil_re_encode_requestforward(reason, oh,
> &request,
> -
> ofconn_get_protocol(ofconn));
> -                if (error) {
> -                    return error;
> -                }
> -                rf.request = request;
> -            }
> -            msg = ofputil_encode_requestforward(&rf);
> -            ofconn_send(ofconn, msg, NULL);
> +        if (ofconn_receives_async_msg(ofconn, OAM_REQUESTFORWARD,
> rf->reason)
> +            && rconn_get_version(ofconn->rconn) >= OFP14_VERSION
> +            && ofconn != source) {
> +            enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
> +            ofconn_send(ofconn, ofputil_encode_requestforward(rf,
> protocol),
> +                        NULL);
>          }
>      }
> -    return 0;
>  }
>
>  /* Sends an OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED message based on 'fr' to
> diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
> index 2de7f6f..8048424 100644
> --- a/ofproto/connmgr.h
> +++ b/ofproto/connmgr.h
> @@ -168,9 +168,8 @@ void connmgr_send_packet_in(struct connmgr *,
>  void ofconn_send_role_status(struct ofconn *ofconn, uint32_t role,
>                               uint8_t reason);
>
> -enum ofperr
> -connmgr_send_requestforward(struct connmgr *, struct ofconn *source,
> -                            uint8_t reason, const struct ofp_header *);
> +void connmgr_send_requestforward(struct connmgr *, const struct ofconn
> *source,
> +                                 const struct ofputil_requestforward *);
>
>  /* Fail-open settings. */
>  enum ofproto_fail_mode connmgr_get_fail_mode(const struct connmgr *);
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 99b743c..c1301b5 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -5909,8 +5909,11 @@ handle_meter_mod(struct ofconn *ofconn, const
> struct ofp_header *oh)
>      }
>
>      if (!error) {
> -        error = connmgr_send_requestforward(ofproto->connmgr, ofconn,
> -                                           OFPRFR_METER_MOD, oh);
> +        struct ofputil_requestforward rf;
> +        rf.xid = oh->xid;
> +        rf.reason = OFPRFR_METER_MOD;
> +        rf.meter_mod = &mm;
> +        connmgr_send_requestforward(ofproto->connmgr, ofconn, &rf);
>      }
>
>  exit_free_bands:
> @@ -6609,8 +6612,11 @@ handle_group_mod(struct ofconn *ofconn, const
> struct ofp_header *oh)
>      }
>
>      if (!error) {
> -        error = connmgr_send_requestforward(ofproto->connmgr, ofconn,
> -                                            OFPRFR_GROUP_MOD, oh);
> +        struct ofputil_requestforward rf;
> +        rf.xid = oh->xid;
> +        rf.reason = OFPRFR_GROUP_MOD;
> +        rf.group_mod = &gm;
> +        connmgr_send_requestforward(ofproto->connmgr, ofconn, &rf);
>      }
>      return error;
>  }
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 38d8ba6..7e80293 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -2904,7 +2904,7 @@ dnl command is sent from one controller and the
> reply is received by
>  dnl other controllers.
>  AT_SETUP([ofproto - requestforward (OpenFlow 1.4)])
>  OVS_VSWITCHD_START
> -ON_EXIT([kill `cat c1.pid c2.pid c3.pid`])
> +on_exit 'kill `cat c1.pid c2.pid c3.pid`'
>
>  # Start two ovs-ofctl controller processes.
>  AT_CAPTURE_FILE([monitor1.log])
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to