diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index a768117ce..0fbb4038e 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, @@ -504,14 +508,17 @@ static void dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, unsigned core_id, int numa_id); static void dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd); -static void dp_netdev_set_nonpmd(struct dp_netdev *dp); +static void dp_netdev_set_nonpmd(struct dp_netdev *dp) + OVS_REQUIRES(dp->port_mutex); + static struct dp_netdev_pmd_thread *dp_netdev_get_pmd(struct dp_netdev *dp, unsigned core_id); static struct dp_netdev_pmd_thread * dp_netdev_pmd_get_next(struct dp_netdev *dp, struct cmap_position *pos); static void dp_netdev_destroy_all_pmds(struct dp_netdev *dp); static void dp_netdev_del_pmds_on_numa(struct dp_netdev *dp, int numa_id); -static void dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id); +static void dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id) + OVS_REQUIRES(dp->port_mutex); static void dp_netdev_pmd_clear_ports(struct dp_netdev_pmd_thread *pmd); static void dp_netdev_del_port_from_all_pmds(struct dp_netdev *dp, struct dp_netdev_port *port); @@ -524,7 +531,8 @@ static void dp_netdev_add_rxq_to_pmd(struct dp_netdev_pmd_thread *pmd, 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); @@ -915,7 +923,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); @@ -928,9 +936,9 @@ create_dp_netdev(const char *name, const struct dpif_class *class, ovs_mutex_init_recursive(&dp->non_pmd_mutex); ovsthread_key_create(&dp->per_pmd_key, NULL); + ovs_mutex_lock(&dp->port_mutex); dp_netdev_set_nonpmd(dp); - ovs_mutex_lock(&dp->port_mutex); error = do_add_port(dp, name, "internal", ODPP_LOCAL); ovs_mutex_unlock(&dp->port_mutex); if (error) { @@ -986,7 +994,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); @@ -995,15 +1003,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 */ @@ -1233,7 +1240,7 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type, dp_netdev_add_port_to_pmds(dp, port); - cmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); + hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); seq_change(dp->port_seq); return 0; @@ -1297,10 +1304,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; } @@ -1311,6 +1319,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; @@ -1347,7 +1356,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; @@ -1382,10 +1391,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; @@ -1400,7 +1410,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); @@ -1437,10 +1447,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; } @@ -1523,7 +1535,7 @@ dpif_netdev_flow_flush(struct dpif *dpif) } struct dp_netdev_port_state { - struct cmap_position position; + struct hmap_position position; char *name; }; @@ -1540,10 +1552,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; @@ -1559,6 +1572,7 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, } else { retval = EOF; } + ovs_mutex_unlock(&dp->port_mutex); return retval; } @@ -2425,8 +2439,8 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) 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; @@ -2467,14 +2481,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); @@ -2494,7 +2511,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) @@ -2516,6 +2534,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; @@ -2536,6 +2555,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; @@ -2646,8 +2666,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; @@ -2657,6 +2678,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(); @@ -2677,7 +2699,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; @@ -2686,6 +2709,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); } @@ -2869,6 +2893,7 @@ dp_netdev_get_pmd(struct dp_netdev *dp, unsigned core_id) /* Sets the 'struct dp_netdev_pmd_thread' for non-pmd threads. */ static void dp_netdev_set_nonpmd(struct dp_netdev *dp) + OVS_REQUIRES(dp->port_mutex) { struct dp_netdev_pmd_thread *non_pmd; struct dp_netdev_port *port; @@ -2876,7 +2901,7 @@ dp_netdev_set_nonpmd(struct dp_netdev *dp) non_pmd = xzalloc(sizeof *non_pmd); dp_netdev_configure_pmd(non_pmd, dp, NON_PMD_CORE_ID, OVS_NUMA_UNSPEC); - CMAP_FOR_EACH (port, node, &dp->ports) { + HMAP_FOR_EACH (port, node, &dp->ports) { dp_netdev_add_port_tx_to_pmd(non_pmd, port); } @@ -3299,6 +3324,7 @@ dp_netdev_add_port_to_pmds(struct dp_netdev *dp, struct dp_netdev_port *port) * The function takes care of filling the threads tx port cache. */ static void dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id) + OVS_REQUIRES(dp->port_mutex) { int n_pmds; @@ -3333,7 +3359,7 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id) dp_netdev_configure_pmd(pmd, dp, core_id, numa_id); - CMAP_FOR_EACH (port, node, &dp->ports) { + HMAP_FOR_EACH (port, node, &dp->ports) { dp_netdev_add_port_tx_to_pmd(pmd, port); } @@ -3348,13 +3374,14 @@ 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 hmapx to_reload = HMAPX_INITIALIZER(&to_reload); struct dp_netdev_pmd_thread *pmd; struct dp_netdev_port *port; struct hmapx_node *node; - CMAP_FOR_EACH (port, node, &dp->ports) { + HMAP_FOR_EACH (port, node, &dp->ports) { if (netdev_is_pmd(port->netdev)) { int numa_id = netdev_get_numa_id(port->netdev); @@ -3939,7 +3966,6 @@ dp_execute_userspace_action(struct dp_netdev_pmd_thread *pmd, static void dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, 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(); @@ -4146,8 +4172,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; @@ -4162,7 +4187,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; } @@ -4177,16 +4202,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);