On Tue, 1 Mar 2022 10:47:32 +0100 David Marchand <david.march...@redhat.com> wrote:
> On Tue, Mar 1, 2022 at 10:02 AM Zhang, Yuying <yuying.zh...@intel.com> wrote: > > > -----Original Message----- > > > From: David Marchand <david.march...@redhat.com> > > > Sent: Tuesday, March 1, 2022 4:44 PM > > > To: Zhang, Yuying <yuying.zh...@intel.com> > > > Cc: dev <dev@dpdk.org>; Maxime Coquelin <maxime.coque...@redhat.com>; > > > Xia, Chenbo <chenbo....@intel.com>; dpdk stable <sta...@dpdk.org> > > > Subject: Re: [PATCH v1] net/vhost: clear data of packet mbuf after > > > sending pkts > > > > > > On Tue, Mar 1, 2022 at 8:29 AM Yuying Zhang <yuying.zh...@intel.com> > > > wrote: > > > > > > > > The PMD frees a packet mbuf back into its original mempool after > > > > sending a packet. However, old data is not cleaned up which causes > > > > error in payload of new packets. This patch clear data of packet mbuf > > > > before freeing mbuf. > > > > > > This patch looks wrong to me. > > > What is the actual issue you want to fix? > > > > eth_vhost_tx() frees the packet mbuf back into its original mempool every > > time after a packet sent without clearing the data field. > > Then packet transmit function will get bulk directly without reset. New > > generated packet contains old data of previous packet. This is wrong. > > With the proposed patch, if the mbuf refcnt is != 0, you are shooting > the data while some other part of the application might be needing it. > > Plus, there should be no expectation about a mbuf data content when > retrieving one from a mempool. > The only bytes that are guaranteed to be initialised by the mbuf API > are its metadata. > > > If there is an issue somewhere in dpdk where the mbuf data content is > expected to be 0 on allocation, please point at it. > Or share the full test that failed. > > Agree. There is no guarantee that mbuf you get was not just used by some other driver or library. Only the fields in rte_pktmbuf_reset are guaranteed to be set.
