[ovs-dev] Returned mail: Data format error
The original message was received at Sun, 1 Jan 2006 02:21:04 +0530 from cgocable.ca [123.186.220.110] - The following addresses had permanent fatal errors - dev@openvswitch.org ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH RFC v3] netdev-dpdk: vhost-user: Fix sending packets to queues not enabled by guest.
Currently virtio driver in guest operating system have to be configured
to use exactly same number of queues. If number of queues will be less,
some packets will get stuck in queues unused by guest and will not be
received.
Fix that by using new 'vring_state_changed' callback, which is
available for vhost-user since DPDK 2.2.
Implementation uses additional mapping from configured tx queues to
enabled by virtio driver. This requires mandatory locking of TX queues
in __netdev_dpdk_vhost_send(), but this locking was almost always anyway
because of calling set_multiq with n_txq = 'ovs_numa_get_n_cores() + 1'.
Fixes: 4573fbd38fa1 ("netdev-dpdk: Add vhost-user multiqueue support")
Signed-off-by: Ilya Maximets
Reviewed-by: Aaron Conole
Acked-by: Flavio Leitner
---
Version 3:
* Fixed size in dpdk_rte_mzalloc() for enabled_queues.
* Fixed possible segfault because of unallocated tx_q[].
* Added remap of tx queues after real_n_txq modification
in netdev_dpdk_vhost_set_queues().
lib/netdev-dpdk.c | 112 ++
1 file changed, 97 insertions(+), 15 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e4f789b..3e69e50 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -173,6 +173,8 @@ struct dpdk_tx_queue {
* from concurrent access. It is used only
* if the queue is shared among different
* pmd threads (see 'txq_needs_locking'). */
+int map; /* Mapping of configured vhost-user queues
+* to enabled by guest. */
uint64_t tsc;
struct rte_mbuf *burst_pkts[MAX_TX_QUEUE_LEN];
};
@@ -572,6 +574,9 @@ netdev_dpdk_alloc_txq(struct netdev_dpdk *netdev, unsigned
int n_txqs)
/* Queues are shared among CPUs. Always flush */
netdev->tx_q[i].flush_tx = true;
}
+
+/* Initialize map for vhost devices. */
+netdev->tx_q[i].map = -1;
rte_spinlock_init(&netdev->tx_q[i].tx_lock);
}
}
@@ -623,6 +628,8 @@ netdev_dpdk_init(struct netdev *netdev_, unsigned int
port_no,
if (err) {
goto unlock;
}
+} else {
+netdev_dpdk_alloc_txq(netdev, VHOST_MAX_QUEUE_PAIRS);
}
list_push_back(&dpdk_list, &netdev->list_node);
@@ -889,10 +896,8 @@ netdev_dpdk_vhost_set_multiq(struct netdev *netdev_,
unsigned int n_txq,
ovs_mutex_lock(&dpdk_mutex);
ovs_mutex_lock(&netdev->mutex);
-rte_free(netdev->tx_q);
netdev->up.n_txq = n_txq;
netdev->up.n_rxq = n_rxq;
-netdev_dpdk_alloc_txq(netdev, netdev->up.n_txq);
ovs_mutex_unlock(&netdev->mutex);
ovs_mutex_unlock(&dpdk_mutex);
@@ -1117,17 +1122,16 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
unsigned int total_pkts = cnt;
uint64_t start = 0;
-if (OVS_UNLIKELY(!is_vhost_running(virtio_dev))) {
+qid = vhost_dev->tx_q[qid % vhost_dev->real_n_txq].map;
+
+if (OVS_UNLIKELY(!is_vhost_running(virtio_dev) || qid == -1)) {
rte_spinlock_lock(&vhost_dev->stats_lock);
vhost_dev->stats.tx_dropped+= cnt;
rte_spinlock_unlock(&vhost_dev->stats_lock);
goto out;
}
-if (vhost_dev->txq_needs_locking) {
-qid = qid % vhost_dev->real_n_txq;
-rte_spinlock_lock(&vhost_dev->tx_q[qid].tx_lock);
-}
+rte_spinlock_lock(&vhost_dev->tx_q[qid].tx_lock);
do {
int vhost_qid = qid * VIRTIO_QNUM + VIRTIO_RXQ;
@@ -1165,9 +1169,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
}
} while (cnt);
-if (vhost_dev->txq_needs_locking) {
-rte_spinlock_unlock(&vhost_dev->tx_q[qid].tx_lock);
-}
+rte_spinlock_unlock(&vhost_dev->tx_q[qid].tx_lock);
rte_spinlock_lock(&vhost_dev->stats_lock);
netdev_dpdk_vhost_update_tx_counters(&vhost_dev->stats, pkts, total_pkts,
@@ -1827,6 +1829,46 @@ set_irq_status(struct virtio_net *dev)
}
}
+/*
+ * Fixes mapping for vhost-user tx queues. Must be called after each
+ * enabling/disabling of queues and real_n_txq modifications.
+ */
+static void
+netdev_dpdk_remap_txqs(struct netdev_dpdk *netdev)
+OVS_REQUIRES(netdev->mutex)
+{
+int *enabled_queues, n_enabled = 0;
+int i, k, total_txqs = netdev->real_n_txq;
+
+enabled_queues = dpdk_rte_mzalloc(total_txqs * sizeof *enabled_queues);
+
+for (i = 0; i < total_txqs; i++) {
+/* Enabled queues always mapped to themselves. */
+if (netdev->tx_q[i].map == i) {
+enabled_queues[n_enabled++] = i;
+}
+}
+
+if (n_enabled == 0 && total_txqs != 0) {
+enabled_queues[0] = -1;
+n_enabled = 1;
+}
+
+k = 0;
+for (i = 0; i < total_txqs; i++) {
+if (netdev->tx_q[i].map != i) {
+netdev->tx_q[i].map = enabled_queues[k];
+k = (k + 1) % n_enabled;
+
[ovs-dev] [PATCH 2/2] ovsdb: Return NULL if prev_txn == next_txn
In case that we flushed everything already, we can immeidately return NULL.
Signed-off-by: Liran Schour
---
ovsdb/monitor.c | 4
1 file changed, 4 insertions(+)
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 5ae9cdb..1a07f19 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -733,6 +733,10 @@ ovsdb_monitor_get_update(struct ovsdb_monitor *dbmon,
uint64_t prev_txn = *unflushed;
uint64_t next_txn = dbmon->n_transactions + 1;
+if (prev_txn == next_txn) {
+return NULL;
+}
+
/* Return a clone of cached json if one exists. Otherwise,
* generate a new one and add it to the cache. */
cache_node = ovsdb_monitor_json_cache_search(dbmon, version, prev_txn);
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] tests: Fix inconsistency tests in monitor
In case of delete only: !initial,!insert,!modify. We can not be sure that we
will see X if we have: insert X, delete X.
In case of modify only: !initial,!insert,!delete. We can not be sure that we
will see X modified if we have: insert X, modify X, delete X.
Signed-off-by: Liran Schour
---
tests/ovsdb-monitor.at | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/tests/ovsdb-monitor.at b/tests/ovsdb-monitor.at
index 0dbf5b0..5a85ea2 100644
--- a/tests/ovsdb-monitor.at
+++ b/tests/ovsdb-monitor.at
@@ -271,7 +271,7 @@ m4_define([OVSDB_MONITOR_TXNS],
[[["ordinals",
{"op": "delete",
"table": "ordinals",
- "where": []})
+ "where": [["name","==","ten"]]})
OVSDB_CHECK_MONITOR([monitor all operations],
[ordinal_schema], [OVSDB_MONITOR_INITIAL],
@@ -287,7 +287,6 @@ row,action,name,number,_version
,new,"""FIVE""",5,"[""uuid"",""<4>""]"
row,action,name,number,_version
-<2>,delete,"""FIVE""",5,"[""uuid"",""<4>""]"
<0>,delete,"""ten""",10,"[""uuid"",""<1>""]"
]])
@@ -311,14 +310,17 @@ OVSDB_CHECK_MONITOR([monitor delete only],
[ordinal_schema], [OVSDB_MONITOR_INITIAL],
[ordinals], [ordinals], [OVSDB_MONITOR_TXNS],
[[row,action,name,number,_version
-<0>,delete,"""FIVE""",5,"[""uuid"",""<1>""]"
-<2>,delete,"""ten""",10,"[""uuid"",""<3>""]"
+<0>,delete,"""ten""",10,"[""uuid"",""<1>""]"
]], [!initial,!insert,!modify])
OVSDB_CHECK_MONITOR([monitor modify only],
[ordinal_schema], [OVSDB_MONITOR_INITIAL],
- [ordinals], [ordinals], [OVSDB_MONITOR_TXNS],
+ [ordinals], [ordinals], "ordinals",
+ {"op": "update",
+ "table": "ordinals",
+ "where": [["name", "==", "ten"]],
+ "row": {"name": "TEN"}},
[[row,action,name,number,_version
-<0>,old,"""five""",,"[""uuid"",""<1>""]"
-,new,"""FIVE""",5,"[""uuid"",""<2>""]"
+<0>,old,"""ten""",,"[""uuid"",""<1>""]"
+,new,"""TEN""",10,"[""uuid"",""<2>""]"
]], [!initial,!insert,!delete])
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v8 0/5] Convert DPDK configuration from command line to DB based
Aaron Conole writes: > Daniele Di Proietto writes: > >> On 09/02/2016 09:48, "Traynor, Kevin" wrote: >> >>> >>> -Original Message- From: Aaron Conole [mailto:acon...@redhat.com] Sent: Friday, January 29, 2016 5:57 PM To: dev@openvswitch.org Cc: Flavio Leitner ; Panu Matilainen ; Traynor, Kevin ; Zoltan Kiss ; Christian Ehrhardt Subject: [PATCH v8 0/5] Convert DPDK configuration from command line to DB based Currently, configuration of DPDK parameters is done via the command line through a --dpdk **OPTIONS** -- command line argument. This has a number of challenges, including: * It must be the first option passed to ovs-vswitchd * It breaks from the way most other things are configured in OVS * It doesn't allow an easy way to populate defaults This series brings the following changes to openvswitch: * All DPDK options are taken from the ovs database rather than the command line * DPDK lcores are optionally auto-assigned to a single core based on the bridge coremask. * Updated documentation v2: * Dropped the vhost-user socket configuration options. Those can be re-added as an extension * Incorporated feedback from Kevin Traynor. v3: * Went back to a global dpdk-init * Language cleanup and various minor fixes v4: * Added a way to pass arbitrary eal arguments v5: * Restore the socket-mem default, and fix up the ovs-dev.py script, along with the manpage for ovsdb-server v6: * Correct a documentation issue with INSTALL.DPDK.md * Correct a non-dpdk enabled OVS incorrect warning variable * Remove an excess whitespace v7: * After testing by Christian with dpdk-alloc-mem v8: * Confirmed ``make check`` operation with and without dpdk. Retested on live-host >>> >>>Hi, >>> >>>I've done some testing on this patchset and I couldn't find any issues. >>> - tested that -c and -n defaults and explicit values are catered for >>> - tested dpdk-init=t/f leads to dpdk initialization or not >>> - tested that use of both dpdk-socket-mem and dpdk-alloc-mem is caught >>> - tested that a string can be passed in through extra_args >>> - tested the code won't catch using a db entry dpdk-socket-mem and also >>> putting --socket-mem in extra_args, however dpdk will barf >>> >>>On command line args vs. db entries vs. a string of args in the db, if >>>there >>>is doubt on this then let's debate further. This will change how ovs with >>>dpdk is used, so better debate it out and get it right. >> >> I don't have any particular preference on command line vs database. As >> Jesse >> pointed out having them in the database might help making them run-time >> configurable (even though they're not today). > > I agree with this very much :). > >>> >>>There's one or two of the db entries that may be able to reused later for >>>other things e.g. vhostuser socket location, so that would be a + for >>>them. >>>Backwards compatibility would be a + for command line args. Daniele has >>>mentioned scripting also. I'm sure there's other +/-'s. >> >> Again, IMHO, this is quite important. Having to start OVS just to write a >> value in the database and the restart it again seems wasteful. If we want >> to go with the database, we should figure out how to integrate with >> ovs-ctl. > > Perhaps there could be something like (just brainstorming): > > ovs-ctl set-db-value "key=value" which would start the database, set the > parameter, and stop the database. Then there could be a > --restart-vswitchd flag? > > Just throwing out a wild idea. I honestly don't know what makes the most > sense from a "user experience" perspective. Thinking further on this, would it make sense for ovs-ctl to have a separate start-db and start-forwarding commands? At this point, both of those actions exist as shell functions, so it's really just a matter of a bit of cleanup and calling the appropriate start function. At that point, the user can actually start the db, make all the dpdk changes required, and then start the switching path. If that makes sense I can submit a quick patch to enable this kind of workflow. I think it will also enable a systemd integration in the sense that systemd wants one service per service file. Current approach starts both in the nonetwork script (which really doesn't make sense anyway... nonetwork actually does enable networking), but with ovs-ctl supporting both a start-db and start-forwarding call, there could be two services. Just another thought. Would be good to know what folks think. >>> >>>Kevin. >>> Aaron Conole (5): netdev-dpdk: Restore thread affinity after DPDK init netdev-dpdk: Convert initialization from cmdline to db netdev-dpdk: Autofill lcore coremask if absent netdev-dpdk: Allow
