[ovs-dev] [PATCH v4] ofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate.
Alex Wang
alexw at nicira.com
Fri May 16 22:23:11 UTC 2014
Acked-by: Alex Wang <alexw at nicira.com>
Hey Ben, Ethan,
Could either of you help review this?
I think it is ready.
Thanks,
Alex Wang,
On Fri, May 16, 2014 at 6:59 AM, 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>
> ---
> 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
> ---
> ofproto/ofproto-dpif-xlate.c | 561
> ++++++++++++++++++++++++++++++++----------
> ofproto/ofproto-dpif-xlate.h | 24 +-
> ofproto/ofproto-dpif.c | 16 +-
> ofproto/ofproto-dpif.h | 5 +-
> 4 files changed, 450 insertions(+), 156 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index da538b7..9b0023d 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. */
> @@ -292,15 +290,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,
> @@ -312,9 +317,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(const struct ofproto_dpif *,
> + struct xlate_cfg *);
> +static struct xbundle *xbundle_lookup(const struct ofbundle *,
> + struct xlate_cfg *);
> +static struct xport *xport_lookup(const struct ofport_dpif *,
> + struct xlate_cfg *);
> 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);
> @@ -324,10 +332,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 xbridge *, struct xlate_cfg *);
> +static void xlate_xbundle_init(struct xbundle *, struct xlate_cfg *);
> +static void xlate_xport_init(struct xport *, struct xlate_cfg *);
> +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 xbridge *, struct xlate_cfg *);
> +static void xlate_xbundle_remove(struct xbundle *, struct xlate_cfg *);
> +static void xlate_xport_remove(struct xport *, struct xlate_cfg *);
> +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 xbridge *xbridge, struct xlate_cfg *xcfg)
> +{
> + 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 xbundle *xbundle, struct xlate_cfg *xcfg)
> +{
> + 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 xport *xport, struct xlate_cfg *xcfg)
> +{
> + 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,
> @@ -339,17 +413,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);
> @@ -380,9 +443,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;
> @@ -394,10 +454,254 @@ 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_xbridge, new_xcfg);
> +
> + 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_xbundle, new_xcfg);
> +
> + 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_xport, new_xcfg);
> +
> + 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(xport->peer->ofport, new_xcfg);
> + 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_init_new_xcfg();
> + * ...
> + * edit_xlate_configuration();
> + * ...
> + * xlate_set_new_xcfg(); */
> void
> -xlate_remove_ofproto(struct ofproto_dpif *ofproto)
> +xlate_set_new_xcfg()
> +{
> + struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
> +
> + ovsrcu_set(&xcfgp, new_xcfg);
> + ovsrcu_postpone(xlate_xcfg_free, xcfg);
> +}
> +
> +/* Copies the current xlate configuration in xcfgp to new_xcfg.
> + *
> + * This needs to be called prior to editing the xlate configuration. */
> +void
> +xlate_init_new_xcfg(void)
> +{
> + struct xbridge *xbridge;
> + struct xlate_cfg *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(xbridge, xcfg);
> + }
> +
> + 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(ofproto, new_xcfg);
> + if (!xbridge) {
> + xbridge = xzalloc(sizeof *xbridge);
> + xbridge->ofproto = ofproto;
> +
> + xlate_xbridge_init(xbridge, new_xcfg);
> + }
> +
> + 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 xbridge *xbridge, struct xlate_cfg *xcfg)
> {
> - struct xbridge *xbridge = xbridge_lookup(ofproto);
> struct xbundle *xbundle, *next_xbundle;
> struct xport *xport, *next_xport;
>
> @@ -406,14 +710,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(xport, xcfg);
> }
>
> LIST_FOR_EACH_SAFE (xbundle, next_xbundle, list_node,
> &xbridge->xbundles) {
> - xlate_bundle_remove(xbundle->ofbundle);
> + xlate_xbundle_remove(xbundle, xcfg);
> }
>
> - 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);
> @@ -425,50 +729,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(ofproto, new_xcfg);
> + xlate_xbridge_remove(xbridge, new_xcfg);
> +}
> +
> +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(ofbundle, new_xcfg);
> if (!xbundle) {
> xbundle = xzalloc(sizeof *xbundle);
> xbundle->ofbundle = ofbundle;
> - xbundle->xbridge = xbridge_lookup(ofproto);
> + xbundle->xbridge = xbridge_lookup(ofproto, new_xcfg);
>
> - 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(xbundle, new_xcfg);
> }
>
> - 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 xbundle *xbundle, struct xlate_cfg *xcfg)
> {
> - struct xbundle *xbundle = xbundle_lookup(ofbundle);
> struct xport *xport, *next;
>
> if (!xbundle) {
> @@ -480,7 +780,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);
> @@ -489,6 +789,17 @@ xlate_bundle_remove(struct ofbundle *ofbundle)
> }
>
> void
> +xlate_bundle_remove(struct ofbundle *ofbundle)
> +{
> + struct xbundle *xbundle;
> +
> + ovs_assert(new_xcfg);
> +
> + xbundle = xbundle_lookup(ofbundle, new_xcfg);
> + xlate_xbundle_remove(xbundle, new_xcfg);
> +}
> +
> +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,
> @@ -499,49 +810,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(ofport, new_xcfg);
> if (!xport) {
> xport = xzalloc(sizeof *xport);
> xport->ofport = ofport;
> - xport->xbridge = xbridge_lookup(ofproto);
> + xport->xbridge = xbridge_lookup(ofproto, new_xcfg);
> 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(xport, new_xcfg);
> }
>
> 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(peer, new_xcfg);
> if (xport->peer) {
> xport->peer->peer = xport;
> }
> @@ -549,7 +841,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(ofbundle, new_xcfg);
> if (xport->xbundle) {
> list_insert(&xport->xbundle->xports, &xport->bundle_node);
> }
> @@ -572,11 +864,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 xport *xport, struct xlate_cfg *xcfg)
> {
> - struct xport *xport = xport_lookup(ofport);
> -
> if (!xport) {
> return;
> }
> @@ -593,7 +883,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);
> @@ -602,6 +892,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(ofport, new_xcfg);
> + xlate_xport_remove(xport, new_xcfg);
> +}
> +
> /* 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
> @@ -634,11 +935,11 @@ xlate_receive(const struct dpif_backer *backer,
> struct ofpbuf *packet,
> {
> const struct xport *xport;
> int error = ENODEV;
> + struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>
> - 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) {
> @@ -647,11 +948,12 @@ xlate_receive(const struct dpif_backer *backer,
> struct ofpbuf *packet,
>
> xport = xport_lookup(tnl_port_should_receive(flow)
> ? tnl_port_receive(flow)
> - : odp_port_to_ofport(backer,
> flow->in_port.odp_port));
> + : odp_port_to_ofport(backer,
> flow->in_port.odp_port),
> + xcfg);
>
> 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)) {
> @@ -680,23 +982,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(const struct ofproto_dpif *ofproto, struct xlate_cfg *xcfg)
> {
> + 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;
> }
> @@ -705,16 +1007,19 @@ xbridge_lookup(const struct ofproto_dpif *ofproto)
> }
>
> static struct xbundle *
> -xbundle_lookup(const struct ofbundle *ofbundle)
> +xbundle_lookup(const struct ofbundle *ofbundle, struct xlate_cfg *xcfg)
> {
> + 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;
> }
> @@ -723,16 +1028,19 @@ xbundle_lookup(const struct ofbundle *ofbundle)
> }
>
> static struct xport *
> -xport_lookup(const struct ofport_dpif *ofport)
> +xport_lookup(const struct ofport_dpif *ofport, struct xlate_cfg *xcfg)
> {
> + 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;
> }
> @@ -1075,7 +1383,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(out, xcfg);
> if (out_xbundle) {
> output_normal(ctx, out_xbundle, vlan);
> }
> @@ -1220,6 +1529,7 @@ output_normal(struct xlate_ctx *ctx, const struct
> xbundle *out_xbundle,
> struct ofport_dpif *ofport;
> struct xlate_recirc *xr = &ctx->recirc;
> struct flow_wildcards *wc = &ctx->xout->wc;
> + struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>
> if (ctx->xbridge->enable_recirc) {
> ctx->use_recirc = bond_may_recirc(
> @@ -1237,7 +1547,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(ofport, xcfg);
>
> if (!xport) {
> /* No slaves enabled, so drop packet. */
> @@ -1565,7 +1875,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(mac_port, xcfg);
> if (mac_xbundle && mac_xbundle != in_xbundle) {
> xlate_report(ctx, "forwarding to learned port");
> output_normal(ctx, mac_xbundle, vlan);
> @@ -1889,7 +2200,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;
> }
>
> @@ -3169,28 +3479,18 @@ 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 flow_wildcards *wc = &xout->wc;
> struct flow *flow = &xin->flow;
> struct rule_dpif *rule = NULL;
> + struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>
> const struct rule_actions *actions = NULL;
> enum slow_path_reason special;
> @@ -3237,7 +3537,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(xin->ofproto, xcfg);
> if (!ctx.xbridge) {
> return;
> }
> @@ -3475,21 +3775,19 @@ xlate_send_packet(const struct ofport_dpif
> *ofport, struct ofpbuf *packet)
> struct xport *xport;
> struct ofpact_output output;
> struct flow flow;
> + struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>
> ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
> /* Use OFPP_NONE as the in_port to avoid special packet processing. */
> flow_extract(packet, NULL, &flow);
> flow.in_port.ofp_port = OFPP_NONE;
>
> - ovs_rwlock_rdlock(&xlate_rwlock);
> - xport = xport_lookup(ofport);
> + xport = xport_lookup(ofport, xcfg);
> 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,
> @@ -3536,8 +3834,9 @@ xlate_cache_normal(struct ofproto_dpif *ofproto,
> struct flow *flow, int vlan)
> struct xbridge *xbridge;
> struct xbundle *xbundle;
> struct flow_wildcards wc;
> + struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>
> - xbridge = xbridge_lookup(ofproto);
> + xbridge = xbridge_lookup(ofproto, xcfg);
> if (!xbridge) {
> return;
> }
> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
> index 760736a..f5a0c5c 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_init_new_xcfg(void);
> +void xlate_set_new_xcfg(void);
> +
> #endif /* ofproto-dpif-xlate.h */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 926f3d6..d0c5687 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -600,7 +600,7 @@ type_run(const char *type)
> continue;
> }
>
> - ovs_rwlock_wrlock(&xlate_rwlock);
> + xlate_init_new_xcfg();
> xlate_ofproto_set(ofproto, ofproto->up.name,
> ofproto->backer->dpif, ofproto->miss_rule,
> ofproto->no_packet_in_rule, ofproto->ml,
> @@ -633,7 +633,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_set_new_xcfg();
> }
>
> udpif_revalidate(backer->udpif);
> @@ -1293,9 +1293,9 @@ destruct(struct ofproto *ofproto_)
> struct list pins;
>
> ofproto->backer->need_revalidate = REV_RECONFIGURE;
> - ovs_rwlock_wrlock(&xlate_rwlock);
> + xlate_init_new_xcfg();
> xlate_remove_ofproto(ofproto);
> - ovs_rwlock_unlock(&xlate_rwlock);
> + xlate_set_new_xcfg();
>
> /* Ensure that the upcall processing threads have no remaining
> references
> * to the ofproto or anything in it. */
> @@ -1646,9 +1646,9 @@ port_destruct(struct ofport *port_)
> const char *dp_port_name;
>
> ofproto->backer->need_revalidate = REV_RECONFIGURE;
> - ovs_rwlock_wrlock(&xlate_rwlock);
> + xlate_init_new_xcfg();
> xlate_ofport_remove(port);
> - ovs_rwlock_unlock(&xlate_rwlock);
> + xlate_set_new_xcfg();
>
> dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,
> sizeof namebuf);
> @@ -2291,9 +2291,9 @@ bundle_destroy(struct ofbundle *bundle)
> ofproto = bundle->ofproto;
> mbridge_unregister_bundle(ofproto->mbridge, bundle->aux);
>
> - ovs_rwlock_wrlock(&xlate_rwlock);
> + xlate_init_new_xcfg();
> xlate_bundle_remove(bundle);
> - ovs_rwlock_unlock(&xlate_rwlock);
> + xlate_set_new_xcfg();
>
> 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 679a41e..d343d82 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.
> *
> @@ -142,8 +140,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/20140516/b00fcb8a/attachment-0005.html>
More information about the dev
mailing list