RE: [dpdk-dev] [PATCH] config/x86: add support for AMD platform

["Song, Keesang" <Keesang.Song@amd.com>]

[AMD Official Use Only]

I don't see any update since Thomas's.
Do we have this patch taken care of submitting to 22.03 and backporting it?

-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Wednesday, November 24, 2021 4:37 AM
To: techboard@dpdk.org
Cc: Bruce Richardson <bruce.richardson@intel.com>; Aman Kumar <aman.kumar@vvdntech.in>; David Marchand <david.marchand@redhat.com>; Song, Keesang <Keesang.Song@amd.com>; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] config/x86: add support for AMD platform

[CAUTION: External Email]

Ping techboard for comments

18/11/2021 15:05, Thomas Monjalon:
> 18/11/2021 14:52, Bruce Richardson:
> > On Thu, Nov 18, 2021 at 01:25:38PM +0100, Thomas Monjalon wrote:
> > > I request a techboard decision for this patch.
> > >
> > >
> > > 02/11/2021 20:04, Thomas Monjalon:
> > > > 02/11/2021 19:45, David Marchand:
> > > > > On Tue, Nov 2, 2021 at 3:53 PM Aman Kumar <aman.kumar@vvdntech.in> wrote:
> > > > > >
> > > > > > -Dcpu_instruction_set=znverX meson option can be used to
> > > > > > build dpdk for AMD platforms. Supported options are znver1,
> > > > > > znver2 and znver3.
> > > > > >
> > > > > > Signed-off-by: Aman Kumar <aman.kumar@vvdntech.in>
> > > > > > ---
> > > > > >  dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > > > > > dpdk_conf.set('RTE_MAX_LCORE', 128)
> > > > > > dpdk_conf.set('RTE_MAX_NUMA_NODES', 32)
> > > > > > +
> > > > > > +# AMD platform support
> > > > > > +if get_option('cpu_instruction_set') == 'znver1'
> > > > > > +    dpdk_conf.set('RTE_MAX_LCORE', 256) elif
> > > > > > +get_option('cpu_instruction_set') == 'znver2'
> > > > > > +    dpdk_conf.set('RTE_MAX_LCORE', 512) elif
> > > > > > +get_option('cpu_instruction_set') == 'znver3'
> > > > > > +    dpdk_conf.set('RTE_MAX_LCORE', 512) endif
> > > > >
> > > > > I already replied to a similar patch earlier in this release.
> > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2
> > > > > F%2Finbox.dpdk.org%2Fdev%2FCAJFAV8z-5amvEnr3mazkTqH-7SZX_C6EqC
> > > > > ua6UdMXXHgrcmT6g%40mail.gmail.com%2F&amp;data=04%7C01%7Ckeesan
> > > > > g.song%40amd.com%7C621ed6a39c304c905dbb08d9af4717c5%7C3dd8961f
> > > > > e4884e608e11a82d994e183d%7C0%7C0%7C637733542157359380%7CUnknow
> > > > > n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
> > > > > 1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=L9IM4ZtukVwY5qdbGgkt2zlu
> > > > > sGm0ZmrQmE9oMeg1RDo%3D&amp;reserved=0
> > > > >
> > > > > So repeating the same: do you actually _need_ more than 128
> > > > > lcores in a single DPDK application?
> > >
> > > We did not receive an answer to this question.
> > >
> > > > Yes I forgot this previous discussion concluding that we should
> > > > not increase more than 128 threads.
> > >
> > > We had a discussion yesterday in techboard meeting.
> > > The consensus is that we didn't hear for real need of more than
> > > 128 threads, except for configuration usability convenience.
> > >
> > > Now looking again at the code, this is how it is defined:
> > >
> > > option('max_lcores', type: 'string', value: 'default', description:
> > >        'Set maximum number of cores/threads supported by EAL;
> > >        "default" is different per-arch, "detect" detects the
> > > number of cores on the build machine.')
> > > config/x86/meson.build: dpdk_conf.set('RTE_MAX_LCORE', 128)
> > > config/ppc/meson.build: dpdk_conf.set('RTE_MAX_LCORE', 128)
> > > config/arm/meson.build: it goes from 4 to 1280!
> > >
> > > So I feel it is not fair to reject this AMD patch if we allow Arm to go beyond.
> > > Techboard, let's have a quick decision please for 21.11-rc4.
> > >
> > I would support increasing the default value for x86 in this release.
>
> This patch is not increasing the default for all x86, only for some
> CPUs as given at compilation time.
> I think it is the same logic as Arm CPU-specific compilation.
>
> > I believe Dave H. had some patches to decrease the memory footprint
> > overhead of such a change. I don't believe that they were merged,
> > and while it's a bit late for 21.11 now, those should be considered
> > for 22.03 release and then maybe for backport.