<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;font-style:normal;font-weight:normal;text-decoration:none;" align="Left">
[AMD Official Use Only - General]<br>
</p>
<br>
<div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<span style="font-family: Calibri, sans-serif; font-size: 11pt;"><b>From:</b> Bruce Richardson <bruce.richardson@intel.com><br>
<b>Sent:</b> 06 December 2023 21:20<br>
<b>To:</b> Varghese, Vipin <Vipin.Varghese@amd.com><br>
<b>Cc:</b> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org <dev@dpdk.org>; Yigit, Ferruh <Ferruh.Yigit@amd.com>; Parikh, Neerav <Neerav.Parikh@amd.com><br>
<b>Subject:</b> Re: [PATCH] cfgfile: increase value length</span></div>
<div id="divRplyFwdMsg" dir="ltr" class="elementToProof">
<div> </div>
</div>
<div><span style="font-size: 11pt;">Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.<br>
<br>
<br>
On Wed, Dec 06, 2023 at 03:22:41PM +0000, Varghese, Vipin wrote:<br>
> [AMD Official Use Only - General]<br>
><br>
> Thanks Bruce & Cristian for the comments.<br>
> An increase seems ok to me, but is an 8x increase really necessary? If<br>
> lines in the config files are over 1k in size, then it implies that<br>
> some<br>
> other mechanism would surely be better for configuration.<br>
> Can we make do with an increase to 512 only?<br>
> VV> We encountered this issue<br>
> [1]https://bugs.dpdk.org/show_bug.cgi?id=1333 trying to use multiple<br>
> queue with DSA. But I hear you 256 to 2048 is big jump.<br>
> Happy to compromise with 1K.<br>
> VV> Sure let me update this is v2.<br>
> Since there is a ABI breakage,<br>
> [2]https://patchwork.dpdk.org/project/dpdk/patch/20231206112952.1588-1-<br>
> vipin.varghese@amd.com/ I will re work and share v2.<br>
<br>
Looking at the bugzilla, I still think even an increase to 1k is too much,<br>
and that we should look to adjust the config file format in some way to<br>
accomodate these longer line settings.</span></div>
<div class="elementToProof"><span style="font-size: 11pt;"><br>
</span></div>
<div class="elementToProof"><span style="font-size: 11pt;">Based on my testing 32 devices with the current format 2048B are appropriate.</span></div>
<div><span style="font-size: 11pt;">That 4 DSA devices with 8 queues each.<br>
<br>
Taking the specific example in the bug:<br>
* each entry in the line is wasting 5 characters with the string "lcore"<br>
* each PCI ID is starting with 0000, so we can do like EAL does and assume<br>
PCI domain is "0000" if just the BDF is given. This may involve changes<br>
to the PCI driver in question, since i assume these queues are names<br>
rather than addresses, but it would make them easier to address.<br>
<br>
If we did these two items, then:<br>
lcore_dma=lcore10@0000:00:04.2-q0,lcore12@0000:00:04.2-q1,lcore13@0000:00:04.2-q2,lcore14@0000:00:04.2-q3,lcore15@0000:00:04.2-q4,lcore16@0000:00:04.2-q5,lcore17@0000:00:04.2-q6,lcore18@0000:00:04.2-q7,lcore19@0000:00:04.4-q0,lcore20@0000:00:04.4-q1,lcore21@0000:00:04.4-q2,lcore22@0000:00:04.4-q3,lcore23@0000:00:04.4-q4,lcore24@0000:00:04.4-q5,lcore25@0000:00:04.4-q6,lcore26@0000:00:04.4-q7<br>
<br>
becomes:<br>
lcore_dma=10@00:04.2-q0,12@00:04.2-q1,13@00:04.2-q2,14@00:04.2-q3,15@00:04.2-q4,16@00:04.2-q5,17@00:04.2-q6,18@00:04.2-q7,19@00:04.4-q0,20@00:04.4-q1,21@00:04.4-q2,22@00:04.4-q3,23@00:04.4-q4,24@00:04.4-q5,25@00:04.4-q6,26@00:04.4-q7<br>
<br>
a reduction from 394 chars to 234 (10 chars per device).<br>
<br>
</span></div>
<div class="elementToProof"><span style="font-size: 11pt;">I see what you are suggesting, even though DPDK documentation states
<a href="https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html" id="OWAde744d10-78b2-940d-61e1-1b0256289175" class="OWAAutoLink">
9. EAL parameters — Data Plane Development Kit 23.11.0 documentation (dpdk.org)</a> `</span><span style="letter-spacing: normal; font-family: SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", Courier, monospace; font-size: 12px; font-weight: 400; color: rgb(231, 76, 60); background-color: rgb(255, 255, 255);"><[domain:]bus:devid.func></span><span style="font-size: 11pt;">`,
omit the domain to `</span><span style="letter-spacing: normal; font-family: SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", Courier, monospace; font-size: 12px; font-weight: 400; color: rgb(231, 76, 60); background-color: rgb(255, 255, 255);">bus:devid.func></span><span style="font-size: 11pt;">`.
That will reduce the bytes I agree.</span></div>
<div><span style="font-size: 11pt;"><br>
However, a better alternative may be to split up the lcore_dma item<br>
entirely, so that we always have one element per line:<br>
<br>
lcore_dma0=lcore10@0000:04.2-q0<br>
lcore_dma1=lcore12@0000:04.2-q1<br>
....<br>
lcore_dma15=lcore26@0000:04.4-q7<br>
<br>
What is especially good about using this is a solution is that it can be fully<br>
backward compatible and has no ABI issues. If an "lcore_dma=" line exists,<br>
we can split it, otherwise look for lcore_dma0, lcore_dma1 etc.</span></div>
<div><span style="font-size: 11pt;"><br>
</span></div>
<div><span style="font-size: 11pt;">Ok, but this means the test-dma-perf will need parsing changes and cfgfile library.<br>
<br>
As I stated above, the need for really long lines in a config file is a<br>
symptom of a more serious config problem, rather than an issue with the<br>
cfgfile lib itself.</span></div>
<div class="elementToProof"><span style="font-size: 11pt;"><br>
</span></div>
<div class="elementToProof"><span style="font-size: 11pt;">I do not fully agree to this statement, here are my 2 points</span></div>
<div class="elementToProof"><span style="font-size: 11pt;"><br>
</span></div>
<ol start="1" data-editing-info="{"orderedStyleType":1,"unorderedStyleType":1}" data-listchain="__List_Chain_158" style="margin-block: 0px; list-style-type: decimal;">
<li style="font-size: 11pt;">
<div class="elementToProof"><span style="font-size: 11pt;">current code is `#ifndef CFG_VALUE_LEN` which means, the user can change the value using compiler flag `-DCFG_VALUE_LEN=[user desired value]` as cflags.</span></div>
</li><li style="font-size: 11pt;">
<div class="elementToProof"><span style="font-size: 11pt;">If not declared as CFLAGS configuration value is assumed to be hardened to take only 256</span></div>
</li></ol>
<div class="elementToProof" style="font-size: 11pt;"><br>
</div>
<div class="elementToProof"><span style="font-size: 11pt;">Hence the reasoning would be `it was not assumed there would be change from 256B was not expected or explored. If 256B is the hardlimit then we should not have used
</span><span style="letter-spacing: normal; font-family: "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; font-weight: 400; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">#ifndef</span><span style="font-size: 11pt;">`.</span></div>
<div class="elementToProof"><span style="font-size: 11pt;"><br>
</span></div>
<div class="elementToProof"><span style="font-size: 11pt;">But +1 for changing the parsing logic.</span></div>
<div><span style="font-size: 11pt;"><br>
<br>
/Bruce</span></div>
</div>
</body>
</html>