Re: [ovs-dev] [PATCH v7 16/16] netdev-dpdk: Use ->reconfigure() call to change rx/tx queues.

[Kavanagh, Mark B]

>
>
>
>On 14/04/2016 05:37, "Kavanagh, Mark B"  wrote:
>
>>Hi Daniele,
>>
>>One comment inline.
>>
>>Thanks,
>>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.
>>>* Move a DPDK vhost device to the proper NUMA socket.
>>>
>>>Signed-off-by: Daniele Di Proietto 
>>>---
>>> lib/dpif-netdev.c |  69 +-
>>> lib/netdev-dpdk.c | 195
>>>++
>>> lib/netdev-provider.h |  23 +++---
>>> lib/netdev.c  |  34 +++--
>>> lib/netdev.h  |   3 +-
>>> 5 files changed, 155 insertions(+), 169 deletions(-)
>
>[...]
>
>>>@@ -312,12 +305,12 @@ 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);
>>>+ * The caller will call netdev_reconfigure() (if necessary) before
>>>using
>>>+ * netdev_send() on any of the newly configured queues, giving the
>>>provider
>>>+ * a chance to adjust its settings.
>>>+ *
>>>+ * On error, the tx queue configuration is unchanged. */
>>>+int (*set_multiq)(struct netdev *netdev, unsigned int n_txq);
>>
>>Since this function now deals only with TX queues, an identifier along
>>the lines of 'set_tx_multiq' might more accurately describe its
>>functionality. Specific netdev classes would need to modify the names of
>>their own specific 'set_multiq' functions accordingly.
>
>You're right, 'set_tx_multiq()' is definitely a better name for it. I
>updated it.
>
>Thanks for all your feedback

No problem - thanks for the patchset!

Cheers,
Mark
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v7 16/16] netdev-dpdk: Use ->reconfigure() call to change rx/tx queues.

[Daniele Di Proietto]

On 14/04/2016 05:37, "Kavanagh, Mark B"  wrote:

>Hi Daniele,
>
>One comment inline.
>
>Thanks,
>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.
>>* Move a DPDK vhost device to the proper NUMA socket.
>>
>>Signed-off-by: Daniele Di Proietto 
>>---
>> lib/dpif-netdev.c |  69 +-
>> lib/netdev-dpdk.c | 195
>>++
>> lib/netdev-provider.h |  23 +++---
>> lib/netdev.c  |  34 +++--
>> lib/netdev.h  |   3 +-
>> 5 files changed, 155 insertions(+), 169 deletions(-)

[...]

>>@@ -312,12 +305,12 @@ 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);
>>+ * The caller will call netdev_reconfigure() (if necessary) before
>>using
>>+ * netdev_send() on any of the newly configured queues, giving the
>>provider
>>+ * a chance to adjust its settings.
>>+ *
>>+ * On error, the tx queue configuration is unchanged. */
>>+int (*set_multiq)(struct netdev *netdev, unsigned int n_txq);
>
>Since this function now deals only with TX queues, an identifier along
>the lines of 'set_tx_multiq' might more accurately describe its
>functionality. Specific netdev classes would need to modify the names of
>their own specific 'set_multiq' functions accordingly.

You're right, 'set_tx_multiq()' is definitely a better name for it. I
updated it.

Thanks for all your feedback

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

Re: [ovs-dev] [PATCH v7 16/16] netdev-dpdk: Use ->reconfigure() call to change rx/tx queues.

[Kavanagh, Mark B]

Hi Daniele,

One comment inline.

Thanks,
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.
>* Move a DPDK vhost device to the proper NUMA socket.
>
>Signed-off-by: Daniele Di Proietto 
>---
> lib/dpif-netdev.c |  69 +-
> lib/netdev-dpdk.c | 195 ++
> lib/netdev-provider.h |  23 +++---
> lib/netdev.c  |  34 +++--
> lib/netdev.h  |   3 +-
> 5 files changed, 155 insertions(+), 169 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index fc81741..d9bfe80 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -257,8 +257,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.  */
>@@ -1159,20 +1157,26 @@ port_create(const char *devname, const char 
>*open_type, 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;
> }
> }
>+
>+if (netdev_is_reconf_required(netdev)) {
>+error = netdev_reconfigure(netdev);
>+if (error) {
>+goto out;
>+}
>+}
>+
> port = xzalloc(sizeof *port);
> port->port_no = port_no;
> port->netdev = netdev;
> port->n_rxq = netdev_n_rxq(netdev);
> port->rxq = xcalloc(port->n_rxq, sizeof *port->rxq);
> port->type = xstrdup(type);
>-port->latest_requested_n_rxq = netdev_requested_n_rxq(netdev);
>
> for (i = 0; i < port->n_rxq; i++) {
> error = netdev_rxq_open(netdev, >rxq[i], i);
>@@ -2453,27 +2457,6 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op 
>**ops, size_t
>n_ops)
> }
> }
>
>-/* Returns true if the configuration for rx queues is changed. */
>-static bool
>-pmd_n_rxq_changed(const struct dp_netdev *dp)
>-{
>-struct dp_netdev_port *port;
>-
>-ovs_mutex_lock(>port_mutex);
>-HMAP_FOR_EACH (port, node, >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) {
>-ovs_mutex_unlock(>port_mutex);
>-return true;
>-}
>-}
>-ovs_mutex_unlock(>port_mutex);
>-
>-return false;
>-}
>-
> static bool
> cmask_equals(const char *a, const char *b)
> {
>@@ -2597,11 +2580,9 @@ static int
> port_reconfigure(struct dp_netdev_port *port)
> {
> struct netdev *netdev = port->netdev;
>-int requested_n_rxq = netdev_requested_n_rxq(netdev);
> int i, err;
>
>-if (!netdev_is_pmd(port->netdev)
>-|| port->latest_requested_n_rxq != requested_n_rxq) {
>+if (!netdev_is_reconf_required(netdev)) {
> return 0;
> }
>
>@@ -2612,15 +2593,14 @@ port_reconfigure(struct dp_netdev_port *port)
> }
> 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);
>+/* Allows 'netdev' to apply the pending configuration changes. */
>+err = netdev_reconfigure(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(netdev));
> return err;
> }
>-/* If the set_multiq() above succeeds, reopens the 'rxq's. */
>+/* If the netdev_reconfigure( above succeeds, reopens the 'rxq's. */
> port->rxq = xrealloc(port->rxq, sizeof *port->rxq * netdev_n_rxq(netdev));
> for (i = 0; i < netdev_n_rxq(netdev); i++) {
> err = netdev_rxq_open(netdev, >rxq[i], i);
>@@ -2664,6 +2644,22 @@ 

[ovs-dev] [PATCH v7 16/16] netdev-dpdk: Use ->reconfigure() call to change rx/tx queues.

[Daniele Di Proietto]

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.
* Move a DPDK vhost device to the proper NUMA socket.

Signed-off-by: Daniele Di Proietto 
---
 lib/dpif-netdev.c |  69 +-
 lib/netdev-dpdk.c | 195 ++
 lib/netdev-provider.h |  23 +++---
 lib/netdev.c  |  34 +++--
 lib/netdev.h  |   3 +-
 5 files changed, 155 insertions(+), 169 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index fc81741..d9bfe80 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -257,8 +257,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.  */
@@ -1159,20 +1157,26 @@ port_create(const char *devname, const char *open_type, 
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;
 }
 }
+
+if (netdev_is_reconf_required(netdev)) {
+error = netdev_reconfigure(netdev);
+if (error) {
+goto out;
+}
+}
+
 port = xzalloc(sizeof *port);
 port->port_no = port_no;
 port->netdev = netdev;
 port->n_rxq = netdev_n_rxq(netdev);
 port->rxq = xcalloc(port->n_rxq, sizeof *port->rxq);
 port->type = xstrdup(type);
-port->latest_requested_n_rxq = netdev_requested_n_rxq(netdev);
 
 for (i = 0; i < port->n_rxq; i++) {
 error = netdev_rxq_open(netdev, >rxq[i], i);
@@ -2453,27 +2457,6 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op 
**ops, size_t n_ops)
 }
 }
 
-/* Returns true if the configuration for rx queues is changed. */
-static bool
-pmd_n_rxq_changed(const struct dp_netdev *dp)
-{
-struct dp_netdev_port *port;
-
-ovs_mutex_lock(>port_mutex);
-HMAP_FOR_EACH (port, node, >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) {
-ovs_mutex_unlock(>port_mutex);
-return true;
-}
-}
-ovs_mutex_unlock(>port_mutex);
-
-return false;
-}
-
 static bool
 cmask_equals(const char *a, const char *b)
 {
@@ -2597,11 +2580,9 @@ static int
 port_reconfigure(struct dp_netdev_port *port)
 {
 struct netdev *netdev = port->netdev;
-int requested_n_rxq = netdev_requested_n_rxq(netdev);
 int i, err;
 
-if (!netdev_is_pmd(port->netdev)
-|| port->latest_requested_n_rxq != requested_n_rxq) {
+if (!netdev_is_reconf_required(netdev)) {
 return 0;
 }
 
@@ -2612,15 +2593,14 @@ port_reconfigure(struct dp_netdev_port *port)
 }
 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);
+/* Allows 'netdev' to apply the pending configuration changes. */
+err = netdev_reconfigure(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(netdev));
 return err;
 }
-/* If the set_multiq() above succeeds, reopens the 'rxq's. */
+/* If the netdev_reconfigure( above succeeds, reopens the 'rxq's. */
 port->rxq = xrealloc(port->rxq, sizeof *port->rxq * netdev_n_rxq(netdev));
 for (i = 0; i < netdev_n_rxq(netdev); i++) {
 err = netdev_rxq_open(netdev, >rxq[i], i);
@@ -2664,6 +2644,22 @@ 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