| Message ID | 1461104928-33500-11-git-send-email-diproiettod@vmware.com |
|---|---|
| State | Superseded |
| Headers | show |
On 20.04.2016 01:28, diproiettod at vmware.com (Daniele Di Proietto) wrote: > netdev objects are hard to use with RCU, because it's not possible to > split removal and reclamation. Postponing the removal means that the > port is not removed and cannot be readded immediately. Waiting for > reclamation means introducing a quiescent state, and that may introduce > subtle bugs, due to the RCU model we use in userspace. > > This commit changes the port container from cmap to hmap. 'port_mutex' > must be held by readers and writers. This shouldn't have performace > impact, as readers in the fast path use a thread local cache. > > Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com> > --- > lib/dpif-netdev.c | 96 +++++++++++++++++++++++++++++++++---------------------- > 1 file changed, 57 insertions(+), 39 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index bd2249e..8cc37e2 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -195,9 +195,10 @@ struct dp_netdev { > > /* Ports. > * > - * Protected by RCU. Take the mutex to add or remove ports. */ > + * Any lookup into 'ports' or any access to the dp_netdev_ports found > + * through 'ports' requires taking 'port_mutex'. */ > struct ovs_mutex port_mutex; > - struct cmap ports; > + struct hmap ports; > struct seq *port_seq; /* Incremented whenever a port changes. */ > > /* Protects access to ofproto-dpif-upcall interface during revalidator > @@ -228,7 +229,8 @@ struct dp_netdev { > }; > > static struct dp_netdev_port *dp_netdev_lookup_port(const struct dp_netdev *dp, > - odp_port_t); > + odp_port_t) > + OVS_REQUIRES(&dp->port_mutex); OVS_REQUIRES(dp->port_mutex); here and 2 times more below. > > enum dp_stat_type { > DP_STAT_EXACT_HIT, /* Packets that had an exact match (emc). */ > @@ -248,7 +250,7 @@ enum pmd_cycles_counter_type { > struct dp_netdev_port { > odp_port_t port_no; > struct netdev *netdev; > - struct cmap_node node; /* Node in dp_netdev's 'ports'. */ > + struct hmap_node node; /* Node in dp_netdev's 'ports'. */ > struct netdev_saved_flags *sf; > unsigned n_rxq; /* Number of elements in 'rxq' */ > struct netdev_rxq **rxq; > @@ -476,9 +478,11 @@ struct dpif_netdev { > }; > > static int get_port_by_number(struct dp_netdev *dp, odp_port_t port_no, > - struct dp_netdev_port **portp); > + struct dp_netdev_port **portp) > + OVS_REQUIRES(dp->port_mutex); > static int get_port_by_name(struct dp_netdev *dp, const char *devname, > - struct dp_netdev_port **portp); > + struct dp_netdev_port **portp) > + OVS_REQUIRES(dp->port_mutex); > static void dp_netdev_free(struct dp_netdev *) > OVS_REQUIRES(dp_netdev_mutex); > static int do_add_port(struct dp_netdev *dp, const char *devname, > @@ -522,7 +526,8 @@ dp_netdev_add_rxq_to_pmd(struct dp_netdev_pmd_thread *pmd, > struct dp_netdev_port *port, struct netdev_rxq *rx); > static struct dp_netdev_pmd_thread * > dp_netdev_less_loaded_pmd_on_numa(struct dp_netdev *dp, int numa_id); > -static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp); > +static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp) > + OVS_REQUIRES(dp->port_mutex); > static bool dp_netdev_pmd_try_ref(struct dp_netdev_pmd_thread *pmd); > static void dp_netdev_pmd_unref(struct dp_netdev_pmd_thread *pmd); > static void dp_netdev_pmd_flow_flush(struct dp_netdev_pmd_thread *pmd); > @@ -913,7 +918,7 @@ create_dp_netdev(const char *name, const struct dpif_class *class, > atomic_flag_clear(&dp->destroyed); > > ovs_mutex_init(&dp->port_mutex); > - cmap_init(&dp->ports); > + hmap_init(&dp->ports); > dp->port_seq = seq_create(); > fat_rwlock_init(&dp->upcall_rwlock); > > @@ -984,7 +989,7 @@ static void > dp_netdev_free(struct dp_netdev *dp) > OVS_REQUIRES(dp_netdev_mutex) > { > - struct dp_netdev_port *port; > + struct dp_netdev_port *port, *next; > > shash_find_and_delete(&dp_netdevs, dp->name); > > @@ -993,15 +998,14 @@ dp_netdev_free(struct dp_netdev *dp) > ovsthread_key_delete(dp->per_pmd_key); > > ovs_mutex_lock(&dp->port_mutex); > - CMAP_FOR_EACH (port, node, &dp->ports) { > - /* PMD threads are destroyed here. do_del_port() cannot quiesce */ > + HMAP_FOR_EACH_SAFE (port, next, node, &dp->ports) { > do_del_port(dp, port); > } > ovs_mutex_unlock(&dp->port_mutex); > cmap_destroy(&dp->poll_threads); > > seq_destroy(dp->port_seq); > - cmap_destroy(&dp->ports); > + hmap_destroy(&dp->ports); > ovs_mutex_destroy(&dp->port_mutex); > > /* Upcalls must be disabled at this point */ > @@ -1222,7 +1226,7 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type, > return error; > } > > - cmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); > + hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); > > dp_netdev_add_port_to_pmds(dp, port); > seq_change(dp->port_seq); > @@ -1288,10 +1292,11 @@ is_valid_port_number(odp_port_t port_no) > > static struct dp_netdev_port * > dp_netdev_lookup_port(const struct dp_netdev *dp, odp_port_t port_no) > + OVS_REQUIRES(&dp->port_mutex) here > { > struct dp_netdev_port *port; > > - CMAP_FOR_EACH_WITH_HASH (port, node, hash_port_no(port_no), &dp->ports) { > + HMAP_FOR_EACH_WITH_HASH (port, node, hash_port_no(port_no), &dp->ports) { > if (port->port_no == port_no) { > return port; > } > @@ -1302,6 +1307,7 @@ dp_netdev_lookup_port(const struct dp_netdev *dp, odp_port_t port_no) > static int > get_port_by_number(struct dp_netdev *dp, > odp_port_t port_no, struct dp_netdev_port **portp) > + OVS_REQUIRES(&dp->port_mutex) and here. > { > if (!is_valid_port_number(port_no)) { > *portp = NULL; > @@ -1338,7 +1344,7 @@ get_port_by_name(struct dp_netdev *dp, > { > struct dp_netdev_port *port; > > - CMAP_FOR_EACH (port, node, &dp->ports) { > + HMAP_FOR_EACH (port, node, &dp->ports) { > if (!strcmp(netdev_get_name(port->netdev), devname)) { > *portp = port; > return 0; > @@ -1373,10 +1379,11 @@ get_n_pmd_threads_on_numa(struct dp_netdev *dp, int numa_id) > * is on numa node 'numa_id'. */ > static bool > has_pmd_port_for_numa(struct dp_netdev *dp, int numa_id) > + OVS_REQUIRES(dp->port_mutex) > { > struct dp_netdev_port *port; > > - CMAP_FOR_EACH (port, node, &dp->ports) { > + HMAP_FOR_EACH (port, node, &dp->ports) { > if (netdev_is_pmd(port->netdev) > && netdev_get_numa_id(port->netdev) == numa_id) { > return true; > @@ -1391,7 +1398,7 @@ static void > do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port) > OVS_REQUIRES(dp->port_mutex) > { > - cmap_remove(&dp->ports, &port->node, hash_odp_port(port->port_no)); > + hmap_remove(&dp->ports, &port->node); > seq_change(dp->port_seq); > > dp_netdev_del_port_from_all_pmds(dp, port); > @@ -1428,10 +1435,12 @@ dpif_netdev_port_query_by_number(const struct dpif *dpif, odp_port_t port_no, > struct dp_netdev_port *port; > int error; > > + ovs_mutex_lock(&dp->port_mutex); > error = get_port_by_number(dp, port_no, &port); > if (!error && dpif_port) { > answer_port_query(port, dpif_port); > } > + ovs_mutex_unlock(&dp->port_mutex); > > return error; > } > @@ -1516,7 +1525,7 @@ dpif_netdev_flow_flush(struct dpif *dpif) > } > > struct dp_netdev_port_state { > - struct cmap_position position; > + struct hmap_position position; > char *name; > }; > > @@ -1533,10 +1542,11 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, > { > struct dp_netdev_port_state *state = state_; > struct dp_netdev *dp = get_dp_netdev(dpif); > - struct cmap_node *node; > + struct hmap_node *node; > int retval; > > - node = cmap_next_position(&dp->ports, &state->position); > + ovs_mutex_lock(&dp->port_mutex); > + node = hmap_at_position(&dp->ports, &state->position); > if (node) { > struct dp_netdev_port *port; > > @@ -1552,6 +1562,7 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, > } else { > retval = EOF; > } > + ovs_mutex_unlock(&dp->port_mutex); > > return retval; > } > @@ -2407,8 +2418,8 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) > dp_netdev_execute_actions(pmd, &pp, 1, false, execute->actions, > execute->actions_len); > if (pmd->core_id == NON_PMD_CORE_ID) { > - dp_netdev_pmd_unref(pmd); > ovs_mutex_unlock(&dp->non_pmd_mutex); > + dp_netdev_pmd_unref(pmd); > } > > return 0; > @@ -2449,14 +2460,17 @@ pmd_config_changed(const struct dp_netdev *dp, const char *cmask) > { > struct dp_netdev_port *port; > > - CMAP_FOR_EACH (port, node, &dp->ports) { > + ovs_mutex_lock(&dp->port_mutex); > + HMAP_FOR_EACH (port, node, &dp->ports) { > struct netdev *netdev = port->netdev; > int requested_n_rxq = netdev_requested_n_rxq(netdev); > if (netdev_is_pmd(netdev) > && port->latest_requested_n_rxq != requested_n_rxq) { > + ovs_mutex_unlock(&dp->port_mutex); > return true; > } > } > + ovs_mutex_unlock(&dp->port_mutex); > > if (dp->pmd_cmask != NULL && cmask != NULL) { > return strcmp(dp->pmd_cmask, cmask); > @@ -2476,7 +2490,8 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask) > > dp_netdev_destroy_all_pmds(dp); > > - CMAP_FOR_EACH (port, node, &dp->ports) { > + ovs_mutex_lock(&dp->port_mutex); > + HMAP_FOR_EACH (port, node, &dp->ports) { > struct netdev *netdev = port->netdev; > int requested_n_rxq = netdev_requested_n_rxq(netdev); > if (netdev_is_pmd(port->netdev) > @@ -2498,6 +2513,7 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask) > VLOG_ERR("Failed to set dpdk interface %s rx_queue to:" > " %u", netdev_get_name(port->netdev), > requested_n_rxq); > + ovs_mutex_unlock(&dp->port_mutex); > return err; > } > port->latest_requested_n_rxq = requested_n_rxq; > @@ -2518,6 +2534,7 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask) > dp_netdev_set_nonpmd(dp); > /* Restores all pmd threads. */ > dp_netdev_reset_pmd_threads(dp); > + ovs_mutex_unlock(&dp->port_mutex); > } > > return 0; > @@ -2627,8 +2644,9 @@ dpif_netdev_run(struct dpif *dpif) > NON_PMD_CORE_ID); > uint64_t new_tnl_seq; > > + ovs_mutex_lock(&dp->port_mutex); > ovs_mutex_lock(&dp->non_pmd_mutex); > - CMAP_FOR_EACH (port, node, &dp->ports) { > + HMAP_FOR_EACH (port, node, &dp->ports) { > if (!netdev_is_pmd(port->netdev)) { > int i; > > @@ -2638,6 +2656,7 @@ dpif_netdev_run(struct dpif *dpif) > } > } > ovs_mutex_unlock(&dp->non_pmd_mutex); > + ovs_mutex_unlock(&dp->port_mutex); > dp_netdev_pmd_unref(non_pmd); > > tnl_neigh_cache_run(); > @@ -2658,7 +2677,8 @@ dpif_netdev_wait(struct dpif *dpif) > struct dp_netdev *dp = get_dp_netdev(dpif); > > ovs_mutex_lock(&dp_netdev_mutex); > - CMAP_FOR_EACH (port, node, &dp->ports) { > + ovs_mutex_lock(&dp->port_mutex); > + HMAP_FOR_EACH (port, node, &dp->ports) { > if (!netdev_is_pmd(port->netdev)) { > int i; > > @@ -2667,6 +2687,7 @@ dpif_netdev_wait(struct dpif *dpif) > } > } > } > + ovs_mutex_unlock(&dp->port_mutex); > ovs_mutex_unlock(&dp_netdev_mutex); > seq_wait(tnl_conf_seq, dp->last_tnl_conf_seq); > } > @@ -3296,12 +3317,13 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id) > * new configuration. */ > static void > dp_netdev_reset_pmd_threads(struct dp_netdev *dp) > + OVS_REQUIRES(dp->port_mutex) > { > struct dp_netdev_port *port; > struct hmapx to_reload = HMAPX_INITIALIZER(&to_reload); > struct hmapx_node *node; > > - CMAP_FOR_EACH (port, node, &dp->ports) { > + HMAP_FOR_EACH (port, node, &dp->ports) { > if (netdev_is_pmd(port->netdev)) { > dp_netdev_add_port_to_pmds__(dp, port, &to_reload); > } > @@ -3857,7 +3879,6 @@ dp_netdev_clone_pkt_batch(struct dp_packet **dst_pkts, > static void > dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt, > const struct nlattr *a, bool may_steal) > - OVS_NO_THREAD_SAFETY_ANALYSIS > { > struct dp_netdev_execute_aux *aux = aux_; > uint32_t *depth = recirc_depth_get(); > @@ -4076,8 +4097,7 @@ static void > dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, > const char *argv[], void *aux OVS_UNUSED) > { > - struct dp_netdev_port *old_port; > - struct dp_netdev_port *new_port; > + struct dp_netdev_port *port; > struct dp_netdev *dp; > odp_port_t port_no; > > @@ -4092,7 +4112,7 @@ dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, > ovs_mutex_unlock(&dp_netdev_mutex); > > ovs_mutex_lock(&dp->port_mutex); > - if (get_port_by_name(dp, argv[2], &old_port)) { > + if (get_port_by_name(dp, argv[2], &port)) { > unixctl_command_reply_error(conn, "unknown port"); > goto exit; > } > @@ -4107,16 +4127,14 @@ dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, > goto exit; > } > > - /* Remove old port. */ > - cmap_remove(&dp->ports, &old_port->node, hash_port_no(old_port->port_no)); > - dp_netdev_del_port_from_all_pmds(dp, old_port); > - ovsrcu_postpone(free, old_port); > + /* Remove port. */ > + hmap_remove(&dp->ports, &port->node); > + dp_netdev_del_port_from_all_pmds(dp, port); > > - /* Insert new port (cmap semantics mean we cannot re-insert 'old_port'). */ > - new_port = xmemdup(old_port, sizeof *old_port); > - new_port->port_no = port_no; > - cmap_insert(&dp->ports, &new_port->node, hash_port_no(port_no)); > - dp_netdev_add_port_to_pmds(dp, new_port); > + /* Reinsert with new port number. */ > + port->port_no = port_no; > + hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); > + dp_netdev_add_port_to_pmds(dp, port); > > seq_change(dp->port_seq); > unixctl_command_reply(conn, NULL); >
On 20/04/2016 07:21, "Ilya Maximets" <i.maximets@samsung.com> wrote: >On 20.04.2016 01:28, diproiettod at vmware.com (Daniele Di Proietto) >wrote: >> netdev objects are hard to use with RCU, because it's not possible to >> split removal and reclamation. Postponing the removal means that the >> port is not removed and cannot be readded immediately. Waiting for >> reclamation means introducing a quiescent state, and that may introduce >> subtle bugs, due to the RCU model we use in userspace. >> >> This commit changes the port container from cmap to hmap. 'port_mutex' >> must be held by readers and writers. This shouldn't have performace >> impact, as readers in the fast path use a thread local cache. >> >> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com> >> --- >> lib/dpif-netdev.c | 96 >>+++++++++++++++++++++++++++++++++---------------------- >> 1 file changed, 57 insertions(+), 39 deletions(-) >> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> index bd2249e..8cc37e2 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -195,9 +195,10 @@ struct dp_netdev { >> >> /* Ports. >> * >> - * Protected by RCU. Take the mutex to add or remove ports. */ >> + * Any lookup into 'ports' or any access to the dp_netdev_ports >>found >> + * through 'ports' requires taking 'port_mutex'. */ >> struct ovs_mutex port_mutex; >> - struct cmap ports; >> + struct hmap ports; >> struct seq *port_seq; /* Incremented whenever a port >>changes. */ >> >> /* Protects access to ofproto-dpif-upcall interface during >>revalidator >> @@ -228,7 +229,8 @@ struct dp_netdev { >> }; >> >> static struct dp_netdev_port *dp_netdev_lookup_port(const struct >>dp_netdev *dp, >> - odp_port_t); >> + odp_port_t) >> + OVS_REQUIRES(&dp->port_mutex); > >OVS_REQUIRES(dp->port_mutex); >here and 2 times more below. I've changed them, thanks. I think the analyzer accepts both (a pointer or the object itself), but I prefer the syntax you suggested. > >> >> enum dp_stat_type { >> DP_STAT_EXACT_HIT, /* Packets that had an exact match >>(emc). */ >> @@ -248,7 +250,7 @@ enum pmd_cycles_counter_type { >> struct dp_netdev_port { >> odp_port_t port_no; >> struct netdev *netdev; >> - struct cmap_node node; /* Node in dp_netdev's 'ports'. */ >> + struct hmap_node node; /* Node in dp_netdev's 'ports'. */ >> struct netdev_saved_flags *sf; >> unsigned n_rxq; /* Number of elements in 'rxq' */ >> struct netdev_rxq **rxq; >> @@ -476,9 +478,11 @@ struct dpif_netdev { >> }; >> >> static int get_port_by_number(struct dp_netdev *dp, odp_port_t port_no, >> - struct dp_netdev_port **portp); >> + struct dp_netdev_port **portp) >> + OVS_REQUIRES(dp->port_mutex); >> static int get_port_by_name(struct dp_netdev *dp, const char *devname, >> - struct dp_netdev_port **portp); >> + struct dp_netdev_port **portp) >> + OVS_REQUIRES(dp->port_mutex); >> static void dp_netdev_free(struct dp_netdev *) >> OVS_REQUIRES(dp_netdev_mutex); >> static int do_add_port(struct dp_netdev *dp, const char *devname, >> @@ -522,7 +526,8 @@ dp_netdev_add_rxq_to_pmd(struct >>dp_netdev_pmd_thread *pmd, >> struct dp_netdev_port *port, struct >>netdev_rxq *rx); >> static struct dp_netdev_pmd_thread * >> dp_netdev_less_loaded_pmd_on_numa(struct dp_netdev *dp, int numa_id); >> -static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp); >> +static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp) >> + OVS_REQUIRES(dp->port_mutex); >> static bool dp_netdev_pmd_try_ref(struct dp_netdev_pmd_thread *pmd); >> static void dp_netdev_pmd_unref(struct dp_netdev_pmd_thread *pmd); >> static void dp_netdev_pmd_flow_flush(struct dp_netdev_pmd_thread *pmd); >> @@ -913,7 +918,7 @@ create_dp_netdev(const char *name, const struct >>dpif_class *class, >> atomic_flag_clear(&dp->destroyed); >> >> ovs_mutex_init(&dp->port_mutex); >> - cmap_init(&dp->ports); >> + hmap_init(&dp->ports); >> dp->port_seq = seq_create(); >> fat_rwlock_init(&dp->upcall_rwlock); >> >> @@ -984,7 +989,7 @@ static void >> dp_netdev_free(struct dp_netdev *dp) >> OVS_REQUIRES(dp_netdev_mutex) >> { >> - struct dp_netdev_port *port; >> + struct dp_netdev_port *port, *next; >> >> shash_find_and_delete(&dp_netdevs, dp->name); >> >> @@ -993,15 +998,14 @@ dp_netdev_free(struct dp_netdev *dp) >> ovsthread_key_delete(dp->per_pmd_key); >> >> ovs_mutex_lock(&dp->port_mutex); >> - CMAP_FOR_EACH (port, node, &dp->ports) { >> - /* PMD threads are destroyed here. do_del_port() cannot >>quiesce */ >> + HMAP_FOR_EACH_SAFE (port, next, node, &dp->ports) { >> do_del_port(dp, port); >> } >> ovs_mutex_unlock(&dp->port_mutex); >> cmap_destroy(&dp->poll_threads); >> >> seq_destroy(dp->port_seq); >> - cmap_destroy(&dp->ports); >> + hmap_destroy(&dp->ports); >> ovs_mutex_destroy(&dp->port_mutex); >> >> /* Upcalls must be disabled at this point */ >> @@ -1222,7 +1226,7 @@ do_add_port(struct dp_netdev *dp, const char >>*devname, const char *type, >> return error; >> } >> >> - cmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); >> + hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); >> >> dp_netdev_add_port_to_pmds(dp, port); >> seq_change(dp->port_seq); >> @@ -1288,10 +1292,11 @@ is_valid_port_number(odp_port_t port_no) >> >> static struct dp_netdev_port * >> dp_netdev_lookup_port(const struct dp_netdev *dp, odp_port_t port_no) >> + OVS_REQUIRES(&dp->port_mutex) > >here > >> { >> struct dp_netdev_port *port; >> >> - CMAP_FOR_EACH_WITH_HASH (port, node, hash_port_no(port_no), >>&dp->ports) { >> + HMAP_FOR_EACH_WITH_HASH (port, node, hash_port_no(port_no), >>&dp->ports) { >> if (port->port_no == port_no) { >> return port; >> } >> @@ -1302,6 +1307,7 @@ dp_netdev_lookup_port(const struct dp_netdev *dp, >>odp_port_t port_no) >> static int >> get_port_by_number(struct dp_netdev *dp, >> odp_port_t port_no, struct dp_netdev_port **portp) >> + OVS_REQUIRES(&dp->port_mutex) > >and here. > >> { >> if (!is_valid_port_number(port_no)) { >> *portp = NULL; >> @@ -1338,7 +1344,7 @@ get_port_by_name(struct dp_netdev *dp, >> { >> struct dp_netdev_port *port; >> >> - CMAP_FOR_EACH (port, node, &dp->ports) { >> + HMAP_FOR_EACH (port, node, &dp->ports) { >> if (!strcmp(netdev_get_name(port->netdev), devname)) { >> *portp = port; >> return 0; >> @@ -1373,10 +1379,11 @@ get_n_pmd_threads_on_numa(struct dp_netdev *dp, >>int numa_id) >> * is on numa node 'numa_id'. */ >> static bool >> has_pmd_port_for_numa(struct dp_netdev *dp, int numa_id) >> + OVS_REQUIRES(dp->port_mutex) >> { >> struct dp_netdev_port *port; >> >> - CMAP_FOR_EACH (port, node, &dp->ports) { >> + HMAP_FOR_EACH (port, node, &dp->ports) { >> if (netdev_is_pmd(port->netdev) >> && netdev_get_numa_id(port->netdev) == numa_id) { >> return true; >> @@ -1391,7 +1398,7 @@ static void >> do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port) >> OVS_REQUIRES(dp->port_mutex) >> { >> - cmap_remove(&dp->ports, &port->node, hash_odp_port(port->port_no)); >> + hmap_remove(&dp->ports, &port->node); >> seq_change(dp->port_seq); >> >> dp_netdev_del_port_from_all_pmds(dp, port); >> @@ -1428,10 +1435,12 @@ dpif_netdev_port_query_by_number(const struct >>dpif *dpif, odp_port_t port_no, >> struct dp_netdev_port *port; >> int error; >> >> + ovs_mutex_lock(&dp->port_mutex); >> error = get_port_by_number(dp, port_no, &port); >> if (!error && dpif_port) { >> answer_port_query(port, dpif_port); >> } >> + ovs_mutex_unlock(&dp->port_mutex); >> >> return error; >> } >> @@ -1516,7 +1525,7 @@ dpif_netdev_flow_flush(struct dpif *dpif) >> } >> >> struct dp_netdev_port_state { >> - struct cmap_position position; >> + struct hmap_position position; >> char *name; >> }; >> >> @@ -1533,10 +1542,11 @@ dpif_netdev_port_dump_next(const struct dpif >>*dpif, void *state_, >> { >> struct dp_netdev_port_state *state = state_; >> struct dp_netdev *dp = get_dp_netdev(dpif); >> - struct cmap_node *node; >> + struct hmap_node *node; >> int retval; >> >> - node = cmap_next_position(&dp->ports, &state->position); >> + ovs_mutex_lock(&dp->port_mutex); >> + node = hmap_at_position(&dp->ports, &state->position); >> if (node) { >> struct dp_netdev_port *port; >> >> @@ -1552,6 +1562,7 @@ dpif_netdev_port_dump_next(const struct dpif >>*dpif, void *state_, >> } else { >> retval = EOF; >> } >> + ovs_mutex_unlock(&dp->port_mutex); >> >> return retval; >> } >> @@ -2407,8 +2418,8 @@ dpif_netdev_execute(struct dpif *dpif, struct >>dpif_execute *execute) >> dp_netdev_execute_actions(pmd, &pp, 1, false, execute->actions, >> execute->actions_len); >> if (pmd->core_id == NON_PMD_CORE_ID) { >> - dp_netdev_pmd_unref(pmd); >> ovs_mutex_unlock(&dp->non_pmd_mutex); >> + dp_netdev_pmd_unref(pmd); >> } >> >> return 0; >> @@ -2449,14 +2460,17 @@ pmd_config_changed(const struct dp_netdev *dp, >>const char *cmask) >> { >> struct dp_netdev_port *port; >> >> - CMAP_FOR_EACH (port, node, &dp->ports) { >> + ovs_mutex_lock(&dp->port_mutex); >> + HMAP_FOR_EACH (port, node, &dp->ports) { >> struct netdev *netdev = port->netdev; >> int requested_n_rxq = netdev_requested_n_rxq(netdev); >> if (netdev_is_pmd(netdev) >> && port->latest_requested_n_rxq != requested_n_rxq) { >> + ovs_mutex_unlock(&dp->port_mutex); >> return true; >> } >> } >> + ovs_mutex_unlock(&dp->port_mutex); >> >> if (dp->pmd_cmask != NULL && cmask != NULL) { >> return strcmp(dp->pmd_cmask, cmask); >> @@ -2476,7 +2490,8 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char >>*cmask) >> >> dp_netdev_destroy_all_pmds(dp); >> >> - CMAP_FOR_EACH (port, node, &dp->ports) { >> + ovs_mutex_lock(&dp->port_mutex); >> + HMAP_FOR_EACH (port, node, &dp->ports) { >> struct netdev *netdev = port->netdev; >> int requested_n_rxq = netdev_requested_n_rxq(netdev); >> if (netdev_is_pmd(port->netdev) >> @@ -2498,6 +2513,7 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char >>*cmask) >> VLOG_ERR("Failed to set dpdk interface %s rx_queue >>to:" >> " %u", netdev_get_name(port->netdev), >> requested_n_rxq); >> + ovs_mutex_unlock(&dp->port_mutex); >> return err; >> } >> port->latest_requested_n_rxq = requested_n_rxq; >> @@ -2518,6 +2534,7 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char >>*cmask) >> dp_netdev_set_nonpmd(dp); >> /* Restores all pmd threads. */ >> dp_netdev_reset_pmd_threads(dp); >> + ovs_mutex_unlock(&dp->port_mutex); >> } >> >> return 0; >> @@ -2627,8 +2644,9 @@ dpif_netdev_run(struct dpif *dpif) >> >>NON_PMD_CORE_ID); >> uint64_t new_tnl_seq; >> >> + ovs_mutex_lock(&dp->port_mutex); >> ovs_mutex_lock(&dp->non_pmd_mutex); >> - CMAP_FOR_EACH (port, node, &dp->ports) { >> + HMAP_FOR_EACH (port, node, &dp->ports) { >> if (!netdev_is_pmd(port->netdev)) { >> int i; >> >> @@ -2638,6 +2656,7 @@ dpif_netdev_run(struct dpif *dpif) >> } >> } >> ovs_mutex_unlock(&dp->non_pmd_mutex); >> + ovs_mutex_unlock(&dp->port_mutex); >> dp_netdev_pmd_unref(non_pmd); >> >> tnl_neigh_cache_run(); >> @@ -2658,7 +2677,8 @@ dpif_netdev_wait(struct dpif *dpif) >> struct dp_netdev *dp = get_dp_netdev(dpif); >> >> ovs_mutex_lock(&dp_netdev_mutex); >> - CMAP_FOR_EACH (port, node, &dp->ports) { >> + ovs_mutex_lock(&dp->port_mutex); >> + HMAP_FOR_EACH (port, node, &dp->ports) { >> if (!netdev_is_pmd(port->netdev)) { >> int i; >> >> @@ -2667,6 +2687,7 @@ dpif_netdev_wait(struct dpif *dpif) >> } >> } >> } >> + ovs_mutex_unlock(&dp->port_mutex); >> ovs_mutex_unlock(&dp_netdev_mutex); >> seq_wait(tnl_conf_seq, dp->last_tnl_conf_seq); >> } >> @@ -3296,12 +3317,13 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev >>*dp, int numa_id) >> * new configuration. */ >> static void >> dp_netdev_reset_pmd_threads(struct dp_netdev *dp) >> + OVS_REQUIRES(dp->port_mutex) >> { >> struct dp_netdev_port *port; >> struct hmapx to_reload = HMAPX_INITIALIZER(&to_reload); >> struct hmapx_node *node; >> >> - CMAP_FOR_EACH (port, node, &dp->ports) { >> + HMAP_FOR_EACH (port, node, &dp->ports) { >> if (netdev_is_pmd(port->netdev)) { >> dp_netdev_add_port_to_pmds__(dp, port, &to_reload); >> } >> @@ -3857,7 +3879,6 @@ dp_netdev_clone_pkt_batch(struct dp_packet >>**dst_pkts, >> static void >> dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt, >> const struct nlattr *a, bool may_steal) >> - OVS_NO_THREAD_SAFETY_ANALYSIS >> { >> struct dp_netdev_execute_aux *aux = aux_; >> uint32_t *depth = recirc_depth_get(); >> @@ -4076,8 +4097,7 @@ static void >> dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc >>OVS_UNUSED, >> const char *argv[], void *aux OVS_UNUSED) >> { >> - struct dp_netdev_port *old_port; >> - struct dp_netdev_port *new_port; >> + struct dp_netdev_port *port; >> struct dp_netdev *dp; >> odp_port_t port_no; >> >> @@ -4092,7 +4112,7 @@ dpif_dummy_change_port_number(struct unixctl_conn >>*conn, int argc OVS_UNUSED, >> ovs_mutex_unlock(&dp_netdev_mutex); >> >> ovs_mutex_lock(&dp->port_mutex); >> - if (get_port_by_name(dp, argv[2], &old_port)) { >> + if (get_port_by_name(dp, argv[2], &port)) { >> unixctl_command_reply_error(conn, "unknown port"); >> goto exit; >> } >> @@ -4107,16 +4127,14 @@ dpif_dummy_change_port_number(struct >>unixctl_conn *conn, int argc OVS_UNUSED, >> goto exit; >> } >> >> - /* Remove old port. */ >> - cmap_remove(&dp->ports, &old_port->node, >>hash_port_no(old_port->port_no)); >> - dp_netdev_del_port_from_all_pmds(dp, old_port); >> - ovsrcu_postpone(free, old_port); >> + /* Remove port. */ >> + hmap_remove(&dp->ports, &port->node); >> + dp_netdev_del_port_from_all_pmds(dp, port); >> >> - /* Insert new port (cmap semantics mean we cannot re-insert >>'old_port'). */ >> - new_port = xmemdup(old_port, sizeof *old_port); >> - new_port->port_no = port_no; >> - cmap_insert(&dp->ports, &new_port->node, hash_port_no(port_no)); >> - dp_netdev_add_port_to_pmds(dp, new_port); >> + /* Reinsert with new port number. */ >> + port->port_no = port_no; >> + hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); >> + dp_netdev_add_port_to_pmds(dp, port); >> >> seq_change(dp->port_seq); >> unixctl_command_reply(conn, NULL); >>
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index bd2249e..8cc37e2 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -195,9 +195,10 @@ struct dp_netdev { /* Ports. * - * Protected by RCU. Take the mutex to add or remove ports. */ + * Any lookup into 'ports' or any access to the dp_netdev_ports found + * through 'ports' requires taking 'port_mutex'. */ struct ovs_mutex port_mutex; - struct cmap ports; + struct hmap ports; struct seq *port_seq; /* Incremented whenever a port changes. */ /* Protects access to ofproto-dpif-upcall interface during revalidator @@ -228,7 +229,8 @@ struct dp_netdev { }; static struct dp_netdev_port *dp_netdev_lookup_port(const struct dp_netdev *dp, - odp_port_t); + odp_port_t) + OVS_REQUIRES(&dp->port_mutex); enum dp_stat_type { DP_STAT_EXACT_HIT, /* Packets that had an exact match (emc). */ @@ -248,7 +250,7 @@ enum pmd_cycles_counter_type { struct dp_netdev_port { odp_port_t port_no; struct netdev *netdev; - struct cmap_node node; /* Node in dp_netdev's 'ports'. */ + struct hmap_node node; /* Node in dp_netdev's 'ports'. */ struct netdev_saved_flags *sf; unsigned n_rxq; /* Number of elements in 'rxq' */ struct netdev_rxq **rxq; @@ -476,9 +478,11 @@ struct dpif_netdev { }; static int get_port_by_number(struct dp_netdev *dp, odp_port_t port_no, - struct dp_netdev_port **portp); + struct dp_netdev_port **portp) + OVS_REQUIRES(dp->port_mutex); static int get_port_by_name(struct dp_netdev *dp, const char *devname, - struct dp_netdev_port **portp); + struct dp_netdev_port **portp) + OVS_REQUIRES(dp->port_mutex); static void dp_netdev_free(struct dp_netdev *) OVS_REQUIRES(dp_netdev_mutex); static int do_add_port(struct dp_netdev *dp, const char *devname, @@ -522,7 +526,8 @@ dp_netdev_add_rxq_to_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev_port *port, struct netdev_rxq *rx); static struct dp_netdev_pmd_thread * dp_netdev_less_loaded_pmd_on_numa(struct dp_netdev *dp, int numa_id); -static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp); +static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp) + OVS_REQUIRES(dp->port_mutex); static bool dp_netdev_pmd_try_ref(struct dp_netdev_pmd_thread *pmd); static void dp_netdev_pmd_unref(struct dp_netdev_pmd_thread *pmd); static void dp_netdev_pmd_flow_flush(struct dp_netdev_pmd_thread *pmd); @@ -913,7 +918,7 @@ create_dp_netdev(const char *name, const struct dpif_class *class, atomic_flag_clear(&dp->destroyed); ovs_mutex_init(&dp->port_mutex); - cmap_init(&dp->ports); + hmap_init(&dp->ports); dp->port_seq = seq_create(); fat_rwlock_init(&dp->upcall_rwlock); @@ -984,7 +989,7 @@ static void dp_netdev_free(struct dp_netdev *dp) OVS_REQUIRES(dp_netdev_mutex) { - struct dp_netdev_port *port; + struct dp_netdev_port *port, *next; shash_find_and_delete(&dp_netdevs, dp->name); @@ -993,15 +998,14 @@ dp_netdev_free(struct dp_netdev *dp) ovsthread_key_delete(dp->per_pmd_key); ovs_mutex_lock(&dp->port_mutex); - CMAP_FOR_EACH (port, node, &dp->ports) { - /* PMD threads are destroyed here. do_del_port() cannot quiesce */ + HMAP_FOR_EACH_SAFE (port, next, node, &dp->ports) { do_del_port(dp, port); } ovs_mutex_unlock(&dp->port_mutex); cmap_destroy(&dp->poll_threads); seq_destroy(dp->port_seq); - cmap_destroy(&dp->ports); + hmap_destroy(&dp->ports); ovs_mutex_destroy(&dp->port_mutex); /* Upcalls must be disabled at this point */ @@ -1222,7 +1226,7 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type, return error; } - cmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); + hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); dp_netdev_add_port_to_pmds(dp, port); seq_change(dp->port_seq); @@ -1288,10 +1292,11 @@ is_valid_port_number(odp_port_t port_no) static struct dp_netdev_port * dp_netdev_lookup_port(const struct dp_netdev *dp, odp_port_t port_no) + OVS_REQUIRES(&dp->port_mutex) { struct dp_netdev_port *port; - CMAP_FOR_EACH_WITH_HASH (port, node, hash_port_no(port_no), &dp->ports) { + HMAP_FOR_EACH_WITH_HASH (port, node, hash_port_no(port_no), &dp->ports) { if (port->port_no == port_no) { return port; } @@ -1302,6 +1307,7 @@ dp_netdev_lookup_port(const struct dp_netdev *dp, odp_port_t port_no) static int get_port_by_number(struct dp_netdev *dp, odp_port_t port_no, struct dp_netdev_port **portp) + OVS_REQUIRES(&dp->port_mutex) { if (!is_valid_port_number(port_no)) { *portp = NULL; @@ -1338,7 +1344,7 @@ get_port_by_name(struct dp_netdev *dp, { struct dp_netdev_port *port; - CMAP_FOR_EACH (port, node, &dp->ports) { + HMAP_FOR_EACH (port, node, &dp->ports) { if (!strcmp(netdev_get_name(port->netdev), devname)) { *portp = port; return 0; @@ -1373,10 +1379,11 @@ get_n_pmd_threads_on_numa(struct dp_netdev *dp, int numa_id) * is on numa node 'numa_id'. */ static bool has_pmd_port_for_numa(struct dp_netdev *dp, int numa_id) + OVS_REQUIRES(dp->port_mutex) { struct dp_netdev_port *port; - CMAP_FOR_EACH (port, node, &dp->ports) { + HMAP_FOR_EACH (port, node, &dp->ports) { if (netdev_is_pmd(port->netdev) && netdev_get_numa_id(port->netdev) == numa_id) { return true; @@ -1391,7 +1398,7 @@ static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port) OVS_REQUIRES(dp->port_mutex) { - cmap_remove(&dp->ports, &port->node, hash_odp_port(port->port_no)); + hmap_remove(&dp->ports, &port->node); seq_change(dp->port_seq); dp_netdev_del_port_from_all_pmds(dp, port); @@ -1428,10 +1435,12 @@ dpif_netdev_port_query_by_number(const struct dpif *dpif, odp_port_t port_no, struct dp_netdev_port *port; int error; + ovs_mutex_lock(&dp->port_mutex); error = get_port_by_number(dp, port_no, &port); if (!error && dpif_port) { answer_port_query(port, dpif_port); } + ovs_mutex_unlock(&dp->port_mutex); return error; } @@ -1516,7 +1525,7 @@ dpif_netdev_flow_flush(struct dpif *dpif) } struct dp_netdev_port_state { - struct cmap_position position; + struct hmap_position position; char *name; }; @@ -1533,10 +1542,11 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, { struct dp_netdev_port_state *state = state_; struct dp_netdev *dp = get_dp_netdev(dpif); - struct cmap_node *node; + struct hmap_node *node; int retval; - node = cmap_next_position(&dp->ports, &state->position); + ovs_mutex_lock(&dp->port_mutex); + node = hmap_at_position(&dp->ports, &state->position); if (node) { struct dp_netdev_port *port; @@ -1552,6 +1562,7 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, } else { retval = EOF; } + ovs_mutex_unlock(&dp->port_mutex); return retval; } @@ -2407,8 +2418,8 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) dp_netdev_execute_actions(pmd, &pp, 1, false, execute->actions, execute->actions_len); if (pmd->core_id == NON_PMD_CORE_ID) { - dp_netdev_pmd_unref(pmd); ovs_mutex_unlock(&dp->non_pmd_mutex); + dp_netdev_pmd_unref(pmd); } return 0; @@ -2449,14 +2460,17 @@ pmd_config_changed(const struct dp_netdev *dp, const char *cmask) { struct dp_netdev_port *port; - CMAP_FOR_EACH (port, node, &dp->ports) { + ovs_mutex_lock(&dp->port_mutex); + HMAP_FOR_EACH (port, node, &dp->ports) { struct netdev *netdev = port->netdev; int requested_n_rxq = netdev_requested_n_rxq(netdev); if (netdev_is_pmd(netdev) && port->latest_requested_n_rxq != requested_n_rxq) { + ovs_mutex_unlock(&dp->port_mutex); return true; } } + ovs_mutex_unlock(&dp->port_mutex); if (dp->pmd_cmask != NULL && cmask != NULL) { return strcmp(dp->pmd_cmask, cmask); @@ -2476,7 +2490,8 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask) dp_netdev_destroy_all_pmds(dp); - CMAP_FOR_EACH (port, node, &dp->ports) { + ovs_mutex_lock(&dp->port_mutex); + HMAP_FOR_EACH (port, node, &dp->ports) { struct netdev *netdev = port->netdev; int requested_n_rxq = netdev_requested_n_rxq(netdev); if (netdev_is_pmd(port->netdev) @@ -2498,6 +2513,7 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask) VLOG_ERR("Failed to set dpdk interface %s rx_queue to:" " %u", netdev_get_name(port->netdev), requested_n_rxq); + ovs_mutex_unlock(&dp->port_mutex); return err; } port->latest_requested_n_rxq = requested_n_rxq; @@ -2518,6 +2534,7 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask) dp_netdev_set_nonpmd(dp); /* Restores all pmd threads. */ dp_netdev_reset_pmd_threads(dp); + ovs_mutex_unlock(&dp->port_mutex); } return 0; @@ -2627,8 +2644,9 @@ dpif_netdev_run(struct dpif *dpif) NON_PMD_CORE_ID); uint64_t new_tnl_seq; + ovs_mutex_lock(&dp->port_mutex); ovs_mutex_lock(&dp->non_pmd_mutex); - CMAP_FOR_EACH (port, node, &dp->ports) { + HMAP_FOR_EACH (port, node, &dp->ports) { if (!netdev_is_pmd(port->netdev)) { int i; @@ -2638,6 +2656,7 @@ dpif_netdev_run(struct dpif *dpif) } } ovs_mutex_unlock(&dp->non_pmd_mutex); + ovs_mutex_unlock(&dp->port_mutex); dp_netdev_pmd_unref(non_pmd); tnl_neigh_cache_run(); @@ -2658,7 +2677,8 @@ dpif_netdev_wait(struct dpif *dpif) struct dp_netdev *dp = get_dp_netdev(dpif); ovs_mutex_lock(&dp_netdev_mutex); - CMAP_FOR_EACH (port, node, &dp->ports) { + ovs_mutex_lock(&dp->port_mutex); + HMAP_FOR_EACH (port, node, &dp->ports) { if (!netdev_is_pmd(port->netdev)) { int i; @@ -2667,6 +2687,7 @@ dpif_netdev_wait(struct dpif *dpif) } } } + ovs_mutex_unlock(&dp->port_mutex); ovs_mutex_unlock(&dp_netdev_mutex); seq_wait(tnl_conf_seq, dp->last_tnl_conf_seq); } @@ -3296,12 +3317,13 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id) * new configuration. */ static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp) + OVS_REQUIRES(dp->port_mutex) { struct dp_netdev_port *port; struct hmapx to_reload = HMAPX_INITIALIZER(&to_reload); struct hmapx_node *node; - CMAP_FOR_EACH (port, node, &dp->ports) { + HMAP_FOR_EACH (port, node, &dp->ports) { if (netdev_is_pmd(port->netdev)) { dp_netdev_add_port_to_pmds__(dp, port, &to_reload); } @@ -3857,7 +3879,6 @@ dp_netdev_clone_pkt_batch(struct dp_packet **dst_pkts, static void dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt, const struct nlattr *a, bool may_steal) - OVS_NO_THREAD_SAFETY_ANALYSIS { struct dp_netdev_execute_aux *aux = aux_; uint32_t *depth = recirc_depth_get(); @@ -4076,8 +4097,7 @@ static void dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[], void *aux OVS_UNUSED) { - struct dp_netdev_port *old_port; - struct dp_netdev_port *new_port; + struct dp_netdev_port *port; struct dp_netdev *dp; odp_port_t port_no; @@ -4092,7 +4112,7 @@ dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, ovs_mutex_unlock(&dp_netdev_mutex); ovs_mutex_lock(&dp->port_mutex); - if (get_port_by_name(dp, argv[2], &old_port)) { + if (get_port_by_name(dp, argv[2], &port)) { unixctl_command_reply_error(conn, "unknown port"); goto exit; } @@ -4107,16 +4127,14 @@ dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, goto exit; } - /* Remove old port. */ - cmap_remove(&dp->ports, &old_port->node, hash_port_no(old_port->port_no)); - dp_netdev_del_port_from_all_pmds(dp, old_port); - ovsrcu_postpone(free, old_port); + /* Remove port. */ + hmap_remove(&dp->ports, &port->node); + dp_netdev_del_port_from_all_pmds(dp, port); - /* Insert new port (cmap semantics mean we cannot re-insert 'old_port'). */ - new_port = xmemdup(old_port, sizeof *old_port); - new_port->port_no = port_no; - cmap_insert(&dp->ports, &new_port->node, hash_port_no(port_no)); - dp_netdev_add_port_to_pmds(dp, new_port); + /* Reinsert with new port number. */ + port->port_no = port_no; + hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); + dp_netdev_add_port_to_pmds(dp, port); seq_change(dp->port_seq); unixctl_command_reply(conn, NULL);
netdev objects are hard to use with RCU, because it's not possible to split removal and reclamation. Postponing the removal means that the port is not removed and cannot be readded immediately. Waiting for reclamation means introducing a quiescent state, and that may introduce subtle bugs, due to the RCU model we use in userspace. This commit changes the port container from cmap to hmap. 'port_mutex' must be held by readers and writers. This shouldn't have performace impact, as readers in the fast path use a thread local cache. Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> --- lib/dpif-netdev.c | 96 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 39 deletions(-)