From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37275) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rxx0h-0002H7-7G for qemu-devel@nongnu.org; Thu, 16 Feb 2012 03:48:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rxx0b-0003ne-Fi for qemu-devel@nongnu.org; Thu, 16 Feb 2012 03:48:15 -0500 Received: from fmmailgate05.web.de ([217.72.192.243]:64190) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rxx0b-0003nO-4b for qemu-devel@nongnu.org; Thu, 16 Feb 2012 03:48:09 -0500 Received: from moweb002.kundenserver.de (moweb002.kundenserver.de [172.19.20.108]) by fmmailgate05.web.de (Postfix) with ESMTP id BA2CE69F9B0D for ; Thu, 16 Feb 2012 09:48:07 +0100 (CET) Message-ID: <4F3CC2C3.9020401@web.de> Date: Thu, 16 Feb 2012 09:48:03 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1329379634-1498-1-git-send-email-zwu.kernel@gmail.com> <4F3CC043.1010607@web.de> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig64E801AE7B64F511D3612630" Subject: Re: [Qemu-devel] [PATCH v2] slirp: fix packet requeue issue in batchq List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhi Yong Wu Cc: aliguori@us.ibm.com, Zhi Yong Wu , qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com, mst@redhat.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig64E801AE7B64F511D3612630 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2012-02-16 09:45, Zhi Yong Wu wrote: > On Thu, Feb 16, 2012 at 4:37 PM, Jan Kiszka wrote: >> On 2012-02-16 09:07, zwu.kernel@gmail.com wrote: >>> From: Zhi Yong Wu >>> >> >> Please summarize in a bit more details what was broken. > Should those bits be put in the message part of this part? or here? Here, as a commit log. >=20 >> >>> Signed-off-by: Zhi Yong Wu >>> --- >>> slirp/if.c | 19 +++++++++++++++++-- >>> slirp/mbuf.c | 3 +-- >>> 2 files changed, 18 insertions(+), 4 deletions(-) >>> >>> diff --git a/slirp/if.c b/slirp/if.c >>> index 8e0cac2..57350d5 100644 >>> --- a/slirp/if.c >>> +++ b/slirp/if.c >>> @@ -22,6 +22,7 @@ ifs_remque(struct mbuf *ifm) >>> { >>> ifm->ifs_prev->ifs_next =3D ifm->ifs_next; >>> ifm->ifs_next->ifs_prev =3D ifm->ifs_prev; >>> + ifs_init(ifm); >>> } >>> >>> void >>> @@ -154,7 +155,7 @@ if_start(Slirp *slirp) >>> { >>> uint64_t now =3D qemu_get_clock_ns(rt_clock); >>> int requeued =3D 0; >>> - struct mbuf *ifm, *ifqt; >>> + struct mbuf *ifm, *ifqt, *ifm_next; >>> >>> DEBUG_CALL("if_start"); >>> >>> @@ -162,6 +163,8 @@ if_start(Slirp *slirp) >>> return; /* Nothing to do */ >>> >>> again: >>> + ifm_next =3D NULL; >>> + >>> /* check if we can really output */ >>> if (!slirp_can_output(slirp->opaque)) >>> return; >>> @@ -190,6 +193,7 @@ if_start(Slirp *slirp) >>> /* If there are more packets for this session, re-queue them */= >>> if (ifm->ifs_next !=3D /* ifm->ifs_prev !=3D */ ifm) { >>> insque(ifm->ifs_next, ifqt); >>> + ifm_next =3D ifm->ifs_next; >>> ifs_remque(ifm); >>> } >>> >>> @@ -209,7 +213,18 @@ if_start(Slirp *slirp) >>> m_free(ifm); >>> } else { >>> /* re-queue */ >>> - insque(ifm, ifqt); >>> + if (ifm_next) { >>> + /*restore the original state of batchq*/ >>> + remque(ifm_next); >>> + insque(ifm, ifqt); >>> + ifm_next->ifs_prev->ifs_next =3D ifm; >>> + ifm->ifs_prev =3D ifm_next->ifs_prev; >>> + ifm->ifs_next =3D ifm_next; >>> + ifm_next->ifs_prev =3D ifm; >>> + } else { >>> + insque(ifm, ifqt); >>> + } >>> + >>> requeued++; >>> } >>> } >>> diff --git a/slirp/mbuf.c b/slirp/mbuf.c >>> index c699c75..f429c0a 100644 >>> --- a/slirp/mbuf.c >>> +++ b/slirp/mbuf.c >>> @@ -68,8 +68,7 @@ m_get(Slirp *slirp) >>> m->m_size =3D SLIRP_MSIZE - offsetof(struct mbuf, m_dat); >>> m->m_data =3D m->m_dat; >>> m->m_len =3D 0; >>> - m->m_nextpkt =3D NULL; >>> - m->m_prevpkt =3D NULL; >>> + ifs_init(m); >>> m->arp_requested =3D false; >>> m->expiration_date =3D (uint64_t)-1; >>> end_error: >> >> Wondering now: Is this hunk required or a cleanup? > The former. I think that the pointer of one raw mbuf which are used to > link the packets for the same session should default to itself, not > NULL. OK. Out of curiosity, is that an older issue or just related to the requeuing we now practice? Jan --------------enig64E801AE7B64F511D3612630 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk88wsMACgkQitSsb3rl5xS0CACeJDkmdPNTQPgiiF5fvrnRn3vi hTQAoIcgLHH8gAw1kjZSRPpqsABqDDGq =m6eU -----END PGP SIGNATURE----- --------------enig64E801AE7B64F511D3612630--