Hi Mark,
I applied your suggestion and sent a v6.
Thanks,
Daniele
On 24/03/2016 09:53, "Kavanagh, Mark B" <mark.b.kavan...@intel.com> wrote:
>Hi Daniele,
>
>One minor comment below.
>
>Cheers,
>Mark
>
>>
>>This commit changes 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>
>>Tested-by: Ilya Maximets <i.maxim...@samsung.com>
>>Acked-by: Ilya Maximets <i.maxim...@samsung.com>
>>---
>> lib/dpif-netdev.c | 78
>>++++++++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 54 insertions(+), 24 deletions(-)
>>
>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>index c66bf29..456cda4 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,58 @@ 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, 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
>
>Shouldn't this be 'If the netdev_set_multiq() above succeeds"?
>
>>+ * 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);
>>--
>>2.1.4
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
