comment inline.
Best regards, Ilya Maximets.
On 03.03.2016 04:33, Daniele Di Proietto wrote:
> This commit change reconfigure_pmd_threads() to interact with the ports
> cmap using RCU semantics (the content of the port structure is not
> altered while concurrent readers might access it) and to fail more
> gracefully in case of a set_multiq fail (now we remove the port from the
> datapath, instead of returning prematurely from the function without
> restarting the pmd threads).
>
> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
> ---
> lib/dpif-netdev.c | 79
> ++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 55 insertions(+), 24 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 8ed6bcb..0c3673b 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2591,6 +2591,11 @@ static void
> reconfigure_pmd_threads(struct dp_netdev *dp)
> {
> struct dp_netdev_port *port;
> + struct hmapx to_reconfigure = HMAPX_INITIALIZER(&to_reconfigure);
> + struct hmapx_node *node;
> + bool failed_config = false;
> +
> + ovs_mutex_lock(&dp->port_mutex);
>
> dp_netdev_destroy_all_pmds(dp);
>
> @@ -2599,33 +2604,59 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
> int requested_n_rxq = netdev_requested_n_rxq(netdev);
> if (netdev_is_pmd(port->netdev)
> && port->latest_requested_n_rxq != requested_n_rxq) {
> - int i, err;
> + cmap_remove(&dp->ports, &port->node,
> hash_odp_port(port->port_no));
> + hmapx_add(&to_reconfigure, port);
> + }
> + }
> + ovs_mutex_unlock(&dp->port_mutex);
>
> - /* Closes the existing 'rxq's. */
> - for (i = 0; i < port->n_rxq; i++) {
> - netdev_rxq_close(port->rxq[i]);
> - port->rxq[i] = NULL;
> - }
> - port->n_rxq = 0;
> -
> - /* Sets the new rx queue config. */
> - err = netdev_set_multiq(port->netdev, ovs_numa_get_n_cores() + 1,
> - requested_n_rxq);
> - if (err && (err != EOPNOTSUPP)) {
> - VLOG_ERR("Failed to set dpdk interface %s rx_queue to: %u",
> - netdev_get_name(port->netdev),
> - requested_n_rxq);
> - return;
> - }
> - port->latest_requested_n_rxq = requested_n_rxq;
> - /* If the set_multiq() above succeeds, reopens the 'rxq's. */
> - port->n_rxq = netdev_n_rxq(port->netdev);
> - port->rxq = xrealloc(port->rxq, sizeof *port->rxq * port->n_rxq);
> - for (i = 0; i < port->n_rxq; i++) {
> - netdev_rxq_open(port->netdev, &port->rxq[i], i);
> - }
> + /* Waits for the other threads to see the ports removed from the cmap,
> + * otherwise we are not allowed to alter them. */
> + ovsrcu_synchronize();
> +
> + ovs_mutex_lock(&dp->port_mutex);
> + HMAPX_FOR_EACH (node, &to_reconfigure) {
> + int requested_n_rxq = netdev_requested_n_rxq(port->netdev);
This will be removed lately but this is a bug in this patch. We should not
call netdev_requested_n_rxq here. Also, this may lead to segmentation fault
because of invalid 'port'.
> + int i, err;
> +
> + port = node->data;
> + requested_n_rxq = netdev_requested_n_rxq(port->netdev);
> + /* Closes the existing 'rxq's. */
> + for (i = 0; i < port->n_rxq; i++) {
> + netdev_rxq_close(port->rxq[i]);
> + port->rxq[i] = NULL;
> + }
> + port->n_rxq = 0;
> +
> + /* Sets the new rx queue config. */
> + err = netdev_set_multiq(port->netdev, ovs_numa_get_n_cores() + 1,
> + requested_n_rxq);
> + if (err && (err != EOPNOTSUPP)) {
> + VLOG_ERR("Failed to set dpdk interface %s rx_queue to: %u",
> + netdev_get_name(port->netdev),
> + requested_n_rxq);
> + do_destroy_port(port);
> + failed_config = true;
> + continue;
> }
> + port->latest_requested_n_rxq = requested_n_rxq;
> + /* If the netdev_reconfigure() above succeeds, reopens the 'rxq's and
> + * inserts the port back in the cmap, to allow transmitting packets.
> */
> + port->n_rxq = netdev_n_rxq(port->netdev);
> + port->rxq = xrealloc(port->rxq, sizeof *port->rxq * port->n_rxq);
> + for (i = 0; i < port->n_rxq; i++) {
> + netdev_rxq_open(port->netdev, &port->rxq[i], i);
> + }
> + cmap_insert(&dp->ports, &port->node, hash_port_no(port->port_no));
> + }
> + ovs_mutex_unlock(&dp->port_mutex);
> +
> + hmapx_destroy(&to_reconfigure);
> +
> + if (failed_config) {
> + seq_change(dp->port_seq);
> }
> +
> /* Reconfigures the cpu mask. */
> ovs_numa_set_cpu_mask(dp->requested_pmd_cmask);
> free(dp->pmd_cmask);
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
