Hi Ting, > -----Original Message----- > From: Xu, Ting <ting...@intel.com> > Sent: Tuesday, May 12, 2020 3:17 AM > To: Yigit, Ferruh <ferruh.yi...@intel.com>; dev@dpdk.org > Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Xing, Beilei > <beilei.x...@intel.com>; Iremonger, Bernard > <bernard.iremon...@intel.com>; sta...@dpdk.org; Andrew Rybchenko > <arybche...@solarflare.com>; Thomas Monjalon <tho...@monjalon.net> > Subject: RE: [dpdk-dev] [PATCH v1] app/testpmd: fix DCB set failure in > FreeBSD by clang > > > > > -----Original Message----- > > From: Yigit, Ferruh <ferruh.yi...@intel.com> > > Sent: Tuesday, May 12, 2020 12:29 AM > > To: Xu, Ting <ting...@intel.com>; dev@dpdk.org > > Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Xing, Beilei > > <beilei.x...@intel.com>; Iremonger, Bernard > > <bernard.iremon...@intel.com>; sta...@dpdk.org; Andrew Rybchenko > > <arybche...@solarflare.com>; Thomas Monjalon > <tho...@monjalon.net> > > Subject: Re: [dpdk-dev] [PATCH v1] app/testpmd: fix DCB set failure in > > FreeBSD by clang > > > > On 5/11/2020 11:25 AM, Ting Xu wrote: > > > When set DCB in testpmd by clang, there is a segmentation fault. > > > It is because the local variable rss_conf in get_eth_dcb_conf() is > > > not cleared, so that the pointer member variable rss_key has a > > > random address, which leads to an error in the following processing. > > > This patch initialized the local variable rss_conf to avoid random > > > address. > > > > This is nothing really FreeBSD or clang issue, although it may be > > reproduced that environment, this is a pointer with random value > > issue. We may drop FreeBSD and clang reference to not create confusion. > > > > OK, I will modify the commit log. > > > > > > > Fixes: b57b66a97ebf ("app/testpmd: support mbuf dynamic flag") > > > > This commit looks unrelated, if not can you please explain why above > > commit causing the issue? > > > > This is the bad commit the validation team find for this issue. Honestly > speaking, I did not find the relation between this commit and the issue > either. > > > > Cc: sta...@dpdk.org > > > > > > Signed-off-by: Ting Xu <ting...@intel.com> > > > --- > > > app/test-pmd/testpmd.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index > > > 99bacddbf..1276476ca 100644 > > > --- a/app/test-pmd/testpmd.c > > > +++ b/app/test-pmd/testpmd.c > > > @@ -3408,6 +3408,7 @@ get_eth_dcb_conf(portid_t pid, struct > > rte_eth_conf *eth_conf, > > > int32_t rc; > > > struct rte_eth_rss_conf rss_conf; > > > > > > +memset(&rss_conf, 0, sizeof(struct rte_eth_rss_conf)); > > > > The variable is used in the 'else' leg, memset can be moved there, but > > more importantly should this be done in the > 'rte_eth_dev_rss_hash_conf_get()' API. > > > > @Andrew, @Thomas, > > > > What do you think 'rte_eth_dev_rss_hash_conf_get()' memset the > 'rss_conf' > > param before passing it to the PMD? To prevent issues like above in > > user application.
Seems like a good idea to me, to fix it in rte_eth_dev_rss_hash_conf_get() instead of in testpmd. > > I will move it to the else leg first in the v2 patch. Wait for more comments. > Thanks! Regards, Bernard.