Hi Daniele,

Comments inline, as usual.

Cheers,
Mark
>
>On 01/03/2016 09:12, "Kavanagh, Mark B" <mark.b.kavan...@intel.com> wrote:
>
>>Hi Daniele,
>>
>>Some comments inline - thanks again for the patchset!
>>
>>Cheers,
>>Mark
>>
>>>
>>>This introduces in dpif-netdev and netdev-dpdk the first use for the
>>>newly introduce reconfigure netdev call.
>>>
>>>When a request to change the number of queues comes, netdev-dpdk will
>>>remember this and notify the upper layer via
>>>netdev_request_reconfigure().
>>>
>>>The datapath, instead of periodically calling netdev_set_multiq(), can
>>>detect this and call reconfigure().
>>>
>>>This mechanism can also be used to:
>>>* Automatically match the number of rxq with the one provided by qemu
>>>  via the new_device callback.
>>>* Provide a way to change the MTU of dpdk devices at runtime.
>>>
>>>Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>>>---
>>> lib/dpif-netdev.c     |  69 ++++++++++-----------
>>> lib/netdev-dpdk.c     | 167
>>>+++++++++++++++++++++++++++++---------------------
>>> lib/netdev-provider.h |  19 ++----
>>> lib/netdev.c          |  30 ++-------
>>> lib/netdev.h          |   3 +-
>>> 5 files changed, 139 insertions(+), 149 deletions(-)
>>>
>>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>>index 2adba89..3421df7 100644
>>>--- a/lib/dpif-netdev.c
>>>+++ b/lib/dpif-netdev.c
>>>@@ -256,8 +256,6 @@ struct dp_netdev_port {
>>>     unsigned n_rxq;             /* Number of elements in 'rxq' */
>>>     struct netdev_rxq **rxq;
>>>     char *type;                 /* Port type as requested by user. */
>>>-    int latest_requested_n_rxq; /* Latest requested from netdev number
>>>-                                   of rx queues. */
>>> };
>>>
>>> /* Contained by struct dp_netdev_flow's 'stats' member.  */
>>>@@ -1161,8 +1159,7 @@ do_add_port(struct dp_netdev *dp, const char
>>>*devname, const char
>>>*type,
>>>         /* There can only be ovs_numa_get_n_cores() pmd threads,
>>>          * so creates a txq for each, and one extra for the non
>>>          * pmd threads. */
>>>-        error = netdev_set_multiq(netdev, n_cores + 1,
>>>-                                  netdev_requested_n_rxq(netdev));
>>>+        error = netdev_set_multiq(netdev, n_cores + 1);
>>>         if (error && (error != EOPNOTSUPP)) {
>>>             VLOG_ERR("%s, cannot set multiq", devname);
>>>             goto out_close;
>>>@@ -1174,7 +1171,14 @@ do_add_port(struct dp_netdev *dp, const char
>>>*devname, const char
>>>*type,
>>>     port->n_rxq = netdev_n_rxq(netdev);
>>>     port->rxq = xmalloc(sizeof *port->rxq * port->n_rxq);
>>>     port->type = xstrdup(type);
>>>-    port->latest_requested_n_rxq = netdev_requested_n_rxq(netdev);
>>>+
>>>+    if (netdev_is_reconf_required(netdev)) {
>>>+        error = netdev_reconfigure(netdev);
>>>+        if (error) {
>>>+            goto out_close;
>>
>>This causes a memory leak, as port->rxq has already been allocated.
>>
>>Perhaps if you initialize n_open_rxqs=0 before this block, you can then
>>use 'goto out_rx_close' instead.
>
>Oops, you're right. Thanks for noticing
>
>I've decided to move the reconfigure call before the rxq allocation
>in case netdev_reconfigure() changes the number of rxqs

Good catch.

>
>>
>>>+        }
>>>+    }
>>>+
>>>     n_open_rxqs = 0;
>>>     for (i = 0; i < port->n_rxq; i++) {
>>>         error = netdev_rxq_open(netdev, &port->rxq[i], i);
>>>@@ -2450,25 +2454,6 @@ dpif_netdev_operate(struct dpif *dpif, struct
>>>dpif_op **ops, size_t
>>>n_ops)
>>>     }
>>> }
>>>
>>>-/* Returns true if the configuration for rx queues or cpu mask
>>>- * is changed. */
>>>-static bool
>>>-pmd_n_rxq_changed(const struct dp_netdev *dp)
>>>-{
>>>-    struct dp_netdev_port *port;
>>>-
>>>-    CMAP_FOR_EACH (port, node, &dp->ports) {
>>>-        int requested_n_rxq = netdev_requested_n_rxq(port->netdev);
>>>-
>>>-        if (netdev_is_pmd(port->netdev)
>>>-            && port->latest_requested_n_rxq != requested_n_rxq) {
>>>-            return true;
>>>-        }
>>>-    }
>>>-
>>>-    return false;
>>>-}
>>>-
>>> static bool
>>> cmask_equals(const char *a, const char *b)
>>> {
>>>@@ -2601,14 +2586,12 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>>>     dp_netdev_destroy_all_pmds(dp);
>>>
>>>     CMAP_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)
>>>-            && port->latest_requested_n_rxq != requested_n_rxq) {
>>>+        if (netdev_is_reconf_required(port->netdev)) {
>>>             cmap_remove(&dp->ports, &port->node,
>>>hash_odp_port(port->port_no));
>>>             hmapx_add(&to_reconfigure, port);
>>>         }
>>>     }
>>>+
>>>     ovs_mutex_unlock(&dp->port_mutex);
>>>
>>>     /* Waits for the other threads to see the ports removed from the
>>>cmap,
>>>@@ -2617,11 +2600,9 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>>>
>>>     ovs_mutex_lock(&dp->port_mutex);
>>>     HMAPX_FOR_EACH (node, &to_reconfigure) {
>>>-        int requested_n_rxq = netdev_requested_n_rxq(port->netdev);
>>>         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]);
>>>@@ -2630,17 +2611,14 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>>>         port->n_rxq = 0;
>>>
>>>         /* Sets the new rx queue config. */
>>
>>Given that netdev_reconfigure can be used to set the config of more than
>>just the Rx queues, this comment should probably be made more general.
>
>You're right, I changed it to
>
>/* Allows the netdev to apply the pending configuration changes. */
>
>>
>>>-        err = netdev_set_multiq(port->netdev, ovs_numa_get_n_cores() +
>>>1,
>>>-                                requested_n_rxq);
>>>+        err = netdev_reconfigure(port->netdev);
>>>         if (err && (err != EOPNOTSUPP)) {
>>>-            VLOG_ERR("Failed to set dpdk interface %s rx_queue to: %u",
>>>-                     netdev_get_name(port->netdev),
>>>-                     requested_n_rxq);
>>>+            VLOG_ERR("Failed to set interface %s new configuration",
>>>+                     netdev_get_name(port->netdev));
>>>             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);
>>>@@ -2671,6 +2649,21 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>>>     dp_netdev_reset_pmd_threads(dp);
>>> }
>>>
>>>+/* Returns true if one of the netdevs in 'dp' requires a
>>>reconfiguration */
>>>+static bool
>>>+ports_require_restart(const struct dp_netdev *dp)
>>>+{
>>>+    struct dp_netdev_port *port;
>>>+
>>>+    CMAP_FOR_EACH (port, node, &dp->ports) {
>>>+        if (netdev_is_reconf_required(port->netdev)) {
>>>+            return true;
>>>+        }
>>>+    }
>>>+
>>>+    return false;
>>>+}
>>>+
>>> /* Return true if needs to revalidate datapath flows. */
>>> static bool
>>> dpif_netdev_run(struct dpif *dpif)
>>>@@ -2696,7 +2689,7 @@ dpif_netdev_run(struct dpif *dpif)
>>>     ovs_mutex_unlock(&dp->non_pmd_mutex);
>>>
>>>     if (!cmask_equals(dp->pmd_cmask, dp->requested_pmd_cmask)
>>>-        || pmd_n_rxq_changed(dp)) {
>>>+        || ports_require_restart(dp)) {
>>>         reconfigure_pmd_threads(dp);
>>>     }
>>>
>>>@@ -2719,6 +2712,8 @@ dpif_netdev_wait(struct dpif *dpif)
>>>
>>>     ovs_mutex_lock(&dp_netdev_mutex);
>>>     CMAP_FOR_EACH (port, node, &dp->ports) {
>>>+        netdev_wait_reconf_required(port->netdev);
>>>+
>>>         if (!netdev_is_pmd(port->netdev)) {
>>>             int i;
>>>
>>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>>index a48ca71..17b8d51 100644
>>>--- a/lib/netdev-dpdk.c
>>>+++ b/lib/netdev-dpdk.c
>>>@@ -237,6 +237,12 @@ struct netdev_dpdk {
>>>
>>>     /* In dpdk_list. */
>>>     struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
>>>+
>>>+    /* The following properties cannot be changed when a device is
>>>running,
>>>+     * so we remember the request and update them next time
>>>+     * netdev_dpdk*_reconfigure() is called */
>>>+    int requested_n_txq;
>>>+    int requested_n_rxq;
>>> };
>>>
>>> struct netdev_rxq_dpdk {
>>>@@ -614,7 +620,8 @@ netdev_dpdk_init(struct netdev *netdev_, unsigned
>>>int port_no,
>>>
>>>     netdev_->n_txq = NR_QUEUE;
>>>     netdev_->n_rxq = NR_QUEUE;
>>>-    netdev_->requested_n_rxq = NR_QUEUE;
>>>+    netdev->requested_n_rxq = NR_QUEUE;
>>>+    netdev->requested_n_txq = NR_QUEUE;
>>>     netdev->real_n_txq = NR_QUEUE;
>>>
>>>     if (type == DPDK_DEV_ETH) {
>>>@@ -796,7 +803,7 @@ netdev_dpdk_get_config(const struct netdev *netdev,
>>>struct smap *args)
>>>
>>>     ovs_mutex_lock(&dev->mutex);
>>>
>>>-    smap_add_format(args, "requested_rx_queues", "%d",
>>>netdev->requested_n_rxq);
>>>+    smap_add_format(args, "requested_rx_queues", "%d",
>>>dev->requested_n_rxq);
>>>     smap_add_format(args, "configured_rx_queues", "%d", netdev->n_rxq);
>>>     smap_add_format(args, "requested_tx_queues", "%d", netdev->n_txq);
>>>     smap_add_format(args, "configured_tx_queues", "%d",
>>>dev->real_n_txq);
>>>@@ -809,11 +816,13 @@ static int
>>> netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args)
>>> {
>>>     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>>+    int new_n_rxq = MAX(smap_get_int(args, "n_rxq",
>>>dev->requested_n_rxq), 1);
>>
>>Should we provide some parameter checking here? i.e. what happens if the
>>user provides a negative value?
>>Alternatively, the string may be terminated with junk; smap_get_int
>>relies on atoi to convert the user argument, so it may be better to use
>>strtoul to sanitize the input instead.
>
>Negative values shouldn't be a problem because we use MAX(..., 1)
>
>That's true the string may be terminated with junk, maybe we should fix
>smap_get_int? I'm not sure we should do more here.
>
>I've decided to leave it as it is for now

Sounds good. If I have time, I _might_ log a patch for smap_get_int.

>
>>
>>>
>>>     ovs_mutex_lock(&dev->mutex);
>>>-    netdev->requested_n_rxq = MAX(smap_get_int(args, "n_rxq",
>>>-
>>>netdev->requested_n_rxq), 1);
>>>-    netdev_change_seq_changed(netdev);
>>>+    if (new_n_rxq != dev->requested_n_rxq) {
>>>+        dev->requested_n_rxq = new_n_rxq;
>>>+        netdev_request_reconfigure(netdev);
>>>+    }
>>>     ovs_mutex_unlock(&dev->mutex);
>>>
>>>     return 0;
>>>@@ -827,93 +836,44 @@ netdev_dpdk_get_numa_id(const struct netdev
>>>*netdev_)
>>>     return netdev->socket_id;
>>> }
>>>
>>>-/* Sets the number of tx queues and rx queues for the dpdk interface.
>>>- * If the configuration fails, do not try restoring its old
>>>configuration
>>>- * and just returns the error. */
>>>+/* Sets the number of tx queues for the dpdk interface. */
>>> static int
>>>-netdev_dpdk_set_multiq(struct netdev *netdev_, unsigned int n_txq,
>>>-                       unsigned int n_rxq)
>>>+netdev_dpdk_set_multiq(struct netdev *netdev_, unsigned int n_txq)
>>> {
>>>     struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
>>>     int err = 0;
>>>-    int old_rxq, old_txq;
>>>
>>>-    if (netdev->up.n_txq == n_txq && netdev->up.n_rxq == n_rxq) {
>>>-        return err;
>>>-    }
>>>-
>>>-    ovs_mutex_lock(&dpdk_mutex);
>>>     ovs_mutex_lock(&netdev->mutex);
>>>
>>>-    rte_eth_dev_stop(netdev->port_id);
>>>-
>>>-    old_txq = netdev->up.n_txq;
>>>-    old_rxq = netdev->up.n_rxq;
>>>-    netdev->up.n_txq = n_txq;
>>>-    netdev->up.n_rxq = n_rxq;
>>>-
>>>-    rte_free(netdev->tx_q);
>>>-    err = dpdk_eth_dev_init(netdev);
>>>-    netdev_dpdk_alloc_txq(netdev, netdev->real_n_txq);
>>>-    if (err) {
>>>-        /* If there has been an error, it means that the requested
>>>queues
>>>-         * have not been created.  Restore the old numbers. */
>>>-        netdev->up.n_txq = old_txq;
>>>-        netdev->up.n_rxq = old_rxq;
>>>+    if (netdev->up.n_txq == n_txq) {
>>>+        goto out;
>>>     }
>>>
>>>-    netdev->txq_needs_locking = netdev->real_n_txq != netdev->up.n_txq;
>>>+    netdev->requested_n_txq = n_txq;
>>>+    netdev_request_reconfigure(netdev_);
>>>
>>>+out:
>>>     ovs_mutex_unlock(&netdev->mutex);
>>>-    ovs_mutex_unlock(&dpdk_mutex);
>>>-
>>>     return err;
>>>+
>>> }
>>>
>>> static int
>>>-netdev_dpdk_vhost_cuse_set_multiq(struct netdev *netdev_, unsigned int
>>>n_txq,
>>>-                             unsigned int n_rxq)
>>>+netdev_dpdk_vhost_set_multiq(struct netdev *netdev_, unsigned int n_txq)
>>> {
>>>     struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
>>>     int err = 0;
>>>
>>>-    if (netdev->up.n_txq == n_txq && netdev->up.n_rxq == n_rxq) {
>>>-        return err;
>>>-    }
>>>-
>>>-    ovs_mutex_lock(&dpdk_mutex);
>>>     ovs_mutex_lock(&netdev->mutex);
>>>
>>>-    netdev->up.n_txq = n_txq;
>>>-    netdev->real_n_txq = 1;
>>>-    netdev->up.n_rxq = 1;
>>>-    netdev->txq_needs_locking = netdev->real_n_txq != netdev->up.n_txq;
>>>-
>>>-    ovs_mutex_unlock(&netdev->mutex);
>>>-    ovs_mutex_unlock(&dpdk_mutex);
>>>-
>>>-    return err;
>>>-}
>>>-
>>>-static int
>>>-netdev_dpdk_vhost_set_multiq(struct netdev *netdev_, unsigned int n_txq,
>>>-                             unsigned int n_rxq)
>>>-{
>>>-    struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
>>>-    int err = 0;
>>>-
>>>-    if (netdev->up.n_txq == n_txq && netdev->up.n_rxq == n_rxq) {
>>>-        return err;
>>>+    if (netdev->up.n_txq == n_txq) {
>>>+        goto out;
>>>     }
>>>
>>>-    ovs_mutex_lock(&dpdk_mutex);
>>>-    ovs_mutex_lock(&netdev->mutex);
>>>-
>>>-    netdev->up.n_txq = n_txq;
>>>-    netdev->up.n_rxq = n_rxq;
>>>+    netdev->requested_n_txq = n_txq;
>>
>>Do we need to request a reconfigure here?
>
>You're right, thanks for noticing!
>
>That change makes it equal to netdev_dpdk_set_multiq(), so I've merged the
>two functions
>
>>
>>>
>>>+out:
>>>     ovs_mutex_unlock(&netdev->mutex);
>>>-    ovs_mutex_unlock(&dpdk_mutex);
>>>
>>>     return err;
>>> }
>>>@@ -2239,8 +2199,69 @@ unlock_dpdk:
>>>     return err;
>>> }
>>>
>>>-#define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT, MULTIQ,
>>>SEND, \
>>>-    GET_CARRIER, GET_STATS, GET_FEATURES, GET_STATUS, RXQ_RECV)
>>> \
>>>+static int
>>>+netdev_dpdk_reconfigure(struct netdev *netdev_)
>>>+{
>>>+    struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
>>>+    int err = 0;
>>>+
>>>+    ovs_mutex_lock(&dpdk_mutex);
>>>+    ovs_mutex_lock(&netdev->mutex);
>>>+    rte_eth_dev_stop(netdev->port_id);
>>>+
>>>+    netdev_->n_txq = netdev->requested_n_txq;
>>>+    netdev_->n_rxq = netdev->requested_n_rxq;
>>>+
>>>+    rte_free(netdev->tx_q);
>>>+    err = dpdk_eth_dev_init(netdev);
>>>+    netdev_dpdk_alloc_txq(netdev, netdev->real_n_txq);
>>>+
>>>+    netdev->txq_needs_locking = netdev->real_n_txq != netdev->up.n_txq;
>>>+
>>>+    ovs_mutex_unlock(&netdev->mutex);
>>>+    ovs_mutex_unlock(&dpdk_mutex);
>>>+
>>>+    return err;
>>>+}
>>>+
>>>+static int
>>>+netdev_dpdk_vhost_user_reconfigure(struct netdev *netdev_)
>>>+{
>>>+    struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
>>>+
>>>+    ovs_mutex_lock(&dpdk_mutex);
>>>+    ovs_mutex_lock(&netdev->mutex);
>>>+
>>>+    netdev->up.n_txq = netdev->requested_n_txq;
>>>+    netdev->up.n_rxq = netdev->requested_n_rxq;
>>>+
>>>+    ovs_mutex_unlock(&netdev->mutex);
>>>+    ovs_mutex_unlock(&dpdk_mutex);
>>>+
>>>+    return 0;
>>>+}
>>>+
>>>+static int
>>>+netdev_dpdk_vhost_cuse_reconfigure(struct netdev *netdev_)
>>>+{
>>>+    struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
>>>+
>>>+    ovs_mutex_lock(&dpdk_mutex);
>>>+    ovs_mutex_lock(&netdev->mutex);
>>>+
>>>+    netdev->up.n_txq = netdev->requested_n_txq;
>>>+    netdev->real_n_txq = 1;
>>>+    netdev->up.n_rxq = 1;
>>>+    netdev->txq_needs_locking = netdev->real_n_txq != netdev->up.n_txq;
>>>+
>>>+    ovs_mutex_unlock(&netdev->mutex);
>>>+    ovs_mutex_unlock(&dpdk_mutex);
>>>+
>>>+    return 0;
>>>+}
>>>+
>>>+#define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT, MULTIQ,
>>>SEND,      \
>>>+    GET_CARRIER, GET_STATS, GET_FEATURES, GET_STATUS, RECONFIGURE,
>>>RXQ_RECV)  \
>>> {                                                             \
>>>     NAME,                                                     \
>>>     INIT,                       /* init */                    \
>>>@@ -2298,7 +2319,7 @@ unlock_dpdk:
>>>     NULL,                       /* arp_lookup */              \
>>>                                                               \
>>>     netdev_dpdk_update_flags,                                 \
>>>-    NULL,                       /* reconfigure */             \
>>>+    RECONFIGURE,                                              \
>>>                                                               \
>>>     netdev_dpdk_rxq_alloc,                                    \
>>>     netdev_dpdk_rxq_construct,                                \
>>>@@ -2419,6 +2440,7 @@ static const struct netdev_class dpdk_class =
>>>         netdev_dpdk_get_stats,
>>>         netdev_dpdk_get_features,
>>>         netdev_dpdk_get_status,
>>>+        netdev_dpdk_reconfigure,
>>>         netdev_dpdk_rxq_recv);
>>>
>>> static const struct netdev_class dpdk_ring_class =
>>>@@ -2433,6 +2455,7 @@ static const struct netdev_class dpdk_ring_class =
>>>         netdev_dpdk_get_stats,
>>>         netdev_dpdk_get_features,
>>>         netdev_dpdk_get_status,
>>>+        netdev_dpdk_reconfigure,
>>>         netdev_dpdk_rxq_recv);
>>>
>>> static const struct netdev_class OVS_UNUSED dpdk_vhost_cuse_class =
>>>@@ -2441,12 +2464,13 @@ static const struct netdev_class OVS_UNUSED
>>>dpdk_vhost_cuse_class =
>>>         dpdk_vhost_cuse_class_init,
>>>         netdev_dpdk_vhost_cuse_construct,
>>>         netdev_dpdk_vhost_destruct,
>>>-        netdev_dpdk_vhost_cuse_set_multiq,
>>>+        netdev_dpdk_vhost_set_multiq,
>>>         netdev_dpdk_vhost_send,
>>>         netdev_dpdk_vhost_get_carrier,
>>>         netdev_dpdk_vhost_get_stats,
>>>         NULL,
>>>         NULL,
>>>+        netdev_dpdk_vhost_cuse_reconfigure,
>>>         netdev_dpdk_vhost_rxq_recv);
>>>
>>> static const struct netdev_class OVS_UNUSED dpdk_vhost_user_class =
>>>@@ -2461,6 +2485,7 @@ static const struct netdev_class OVS_UNUSED
>>>dpdk_vhost_user_class =
>>>         netdev_dpdk_vhost_get_stats,
>>>         NULL,
>>>         NULL,
>>>+        netdev_dpdk_vhost_user_reconfigure,
>>>         netdev_dpdk_vhost_rxq_recv);
>>>
>>> void
>>>diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
>>>index 9646cca..29b3949 100644
>>>--- a/lib/netdev-provider.h
>>>+++ b/lib/netdev-provider.h
>>>@@ -67,8 +67,6 @@ struct netdev {
>>>      * modify them. */
>>>     int n_txq;
>>>     int n_rxq;
>>>-    /* Number of rx queues requested by user. */
>>>-    int requested_n_rxq;
>>>     int ref_cnt;                        /* Times this devices was
>>>opened. */
>>>     struct shash_node *node;            /* Pointer to element in global
>>>map. */
>>>     struct ovs_list saved_flags_list; /* Contains "struct
>>>netdev_saved_flags". */
>>>@@ -295,13 +293,8 @@ struct netdev_class {
>>>      * such info, returns NETDEV_NUMA_UNSPEC. */
>>>     int (*get_numa_id)(const struct netdev *netdev);
>>>
>>>-    /* Configures the number of tx queues and rx queues of 'netdev'.
>>>-     * Return 0 if successful, otherwise a positive errno value.
>>>-     *
>>>-     * 'n_rxq' specifies the maximum number of receive queues to create.
>>>-     * The netdev provider might choose to create less (e.g. if the
>>>hardware
>>>-     * supports only a smaller number).  The actual number of queues
>>>created
>>>-     * is stored in the 'netdev->n_rxq' field.
>>>+    /* Configures the number of tx queues of 'netdev'. Returns 0 if
>>>successful,
>>>+     * otherwise a positive errno value.
>>>      *
>>>      * 'n_txq' specifies the exact number of transmission queues to
>>>create.
>>>      * The caller will call netdev_send() concurrently from 'n_txq'
>>>different
>>>@@ -309,12 +302,8 @@ struct netdev_class {
>>>      * making sure that these concurrent calls do not create a race
>>>condition
>>>      * by using multiple hw queues or locking.
>>>      *
>>>-     * On error, the tx queue and rx queue configuration is
>>>indeterminant.
>>>-     * Caller should make decision on whether to restore the previous or
>>>-     * the default configuration.  Also, caller must make sure there is
>>>no
>>>-     * other thread accessing the queues at the same time. */
>>>-    int (*set_multiq)(struct netdev *netdev, unsigned int n_txq,
>>>-                      unsigned int n_rxq);
>>>+     * On error, the tx queue and rx queue configuration is unchanged.
>>>*/
>>
>>It's implicit that the Rx queue config is unchanged, since the set_multiq
>>function now only affects Tx queues, so there's probably no need to state
>>it explicitly.
>
>You're right, I removed the reference to the rx queues.
>
>Thank you for your review, Mark.
>
>I've incorporated all your comments.  I hope to get more reviews,
>otherwise I'll post a v2 soon

No problem - looking forward to V2!



_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to