From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AA0EE425F0; Wed, 20 Sep 2023 15:05:44 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7658640ECF; Wed, 20 Sep 2023 15:05:44 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id C8D2240A79 for ; Wed, 20 Sep 2023 15:05:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695215143; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zgXSAq+JwUm4Xbg84X1deL0ZSdiP4GCL74z1O2/BjrY=; b=M/Papz62i8Fti84qtWvOgWX9ng9ZKwBYn9qDwUkdzRKzeSioFz9zyXv5JZhFR6bulZNqvK O6r0fFWOb7kfnm6QgawJknypB49J4XKMETxjwC6zWYy6QnABUNtkbMJTZJZkLfppYSvIof 5CTAuH0WBxMg1kQERd+oK3Ye2xKXn88= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-61-xigv2NFtPsi60ZNTfEe17Q-1; Wed, 20 Sep 2023 09:05:42 -0400 X-MC-Unique: xigv2NFtPsi60ZNTfEe17Q-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 58A38280D207; Wed, 20 Sep 2023 13:05:40 +0000 (UTC) Received: from [10.39.208.35] (unknown [10.39.208.35]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 87CCC1686D; Wed, 20 Sep 2023 13:05:38 +0000 (UTC) Message-ID: Date: Wed, 20 Sep 2023 15:05:37 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: Commit broke 32-bit testpmd app From: Maxime Coquelin To: "Roger Melton (rmelton)" , "Dave Johnson (davejo)" , "dev@dpdk.org" , "Sampath Peechu (speechu)" , chenbo.xia@outlook.com Cc: "Malcolm Bumgardner (mbumgard)" , "Chris Brezovec (cbrezove)" , David Marchand References: <63486764-3b44-3299-6830-05435dfd78f3@redhat.com> <827f912f-fc2d-6d41-ba8c-e7f3f9f2e24b@redhat.com> In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 9/20/23 09:35, Maxime Coquelin wrote: > Hi, > > I tried to reproduce without success(see attached log). > > I fail to reproduce because buf_iova fits into 32 bits in my case: > (gdb) p /x *tx_pkts[0] > > $4 = { >   cacheline0 = 0x77b19ec0, >   buf_addr = 0x77b19f40, >   buf_iova = 0x49519f40, >   rearm_data = 0x77b19ed0, > > However, looking at your report, something like this would work for you: > > diff --git a/drivers/net/virtio/virtqueue.h > b/drivers/net/virtio/virtqueue.h > index 9d4aba11a3..38efbc517a 100644 > --- a/drivers/net/virtio/virtqueue.h > +++ b/drivers/net/virtio/virtqueue.h > @@ -124,7 +124,7 @@ virtqueue_store_flags_packed(struct > vring_packed_desc *dp, >   * (virtio-pci and virtio-user). >   */ >  #define VIRTIO_MBUF_ADDR(mb, vq) \ > -       ((uint64_t)(*(uintptr_t *)((uintptr_t)(mb) + > (vq)->mbuf_addr_offset))) > +       (*(uint64_t *)((uintptr_t)(mb) + (vq)->mbuf_addr_offset)) > > > The problem is that it would likely break Virtio-user en 32bits mode, as > this is how it was initially implemented, and got fixed few years ago, > as David hinted to me: > > commit 260aae9ad9621e3e758f1443abb8fcbc25ece07c > Author: Jianfeng Tan > Date:   Wed Apr 19 02:30:33 2017 +0000 > >     net/virtio-user: fix address on 32-bit system > >     virtio-user cannot work on 32-bit system as higher 32-bit of the >     addr field (64-bit) in the desc is filled with non-zero value >     which should not happen for a 32-bit system. > >     In case of virtio-user, we use buf_addr of mbuf to fill the >     virtqueue desc addr. This is a regression bug. For 32-bit system, >     the first 4 bytes of mbuf is buf_addr, with following 8 bytes for >     buf_phyaddr. With below wrong definition, both buf_addr and lower >     4 bytes buf_phyaddr are obtained to fill the virtqueue desc. >       #define VIRTIO_MBUF_ADDR(mb, vq) \ >             (*(uint64_t *)((uintptr_t)(mb) + (vq)->offset)) > >     Fixes: 25f80d108780 ("net/virtio: fix packet corruption") >     Cc: stable@dpdk.org > >     Signed-off-by: Jianfeng Tan >     Acked-by: Yuanhan Liu > > If my understanding is correct, on 32 bits, when mbuf->buf_addr is used > (Virtio-user), we need to mask out the higher 4 bytes, while when using > Virtio-pci we need the full 64 bits (as the physical addresses used as > IOVA on the guest are 64 bits). I posted a fix aiming at making it work for both Virtio-user and Virtio- PCI 32 bits builds while not impacting 64 bits performance. Could you please have a try and report feedback by replying to the patch? Regards, Maxime > Regards, > Maxime > > On 9/13/23 15:24, Roger Melton (rmelton) wrote: >> +Chris Brezovec >> >> Hi Maxime, >> >> Chris from our team is attending the DPDK Summit in Dublin this week. >> If you have some time available, we'd appreciate it if he could meet >> with you to discuss the 32bit virtio issue we are seeing. >> >> Regards, >> Roger Melton >> >> On 9/6/23 2:57 PM, Dave Johnson (davejo) wrote: >>> >>> Hi Maxime, >>> >>> This email is regarding the following commit: >>> >>> https://github.com/DPDK/dpdk/commit/ba55c94a7ebc386d2288d6578ed57aad6cb92657 >>> >>> A query had been sent previously on this topic (see below) indicating >>> this commit appears to have broken the 32-bit testpmd app and >>> impacted one of our products that runs as a 32-bit DPDK application. >>> We consequently backed the commit out of our product but would prefer >>> to get a fix for it.  In the earlier exchange, you had asked if we >>> were using virtio-pci or virtio-user (we are using virtio-pci) and >>> asked for logs which Sampath provided.  It’s been a while, so let me >>> now if you need me to send resend those logs or need any other >>> information. >>> >>> FWIW, I reproduced this using testpmd and noticed that this part of >>> the change seems to be the interesting part (in >>> drivers/net/virtio/virtqueue.h): >>> >>> /** >>> >>> * Return the IOVA (or virtual address in case of virtio-user) of mbuf >>> >>> * data buffer. >>> >>> * >>> >>> * The address is firstly casted to the word size (sizeof(uintptr_t)) >>> >>> * before casting it to uint64_t. This is to make it work with different >>> >>> * combination of word size (64 bit and 32 bit) and virtio device >>> >>> * (virtio-pci and virtio-user). >>> >>> */ >>> >>> #define VIRTIO_MBUF_ADDR(mb, vq) \ >>> >>>       ((uint64_t)(*(uintptr_t *)((uintptr_t)(mb) + >>> (vq)->mbuf_addr_offset)) >>> >>> If I revert just this part of the changeset (by re-using the >>> VIRTIO_MBUF_ADDR to return buf_iova which matches what it had used >>> previously), then 32-bit testpmd is able to receive traffic again: >>> >>> #define VIRTIO_MBUF_ADDR(mb, vq) (mb->buf_iova) >>> >>> Looking at the address produced by each of these, I see the address >>> is the same except that the casting results in the upper bits getting >>> cleared: >>> >>> Address from patch (nonworking case) = 0x58e7c900 >>> >>> Address using buf_iova (working case) = 0x158e7c900 >>> >>> :: >>> >>> Address from patch (nonworking case) = 0x58e7bfc0 >>> >>> Address using buf_iova (working case) = 0x158e7bfc0 >>> >>> :: >>> >>> Address from patch (nonworking case) = 0x58e7b680 >>> >>> Address using buf_iova (working case) = 0x158e7b680 >>> >>> :: >>> >>> Regards, Dave >>> >>> *From: *Sampath Peechu (speechu) >>> *Date: *Monday, January 30, 2023 at 3:29 PM >>> *To: *Maxime Coquelin , >>> chenbo.xia@intel.com , dev@dpdk.org >>> *Cc: *Roger Melton (rmelton) , Malcolm Bumgardner >>> (mbumgard) >>> *Subject: *Re: Commit broke 32-bit testpmd app >>> >>> Hi Maxime, >>> >>> Could you please let us know if you got a chance to look at the >>> debugs logs I provided? >>> >>> Thanks, >>> >>> Sampath >>> >>> *From: *Sampath Peechu (speechu) >>> *Date: *Tuesday, December 6, 2022 at 1:08 PM >>> *To: *Maxime Coquelin , >>> chenbo.xia@intel.com , dev@dpdk.org >>> *Cc: *Roger Melton (rmelton) >>> *Subject: *Re: Commit broke 32-bit testpmd app >>> >>> Hi Maxime, >>> >>> Did you get a chance to look into this? >>> >>> Please let me know if you need anything else. >>> >>> Thanks, >>> >>> Sampath >>> >>> *From: *Sampath Peechu (speechu) >>> *Date: *Wednesday, November 23, 2022 at 5:15 PM >>> *To: *Maxime Coquelin , >>> chenbo.xia@intel.com , dev@dpdk.org >>> *Cc: *Roger Melton (rmelton) >>> *Subject: *Re: Commit broke 32-bit testpmd app >>> >>> Hi Maxime, >>> >>> I’m attaching the following for reference. >>> >>>   * Instructions for Centos8 test setup >>>   * Diffs between the working and non-working versions (working >>>     version has the problem commit backed out) >>>   * Working logs (stats show that ping packets from neighbor VM can be >>>     seen with both 64-bit and 32-bit apps) >>>   * Non-working logs (stats show that ping packets from neighbor VM >>>     are seen with 64-bit app but NOT seen with 32-bit app) >>> >>> ============================ >>> >>> $ sudo ./usertools/dpdk-devbind.py --status >>> >>> Network devices using DPDK-compatible driver >>> >>> ============================================ >>> >>> 0000:07:00.0 'Virtio network device 1041' drv=igb_uio unused= >>> >>> 0000:08:00.0 'Virtio network device 1041' drv=igb_uio unused= >>> >>> Network devices using kernel driver >>> >>> =================================== >>> >>> 0000:01:00.0 'Virtio network device 1041' if=enp1s0 drv=virtio-pci >>> unused=igb_uio *Active* >>> >>> … >>> >>> =========================== >>> >>> Thanks, >>> >>> Sampath >>> >>> *From: *Maxime Coquelin >>> *Date: *Tuesday, November 22, 2022 at 4:24 AM >>> *To: *Sampath Peechu (speechu) , >>> chenbo.xia@intel.com , dev@dpdk.org >>> *Cc: *Roger Melton (rmelton) >>> *Subject: *Re: Commit broke 32-bit testpmd app >>> >>> Hi, >>> >>> In my initial reply (see below), I also asked if you had logs to share. >>> And wondered whether it happens with Virtio PCI or Virtio-user? >>> >>> Regards, >>> Maxime >>> >>> On 11/16/22 00:30, Sampath Peechu (speechu) wrote: >>> > ++ dev@dpdk.org > >>> > >>> > *From: *Maxime Coquelin >>> > *Date: *Tuesday, November 15, 2022 at 3:19 AM >>> > *To: *Sampath Peechu (speechu) , >>> chenbo.xia@intel.com >>> > >>> > *Cc: *Roger Melton (rmelton) >>> > *Subject: *Re: Commit broke 32-bit testpmd app >>> > >>> > Hi Sampath, >>> > >>> > >>> > Please add dev@dpdk.org, the upstream mailing list, if this is related >>> > to the upstream DPDK project.If it is using RHEL DPDK package, please >>> > use the appropriate support channels. >>> > >>> > On 11/14/22 23:55, Sampath Peechu (speechu) wrote: >>> >  > Hi Virtio Maintainers team, >>> >  > >>> >  > This email is regarding the following commit. >>> >  > >>> >  > >>> > >>> https://github.com/DPDK/dpdk/commit/ba55c94a7ebc386d2288d6578ed57aad6cb92657 > >>> >  > >>> >  > The above commit appears to have broken the 32-bit testpmd app (and >>> >  > consequently impacted one of our products that runs as a 32-bit >>> DPDK >>> >  > app). The 64-bit testpmd app does not appear to be impacted though. >>> > >>> > We'll need some logs to understand what is going on. >>> > Does it happen with virtio-pci or virtio-user? >>> > >>> > Regards, >>> > Maxime >>> > >>> >  > With the commit in place, we didn’t see any packets going >>> through at >>> >  > all. After backing out the commit and rebuilding the 32-bit >>> testpmd app >>> >  > in our test setup, we were able to pass traffic as expected. >>> >  > >>> >  > Could you please let us know if this is a known issue? And if >>> there is a >>> >  > fix available for it? >>> >  > >>> >  > Thank you, >>> >  > >>> >  > Sampath Peechu >>> >  > >>> >  > Cisco Systems >>> >  > >>> > >>> >>