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