Hey IIya,
Thx for the reply, Daniele helped me reproduced the issue offline and we
confirmed that this is not DPDK issue.
However, this is also not a ufid issue either...
Let me explain,
First to clear up, with "packets from the same flow (or stream)", I mean
packets with same src/dst MAC, IP, and TCP/UDP port... Packets with such
same info must be hashed to the same rx queue of the receiving port and be
handled by the same pmd threads,... Otherwise, we may have serious issue
with reordering.
Then, turns out this issue can be reproduced whenever we reconfigure the
pmd threads (i.e. change number of pmd threads or change cpu pin) or the
port receiving queues... And when such reconfiguration happens, the
following two things happen:
- all pmd threads will be recreated, so after recreation, there is no
flow
flow exists in any pmd thread's classifier and exact match cache.
- the rx queues of each port will be re-mapped (highly likely to a
different
pmd thread).
So, after the queue re-mapping, the pmd thread will fire a new upcall
handling request to the ofproto-dpif-upcall module. However, the module has
no knowledge of the reconfiguration in the dpif-netdev layer, and still
think
the same packet has already been installed to in dpif-netdev module
(even though the old pmd thread has been reset). And thusly, the ofproto-
dpif-upcall module will fail the flow installation and increment the
handler_duplicate_upcall coverage counter.
Expectedly, this will only be a transient state, since the revalidator
should
cleans up the old ukey (with the ufid) since it can no longer be dumped
from the dpif-netdev module. But there is a bug that makes the revalidators
skip doing so.
I'd like to explain more if things are still unclear. Meanwhile, we will
discuss further on the fix and post the patch asap...
Thanks,
Alex Wang,
On Mon, Aug 10, 2015 at 7:57 AM, Ilya Maximets <i.maxim...@samsung.com>
wrote:
> Hello, Alex.
> I think, that this problem doesn't depend on dpdk. It is generic problem
> with ukeys.
> I found it while testing ODP-OVS from (Linaro Networking Group) without
> dpdk.
> IMHO, there must be opportunity to install ukeys for same flow by
> different pmd threads
> anyway, because they are reading from different rx queues.
>
> Sorry for delay, was busy fixing coverage to show it to you.)
> ( http://openvswitch.org/pipermail/dev/2015-August/058743.html )
>
> # ./bin/ovs-appctl coverage/show
> Event coverage, avg rate over last: 5 seconds, last minute, last hour,
> hash=09437d7d:
> bridge_reconfigure 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> ofproto_flush 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> ofproto_recv_openflow 0.0/sec 0.000/sec 0.0017/sec
> total: 6
> ofproto_update_port 0.0/sec 0.000/sec 0.0011/sec
> total: 4
> rev_reconfigure 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> rev_flow_table 0.0/sec 0.000/sec 0.0011/sec
> total: 4
> handler_duplicate_upcall 243016.0/sec 268180.317/sec 5728.5375/sec
> total: 20622735
> xlate_actions 243016.2/sec 259283.650/sec 5580.2606/sec
> total: 20088938
> cmap_expand 0.0/sec 0.000/sec 0.0019/sec
> total: 7
> cmap_shrink 0.0/sec 0.000/sec 0.0008/sec
> total: 3
> dpif_port_add 0.0/sec 0.000/sec 0.0008/sec
> total: 3
> dpif_flow_flush 0.0/sec 0.000/sec 0.0006/sec
> total: 2
> dpif_flow_get 0.0/sec 0.000/sec 0.0014/sec
> total: 5
> dpif_flow_put 0.0/sec 0.000/sec 0.0014/sec
> total: 5
> dpif_flow_del 0.0/sec 0.000/sec 0.0014/sec
> total: 5
> dpif_execute 0.0/sec 0.000/sec 0.0006/sec
> total: 2
> flow_extract 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> hmap_pathological 0.0/sec 0.000/sec 0.0050/sec
> total: 18
> hmap_expand 13.4/sec 19.467/sec 0.8011/sec
> total: 2884
> netdev_received 71024.6/sec 77401.583/sec 1678.5072/sec
> total: 6042626
> netdev_sent 306446.4/sec 328582.933/sec 7084.3894/sec
> total: 25503802
> netdev_get_stats 0.6/sec 0.600/sec 0.0200/sec
> total: 72
> txn_unchanged 0.2/sec 0.200/sec 0.0067/sec
> total: 24
> txn_incomplete 0.0/sec 0.000/sec 0.0006/sec
> total: 2
> txn_success 0.0/sec 0.000/sec 0.0006/sec
> total: 2
> poll_create_node 63.8/sec 89.917/sec 3.0958/sec
> total: 11145
> poll_zero_timeout 0.0/sec 0.000/sec 0.0017/sec
> total: 6
> rconn_queued 0.0/sec 0.000/sec 0.0008/sec
> total: 3
> rconn_sent 0.0/sec 0.000/sec 0.0008/sec
> total: 3
> pstream_open 0.0/sec 0.000/sec 0.0008/sec
> total: 3
> stream_open 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> unixctl_received 0.0/sec 0.383/sec 0.0131/sec
> total: 47
> unixctl_replied 0.0/sec 0.383/sec 0.0131/sec
> total: 47
> util_xalloc 729151.8/sec 778037.800/sec 16750.5969/sec
> total: 60302149
> vconn_received 0.0/sec 0.000/sec 0.0025/sec
> total: 9
> vconn_sent 0.0/sec 0.000/sec 0.0017/sec
> total: 6
> netdev_set_policing 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> netdev_get_ifindex 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> netdev_get_hwaddr 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> netdev_set_hwaddr 0.0/sec 0.000/sec 0.0003/sec
> total: 1
> netdev_get_ethtool 0.0/sec 0.000/sec 0.0008/sec
> total: 3
> netlink_received 0.2/sec 0.200/sec 0.0128/sec
> total: 46
> netlink_recv_jumbo 0.2/sec 0.200/sec 0.0067/sec
> total: 24
> netlink_sent 0.2/sec 0.200/sec 0.0097/sec
> total: 35
> nln_changed 0.0/sec 0.000/sec 0.0014/sec
> total: 5
> 54 events never hit
>
> But also, I think, that "packets from the same flow (or stream) should
> only be
> handled by one pmd thread" is a very bad behavior. I don't know exactly
> how dpdk works,
> but it shouldn't work so.
>
> Best regards, Ilya Maximets.
>
> On 08.08.2015 01:28, Alex Wang wrote:
> > Thx for reporting this, I worry this may not be an ovs issue, but to do
> with
> > the dpdk driver... Here is my analysis, please correct me if wrong,
> >
> > When pmd threads are spawned, each of them will take ownership of port
> > queues in a round robin fashion... In other words, each port queue will
> > be read by only one pmd thread.
> >
> > Then based on my understanding the dpdk module will hash packets to one
> > particular queue on the receiving port. Thusly packets from the same
> flow
> > (or stream) should only be handled by one pmd thread.
> >
> > Next, the issue you reported will only happen when the packets from same
> > flow (or stream) are handled by multiple pmd threads. [In that case,
> only one
> > pmd thread will have the flow installed in its classifier (and exact
> match
> > cache)... and all other pmd threads fail to install the flow due to
> duplicate
> > ufid]
> >
> > So, based on the analysis, this really should not happen, and if happens,
> > indicate a packet queueing issue in dpdk.
> >
> > Ilya, could you reproduce this issue and provide the `ovs-appctl
> > coverage/show` output? Want to confirm that the
> 'handler_duplicate_upcall'
> > counter is increasing when this issue happens.
> >
> > Daniele, could you also provide thoughts on this? Since I'm not sure if
> the
> > ovs dpdk code changes (since i last touched it long time ago) affect the
> issue
> > here.
> >
> >
> > Thanks,
> > Alex Wang,
> >
> >
> >
> >
> > On Fri, Aug 7, 2015 at 2:35 AM, Ilya Maximets <i.maxim...@samsung.com
> <mailto:i.maxim...@samsung.com>> wrote:
> >
> > In multiqueue mode several pmd threads may process one
> > port, but different queues. Flow doesn't depend on queue.
> >
> > So, while miss upcall processing, all threads (except first
> > for that port) will receive error = ENOSPC due to
> > ukey_install failure. Therefore they will not add the flow
> > to flow_table and will not insert it to exact match cache.
> >
> > As a result all threads (except first for that port) will
> > always execute a miss.
> >
> > Fix that by mixing pmd_id with the bits from the ufid
> > for ukey->hash calculation.
> >
> > Signed-off-by: Ilya Maximets <i.maxim...@samsung.com <mailto:
> i.maxim...@samsung.com>>
> > ---
> > ofproto/ofproto-dpif-upcall.c | 22 ++++++++++++----------
> > 1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif-upcall.c
> b/ofproto/ofproto-dpif-upcall.c
> > index 0f2e186..57a7f34 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -291,7 +291,8 @@ static bool ukey_install_start(struct udpif *,
> struct udpif_key *ukey);
> > static bool ukey_install_finish(struct udpif_key *ukey, int error);
> > static bool ukey_install(struct udpif *udpif, struct udpif_key
> *ukey);
> > static struct udpif_key *ukey_lookup(struct udpif *udpif,
> > - const ovs_u128 *ufid);
> > + const ovs_u128 *ufid,
> > + const unsigned pmd_id);
> > static int ukey_acquire(struct udpif *, const struct dpif_flow *,
> > struct udpif_key **result, int *error);
> > static void ukey_delete__(struct udpif_key *);
> > @@ -1143,7 +1144,8 @@ process_upcall(struct udpif *udpif, struct
> upcall *upcall,
> > }
> > if (actions_len == 0) {
> > /* Lookup actions in userspace cache. */
> > - struct udpif_key *ukey = ukey_lookup(udpif,
> upcall->ufid);
> > + struct udpif_key *ukey = ukey_lookup(udpif,
> upcall->ufid,
> > +
> upcall->pmd_id);
> > if (ukey) {
> > actions = ukey->actions->data;
> > actions_len = ukey->actions->size;
> > @@ -1320,19 +1322,19 @@ handle_upcalls(struct udpif *udpif, struct
> upcall *upcalls,
> > }
> >
> > static uint32_t
> > -get_ufid_hash(const ovs_u128 *ufid)
> > +get_ukey_hash(const ovs_u128 *ufid, const unsigned pmd_id)
> > {
> > - return ufid->u32[0];
> > + return ufid->u32[0] + pmd_id;
> > }
> >
> > static struct udpif_key *
> > -ukey_lookup(struct udpif *udpif, const ovs_u128 *ufid)
> > +ukey_lookup(struct udpif *udpif, const ovs_u128 *ufid, const
> unsigned pmd_id)
> > {
> > struct udpif_key *ukey;
> > - int idx = get_ufid_hash(ufid) % N_UMAPS;
> > + int idx = get_ukey_hash(ufid, pmd_id) % N_UMAPS;
> > struct cmap *cmap = &udpif->ukeys[idx].cmap;
> >
> > - CMAP_FOR_EACH_WITH_HASH (ukey, cmap_node, get_ufid_hash(ufid),
> cmap) {
> > + CMAP_FOR_EACH_WITH_HASH (ukey, cmap_node, get_ukey_hash(ufid,
> pmd_id), cmap) {
> > if (ovs_u128_equals(&ukey->ufid, ufid)) {
> > return ukey;
> > }
> > @@ -1362,7 +1364,7 @@ ukey_create__(const struct nlattr *key, size_t
> key_len,
> > ukey->ufid_present = ufid_present;
> > ukey->ufid = *ufid;
> > ukey->pmd_id = pmd_id;
> > - ukey->hash = get_ufid_hash(&ukey->ufid);
> > + ukey->hash = get_ukey_hash(&ukey->ufid, pmd_id);
> > ukey->actions = ofpbuf_clone(actions);
> >
> > ovs_mutex_init(&ukey->mutex);
> > @@ -1490,7 +1492,7 @@ ukey_install_start(struct udpif *udpif, struct
> udpif_key *new_ukey)
> > idx = new_ukey->hash % N_UMAPS;
> > umap = &udpif->ukeys[idx];
> > ovs_mutex_lock(&umap->mutex);
> > - old_ukey = ukey_lookup(udpif, &new_ukey->ufid);
> > + old_ukey = ukey_lookup(udpif, &new_ukey->ufid,
> new_ukey->pmd_id);
> > if (old_ukey) {
> > /* Uncommon case: A ukey is already installed with the same
> UFID. */
> > if (old_ukey->key_len == new_ukey->key_len
> > @@ -1572,7 +1574,7 @@ ukey_acquire(struct udpif *udpif, const struct
> dpif_flow *flow,
> > struct udpif_key *ukey;
> > int retval;
> >
> > - ukey = ukey_lookup(udpif, &flow->ufid);
> > + ukey = ukey_lookup(udpif, &flow->ufid, flow->pmd_id);
> > if (ukey) {
> > retval = ovs_mutex_trylock(&ukey->mutex);
> > } else {
> > --
> > 2.1.4
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org <mailto:dev@openvswitch.org>
> > http://openvswitch.org/mailman/listinfo/dev
> >
> >
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
