Just an oversight. Feel free to fix it. Ethan
On Mon, Aug 19, 2013 at 6:29 PM, Simon Horman <ho...@verge.net.au> wrote: > 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