The comment of the OAM_PACKET_IN definition has a redundant period.

ofproto.at has a redundant newline at the end of the file.

Also this comment may be more appropriate for a previous patch.  I'm
not certain why the new ofctl set-output file option is implemented as
an appctl command.  That would make sense to me if we needed to change
the output file as ofctl monitor was running.  It seems to me, that it
would be a lot cleaner and simpler to make it a command line option.
My understanding of the use case presented in the series leads me to
believe that would be sufficient.  I don't feel super strongly about
it though, so if you have some reason for the current strategy feel
free to leave it.

Looks good,
Ethan



On Thu, Jan 26, 2012 at 15:53, Ben Pfaff <b...@nicira.com> wrote:
> Until now, the rules that cover the asynchronous messages that Open vSwitch
> sends to a controller have been ad hoc.  The new NXT_SET_ASYNC_CONFIG
> message provides systematic, precise control.
>
> Feature #7086.
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
>  DESIGN                        |   50 +++++++++++++-
>  NEWS                          |    4 +
>  include/openflow/nicira-ext.h |   26 +++++++-
>  lib/learning-switch.c         |    3 +-
>  lib/ofp-print.c               |  142 +++++++++++++++++++++++++++++++--------
>  lib/ofp-util.c                |    4 +
>  lib/ofp-util.h                |    1 +
>  ofproto/connmgr.c             |  150 
> +++++++++++++++++++++++++----------------
>  ofproto/connmgr.h             |   14 ++++-
>  ofproto/ofproto.c             |   23 ++++++
>  tests/ofp-print.at            |   20 ++++++
>  tests/ofproto.at              |   88 ++++++++++++++++++++++++
>  12 files changed, 433 insertions(+), 92 deletions(-)
>
> diff --git a/DESIGN b/DESIGN
> index 13f3e9f..872fc5f 100644
> --- a/DESIGN
> +++ b/DESIGN
> @@ -9,8 +9,54 @@ successful deployment.  The end of this document contains 
> contact
>  information that can be used to let us know how we can make Open vSwitch
>  more generally useful.
>
> -OpenFlow
> -========
> +Asynchronous Messages
> +=====================
> +
> +Over time, Open vSwitch has added many knobs that control whether a
> +given controller receives OpenFlow asynchronous messages.  This
> +section describes how all of these features interact.
> +
> +First, a service controller never receives any asynchronous messages
> +unless it explicitly configures a miss_send_len greater than zero with
> +an OFPT_SET_CONFIG message.
> +
> +Second, OFPT_FLOW_REMOVED and NXT_FLOW_REMOVED messages are generated
> +only if the flow that was removed had the OFPFF_SEND_FLOW_REM flag
> +set.
> +
> +Finally, Open vSwitch consults a per-connection table indexed by the
> +message type, reason code, and current role.  The following table
> +shows how this table is initialized by default when an OpenFlow
> +connection is made.  An entry labeled "yes" means that the message is
> +sent, an entry labeled "---" means that the message is suppressed.
> +
> +                                             master/
> +  message and reason code                     other     slave
> +  ----------------------------------------   -------    -----
> +  OFPT_PACKET_IN / NXT_PACKET_IN
> +    OFPR_NO_MATCH                              yes       ---
> +    OFPR_ACTION                                yes       ---
> +    OFPR_INVALID_TTL                           ---       ---
> +
> +  OFPT_FLOW_REMOVED / NXT_FLOW_REMOVED
> +    OFPRR_IDLE_TIMEOUT                         yes       ---
> +    OFPRR_HARD_TIMEOUT                         yes       ---
> +    OFPRR_DELETE                               yes       ---
> +
> +  OFPT_PORT_STATUS
> +    OFPPR_ADD                                  yes       yes
> +    OFPPR_DELETE                               yes       yes
> +    OFPPR_MODIFY                               yes       yes
> +
> +The NXT_SET_ASYNC_CONFIG message directly sets all of the values in
> +this table for the current connection.  The
> +OFPC_INVALID_TTL_TO_CONTROLLER bit in the OFPT_SET_CONFIG message
> +controls the setting for OFPR_INVALID_TTL for the "master" and "other"
> +roles.
> +
> +
> +OFPAT_ENQUEUE
> +=============
>
>  The OpenFlow 1.0 specification requires the output port of the OFPAT_ENQUEUE
>  action to "refer to a valid physical port (i.e. < OFPP_MAX) or OFPP_IN_PORT".
> diff --git a/NEWS b/NEWS
> index d7332f8..0b97149 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -6,6 +6,10 @@ post-v1.5.0
>         - The default bond_mode changed from SLB to active-backup, to protect
>           unsuspecting users from the significant risks of SLB bonds (which 
> are
>           documented in vswitchd/INTERNALS).
> +    - OpenFlow:
> +        - Added an OpenFlow extension NXT_SET_ASYNC_CONFIG that allows
> +          controllers more precise control over which OpenFlow messages they
> +          receive asynchronously.
>
>
>  v1.5.0 - xx xxx xxxx
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index 62180a7..1b951ac 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -108,7 +108,9 @@ enum nicira_type {
>
>     /* Alternative PACKET_IN message formats. */
>     NXT_SET_PACKET_IN_FORMAT = 16, /* Set Packet In format. */
> -    NXT_PACKET_IN = 17             /* Nicira Packet In. */
> +    NXT_PACKET_IN = 17,            /* Nicira Packet In. */
> +
> +    NXT_SET_ASYNC_CONFIG = 18,  /* Configure async messages to receive. */
>  };
>
>  /* Header for Nicira vendor stats request and reply messages. */
> @@ -283,6 +285,28 @@ enum nx_role {
>     NX_ROLE_MASTER,             /* Full access, at most one. */
>     NX_ROLE_SLAVE               /* Read-only access. */
>  };
> +
> +/* NXT_SET_ASYNC_CONFIG.
> + *
> + * Sent by a controller, this message configures the asynchronous messages 
> that
> + * the controller wants to receive.  Element 0 in each array specifies 
> messages
> + * of interest when the controller has an "other" or "master" role; element 
> 1,
> + * when the controller has a "slave" role.
> + *
> + * Each array element is a bitmask in which a 0-bit disables receiving a
> + * particular message and a 1-bit enables receiving it.  Each bit controls 
> the
> + * message whose 'reason' corresponds to the bit index.  For example, the bit
> + * with value 1<<2 == 4 in port_status_mask[1] determines whether the
> + * controller will receive OFPT_PORT_STATUS messages with reason OFPPR_MODIFY
> + * (value 2) when the controller has a "slave" role.
> + */
> +struct nx_async_config {
> +    struct nicira_header nxh;
> +    ovs_be32 packet_in_mask[2];    /* Bitmasks of OFPR_* values. */
> +    ovs_be32 port_status_mask[2];  /* Bitmasks of OFPRR_* values. */
> +    ovs_be32 flow_removed_mask[2]; /* Bitmasks of OFPPR_* values. */
> +};
> +OFP_ASSERT(sizeof(struct nx_async_config) == 40);
>
>  /* Nicira vendor flow actions. */
>
> diff --git a/lib/learning-switch.c b/lib/learning-switch.c
> index b19d8bb..6a9d9ba 100644
> --- a/lib/learning-switch.c
> +++ b/lib/learning-switch.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -262,6 +262,7 @@ lswitch_process_packet(struct lswitch *sw, struct rconn 
> *rconn,
>     case OFPUTIL_NXT_PACKET_IN:
>     case OFPUTIL_NXT_FLOW_MOD:
>     case OFPUTIL_NXT_FLOW_REMOVED:
> +    case OFPUTIL_NXT_SET_ASYNC_CONFIG:
>     case OFPUTIL_NXST_FLOW_REQUEST:
>     case OFPUTIL_NXST_AGGREGATE_REQUEST:
>     case OFPUTIL_NXST_FLOW_REPLY:
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index a0fa7de..5b840a9 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -79,6 +79,24 @@ ofp_packet_to_string(const void *data, size_t len)
>     return ds_cstr(&ds);
>  }
>
> +static const char *
> +ofp_packet_in_reason_to_string(enum ofp_packet_in_reason reason)
> +{
> +    static char s[32];
> +
> +    switch (reason) {
> +    case OFPR_NO_MATCH:
> +        return "no_match";
> +    case OFPR_ACTION:
> +        return "action";
> +    case OFPR_INVALID_TTL:
> +        return "invalid_ttl";
> +    default:
> +        sprintf(s, "%d", (int) reason);
> +        return s;
> +    }
> +}
> +
>  static void
>  ofp_print_packet_in(struct ds *string, const struct ofp_header *oh,
>                     int verbosity)
> @@ -120,20 +138,8 @@ ofp_print_packet_in(struct ds *string, const struct 
> ofp_header *oh,
>         }
>     }
>
> -    switch (pin.reason) {
> -    case OFPR_NO_MATCH:
> -        ds_put_cstr(string, " (via no_match)");
> -        break;
> -    case OFPR_ACTION:
> -        ds_put_cstr(string, " (via action)");
> -        break;
> -    case OFPR_INVALID_TTL:
> -        ds_put_cstr(string, " (via invalid_ttl)");
> -        break;
> -    default:
> -        ds_put_format(string, " (***reason %"PRIu8"***)", pin.reason);
> -        break;
> -    }
> +    ds_put_format(string, " (via %s)",
> +                  ofp_packet_in_reason_to_string(pin.reason));
>
>     ds_put_format(string, " data_len=%zu", pin.packet_len);
>     if (pin.buffer_id == UINT32_MAX) {
> @@ -870,6 +876,24 @@ ofp_print_duration(struct ds *string, unsigned int sec, 
> unsigned int nsec)
>     ds_put_char(string, 's');
>  }
>
> +static const char *
> +ofp_flow_removed_reason_to_string(enum ofp_flow_removed_reason reason)
> +{
> +    static char s[32];
> +
> +    switch (reason) {
> +    case OFPRR_IDLE_TIMEOUT:
> +        return "idle";
> +    case OFPRR_HARD_TIMEOUT:
> +        return "hard";
> +    case OFPRR_DELETE:
> +        return "delete";
> +    default:
> +        sprintf(s, "%d", (int) reason);
> +        return s;
> +    }
> +}
> +
>  static void
>  ofp_print_flow_removed(struct ds *string, const struct ofp_header *oh)
>  {
> @@ -885,21 +909,8 @@ ofp_print_flow_removed(struct ds *string, const struct 
> ofp_header *oh)
>     ds_put_char(string, ' ');
>     cls_rule_format(&fr.rule, string);
>
> -    ds_put_cstr(string, " reason=");
> -    switch (fr.reason) {
> -    case OFPRR_IDLE_TIMEOUT:
> -        ds_put_cstr(string, "idle");
> -        break;
> -    case OFPRR_HARD_TIMEOUT:
> -        ds_put_cstr(string, "hard");
> -        break;
> -    case OFPRR_DELETE:
> -        ds_put_cstr(string, "delete");
> -        break;
> -    default:
> -        ds_put_format(string, "**%"PRIu8"**", fr.reason);
> -        break;
> -    }
> +    ds_put_format(string, " reason=%s",
> +                  ofp_flow_removed_reason_to_string(fr.reason));
>
>     if (fr.cookie != htonll(0)) {
>         ds_put_format(string, " cookie:0x%"PRIx64, ntohll(fr.cookie));
> @@ -1307,6 +1318,75 @@ ofp_print_nxt_set_packet_in_format(struct ds *string,
>     }
>  }
>
> +static const char *
> +ofp_port_reason_to_string(enum ofp_port_reason reason)
> +{
> +    static char s[32];
> +
> +    switch (reason) {
> +    case OFPPR_ADD:
> +        return "add";
> +
> +    case OFPPR_DELETE:
> +        return "delete";
> +
> +    case OFPPR_MODIFY:
> +        return "modify";
> +
> +    default:
> +        sprintf(s, "%d", (int) reason);
> +        return s;
> +    }
> +}
> +
> +static void
> +ofp_print_nxt_set_async_config(struct ds *string,
> +                               const struct nx_async_config *nac)
> +{
> +    int i;
> +
> +    for (i = 0; i < 2; i++) {
> +        int j;
> +
> +        ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave");
> +
> +        ds_put_cstr(string, "       PACKET_IN:");
> +        for (j = 0; j < 32; j++) {
> +            if (nac->packet_in_mask[i] & htonl(1u << j)) {
> +                ds_put_format(string, " %s",
> +                              ofp_packet_in_reason_to_string(j));
> +            }
> +        }
> +        if (!nac->packet_in_mask[i]) {
> +            ds_put_cstr(string, " (off)");
> +        }
> +        ds_put_char(string, '\n');
> +
> +        ds_put_cstr(string, "     PORT_STATUS:");
> +        for (j = 0; j < 32; j++) {
> +            if (nac->port_status_mask[i] & htonl(1u << j)) {
> +                ds_put_format(string, " %s", ofp_port_reason_to_string(j));
> +            }
> +        }
> +        if (!nac->port_status_mask[i]) {
> +            ds_put_cstr(string, " (off)");
> +        }
> +        ds_put_char(string, '\n');
> +
> +        ds_put_cstr(string, "    FLOW_REMOVED:");
> +        for (j = 0; j < 32; j++) {
> +            if (nac->flow_removed_mask[i] & htonl(1u << j)) {
> +                ds_put_format(string, " %s",
> +                              ofp_flow_removed_reason_to_string(j));
> +            }
> +        }
> +        if (!nac->flow_removed_mask[i]) {
> +            ds_put_cstr(string, " (off)");
> +        }
> +        ds_put_char(string, '\n');
> +    }
> +}
> +
>  static void
>  ofp_to_string__(const struct ofp_header *oh,
>                 const struct ofputil_msg_type *type, struct ds *string,
> @@ -1466,6 +1546,10 @@ ofp_to_string__(const struct ofp_header *oh,
>         ofp_print_flow_mod(string, msg, code, verbosity);
>         break;
>
> +    case OFPUTIL_NXT_SET_ASYNC_CONFIG:
> +        ofp_print_nxt_set_async_config(string, msg);
> +        break;
> +
>     case OFPUTIL_NXST_AGGREGATE_REPLY:
>         ofp_print_stats_reply(string, oh);
>         ofp_print_nxst_aggregate_reply(string, msg);
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index fc57b3f..c12d84f 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -384,6 +384,10 @@ ofputil_decode_vendor(const struct ofp_header *oh, 
> size_t length,
>         { OFPUTIL_NXT_FLOW_MOD_TABLE_ID, OFP10_VERSION,
>           NXT_FLOW_MOD_TABLE_ID, "NXT_FLOW_MOD_TABLE_ID",
>           sizeof(struct nx_flow_mod_table_id), 0 },
> +
> +        { OFPUTIL_NXT_SET_ASYNC_CONFIG, OFP10_VERSION,
> +          NXT_SET_ASYNC_CONFIG, "NXT_SET_ASYNC_CONFIG",
> +          sizeof(struct nx_async_config), 0 },
>     };
>
>     static const struct ofputil_msg_category nxt_category = {
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index 05ae009..72abd0b 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -79,6 +79,7 @@ enum ofputil_msg_code {
>     OFPUTIL_NXT_FLOW_REMOVED,
>     OFPUTIL_NXT_SET_PACKET_IN_FORMAT,
>     OFPUTIL_NXT_PACKET_IN,
> +    OFPUTIL_NXT_SET_ASYNC_CONFIG,
>
>     /* NXST_* stat requests. */
>     OFPUTIL_NXST_FLOW_REQUEST,
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index db848fb..5caa6bf 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -80,6 +80,13 @@ struct ofconn {
>      * requests.  */
>  #define OFCONN_REPLY_MAX 100
>     struct rconn_packet_counter *reply_counter;
> +
> +    /* Asynchronous message configuration in each possible roles.
> +     *
> +     * A 1-bit enables sending an asynchronous message for one possible 
> reason
> +     * that the message might be generated, a 0-bit disables it. */
> +    uint32_t master_async_config[OAM_N_TYPES]; /* master, other */
> +    uint32_t slave_async_config[OAM_N_TYPES];  /* slave */
>  };
>
>  static struct ofconn *ofconn_create(struct connmgr *, struct rconn *,
> @@ -100,8 +107,6 @@ static char *ofconn_make_name(const struct connmgr *, 
> const char *target);
>
>  static void ofconn_set_rate_limit(struct ofconn *, int rate, int burst);
>
> -static bool ofconn_receives_async_msgs(const struct ofconn *);
> -
>  static void ofconn_send(const struct ofconn *, struct ofpbuf *,
>                         struct rconn_packet_counter *);
>
> @@ -762,15 +767,21 @@ ofconn_set_role(struct ofconn *ofconn, enum nx_role 
> role)
>  }
>
>  void
> -ofconn_set_invalid_ttl_to_controller(struct ofconn *ofconn, bool val)
> +ofconn_set_invalid_ttl_to_controller(struct ofconn *ofconn, bool enable)
>  {
> -    ofconn->invalid_ttl_to_controller = val;
> +    uint32_t bit = 1u << OFPR_INVALID_TTL;
> +    if (enable) {
> +        ofconn->master_async_config[OAM_PACKET_IN] |= bit;
> +    } else {
> +        ofconn->master_async_config[OAM_PACKET_IN] &= ~bit;
> +    }
>  }
>
>  bool
>  ofconn_get_invalid_ttl_to_controller(struct ofconn *ofconn)
>  {
> -    return ofconn->invalid_ttl_to_controller;
> +    uint32_t bit = 1u << OFPR_INVALID_TTL;
> +    return (ofconn->master_async_config[OAM_PACKET_IN] & bit) != 0;
>  }
>
>  /* Returns the currently configured flow format for 'ofconn', one of NXFF_*.
> @@ -840,6 +851,16 @@ ofconn_set_miss_send_len(struct ofconn *ofconn, int 
> miss_send_len)
>     ofconn->miss_send_len = miss_send_len;
>  }
>
> +void
> +ofconn_set_async_config(struct ofconn *ofconn,
> +                        const uint32_t master_masks[OAM_N_TYPES],
> +                        const uint32_t slave_masks[OAM_N_TYPES])
> +{
> +    size_t size = sizeof ofconn->master_async_config;
> +    memcpy(ofconn->master_async_config, master_masks, size);
> +    memcpy(ofconn->slave_async_config, slave_masks, size);
> +}
> +
>  /* Sends 'msg' on 'ofconn', accounting it as a reply.  (If there is a
>  * sufficient number of OpenFlow replies in-flight on a single ofconn, then 
> the
>  * connmgr will stop accepting new OpenFlow requests on that ofconn until the
> @@ -956,6 +977,8 @@ ofconn_create(struct connmgr *mgr, struct rconn *rconn, 
> enum ofconn_type type)
>  static void
>  ofconn_flush(struct ofconn *ofconn)
>  {
> +    uint32_t *master = ofconn->master_async_config;
> +    uint32_t *slave = ofconn->slave_async_config;
>     int i;
>
>     ofconn->role = NX_ROLE_OTHER;
> @@ -996,6 +1019,24 @@ ofconn_flush(struct ofconn *ofconn)
>
>     rconn_packet_counter_destroy(ofconn->reply_counter);
>     ofconn->reply_counter = rconn_packet_counter_create();
> +
> +    /* "master" and "other" roles get all asynchronous messages by default,
> +     * except that the controller needs to enable nonstandard "packet-in"
> +     * reasons itself. */
> +    master[OAM_PACKET_IN] = (1u << OFPR_NO_MATCH) | (1u << OFPR_ACTION);
> +    master[OAM_PORT_STATUS] = ((1u << OFPPR_ADD)
> +                               | (1u << OFPPR_DELETE)
> +                               | (1u << OFPPR_MODIFY));
> +    master[OAM_FLOW_REMOVED] = ((1u << OFPRR_IDLE_TIMEOUT)
> +                                | (1u << OFPRR_HARD_TIMEOUT)
> +                                | (1u << OFPRR_DELETE));
> +
> +    /* "slave" role gets port status updates by default. */
> +    slave[OAM_PACKET_IN] = 0;
> +    slave[OAM_PORT_STATUS] = ((1u << OFPPR_ADD)
> +                              | (1u << OFPPR_DELETE)
> +                              | (1u << OFPPR_MODIFY));
> +    slave[OAM_FLOW_REMOVED] = 0;
>  }
>
>  static void
> @@ -1100,42 +1141,42 @@ ofconn_wait(struct ofconn *ofconn, bool 
> handling_openflow)
>     }
>  }
>
> -/* Returns true if 'ofconn' should receive asynchronous messages. */
> +/* Returns true if 'ofconn' should receive asynchronous messages of the given
> + * OAM_* 'type' and 'reason', which should be a OFPR_* value for 
> OAM_PACKET_IN,
> + * a OFPPR_* value for OAM_PORT_STATUS, or an OFPRR_* value for
> + * OAM_FLOW_REMOVED.  Returns false if the message should not be sent on
> + * 'ofconn'. */
>  static bool
> -ofconn_receives_async_msgs__(const struct ofconn *ofconn)
> +ofconn_receives_async_msg(const struct ofconn *ofconn,
> +                          enum ofconn_async_msg_type type,
> +                          unsigned int reason)
>  {
> -    if (ofconn->type == OFCONN_PRIMARY) {
> -        /* Primary controllers always get asynchronous messages unless they
> -         * have configured themselves as "slaves".  */
> -        return ofconn->role != NX_ROLE_SLAVE;
> -    } else {
> -        /* Service connections don't get asynchronous messages unless they 
> have
> -         * explicitly asked for them by setting a nonzero miss send length. 
> */
> -        return ofconn->miss_send_len > 0;
> -    }
> -}
> +    const uint32_t *async_config;
> +
> +    assert(reason < 32);
> +    assert((unsigned int) type < OAM_N_TYPES);
>
> -static bool
> -ofconn_receives_async_msgs(const struct ofconn *ofconn)
> -{
>     if (!rconn_is_connected(ofconn->rconn)) {
>         return false;
> -    } else {
> -        return ofconn_receives_async_msgs__(ofconn);
>     }
> -}
>
> -static bool
> -ofconn_interested_in_packet(const struct ofconn *ofconn,
> -                            const struct ofputil_packet_in *pin)
> -{
> -    if (!rconn_is_connected(ofconn->rconn)) {
> +    /* Keep the following code in sync with the documentation in the
> +     * "Asynchronous Messages" section in DESIGN. */
> +
> +    if (ofconn->type == OFCONN_SERVICE && !ofconn->miss_send_len) {
> +        /* Service connections don't get asynchronous messages unless they 
> have
> +         * explicitly asked for them by setting a nonzero miss send length. 
> */
>         return false;
> -    } else if (pin->reason == OFPR_INVALID_TTL) {
> -        return ofconn->invalid_ttl_to_controller;
> -    } else {
> -        return ofconn_receives_async_msgs__(ofconn);
>     }
> +
> +    async_config = (ofconn->role == NX_ROLE_SLAVE
> +                    ? ofconn->slave_async_config
> +                    : ofconn->master_async_config);
> +    if (!(async_config[type] & (1u << reason))) {
> +        return false;
> +    }
> +
> +    return true;
>  }
>
>  /* Returns a human-readable name for an OpenFlow connection between 'mgr' and
> @@ -1195,20 +1236,15 @@ connmgr_send_port_status(struct connmgr *mgr, const 
> struct ofp_phy_port *opp,
>     struct ofconn *ofconn;
>
>     LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
> -        struct ofp_port_status *ops;
> -        struct ofpbuf *b;
> -
> -        /* Primary controllers, even slaves, should always get port status
> -           updates.  Otherwise obey ofconn_receives_async_msgs(). */
> -        if (ofconn->type != OFCONN_PRIMARY
> -            && !ofconn_receives_async_msgs(ofconn)) {
> -            continue;
> +        if (ofconn_receives_async_msg(ofconn, OAM_PORT_STATUS, reason)) {
> +            struct ofp_port_status *ops;
> +            struct ofpbuf *b;
> +
> +            ops = make_openflow_xid(sizeof *ops, OFPT_PORT_STATUS, 0, &b);
> +            ops->reason = reason;
> +            ops->desc = *opp;
> +            ofconn_send(ofconn, b, NULL);
>         }
> -
> -        ops = make_openflow_xid(sizeof *ops, OFPT_PORT_STATUS, 0, &b);
> -        ops->reason = reason;
> -        ops->desc = *opp;
> -        ofconn_send(ofconn, b, NULL);
>     }
>  }
>
> @@ -1221,19 +1257,17 @@ connmgr_send_flow_removed(struct connmgr *mgr,
>     struct ofconn *ofconn;
>
>     LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
> -        struct ofpbuf *msg;
> -
> -        if (!ofconn_receives_async_msgs(ofconn)) {
> -            continue;
> +        if (ofconn_receives_async_msg(ofconn, OAM_FLOW_REMOVED, fr->reason)) 
> {
> +            struct ofpbuf *msg;
> +
> +            /* Account flow expirations as replies to OpenFlow requests.  
> That
> +             * works because preventing OpenFlow requests from being 
> processed
> +             * also prevents new flows from being added (and expiring).  (It
> +             * also prevents processing OpenFlow requests that would not add
> +             * new flows, so it is imperfect.) */
> +            msg = ofputil_encode_flow_removed(fr, ofconn->flow_format);
> +            ofconn_send_reply(ofconn, msg);
>         }
> -
> -        /* Account flow expirations as replies to OpenFlow requests.  That
> -         * works because preventing OpenFlow requests from being processed 
> also
> -         * prevents new flows from being added (and expiring).  (It also
> -         * prevents processing OpenFlow requests that would not add new 
> flows,
> -         * so it is imperfect.) */
> -        msg = ofputil_encode_flow_removed(fr, ofconn->flow_format);
> -        ofconn_send_reply(ofconn, msg);
>     }
>  }
>
> @@ -1247,7 +1281,7 @@ connmgr_send_packet_in(struct connmgr *mgr,
>     struct ofconn *ofconn;
>
>     LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
> -        if (ofconn_interested_in_packet(ofconn, pin)) {
> +        if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->reason)) {
>             schedule_packet_in(ofconn, *pin, flow);
>         }
>     }
> diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
> index 8ff89f3..f9c9f4d 100644
> --- a/ofproto/connmgr.h
> +++ b/ofproto/connmgr.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2009, 2010, 2011 Nicira Networks.
> + * Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -51,6 +51,14 @@ enum ofconn_type {
>     OFCONN_SERVICE              /* A service connection, e.g. "ovs-ofctl". */
>  };
>
> +/* The type of an OpenFlow asynchronous message. */
> +enum ofconn_async_msg_type {
> +    OAM_PACKET_IN,              /* OFPT_PACKET_IN or NXT_PACKET_IN.. */
> +    OAM_PORT_STATUS,            /* OFPT_PORT_STATUS. */
> +    OAM_FLOW_REMOVED,           /* OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED. */
> +    OAM_N_TYPES
> +};
> +
>  /* Basics. */
>  struct connmgr *connmgr_create(struct ofproto *ofproto,
>                                const char *dpif_name, const char *local_name);
> @@ -98,6 +106,10 @@ bool ofconn_get_invalid_ttl_to_controller(struct ofconn 
> *);
>  int ofconn_get_miss_send_len(const struct ofconn *);
>  void ofconn_set_miss_send_len(struct ofconn *, int miss_send_len);
>
> +void ofconn_set_async_config(struct ofconn *,
> +                             const uint32_t master_masks[OAM_N_TYPES],
> +                             const uint32_t slave_masks[OAM_N_TYPES]);
> +
>  void ofconn_send_reply(const struct ofconn *, struct ofpbuf *);
>  void ofconn_send_replies(const struct ofconn *, struct list *);
>  void ofconn_send_error(const struct ofconn *, const struct ofp_header 
> *request,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index b7b0d63..cd9fa3a 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2880,6 +2880,26 @@ handle_nxt_set_packet_in_format(struct ofconn *ofconn,
>  }
>
>  static enum ofperr
> +handle_nxt_set_async_config(struct ofconn *ofconn, const struct ofp_header 
> *oh)
> +{
> +    const struct nx_async_config *msg = (const struct nx_async_config *) oh;
> +    uint32_t master[OAM_N_TYPES];
> +    uint32_t slave[OAM_N_TYPES];
> +
> +    master[OAM_PACKET_IN] = ntohl(msg->packet_in_mask[0]);
> +    master[OAM_PORT_STATUS] = ntohl(msg->port_status_mask[0]);
> +    master[OAM_FLOW_REMOVED] = ntohl(msg->flow_removed_mask[0]);
> +
> +    slave[OAM_PACKET_IN] = ntohl(msg->packet_in_mask[1]);
> +    slave[OAM_PORT_STATUS] = ntohl(msg->port_status_mask[1]);
> +    slave[OAM_FLOW_REMOVED] = ntohl(msg->flow_removed_mask[1]);
> +
> +    ofconn_set_async_config(ofconn, master, slave);
> +
> +    return 0;
> +}
> +
> +static enum ofperr
>  handle_barrier_request(struct ofconn *ofconn, const struct ofp_header *oh)
>  {
>     struct ofp_header *ob;
> @@ -2952,6 +2972,9 @@ handle_openflow__(struct ofconn *ofconn, const struct 
> ofpbuf *msg)
>     case OFPUTIL_NXT_FLOW_MOD:
>         return handle_flow_mod(ofconn, oh);
>
> +    case OFPUTIL_NXT_SET_ASYNC_CONFIG:
> +        return handle_nxt_set_async_config(ofconn, oh);
> +
>         /* Statistics requests. */
>     case OFPUTIL_OFPST_DESC_REQUEST:
>         return handle_desc_stats_request(ofconn, msg->data);
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index cfa8d83..27f878a 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -708,6 +708,26 @@ priority:0,tunnel:0,in_port:0000,tci(vlan:80,pcp:0) 
> mac(80:81:81:81:81:81->82:82
>  ])
>  AT_CLEANUP
>
> +AT_SETUP([NXT_SET_ASYNC_CONFIG])
> +AT_KEYWORDS([ofp-print])
> +AT_CHECK([ovs-ofctl ofp-print "\
> +01 04 00 28 00 00 00 00 00 00 23 20 00 00 00 12 \
> +00 00 10 05 00 00 10 07 00 00 00 03 00 00 00 07 \
> +00 00 00 00 00 00 00 03 \
> +"], [0], [dnl
> +NXT_SET_ASYNC_CONFIG (xid=0x0):
> + master:
> +       PACKET_IN: no_match invalid_ttl 12
> +     PORT_STATUS: add delete
> +    FLOW_REMOVED: (off)
> +
> + slave:
> +       PACKET_IN: no_match action invalid_ttl 12
> +     PORT_STATUS: add delete modify
> +    FLOW_REMOVED: idle hard
> +])
> +AT_CLEANUP
> +
>  AT_SETUP([NXT_SET_FLOW_FORMAT])
>  AT_KEYWORDS([ofp-print])
>  AT_CHECK([ovs-ofctl ofp-print "\
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index b54d1dd..fd5cbe1 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -174,6 +174,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS | 
> STRIP_DURATION | sort], [0], [
>  cookie=0x3, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=3 
> actions=output:0
>  NXST_FLOW reply:
>  ])
> +
>  AT_CHECK([ovs-ofctl del-flows br0 cookie=0x3])
>  AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS | STRIP_DURATION | sort], 
> [0], [dnl
>  cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=1 
> actions=output:0
> @@ -201,3 +202,90 @@ NXST_FLOW reply:
>  ])
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> +
> +AT_SETUP([ofproto - asynchronous message control])
> +OVS_VSWITCHD_START
> +AT_CHECK([ovs-ofctl -P openflow10 monitor br0 --detach --no-chdir --pidfile])
> +check_async () {
> +    printf '\n\n--- check_async %d ---\n\n\n' $1
> +    shift
> +
> +    ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
> +    : > expout
> +
> +    # OFPT_PACKET_IN, OFPR_ACTION
> +    ovs-ofctl -v packet-out br0 none controller 
> '0001020304050010203040501234'
> +    if test X"$1" = X"OFPR_ACTION"; then shift;
> +        echo >>expout "OFPT_PACKET_IN: total_len=14 in_port=NONE (via 
> action) data_len=14 (unbuffered)
> +priority:0,tunnel:0,in_port:0000,tci(0) 
> mac(00:10:20:30:40:50->00:01:02:03:04:05) type:1234 proto:0 tos:0 ttl:0 
> ip(0.0.0.0->0.0.0.0)"
> +    fi
> +
> +    # OFPT_PACKET_IN, OFPR_INVALID_TTL
> +    ovs-ofctl packet-out br0 none dec_ttl 
> '002583dfb4000026b98cb0f908004500003fb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
> +    if test X"$1" = X"OFPR_INVALID_TTL"; then shift;
> +        echo >>expout "OFPT_PACKET_IN: total_len=76 in_port=NONE (via 
> invalid_ttl) data_len=76 (unbuffered)
> +priority:0,tunnel:0,in_port:0000,tci(0) 
> mac(00:26:b9:8c:b0:f9->00:25:83:df:b4:00) type:0800 proto:17 tos:0 ttl:0 
> ip(172.17.55.13->172.16.0.2) port(55155->53) udp_csum:8f6d"
> +    fi
> +
> +    # OFPT_PORT_STATUS, OFPPR_ADD
> +    ovs-vsctl add-port br0 test -- set Interface test type=dummy
> +    if test X"$1" = X"OFPPR_ADD"; then shift;
> +        echo >>expout "OFPT_PORT_STATUS: ADD: 1(test): addr:aa:55:aa:55:00:0x
> +     config:     PORT_DOWN
> +     state:      LINK_DOWN"
> +    fi
> +
> +    # OFPT_PORT_STATUS, OFPPR_DELETE
> +    ovs-vsctl del-port br0 test
> +    if test X"$1" = X"OFPPR_DELETE"; then shift;
> +        echo >>expout "OFPT_PORT_STATUS: DEL: 1(test): addr:aa:55:aa:55:00:0x
> +     config:     PORT_DOWN
> +     state:      LINK_DOWN"
> +    fi
> +
> +    # OFPT_FLOW_REMOVED, OFPRR_DELETE
> +    ovs-ofctl add-flow br0 send_flow_rem,actions=drop
> +    ovs-ofctl --strict del-flows br0 ''
> +    if test X"$1" = X"OFPRR_DELETE"; then shift;
> +        echo >>expout "OFPT_FLOW_REMOVED:  reason=delete"
> +    fi
> +    AT_FAIL_IF([test X"$1" != X])
> +
> +    ovs-appctl -t ovs-ofctl ofctl/barrier
> +    echo >>expout "send: OFPT_BARRIER_REQUEST:
> +OFPT_BARRIER_REPLY:"
> +
> +    AT_CHECK(
> +      [[sed '
> +s/ (xid=0x[0-9a-fA-F]*)//
> +s/ *duration.*//
> +s/00:0.$/00:0x/' < monitor.log]],
> +      [0], [expout])
> +}
> +
> +# It's a service connection so initially there should be no async messages.
> +check_async 1
> +
> +# Set miss_send_len to 128, turning on packet-outs for our service 
> connection.
> +ovs-appctl -t ovs-ofctl ofctl/send 0109000c0123456700000080
> +check_async 2 OFPR_ACTION OFPPR_ADD OFPPR_DELETE OFPRR_DELETE
> +
> +# Set miss_send_len to 128 and enable invalid_ttl.
> +ovs-appctl -t ovs-ofctl ofctl/send 0109000c0123456700040080
> +check_async 3 OFPR_ACTION OFPR_INVALID_TTL OFPPR_ADD OFPPR_DELETE 
> OFPRR_DELETE
> +
> +# Become slave, which should disable everything except port status.
> +ovs-appctl -t ovs-ofctl ofctl/send 0104001400000002000023200000000a00000002
> +check_async 4 OFPPR_ADD OFPPR_DELETE
> +
> +# Use NXT_SET_ASYNC_CONFIG to enable a patchwork of asynchronous messages.
> +ovs-appctl -t ovs-ofctl ofctl/send 
> 01040028000000020000232000000012000000020000000500000005000000020000000200000005
> +check_async 5 OFPR_INVALID_TTL OFPPR_DELETE OFPRR_DELETE
> +
> +# Become master.
> +ovs-appctl -t ovs-ofctl ofctl/send 0104001400000002000023200000000a00000001
> +check_async 6 OFPR_ACTION OFPPR_ADD
> +
> +ovs-appctl -t ovs-ofctl exit
> +AT_CLEANUP
> +
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to