> -----Original Message----- > From: Zhang, Yuying <[email protected]> > Sent: Wednesday, March 2, 2022 4:59 PM > To: David Marchand <[email protected]> > Cc: dev <[email protected]>; Maxime Coquelin <[email protected]>; Xia, > Chenbo <[email protected]>; dpdk stable <[email protected]>; > [email protected] > Subject: RE: [PATCH v1] net/vhost: clear data of packet mbuf after sending > pkts > > Hi Marchand, > > > -----Original Message----- > > From: David Marchand <[email protected]> > > Sent: Tuesday, March 1, 2022 5:48 PM > > To: Zhang, Yuying <[email protected]> > > Cc: dev <[email protected]>; Maxime Coquelin <[email protected]>; > > Xia, Chenbo <[email protected]>; dpdk stable <[email protected]> > > Subject: Re: [PATCH v1] net/vhost: clear data of packet mbuf after sending > pkts > > > > On Tue, Mar 1, 2022 at 10:02 AM Zhang, Yuying <[email protected]> > > wrote: > > ... > > > > > > > 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. > > According to the test_plan guide of dpdk > (https://doc.dpdk.org/dts/test_plans/loopback_virtio_user_server_mode_test_pla > n.html), > Test Case 13 (loopback packed ring all path payload check test using server > mode and multi-queues), the payload of each packet must be the same. > The packet of first stream is initialized value 0. Then this packet is put > back into mempool(actually, the local cache of the core). > The packet of rest stream is got from local_cache directly and contains the > first packet's header data in the payload. Therefore, the payload of the > packets > are different.
Could you explain more about the problem? But anyway I think this fix is wrong. After we're clear about the problem, there should be another solution. Thanks, Chenbo > > > > > > > -- > > David Marchand
