On Sat, Aug 03, 2013 at 06:42:06PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson <et...@nicira.com>
> ---
>  ofproto/ofproto-dpif.c |   69 
> +++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 51 insertions(+), 18 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 17bc12f..e2dcd3f 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -486,8 +486,9 @@ struct ofproto_dpif {
>      long long int stp_last_tick;
>  
>      /* VLAN splinters. */
> -    struct hmap realdev_vid_map; /* (realdev,vid) -> vlandev. */
> -    struct hmap vlandev_map;     /* vlandev -> (realdev,vid). */
> +    struct ovs_mutex vsp_mutex;
> +    struct hmap realdev_vid_map OVS_GUARDED; /* (realdev,vid) -> vlandev. */
> +    struct hmap vlandev_map OVS_GUARDED;     /* vlandev -> (realdev,vid). */
>  
>      /* Ports. */
>      struct sset ports;             /* Set of standard port names. */
> @@ -1273,6 +1274,7 @@ construct(struct ofproto *ofproto_)
>      ofproto->ml = mac_learning_create(MAC_ENTRY_DEFAULT_IDLE_TIME);
>      ofproto->mbridge = mbridge_create();
>      ofproto->has_bonded_bundles = false;
> +    ovs_mutex_init(&ofproto->vsp_mutex, PTHREAD_MUTEX_NORMAL);
>  
>      classifier_init(&ofproto->facets);
>      ofproto->consistency_rl = LLONG_MIN;
> @@ -1459,6 +1461,8 @@ destruct(struct ofproto *ofproto_)
>      sset_destroy(&ofproto->ghost_ports);
>      sset_destroy(&ofproto->port_poll_set);
>  
> +    ovs_mutex_destroy(&ofproto->vsp_mutex);
> +
>      close_dpif_backer(ofproto->backer);
>  }
>  
> @@ -6384,20 +6388,20 @@ hash_realdev_vid(ofp_port_t realdev_ofp_port, int vid)
>  
>  bool
>  ofproto_has_vlan_splinters(const struct ofproto_dpif *ofproto)
> +    OVS_EXCLUDED(ofproto->vsp_mutex)
>  {
> -    return !hmap_is_empty(&ofproto->realdev_vid_map);
> +    bool ret;
> +
> +    ovs_mutex_lock(&ofproto->vsp_mutex);
> +    ret = !hmap_is_empty(&ofproto->realdev_vid_map);
> +    ovs_mutex_unlock(&ofproto->vsp_mutex);
> +    return ret;
>  }
>  
> -/* Returns the OFP port number of the Linux VLAN device that corresponds to
> - * 'vlan_tci' on the network device with port number 'realdev_ofp_port' in
> - * 'struct ofport_dpif'.  For example, given 'realdev_ofp_port' of eth0 and
> - * 'vlan_tci' 9, it would return the port number of eth0.9.
> - *
> - * Unless VLAN splinters are enabled for port 'realdev_ofp_port', this
> - * function just returns its 'realdev_ofp_port' argument. */
> -ofp_port_t
> -vsp_realdev_to_vlandev(const struct ofproto_dpif *ofproto,
> -                       ofp_port_t realdev_ofp_port, ovs_be16 vlan_tci)
> +static ofp_port_t
> +vsp_realdev_to_vlandev__(const struct ofproto_dpif *ofproto,
> +                         ofp_port_t realdev_ofp_port, ovs_be16 vlan_tci)
> +    OVS_REQUIRES(ofproto->vsp_mutex)
>  {
>      if (!hmap_is_empty(&ofproto->realdev_vid_map)) {
>          int vid = vlan_tci_to_vid(vlan_tci);
> @@ -6415,6 +6419,26 @@ vsp_realdev_to_vlandev(const struct ofproto_dpif 
> *ofproto,
>      return realdev_ofp_port;
>  }
>  
> +/* Returns the OFP port number of the Linux VLAN device that corresponds to
> + * 'vlan_tci' on the network device with port number 'realdev_ofp_port' in
> + * 'struct ofport_dpif'.  For example, given 'realdev_ofp_port' of eth0 and
> + * 'vlan_tci' 9, it would return the port number of eth0.9.
> + *
> + * Unless VLAN splinters are enabled for port 'realdev_ofp_port', this
> + * function just returns its 'realdev_ofp_port' argument. */
> +ofp_port_t
> +vsp_realdev_to_vlandev(const struct ofproto_dpif *ofproto,
> +                       ofp_port_t realdev_ofp_port, ovs_be16 vlan_tci)
> +    OVS_EXCLUDED(ofproto->vsp_mutex)
> +{
> +    ofp_port_t ret;
> +
> +    ovs_mutex_lock(&ofproto->vsp_mutex);
> +    ret = vsp_realdev_to_vlandev__(ofproto, realdev_ofp_port, vlan_tci);
> +    ovs_mutex_unlock(&ofproto->vsp_mutex);
> +    return ret;
> +}
> +
>  static struct vlan_splinter *
>  vlandev_find(const struct ofproto_dpif *ofproto, ofp_port_t vlandev_ofp_port)
>  {
> @@ -6443,6 +6467,7 @@ vlandev_find(const struct ofproto_dpif *ofproto, 
> ofp_port_t vlandev_ofp_port)
>  static ofp_port_t
>  vsp_vlandev_to_realdev(const struct ofproto_dpif *ofproto,
>                         ofp_port_t vlandev_ofp_port, int *vid)
> +    OVS_REQ_WRLOCK(ofproto->vsp_mutex)
>  {
>      if (!hmap_is_empty(&ofproto->vlandev_map)) {
>          const struct vlan_splinter *vsp;
> @@ -6466,11 +6491,14 @@ vsp_vlandev_to_realdev(const struct ofproto_dpif 
> *ofproto,
>   * making any changes. */
>  static bool
>  vsp_adjust_flow(const struct ofproto_dpif *ofproto, struct flow *flow)
> +    OVS_EXCLUDED(ofproto->vsp_mutex)
>  {
>      ofp_port_t realdev;
>      int vid;
>  
> +    ovs_mutex_lock(&ofproto->vsp_mutex);
>      realdev = vsp_vlandev_to_realdev(ofproto, flow->in_port.ofp_port, &vid);
> +    ovs_mutex_unlock(&ofproto->vsp_mutex);
>      if (!realdev) {
>          return false;
>      }
> @@ -6488,6 +6516,7 @@ vsp_remove(struct ofport_dpif *port)
>      struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
>      struct vlan_splinter *vsp;

I am curious to know why vsp_remove isn't annotated with
OVS_EXCLUDED(ofproto->vsp_mutex). Likewise for vsp_add();

>  
> +    ovs_mutex_lock(&ofproto->vsp_mutex);
>      vsp = vlandev_find(ofproto, port->up.ofp_port);
>      if (vsp) {
>          hmap_remove(&ofproto->vlandev_map, &vsp->vlandev_node);
> @@ -6498,6 +6527,7 @@ vsp_remove(struct ofport_dpif *port)
>      } else {
>          VLOG_ERR("missing vlan device record");
>      }
> +    ovs_mutex_unlock(&ofproto->vsp_mutex);
>  }
>  
>  static void
> @@ -6505,24 +6535,27 @@ vsp_add(struct ofport_dpif *port, ofp_port_t 
> realdev_ofp_port, int vid)
>  {
>      struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
>  
> +    ovs_mutex_lock(&ofproto->vsp_mutex);
>      if (!vsp_vlandev_to_realdev(ofproto, port->up.ofp_port, NULL)
> -        && (vsp_realdev_to_vlandev(ofproto, realdev_ofp_port, htons(vid))
> +        && (vsp_realdev_to_vlandev__(ofproto, realdev_ofp_port, htons(vid))
>              == realdev_ofp_port)) {
>          struct vlan_splinter *vsp;
>  
>          vsp = xmalloc(sizeof *vsp);
> -        hmap_insert(&ofproto->vlandev_map, &vsp->vlandev_node,
> -                    hash_ofp_port(port->up.ofp_port));
> -        hmap_insert(&ofproto->realdev_vid_map, &vsp->realdev_vid_node,
> -                    hash_realdev_vid(realdev_ofp_port, vid));
>          vsp->realdev_ofp_port = realdev_ofp_port;
>          vsp->vlandev_ofp_port = port->up.ofp_port;
>          vsp->vid = vid;
>  
>          port->realdev_ofp_port = realdev_ofp_port;
> +
> +        hmap_insert(&ofproto->vlandev_map, &vsp->vlandev_node,
> +                    hash_ofp_port(port->up.ofp_port));
> +        hmap_insert(&ofproto->realdev_vid_map, &vsp->realdev_vid_node,
> +                    hash_realdev_vid(realdev_ofp_port, vid));
>      } else {
>          VLOG_ERR("duplicate vlan device record");
>      }
> +    ovs_mutex_unlock(&ofproto->vsp_mutex);
>  }
>  
>  static odp_port_t
> -- 
> 1.7.9.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