[ovs-dev] [PATCH v5] ofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate.

Alex Wang alexw at nicira.com
Wed May 28 00:43:12 UTC 2014


Applied to master.

Thanks,


On Tue, May 27, 2014 at 3:36 PM, Ryan Wilson <wryan at nicira.com> wrote:

> Before, a global read-write lock protected the ofproto-dpif /
> ofproto-dpif-xlate
> interface. Handler and revalidator threads had to wait while configuration
> was
> being changed. This patch implements RCU locking which allows handlers and
> revalidators to operate while configuration is being updated.
>
> Signed-off-by: Ryan Wilson <wryan at nicira.com>
> Acked-by: Alex Wang <alexw at nicira.com>
> Acked-by: Ethan Jackson <ethan at nicira.com>
> ---
> v2: Split 1 commit into 3:
>   - netdev: Free netdev with ovsrcu_postpone.
>   - ovs-rcu: Add OVSRCU_TYPE_INITIALIZER which initializes OVSRCU_TYPE
> variables
>     to NULL.
>   - ofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate.
> v3: Separate netdev patch (netdev: Remove netdev from global shash when
> the user
> is changing interface configuration) from patch group as it is a more
> general
> fix. Edited xlate RCU locking patch based on the netdev patch.
> v4: Addressed Alex's comments
> v5: Addressed Ethan's comments
> ---
>  ofproto/ofproto-dpif-xlate.c |  570
> ++++++++++++++++++++++++++++++++----------
>  ofproto/ofproto-dpif-xlate.h |   24 +-
>  ofproto/ofproto-dpif.c       |   16 +-
>  ofproto/ofproto-dpif.h       |    5 +-
>  4 files changed, 456 insertions(+), 159 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 43d8571..c230216 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -65,8 +65,6 @@ VLOG_DEFINE_THIS_MODULE(ofproto_dpif_xlate);
>   * recursive or not. */
>  #define MAX_RESUBMITS (MAX_RESUBMIT_RECURSION * MAX_RESUBMIT_RECURSION)
>
> -struct ovs_rwlock xlate_rwlock = OVS_RWLOCK_INITIALIZER;
> -
>  struct xbridge {
>      struct hmap_node hmap_node;   /* Node in global 'xbridges' map. */
>      struct ofproto_dpif *ofproto; /* Key in global 'xbridges' map. */
> @@ -296,15 +294,22 @@ struct xlate_cache {
>      struct ofpbuf entries;
>  };
>
> -static struct hmap xbridges = HMAP_INITIALIZER(&xbridges);
> -static struct hmap xbundles = HMAP_INITIALIZER(&xbundles);
> -static struct hmap xports = HMAP_INITIALIZER(&xports);
> +/* Xlate config contains hash maps of all bridges, bundles and ports.
> + * Xcfgp contains the pointer to the current xlate configuration.
> + * When the main thread needs to change the configuration, it copies
> xcfgp to
> + * new_xcfg and edits new_xcfg. This enables the use of RCU locking which
> + * does not block handler and revalidator threads. */
> +struct xlate_cfg {
> +    struct hmap xbridges;
> +    struct hmap xbundles;
> +    struct hmap xports;
> +};
> +OVSRCU_TYPE(struct xlate_cfg *) xcfgp = OVSRCU_TYPE_INITIALIZER;
> +struct xlate_cfg *new_xcfg = NULL;
>
>  static bool may_receive(const struct xport *, struct xlate_ctx *);
>  static void do_xlate_actions(const struct ofpact *, size_t ofpacts_len,
>                               struct xlate_ctx *);
> -static void xlate_actions__(struct xlate_in *, struct xlate_out *)
> -    OVS_REQ_RDLOCK(xlate_rwlock);
>  static void xlate_normal(struct xlate_ctx *);
>  static void xlate_report(struct xlate_ctx *, const char *);
>  static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
> @@ -316,9 +321,12 @@ static void output_normal(struct xlate_ctx *, const
> struct xbundle *,
>                            uint16_t vlan);
>  static void compose_output_action(struct xlate_ctx *, ofp_port_t
> ofp_port);
>
> -static struct xbridge *xbridge_lookup(const struct ofproto_dpif *);
> -static struct xbundle *xbundle_lookup(const struct ofbundle *);
> -static struct xport *xport_lookup(const struct ofport_dpif *);
> +static struct xbridge *xbridge_lookup(struct xlate_cfg *,
> +                                      const struct ofproto_dpif *);
> +static struct xbundle *xbundle_lookup(struct xlate_cfg *,
> +                                      const struct ofbundle *);
> +static struct xport *xport_lookup(struct xlate_cfg *,
> +                                  const struct ofport_dpif *);
>  static struct xport *get_ofp_port(const struct xbridge *, ofp_port_t
> ofp_port);
>  static struct skb_priority_to_dscp *get_skb_priority(const struct xport *,
>                                                       uint32_t
> skb_priority);
> @@ -328,10 +336,76 @@ static bool dscp_from_skb_priority(const struct
> xport *, uint32_t skb_priority,
>
>  static struct xc_entry *xlate_cache_add_entry(struct xlate_cache *xc,
>                                                enum xc_type type);
> +static void xlate_xbridge_init(struct xlate_cfg *, struct xbridge *);
> +static void xlate_xbundle_init(struct xlate_cfg *, struct xbundle *);
> +static void xlate_xport_init(struct xlate_cfg *, struct xport *);
> +static void xlate_xbridge_set(struct xbridge *xbridge,
> +                              struct dpif *dpif,
> +                              struct rule_dpif *miss_rule,
> +                              struct rule_dpif *no_packet_in_rule,
> +                              const struct mac_learning *ml, struct stp
> *stp,
> +                              const struct mbridge *mbridge,
> +                              const struct dpif_sflow *sflow,
> +                              const struct dpif_ipfix *ipfix,
> +                              const struct netflow *netflow,
> +                              enum ofp_config_flags frag,
> +                              bool forward_bpdu, bool has_in_band,
> +                              bool enable_recirc,
> +                              bool variable_length_userdata,
> +                              size_t max_mpls_depth);
> +static void xlate_xbundle_set(struct xbundle *xbundle,
> +                              enum port_vlan_mode vlan_mode, int vlan,
> +                              unsigned long *trunks, bool
> use_priority_tags,
> +                              const struct bond *bond, const struct lacp
> *lacp,
> +                              bool floodable);
> +static void xlate_xport_set(struct xport *xport, odp_port_t odp_port,
> +                            const struct netdev *netdev, const struct cfm
> *cfm,
> +                            const struct bfd *bfd, int stp_port_no,
> +                            enum ofputil_port_config config,
> +                            enum ofputil_port_state state, bool is_tunnel,
> +                            bool may_enable);
> +static void xlate_xbridge_remove(struct xlate_cfg *, struct xbridge *);
> +static void xlate_xbundle_remove(struct xlate_cfg *, struct xbundle *);
> +static void xlate_xport_remove(struct xlate_cfg *, struct xport *);
> +static void xlate_xbridge_copy(struct xbridge *);
> +static void xlate_xbundle_copy(struct xbridge *, struct xbundle *);
> +static void xlate_xport_copy(struct xbridge *, struct xbundle *,
> +                             struct xport *);
> +static void xlate_xcfg_free(struct xlate_cfg *);
>
> -void
> -xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
> -                  struct dpif *dpif, struct rule_dpif *miss_rule,
> +
> +static void
> +xlate_xbridge_init(struct xlate_cfg *xcfg, struct xbridge *xbridge)
> +{
> +    list_init(&xbridge->xbundles);
> +    hmap_init(&xbridge->xports);
> +    hmap_insert(&xcfg->xbridges, &xbridge->hmap_node,
> +                hash_pointer(xbridge->ofproto, 0));
> +}
> +
> +static void
> +xlate_xbundle_init(struct xlate_cfg *xcfg, struct xbundle *xbundle)
> +{
> +    list_init(&xbundle->xports);
> +    list_insert(&xbundle->xbridge->xbundles, &xbundle->list_node);
> +    hmap_insert(&xcfg->xbundles, &xbundle->hmap_node,
> +                hash_pointer(xbundle->ofbundle, 0));
> +}
> +
> +static void
> +xlate_xport_init(struct xlate_cfg *xcfg, struct xport *xport)
> +{
> +    hmap_init(&xport->skb_priorities);
> +    hmap_insert(&xcfg->xports, &xport->hmap_node,
> +                hash_pointer(xport->ofport, 0));
> +    hmap_insert(&xport->xbridge->xports, &xport->ofp_node,
> +                hash_ofp_port(xport->ofp_port));
> +}
> +
> +static void
> +xlate_xbridge_set(struct xbridge *xbridge,
> +                  struct dpif *dpif,
> +                  struct rule_dpif *miss_rule,
>                    struct rule_dpif *no_packet_in_rule,
>                    const struct mac_learning *ml, struct stp *stp,
>                    const struct mbridge *mbridge,
> @@ -343,17 +417,6 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const
> char *name,
>                    bool variable_length_userdata,
>                    size_t max_mpls_depth)
>  {
> -    struct xbridge *xbridge = xbridge_lookup(ofproto);
> -
> -    if (!xbridge) {
> -        xbridge = xzalloc(sizeof *xbridge);
> -        xbridge->ofproto = ofproto;
> -
> -        hmap_insert(&xbridges, &xbridge->hmap_node, hash_pointer(ofproto,
> 0));
> -        hmap_init(&xbridge->xports);
> -        list_init(&xbridge->xbundles);
> -    }
> -
>      if (xbridge->ml != ml) {
>          mac_learning_unref(xbridge->ml);
>          xbridge->ml = mac_learning_ref(ml);
> @@ -384,9 +447,6 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const
> char *name,
>          xbridge->netflow = netflow_ref(netflow);
>      }
>
> -    free(xbridge->name);
> -    xbridge->name = xstrdup(name);
> -
>      xbridge->dpif = dpif;
>      xbridge->forward_bpdu = forward_bpdu;
>      xbridge->has_in_band = has_in_band;
> @@ -398,10 +458,258 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto,
> const char *name,
>      xbridge->max_mpls_depth = max_mpls_depth;
>  }
>
> +static void
> +xlate_xbundle_set(struct xbundle *xbundle,
> +                  enum port_vlan_mode vlan_mode, int vlan,
> +                  unsigned long *trunks, bool use_priority_tags,
> +                  const struct bond *bond, const struct lacp *lacp,
> +                  bool floodable)
> +{
> +    ovs_assert(xbundle->xbridge);
> +
> +    xbundle->vlan_mode = vlan_mode;
> +    xbundle->vlan = vlan;
> +    xbundle->trunks = trunks;
> +    xbundle->use_priority_tags = use_priority_tags;
> +    xbundle->floodable = floodable;
> +
> +    if (xbundle->bond != bond) {
> +        bond_unref(xbundle->bond);
> +        xbundle->bond = bond_ref(bond);
> +    }
> +
> +    if (xbundle->lacp != lacp) {
> +        lacp_unref(xbundle->lacp);
> +        xbundle->lacp = lacp_ref(lacp);
> +    }
> +}
> +
> +static void
> +xlate_xport_set(struct xport *xport, odp_port_t odp_port,
> +                const struct netdev *netdev, const struct cfm *cfm,
> +                const struct bfd *bfd, int stp_port_no,
> +                enum ofputil_port_config config, enum ofputil_port_state
> state,
> +                bool is_tunnel, bool may_enable)
> +{
> +    xport->config = config;
> +    xport->state = state;
> +    xport->stp_port_no = stp_port_no;
> +    xport->is_tunnel = is_tunnel;
> +    xport->may_enable = may_enable;
> +    xport->odp_port = odp_port;
> +
> +    if (xport->cfm != cfm) {
> +        cfm_unref(xport->cfm);
> +        xport->cfm = cfm_ref(cfm);
> +    }
> +
> +    if (xport->bfd != bfd) {
> +        bfd_unref(xport->bfd);
> +        xport->bfd = bfd_ref(bfd);
> +    }
> +
> +    if (xport->netdev != netdev) {
> +        netdev_close(xport->netdev);
> +        xport->netdev = netdev_ref(netdev);
> +    }
> +}
> +
> +static void
> +xlate_xbridge_copy(struct xbridge *xbridge)
> +{
> +    struct xbundle *xbundle;
> +    struct xport *xport;
> +    struct xbridge *new_xbridge = xzalloc(sizeof *xbridge);
> +    new_xbridge->ofproto = xbridge->ofproto;
> +    new_xbridge->name = xstrdup(xbridge->name);
> +    xlate_xbridge_init(new_xcfg, new_xbridge);
> +
> +    xlate_xbridge_set(new_xbridge,
> +                      xbridge->dpif, xbridge->miss_rule,
> +                      xbridge->no_packet_in_rule, xbridge->ml,
> xbridge->stp,
> +                      xbridge->mbridge, xbridge->sflow, xbridge->ipfix,
> +                      xbridge->netflow, xbridge->frag,
> xbridge->forward_bpdu,
> +                      xbridge->has_in_band, xbridge->enable_recirc,
> +                      xbridge->variable_length_userdata,
> +                      xbridge->max_mpls_depth);
> +    LIST_FOR_EACH (xbundle, list_node, &xbridge->xbundles) {
> +        xlate_xbundle_copy(new_xbridge, xbundle);
> +    }
> +
> +    /* Copy xports which are not part of a xbundle */
> +    HMAP_FOR_EACH (xport, ofp_node, &xbridge->xports) {
> +        if (!xport->xbundle) {
> +            xlate_xport_copy(new_xbridge, NULL, xport);
> +        }
> +    }
> +}
> +
> +static void
> +xlate_xbundle_copy(struct xbridge *xbridge, struct xbundle *xbundle)
> +{
> +    struct xport *xport;
> +    struct xbundle *new_xbundle = xzalloc(sizeof *xbundle);
> +    new_xbundle->ofbundle = xbundle->ofbundle;
> +    new_xbundle->xbridge = xbridge;
> +    new_xbundle->name = xstrdup(xbundle->name);
> +    xlate_xbundle_init(new_xcfg, new_xbundle);
> +
> +    xlate_xbundle_set(new_xbundle, xbundle->vlan_mode,
> +                      xbundle->vlan, xbundle->trunks,
> +                      xbundle->use_priority_tags, xbundle->bond,
> xbundle->lacp,
> +                      xbundle->floodable);
> +    LIST_FOR_EACH (xport, bundle_node, &xbundle->xports) {
> +        xlate_xport_copy(xbridge, new_xbundle, xport);
> +    }
> +}
> +
> +static void
> +xlate_xport_copy(struct xbridge *xbridge, struct xbundle *xbundle,
> +                 struct xport *xport)
> +{
> +    struct skb_priority_to_dscp *pdscp, *new_pdscp;
> +    struct xport *new_xport = xzalloc(sizeof *xport);
> +    new_xport->ofport = xport->ofport;
> +    new_xport->ofp_port = xport->ofp_port;
> +    new_xport->xbridge = xbridge;
> +    xlate_xport_init(new_xcfg, new_xport);
> +
> +    xlate_xport_set(new_xport, xport->odp_port, xport->netdev, xport->cfm,
> +                    xport->bfd, xport->stp_port_no, xport->config,
> xport->state,
> +                    xport->is_tunnel, xport->may_enable);
> +
> +    if (xport->peer) {
> +        struct xport *peer = xport_lookup(new_xcfg, xport->peer->ofport);
> +        if (peer) {
> +            new_xport->peer = peer;
> +            new_xport->peer->peer = new_xport;
> +        }
> +    }
> +
> +    if (xbundle) {
> +        new_xport->xbundle = xbundle;
> +        list_insert(&new_xport->xbundle->xports, &new_xport->bundle_node);
> +    }
> +
> +    HMAP_FOR_EACH (pdscp, hmap_node, &xport->skb_priorities) {
> +        new_pdscp = xmalloc(sizeof *pdscp);
> +        new_pdscp->skb_priority = pdscp->skb_priority;
> +        new_pdscp->dscp = pdscp->dscp;
> +        hmap_insert(&new_xport->skb_priorities, &new_pdscp->hmap_node,
> +                    hash_int(new_pdscp->skb_priority, 0));
> +    }
> +}
> +
> +/* Sets the current xlate configuration to new_xcfg and frees the old
> xlate
> + * configuration in xcfgp.
> + *
> + * This needs to be called after editing the xlate configuration.
> + *
> + * Functions that edit the new xlate configuration are
> + * xlate_<ofport/bundle/ofport>_set and
> xlate_<ofport/bundle/ofport>_remove.
> + *
> + * A sample workflow:
> + *
> + * xlate_txn_start();
> + * ...
> + * edit_xlate_configuration();
> + * ...
> + * xlate_txn_commit(); */
>  void
> -xlate_remove_ofproto(struct ofproto_dpif *ofproto)
> +xlate_txn_commit(void)
> +{
> +    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
> +
> +    ovsrcu_set(&xcfgp, new_xcfg);
> +    ovsrcu_postpone(xlate_xcfg_free, xcfg);
> +
> +    new_xcfg = NULL;
> +}
> +
> +/* Copies the current xlate configuration in xcfgp to new_xcfg.
> + *
> + * This needs to be called prior to editing the xlate configuration. */
> +void
> +xlate_txn_start(void)
> +{
> +    struct xbridge *xbridge;
> +    struct xlate_cfg *xcfg;
> +
> +    ovs_assert(!new_xcfg);
> +
> +    new_xcfg = xmalloc(sizeof *new_xcfg);
> +    hmap_init(&new_xcfg->xbridges);
> +    hmap_init(&new_xcfg->xbundles);
> +    hmap_init(&new_xcfg->xports);
> +
> +    xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
> +    if (!xcfg) {
> +        return;
> +    }
> +
> +    HMAP_FOR_EACH (xbridge, hmap_node, &xcfg->xbridges) {
> +        xlate_xbridge_copy(xbridge);
> +    }
> +}
> +
> +
> +static void
> +xlate_xcfg_free(struct xlate_cfg *xcfg)
> +{
> +    struct xbridge *xbridge, *next_xbridge;
> +
> +    if (!xcfg) {
> +        return;
> +    }
> +
> +    HMAP_FOR_EACH_SAFE (xbridge, next_xbridge, hmap_node,
> &xcfg->xbridges) {
> +        xlate_xbridge_remove(xcfg, xbridge);
> +    }
> +
> +    hmap_destroy(&xcfg->xbridges);
> +    hmap_destroy(&xcfg->xbundles);
> +    hmap_destroy(&xcfg->xports);
> +    free(xcfg);
> +}
> +
> +void
> +xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
> +                  struct dpif *dpif, struct rule_dpif *miss_rule,
> +                  struct rule_dpif *no_packet_in_rule,
> +                  const struct mac_learning *ml, struct stp *stp,
> +                  const struct mbridge *mbridge,
> +                  const struct dpif_sflow *sflow,
> +                  const struct dpif_ipfix *ipfix,
> +                  const struct netflow *netflow, enum ofp_config_flags
> frag,
> +                  bool forward_bpdu, bool has_in_band,
> +                  bool enable_recirc,
> +                  bool variable_length_userdata,
> +                  size_t max_mpls_depth)
> +{
> +    struct xbridge *xbridge;
> +
> +    ovs_assert(new_xcfg);
> +
> +    xbridge = xbridge_lookup(new_xcfg, ofproto);
> +    if (!xbridge) {
> +        xbridge = xzalloc(sizeof *xbridge);
> +        xbridge->ofproto = ofproto;
> +
> +        xlate_xbridge_init(new_xcfg, xbridge);
> +    }
> +
> +    free(xbridge->name);
> +    xbridge->name = xstrdup(name);
> +
> +    xlate_xbridge_set(xbridge, dpif, miss_rule, no_packet_in_rule, ml,
> stp,
> +                      mbridge, sflow, ipfix, netflow, frag, forward_bpdu,
> +                      has_in_band, enable_recirc,
> variable_length_userdata,
> +                      max_mpls_depth);
> +}
> +
> +static void
> +xlate_xbridge_remove(struct xlate_cfg *xcfg, struct xbridge *xbridge)
>  {
> -    struct xbridge *xbridge = xbridge_lookup(ofproto);
>      struct xbundle *xbundle, *next_xbundle;
>      struct xport *xport, *next_xport;
>
> @@ -410,14 +718,14 @@ xlate_remove_ofproto(struct ofproto_dpif *ofproto)
>      }
>
>      HMAP_FOR_EACH_SAFE (xport, next_xport, ofp_node, &xbridge->xports) {
> -        xlate_ofport_remove(xport->ofport);
> +        xlate_xport_remove(xcfg, xport);
>      }
>
>      LIST_FOR_EACH_SAFE (xbundle, next_xbundle, list_node,
> &xbridge->xbundles) {
> -        xlate_bundle_remove(xbundle->ofbundle);
> +        xlate_xbundle_remove(xcfg, xbundle);
>      }
>
> -    hmap_remove(&xbridges, &xbridge->hmap_node);
> +    hmap_remove(&xcfg->xbridges, &xbridge->hmap_node);
>      mac_learning_unref(xbridge->ml);
>      mbridge_unref(xbridge->mbridge);
>      dpif_sflow_unref(xbridge->sflow);
> @@ -429,50 +737,46 @@ xlate_remove_ofproto(struct ofproto_dpif *ofproto)
>  }
>
>  void
> +xlate_remove_ofproto(struct ofproto_dpif *ofproto)
> +{
> +    struct xbridge *xbridge;
> +
> +    ovs_assert(new_xcfg);
> +
> +    xbridge = xbridge_lookup(new_xcfg, ofproto);
> +    xlate_xbridge_remove(new_xcfg, xbridge);
> +}
> +
> +void
>  xlate_bundle_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
>                   const char *name, enum port_vlan_mode vlan_mode, int
> vlan,
>                   unsigned long *trunks, bool use_priority_tags,
>                   const struct bond *bond, const struct lacp *lacp,
>                   bool floodable)
>  {
> -    struct xbundle *xbundle = xbundle_lookup(ofbundle);
> +    struct xbundle *xbundle;
>
> +    ovs_assert(new_xcfg);
> +
> +    xbundle = xbundle_lookup(new_xcfg, ofbundle);
>      if (!xbundle) {
>          xbundle = xzalloc(sizeof *xbundle);
>          xbundle->ofbundle = ofbundle;
> -        xbundle->xbridge = xbridge_lookup(ofproto);
> +        xbundle->xbridge = xbridge_lookup(new_xcfg, ofproto);
>
> -        hmap_insert(&xbundles, &xbundle->hmap_node,
> hash_pointer(ofbundle, 0));
> -        list_insert(&xbundle->xbridge->xbundles, &xbundle->list_node);
> -        list_init(&xbundle->xports);
> +        xlate_xbundle_init(new_xcfg, xbundle);
>      }
>
> -    ovs_assert(xbundle->xbridge);
> -
>      free(xbundle->name);
>      xbundle->name = xstrdup(name);
>
> -    xbundle->vlan_mode = vlan_mode;
> -    xbundle->vlan = vlan;
> -    xbundle->trunks = trunks;
> -    xbundle->use_priority_tags = use_priority_tags;
> -    xbundle->floodable = floodable;
> -
> -    if (xbundle->bond != bond) {
> -        bond_unref(xbundle->bond);
> -        xbundle->bond = bond_ref(bond);
> -    }
> -
> -    if (xbundle->lacp != lacp) {
> -        lacp_unref(xbundle->lacp);
> -        xbundle->lacp = lacp_ref(lacp);
> -    }
> +    xlate_xbundle_set(xbundle, vlan_mode, vlan, trunks,
> +                      use_priority_tags, bond, lacp, floodable);
>  }
>
> -void
> -xlate_bundle_remove(struct ofbundle *ofbundle)
> +static void
> +xlate_xbundle_remove(struct xlate_cfg *xcfg, struct xbundle *xbundle)
>  {
> -    struct xbundle *xbundle = xbundle_lookup(ofbundle);
>      struct xport *xport, *next;
>
>      if (!xbundle) {
> @@ -484,7 +788,7 @@ xlate_bundle_remove(struct ofbundle *ofbundle)
>          xport->xbundle = NULL;
>      }
>
> -    hmap_remove(&xbundles, &xbundle->hmap_node);
> +    hmap_remove(&xcfg->xbundles, &xbundle->hmap_node);
>      list_remove(&xbundle->list_node);
>      bond_unref(xbundle->bond);
>      lacp_unref(xbundle->lacp);
> @@ -493,6 +797,17 @@ xlate_bundle_remove(struct ofbundle *ofbundle)
>  }
>
>  void
> +xlate_bundle_remove(struct ofbundle *ofbundle)
> +{
> +    struct xbundle *xbundle;
> +
> +    ovs_assert(new_xcfg);
> +
> +    xbundle = xbundle_lookup(new_xcfg, ofbundle);
> +    xlate_xbundle_remove(new_xcfg, xbundle);
> +}
> +
> +void
>  xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
>                   struct ofport_dpif *ofport, ofp_port_t ofp_port,
>                   odp_port_t odp_port, const struct netdev *netdev,
> @@ -503,49 +818,30 @@ xlate_ofport_set(struct ofproto_dpif *ofproto,
> struct ofbundle *ofbundle,
>                   enum ofputil_port_state state, bool is_tunnel,
>                   bool may_enable)
>  {
> -    struct xport *xport = xport_lookup(ofport);
>      size_t i;
> +    struct xport *xport;
> +
> +    ovs_assert(new_xcfg);
>
> +    xport = xport_lookup(new_xcfg, ofport);
>      if (!xport) {
>          xport = xzalloc(sizeof *xport);
>          xport->ofport = ofport;
> -        xport->xbridge = xbridge_lookup(ofproto);
> +        xport->xbridge = xbridge_lookup(new_xcfg, ofproto);
>          xport->ofp_port = ofp_port;
>
> -        hmap_init(&xport->skb_priorities);
> -        hmap_insert(&xports, &xport->hmap_node, hash_pointer(ofport, 0));
> -        hmap_insert(&xport->xbridge->xports, &xport->ofp_node,
> -                    hash_ofp_port(xport->ofp_port));
> +        xlate_xport_init(new_xcfg, xport);
>      }
>
>      ovs_assert(xport->ofp_port == ofp_port);
>
> -    xport->config = config;
> -    xport->state = state;
> -    xport->stp_port_no = stp_port_no;
> -    xport->is_tunnel = is_tunnel;
> -    xport->may_enable = may_enable;
> -    xport->odp_port = odp_port;
> -
> -    if (xport->netdev != netdev) {
> -        netdev_close(xport->netdev);
> -        xport->netdev = netdev_ref(netdev);
> -    }
> -
> -    if (xport->cfm != cfm) {
> -        cfm_unref(xport->cfm);
> -        xport->cfm = cfm_ref(cfm);
> -    }
> -
> -    if (xport->bfd != bfd) {
> -        bfd_unref(xport->bfd);
> -        xport->bfd = bfd_ref(bfd);
> -    }
> +    xlate_xport_set(xport, odp_port, netdev, cfm, bfd, stp_port_no,
> config,
> +                    state, is_tunnel, may_enable);
>
>      if (xport->peer) {
>          xport->peer->peer = NULL;
>      }
> -    xport->peer = xport_lookup(peer);
> +    xport->peer = xport_lookup(new_xcfg, peer);
>      if (xport->peer) {
>          xport->peer->peer = xport;
>      }
> @@ -553,7 +849,7 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct
> ofbundle *ofbundle,
>      if (xport->xbundle) {
>          list_remove(&xport->bundle_node);
>      }
> -    xport->xbundle = xbundle_lookup(ofbundle);
> +    xport->xbundle = xbundle_lookup(new_xcfg, ofbundle);
>      if (xport->xbundle) {
>          list_insert(&xport->xbundle->xports, &xport->bundle_node);
>      }
> @@ -576,11 +872,9 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct
> ofbundle *ofbundle,
>      }
>  }
>
> -void
> -xlate_ofport_remove(struct ofport_dpif *ofport)
> +static void
> +xlate_xport_remove(struct xlate_cfg *xcfg, struct xport *xport)
>  {
> -    struct xport *xport = xport_lookup(ofport);
> -
>      if (!xport) {
>          return;
>      }
> @@ -597,7 +891,7 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
>      clear_skb_priorities(xport);
>      hmap_destroy(&xport->skb_priorities);
>
> -    hmap_remove(&xports, &xport->hmap_node);
> +    hmap_remove(&xcfg->xports, &xport->hmap_node);
>      hmap_remove(&xport->xbridge->xports, &xport->ofp_node);
>
>      netdev_close(xport->netdev);
> @@ -606,6 +900,17 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
>      free(xport);
>  }
>
> +void
> +xlate_ofport_remove(struct ofport_dpif *ofport)
> +{
> +    struct xport *xport;
> +
> +    ovs_assert(new_xcfg);
> +
> +    xport = xport_lookup(new_xcfg, ofport);
> +    xlate_xport_remove(new_xcfg, xport);
> +}
> +
>  /* Given a datpath, packet, and flow metadata ('backer', 'packet', and
> 'key'
>   * respectively), populates 'flow' with the result of
> odp_flow_key_to_flow().
>   * Optionally populates 'ofproto' with the ofproto_dpif, 'odp_in_port'
> with
> @@ -636,26 +941,26 @@ xlate_receive(const struct dpif_backer *backer,
> struct ofpbuf *packet,
>                struct dpif_sflow **sflow, struct netflow **netflow,
>                odp_port_t *odp_in_port)
>  {
> -    const struct xport *xport;
> +    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>      int error = ENODEV;
> +    const struct xport *xport;
>
> -    ovs_rwlock_rdlock(&xlate_rwlock);
>      if (odp_flow_key_to_flow(key, key_len, flow) == ODP_FIT_ERROR) {
>          error = EINVAL;
> -        goto exit;
> +        return error;
>      }
>
>      if (odp_in_port) {
>          *odp_in_port = flow->in_port.odp_port;
>      }
>
> -    xport = xport_lookup(tnl_port_should_receive(flow)
> +    xport = xport_lookup(xcfg, tnl_port_should_receive(flow)
>                           ? tnl_port_receive(flow)
>                           : odp_port_to_ofport(backer,
> flow->in_port.odp_port));
>
>      flow->in_port.ofp_port = xport ? xport->ofp_port : OFPP_NONE;
>      if (!xport) {
> -        goto exit;
> +        return error;
>      }
>
>      if (vsp_adjust_flow(xport->xbridge->ofproto, flow)) {
> @@ -684,23 +989,23 @@ xlate_receive(const struct dpif_backer *backer,
> struct ofpbuf *packet,
>      if (netflow) {
>          *netflow = netflow_ref(xport->xbridge->netflow);
>      }
> -
> -exit:
> -    ovs_rwlock_unlock(&xlate_rwlock);
>      return error;
>  }
>
>  static struct xbridge *
> -xbridge_lookup(const struct ofproto_dpif *ofproto)
> +xbridge_lookup(struct xlate_cfg *xcfg, const struct ofproto_dpif *ofproto)
>  {
> +    struct hmap *xbridges;
>      struct xbridge *xbridge;
>
> -    if (!ofproto) {
> +    if (!ofproto || !xcfg) {
>          return NULL;
>      }
>
> +    xbridges = &xcfg->xbridges;
> +
>      HMAP_FOR_EACH_IN_BUCKET (xbridge, hmap_node, hash_pointer(ofproto, 0),
> -                             &xbridges) {
> +                             xbridges) {
>          if (xbridge->ofproto == ofproto) {
>              return xbridge;
>          }
> @@ -709,16 +1014,19 @@ xbridge_lookup(const struct ofproto_dpif *ofproto)
>  }
>
>  static struct xbundle *
> -xbundle_lookup(const struct ofbundle *ofbundle)
> +xbundle_lookup(struct xlate_cfg *xcfg, const struct ofbundle *ofbundle)
>  {
> +    struct hmap *xbundles;
>      struct xbundle *xbundle;
>
> -    if (!ofbundle) {
> +    if (!ofbundle || !xcfg) {
>          return NULL;
>      }
>
> +    xbundles = &xcfg->xbundles;
> +
>      HMAP_FOR_EACH_IN_BUCKET (xbundle, hmap_node, hash_pointer(ofbundle,
> 0),
> -                             &xbundles) {
> +                             xbundles) {
>          if (xbundle->ofbundle == ofbundle) {
>              return xbundle;
>          }
> @@ -727,16 +1035,19 @@ xbundle_lookup(const struct ofbundle *ofbundle)
>  }
>
>  static struct xport *
> -xport_lookup(const struct ofport_dpif *ofport)
> +xport_lookup(struct xlate_cfg *xcfg, const struct ofport_dpif *ofport)
>  {
> +    struct hmap *xports;
>      struct xport *xport;
>
> -    if (!ofport) {
> +    if (!ofport || !xcfg) {
>          return NULL;
>      }
>
> +    xports = &xcfg->xports;
> +
>      HMAP_FOR_EACH_IN_BUCKET (xport, hmap_node, hash_pointer(ofport, 0),
> -                             &xports) {
> +                             xports) {
>          if (xport->ofport == ofport) {
>              return xport;
>          }
> @@ -1078,7 +1389,8 @@ add_mirror_actions(struct xlate_ctx *ctx, const
> struct flow *orig_flow)
>          mirrors &= ~dup_mirrors;
>          ctx->xout->mirrors |= dup_mirrors;
>          if (out) {
> -            struct xbundle *out_xbundle = xbundle_lookup(out);
> +            struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *,
> &xcfgp);
> +            struct xbundle *out_xbundle = xbundle_lookup(xcfg, out);
>              if (out_xbundle) {
>                  output_normal(ctx, out_xbundle, vlan);
>              }
> @@ -1220,9 +1532,10 @@ output_normal(struct xlate_ctx *ctx, const struct
> xbundle *out_xbundle,
>          xport = CONTAINER_OF(list_front(&out_xbundle->xports), struct
> xport,
>                               bundle_node);
>      } else {
> -        struct ofport_dpif *ofport;
> -        struct xlate_recirc *xr = &ctx->recirc;
> +        struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>          struct flow_wildcards *wc = &ctx->xout->wc;
> +        struct xlate_recirc *xr = &ctx->recirc;
> +        struct ofport_dpif *ofport;
>
>          if (ctx->xbridge->enable_recirc) {
>              ctx->use_recirc = bond_may_recirc(
> @@ -1240,7 +1553,7 @@ output_normal(struct xlate_ctx *ctx, const struct
> xbundle *out_xbundle,
>
>          ofport = bond_choose_output_slave(out_xbundle->bond,
>                                            &ctx->xin->flow, wc, vid);
> -        xport = xport_lookup(ofport);
> +        xport = xport_lookup(xcfg, ofport);
>
>          if (!xport) {
>              /* No slaves enabled, so drop packet. */
> @@ -1585,7 +1898,8 @@ xlate_normal(struct xlate_ctx *ctx)
>      ovs_rwlock_unlock(&ctx->xbridge->ml->rwlock);
>
>      if (mac_port) {
> -        struct xbundle *mac_xbundle = xbundle_lookup(mac_port);
> +        struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
> +        struct xbundle *mac_xbundle = xbundle_lookup(xcfg, mac_port);
>          if (mac_xbundle && mac_xbundle != in_xbundle) {
>              xlate_report(ctx, "forwarding to learned port");
>              output_normal(ctx, mac_xbundle, vlan);
> @@ -1899,7 +2213,6 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
>              entry->u.dev.rx = netdev_ref(peer->netdev);
>              entry->u.dev.bfd = bfd_ref(peer->bfd);
>          }
> -
>          return;
>      }
>
> @@ -3198,25 +3511,15 @@ actions_output_to_local_port(const struct
> xlate_ctx *ctx)
>      return false;
>  }
>
> -/* Thread safe call to xlate_actions__(). */
> -void
> -xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
> -    OVS_EXCLUDED(xlate_rwlock)
> -{
> -    ovs_rwlock_rdlock(&xlate_rwlock);
> -    xlate_actions__(xin, xout);
> -    ovs_rwlock_unlock(&xlate_rwlock);
> -}
> -
>  /* Translates the 'ofpacts_len' bytes of "struct ofpacts" starting at
> 'ofpacts'
>   * into datapath actions in 'odp_actions', using 'ctx'.
>   *
>   * The caller must take responsibility for eventually freeing 'xout', with
>   * xlate_out_uninit(). */
> -static void
> -xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
> -    OVS_REQ_RDLOCK(xlate_rwlock)
> +void
> +xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
>  {
> +    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>      struct flow_wildcards *wc = &xout->wc;
>      struct flow *flow = &xin->flow;
>      struct rule_dpif *rule = NULL;
> @@ -3266,7 +3569,7 @@ xlate_actions__(struct xlate_in *xin, struct
> xlate_out *xout)
>                      sizeof ctx.xout->odp_actions_stub);
>      ofpbuf_reserve(&ctx.xout->odp_actions, NL_A_U32_SIZE);
>
> -    ctx.xbridge = xbridge_lookup(xin->ofproto);
> +    ctx.xbridge = xbridge_lookup(xcfg, xin->ofproto);
>      if (!ctx.xbridge) {
>          return;
>      }
> @@ -3499,6 +3802,7 @@ xlate_actions__(struct xlate_in *xin, struct
> xlate_out *xout)
>  int
>  xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
>  {
> +    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>      struct xport *xport;
>      struct ofpact_output output;
>      struct flow flow;
> @@ -3508,15 +3812,12 @@ xlate_send_packet(const struct ofport_dpif
> *ofport, struct ofpbuf *packet)
>      flow_extract(packet, NULL, &flow);
>      flow.in_port.ofp_port = OFPP_NONE;
>
> -    ovs_rwlock_rdlock(&xlate_rwlock);
> -    xport = xport_lookup(ofport);
> +    xport = xport_lookup(xcfg, ofport);
>      if (!xport) {
> -        ovs_rwlock_unlock(&xlate_rwlock);
>          return EINVAL;
>      }
>      output.port = xport->ofp_port;
>      output.max_len = 0;
> -    ovs_rwlock_unlock(&xlate_rwlock);
>
>      return ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow,
> NULL,
>                                          &output.ofpact, sizeof output,
> @@ -3560,11 +3861,12 @@ xlate_cache_netdev(struct xc_entry *entry, const
> struct dpif_flow_stats *stats)
>  static void
>  xlate_cache_normal(struct ofproto_dpif *ofproto, struct flow *flow, int
> vlan)
>  {
> +    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>      struct xbridge *xbridge;
>      struct xbundle *xbundle;
>      struct flow_wildcards wc;
>
> -    xbridge = xbridge_lookup(ofproto);
> +    xbridge = xbridge_lookup(xcfg, ofproto);
>      if (!xbridge) {
>          return;
>      }
> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
> index 760736a..6065db3 100644
> --- a/ofproto/ofproto-dpif-xlate.h
> +++ b/ofproto/ofproto-dpif-xlate.h
> @@ -136,8 +136,6 @@ struct xlate_in {
>      struct xlate_cache *xcache;
>  };
>
> -extern struct ovs_rwlock xlate_rwlock;
> -
>  void xlate_ofproto_set(struct ofproto_dpif *, const char *name,
>                         struct dpif *, struct rule_dpif *miss_rule,
>                         struct rule_dpif *no_packet_in_rule,
> @@ -147,16 +145,15 @@ void xlate_ofproto_set(struct ofproto_dpif *, const
> char *name,
>                         enum ofp_config_flags, bool forward_bpdu,
>                         bool has_in_band, bool enable_recirc,
>                         bool variable_length_userdata,
> -                       size_t mpls_label_stack_length)
> -    OVS_REQ_WRLOCK(xlate_rwlock);
> -void xlate_remove_ofproto(struct ofproto_dpif *)
> OVS_REQ_WRLOCK(xlate_rwlock);
> +                       size_t mpls_label_stack_length);
> +void xlate_remove_ofproto(struct ofproto_dpif *);
>
>  void xlate_bundle_set(struct ofproto_dpif *, struct ofbundle *,
>                        const char *name, enum port_vlan_mode, int vlan,
>                        unsigned long *trunks, bool use_priority_tags,
>                        const struct bond *, const struct lacp *,
> -                      bool floodable) OVS_REQ_WRLOCK(xlate_rwlock);
> -void xlate_bundle_remove(struct ofbundle *) OVS_REQ_WRLOCK(xlate_rwlock);
> +                      bool floodable);
> +void xlate_bundle_remove(struct ofbundle *);
>
>  void xlate_ofport_set(struct ofproto_dpif *, struct ofbundle *,
>                        struct ofport_dpif *, ofp_port_t, odp_port_t,
> @@ -165,18 +162,16 @@ void xlate_ofport_set(struct ofproto_dpif *, struct
> ofbundle *,
>                        int stp_port_no, const struct ofproto_port_queue
> *qdscp,
>                        size_t n_qdscp, enum ofputil_port_config,
>                        enum ofputil_port_state, bool is_tunnel,
> -                      bool may_enable) OVS_REQ_WRLOCK(xlate_rwlock);
> -void xlate_ofport_remove(struct ofport_dpif *)
> OVS_REQ_WRLOCK(xlate_rwlock);
> +                      bool may_enable);
> +void xlate_ofport_remove(struct ofport_dpif *);
>
>  int xlate_receive(const struct dpif_backer *, struct ofpbuf *packet,
>                    const struct nlattr *key, size_t key_len,
>                    struct flow *, struct ofproto_dpif **, struct
> dpif_ipfix **,
>                    struct dpif_sflow **, struct netflow **,
> -                  odp_port_t *odp_in_port)
> -    OVS_EXCLUDED(xlate_rwlock);
> +                  odp_port_t *odp_in_port);
>
> -void xlate_actions(struct xlate_in *, struct xlate_out *)
> -    OVS_EXCLUDED(xlate_rwlock);
> +void xlate_actions(struct xlate_in *, struct xlate_out *);
>  void xlate_in_init(struct xlate_in *, struct ofproto_dpif *,
>                     const struct flow *, struct rule_dpif *,
>                     uint16_t tcp_flags, const struct ofpbuf *packet);
> @@ -192,4 +187,7 @@ void xlate_push_stats(struct xlate_cache *, bool
> may_learn,
>  void xlate_cache_clear(struct xlate_cache *);
>  void xlate_cache_delete(struct xlate_cache *);
>
> +void xlate_txn_start(void);
> +void xlate_txn_commit(void);
> +
>  #endif /* ofproto-dpif-xlate.h */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index b95e78a..06be234 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -599,7 +599,7 @@ type_run(const char *type)
>                  continue;
>              }
>
> -            ovs_rwlock_wrlock(&xlate_rwlock);
> +            xlate_txn_start();
>              xlate_ofproto_set(ofproto, ofproto->up.name,
>                                ofproto->backer->dpif, ofproto->miss_rule,
>                                ofproto->no_packet_in_rule, ofproto->ml,
> @@ -632,7 +632,7 @@ type_run(const char *type)
>                                   ofport->up.pp.config,
> ofport->up.pp.state,
>                                   ofport->is_tunnel, ofport->may_enable);
>              }
> -            ovs_rwlock_unlock(&xlate_rwlock);
> +            xlate_txn_commit();
>          }
>
>          udpif_revalidate(backer->udpif);
> @@ -1292,9 +1292,9 @@ destruct(struct ofproto *ofproto_)
>      struct list pins;
>
>      ofproto->backer->need_revalidate = REV_RECONFIGURE;
> -    ovs_rwlock_wrlock(&xlate_rwlock);
> +    xlate_txn_start();
>      xlate_remove_ofproto(ofproto);
> -    ovs_rwlock_unlock(&xlate_rwlock);
> +    xlate_txn_commit();
>
>      /* Ensure that the upcall processing threads have no remaining
> references
>       * to the ofproto or anything in it. */
> @@ -1640,9 +1640,9 @@ port_destruct(struct ofport *port_)
>      const char *dp_port_name;
>
>      ofproto->backer->need_revalidate = REV_RECONFIGURE;
> -    ovs_rwlock_wrlock(&xlate_rwlock);
> +    xlate_txn_start();
>      xlate_ofport_remove(port);
> -    ovs_rwlock_unlock(&xlate_rwlock);
> +    xlate_txn_commit();
>
>      dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,
>                                                sizeof namebuf);
> @@ -2285,9 +2285,9 @@ bundle_destroy(struct ofbundle *bundle)
>      ofproto = bundle->ofproto;
>      mbridge_unregister_bundle(ofproto->mbridge, bundle->aux);
>
> -    ovs_rwlock_wrlock(&xlate_rwlock);
> +    xlate_txn_start();
>      xlate_bundle_remove(bundle);
> -    ovs_rwlock_unlock(&xlate_rwlock);
> +    xlate_txn_commit();
>
>      LIST_FOR_EACH_SAFE (port, next_port, bundle_node, &bundle->ports) {
>          bundle_del_port(port);
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index c238b24..51f943f 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -53,8 +53,6 @@ enum rule_dpif_lookup_verdict {
>                                               * dropped. */
>  };
>
> -/* For lock annotation below only. */
> -extern struct ovs_rwlock xlate_rwlock;
>
>  /* Ofproto-dpif -- DPIF based ofproto implementation.
>   *
> @@ -144,8 +142,7 @@ bool vsp_adjust_flow(const struct ofproto_dpif *,
> struct flow *);
>
>  int ofproto_dpif_execute_actions(struct ofproto_dpif *, const struct flow
> *,
>                                   struct rule_dpif *, const struct ofpact
> *,
> -                                 size_t ofpacts_len, struct ofpbuf *)
> -    OVS_EXCLUDED(xlate_rwlock);
> +                                 size_t ofpacts_len, struct ofpbuf *);
>  void ofproto_dpif_send_packet_in(struct ofproto_dpif *,
>                                   struct ofproto_packet_in *);
>  bool ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *);
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140527/90c2a797/attachment-0005.html>


More information about the dev mailing list