issue: 4633470 fix generate_docs.py +update README #440

tomerdbz · 2025-09-16T08:07:15Z

Description

Fix bug in generate_docs.py where non-dict properties caused processing failures. Add proper type checking and error handling to prevent crashes when encountering unexpected property types in the JSON schema.

Changes in generate_docs.py:

Add type check to skip non-dict properties
Wrap property processing in try-catch block
Add error reporting with property path context

Regenerate README documentation with the fixed script:

Document memory size suffix support (B, KB, MB, GB) for memory params
Simplify worker threads documentation for better clarity
Fix inconsistent default value formatting
Tidying spaces, indents, standardizing the mapping to legacy in the description.

This ensures robust documentation generation and provides users with complete information about memory size formatting and threading options.

What

fix generate_docs.py and update README documentation

Why ?

Fixes 4633470. enables #395

How ?

It is optional but for complex PRs please provide information about the design,
architecture, approach, etc.

Change type

What kind of change does this PR introduce?

Check list

Code follows the style de facto guidelines of this project
Comments have been inserted in hard to understand places
Documentation has been updated (if necessary)
Test has been added (if possible)

Copilot

Pull Request Overview

Fixes a bug in the documentation generation script where non-dict properties caused processing failures, and updates the README with improved documentation for memory parameters and threading options.

Key changes:

Add type checking and error handling to prevent crashes in generate_docs.py
Update README with memory size suffix documentation and threading behavior options
Improve documentation clarity for worker threads configuration

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
generate_docs.py	Adds type checking for properties and error handling to prevent crashes when processing non-dict schema properties
README	Updates memory parameter documentation to include size suffixes and clarifies threading behavior options

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

generate_docs.py

README

tomerdbz · 2025-09-16T11:49:41Z

bot:retest

generate_docs.py

src/core/config/descriptor_providers/xlio_config_schema.json

BasharRadya · 2025-09-17T07:57:15Z

lets add newline in every description (to have consistent readme)

BasharRadya · 2025-09-18T12:54:47Z

Can we add new line for the "Maps to .... Environment variable" line? would be much more readable

README

tomerdbz · 2025-09-18T16:58:41Z

following your comment
"Can we add new line for the "Maps to .... Environment variable" line? would be much more readable"
I have decided to add word wrapping logic - made it the most robust

galnoam · 2025-09-18T17:03:00Z

bot:retest

galnoam · 2025-09-18T17:57:48Z

@dpressle , can you check why CI do not run?

tomerdbz · 2025-09-24T17:32:50Z

bot:retest

galnoam · 2025-09-25T09:02:37Z

@BasharRadya , please check if approved

BasharRadya · 2025-09-25T10:39:22Z

README

+Enable/Disable Striding Receive Queues.
+Maps to **XLIO_STRQ** environment variable.
+Each WQE in a Striding RQ may receive several packets.
+Thus, the WQE buffer size is controlled by XLIO_STRQ_NUM_STRIDES x


You start a new line after every "." punctuation, are you sure that this is the right thing to do?

your example, "Thus" should be in the same paragraph/line as the previous sentence because it's a logical continuation.

undid the change - not related to the bug

BasharRadya · 2025-09-25T10:40:19Z

README

 auto
-    Depends on ethtool setting and adapter ability.
-    See ethtool -k <eth0> | grep large-receive-offload
+Depends on ethtool setting and adapter ability.


it is more readable with the TAP before the sentence, WDYT?

undid the change - not related to the bug

BasharRadya · 2025-09-25T10:44:05Z

README

-Default value is 16000
+Number of Work Request Elements allocated in all RQs.
+Maps to **XLIO_RX_WRE** environment variable.
+Default value is 32 KB


note that this might be different value if its striding RQ, look at the OLD XLIO_RX_WRE

"XLIO_RX_WRE
Number of Work Request Elements allocated in all RQs.
Default value is 128 for XLIO_STRQ=1 (default) or 32768 for XLIO_STRQ=0."

undid the change - not related to the bug

BasharRadya

t

BasharRadya · 2025-09-25T10:47:24Z

README

+XLIO_STRQ_STRIDE_SIZE_BYTES.
+Values: on, off
 Default: on (Enabled)
 Default value is 1 (Enabled)


Why write default twice?

undid the change - not related to the bug

BasharRadya · 2025-09-25T10:47:29Z

README

+Maps to **XLIO_STRQ_STRIDE_SIZE_BYTES** environment variable.
+Must be power of two and in range [64 - 8192].
 Default: 64
 Default value is 64


Why write default twice?

undid the change - not related to the bug

BasharRadya · 2025-09-25T10:48:22Z

README

+Maps to **XLIO_SERVICE_NOTIFY_DIR** environment variable.
+Default value is /tmp/xlio/
 Note: when used xliod must be run with --notify-dir directing the same folder.
 Default value is /tmp/xlio


why default twice?

undid the change - not related to the bug

BasharRadya · 2025-09-28T10:58:52Z

src/core/config/descriptor_providers/xlio_config_schema.json

@@ -1445,27 +1445,27 @@
                            "type": "string",
                            "default": "",
                            "title": "Statistics file path",
-                            "description": "Redirect socket statistics to a specific user defined file. Maps to XLIO_STATS_FILE environment variable. Library will dump each socket's statistics into a file when closing the socket.\nExample: XLIO_STATS_FILE=/tmp/stats"


please keep the example "Example: XLIO_STATS_FILE=/tmp/stats"

BasharRadya · 2025-09-28T11:12:37Z

src/core/config/descriptor_providers/xlio_config_schema.json

                                        },
                                        "byte_threshold": {
                                            "type": "integer",
                                            "default": 0,
                                            "minimum": 0,
                                            "title": "Data threshold for flush",
-                                            "description": "Triggers TCP nodelay only if unsent data is larger than this value. The value is in bytes. Default 0 means no threshold - immediate sending. Maps to XLIO_TCP_NODELAY_TRESHOLD environment variable."
+                                            "description": "Triggers TCP nodelay only if unsent data is larger than this value.\nThe value is in bytes.\n0 means no threshold - immediate sending.\nMaps to XLIO_TCP_NODELAY_TRESHOLD environment variable."


effective only if nodelay=true

BasharRadya · 2025-09-28T11:25:30Z

README

-Default value is 1 KB
+Maximum size of the Data Encryption Key cache for TLS offload operations.
+Maps to **XLIO_HIGH_WMARK_DEK_CACHE_SIZE** environment variable.
+Default value is 1KB


Search KB or smethn else maybe need to add
"Supports suffixes: B, KB, MB, GB." or other supported suffixes

BasharRadya · 2025-09-28T11:39:09Z

README

+When enabled (value > 0), even if the socket receive ready packet queue is
+not empty it will still check the CQ for ready completions for processing.
+This CQ polling rate is controlled in nano-second resolution to prevent CPU consumption because of over CQ polling.
+This will enable a more 'real time' monitoring of the sockets ready packet queue.


"When disabled (0) " & "Disable with 0." is kinda duplication

BasharRadya · 2025-09-28T11:41:25Z

README

+Monitor of the applications socket usage of current,
+max and dropped bytes and packet counters can be done with xlio_stats.
+Maps to **XLIO_RX_BYTES_MIN** environment variable.
+Default value is 64KB


Lets check if this param and other params that uses KB really supports this type of suffixes

BasharRadya · 2025-09-28T11:43:22Z

README

+latency is improved dramatically.
+This comes on account of CPU utilization.
+Value range is -1, 0 to 100,000,000.
+Where value of -1 is used for infinite polling and 0 is used for no polling (interrupt driven).


i prefer this syntax
"Where value of -1 is used for infinite polling
Where value of 0 is used for no polling (interrupt driven)"

BasharRadya · 2025-09-28T11:58:41Z

README

 Default value is 64

 performance.rings.tx.max_inline_size
-Maximum data size sent inline. Setting to 0 disables inlining. Maps to **XLIO_TX_MAX_INLINE** environment variable. Max send inline data set for QP. Data copied into the INLINE space is at least 32 bytes of headers and the rest can be user datagram payload. XLIO_TX_MAX_INLINE=0 disables INLINEing on the Tx transmit path. In older releases this parameter was called: XLIO_MAX_INLINE.


Lets remove this line "Maximum data size sent inline. Setting to 0 disables inlining."
its redundant

BasharRadya · 2025-09-28T12:00:32Z

README


 performance.rings.tx.max_on_device_memory
-Maximum On Device Memory buffer size for each TX ring. 0 means unlimited. Maps to **XLIO_RING_DEV_MEM_TX** environment variable.
+Maximum On Device Memory buffer size for each TX ring.


Lets stick with the original?

XLIO can use the On Device Memory to store the egress packet if it does not fit into
the BF inline buffer. This improves application egress latency by reducing PCI transactions.
Using XLIO_RING_DEV_MEM_TX, the user can set the amount of On Device Memory buffer allocated
for each TX ring.
The total size of the On Device Memory is limited to 256k for a single port HCA and to
128k for dual port HCA.
Default value is 0

BasharRadya · 2025-09-28T12:02:16Z

README

+Number of Work Request Elements allocated in all transmit QPs.
+Maps to **XLIO_TX_WRE** environment variable.
+The number of QPs can change according to the number of network offloaded interfaces.
+Default value is 32KB


check if we can take KB here

BasharRadya · 2025-09-28T12:19:49Z

Lets convert the legacy config params usage in the examples to the new config params (e.g. Interrupt Moderation

The basic idea behind interrupt moderation is that the HW will not generate
interrupt for each packet, but instead only after some amount of packets received
or after the packet was held for some time.

The adaptive interrupt moderation change this packet count and time period
automatically to reach a desired rate of interrupts.

Use XLIO_RX_POLL=0 and XLIO_SELECT_POLL=0 to work in interrupt driven mode.
Control the period and frame count parameters with:
XLIO_CQ_MODERATION_COUNT - hold #count frames before interrupt
XLIO_CQ_MODERATION_PERIOD_USEC - hold #usec before interrupt
Control the adaptive algorithm with the following:)

tomerdbz · 2025-09-28T12:20:54Z

TODO remaining
Maps to - move to begin or end
change other README refs

tomerdbz · 2025-09-28T17:55:07Z

@BasharRadya - ready. please take a look. Thanks <3

BasharRadya · 2025-09-29T08:17:36Z

README

-Default value is 256 KB
+Maps to **XLIO_MAX_TSO_SIZE** environment variable.
+Maximum size in bytes of a TCP segment that can be transmitted with TSO.
+Default value is 262144


it would be much more convenient it to add KB format support

currently we either support B, KB, MB and GB or nothing - for this relatively small value I'd rather not make code change

BasharRadya · 2025-09-29T08:26:08Z

src/core/config/descriptor_providers/xlio_config_schema.json

                                },
                                "spare_buffers": {
                                    "type": "integer",
                                    "default": 32768,
                                    "title": "Spare RX buffers",
-                                    "description": "Number of spare receive buffer a ring holds to allow for filling up QP while full receive buffers are being processed inside XLIO. Maps to XLIO_QP_COMPENSATION_LEVEL environment variable.\nDefault value is XLIO_RX_WRE / 2"
+                                    "description": "Maps to XLIO_QP_COMPENSATION_LEVEL environment variable.\nNumber of spare receive buffer a ring holds to allow for filling up QP while\nfull receive buffers are being processed inside XLIO.\nDefault value is 128 for XLIO_STRQ=1 (default) or 32768 for XLIO_STRQ=0."


why use XLIO_STRQ instead of new config path

BasharRadya · 2025-09-29T08:28:32Z

generate_docs.py

            if value >= DocConstants.MB and value % DocConstants.MB == 0:
-                return f"{value // DocConstants.MB} MB"
-            elif value >= DocConstants.KB and value % DocConstants.KB == 0:
-                return f"{value // DocConstants.KB} KB"


but some support KB "Supports suffixes: B, KB, MB, GB"..

Indeed, this just prettifies numbers - the prettifying to KB seemed redundant. (2048 -> 2KB)

BasharRadya · 2025-09-29T08:54:12Z

README

+socket is a UDP unicast socket and no multicast addresses were added to it.
+Once the first ADD_MEMBERSHIP is called the RX poll duration setting takes effect.
+Value range is similar to the RX poll duration:
+   - -1 means infinite,


"." instead of ","

BasharRadya · 2025-09-29T08:57:58Z

README

-Number of TX buffers fetched by a UDP socket at once. Maps to **TX_BUFS_BATCH_UDP** environment variable.
-Default value is 16
+Number of TX buffers fetched by a UDP socket at once.
+Maps to **TX_BUFS_BATCH_UDP** environment variable.


"Maps" is not immediately after the title.
check all other params please.

my mistake, thanks

tomerdbz · 2025-09-29T10:04:43Z

fixed comments @BasharRadya :)

Fix bug in generate_docs.py where non-dict properties caused processing failures. Add proper type checking and error handling to prevent crashes when encountering unexpected property types in the JSON schema. Changes in generate_docs.py: - Add type check to skip non-dict properties - Wrap property processing in try-catch block - Add error reporting with property path context Regenerate README documentation with the fixed script: - Document memory size suffix support (B, KB, MB, GB) for memory params - Simplify worker threads documentation for better clarity - Fix inconsistent default value formatting - Tidying spaces, indents - Standardizing the mapping to legacy in the description. This ensures robust documentation generation and provides users with complete information about memory size formatting and threading options. Signed-off-by: Tomer Cabouly <[email protected]>

BasharRadya · 2025-09-29T10:21:08Z

Looks good, will approve once CI passes.
@galnoam do some random check on 3 of the xlio params in the new read me (just extra double check)

tomerdbz · 2025-09-29T17:59:42Z

bot:retest

tomerdbz requested review from AlexanderGrissik and Copilot September 16, 2025 08:07

Copilot AI reviewed Sep 16, 2025

View reviewed changes

generate_docs.py Outdated Show resolved Hide resolved

README Show resolved Hide resolved

tomerdbz force-pushed the 4633470_config_gen_docs branch 2 times, most recently from 99e4e65 to 30e2175 Compare September 16, 2025 08:36

BasharRadya reviewed Sep 17, 2025

View reviewed changes

generate_docs.py Outdated Show resolved Hide resolved

BasharRadya reviewed Sep 17, 2025

View reviewed changes

src/core/config/descriptor_providers/xlio_config_schema.json Outdated Show resolved Hide resolved

tomerdbz force-pushed the 4633470_config_gen_docs branch from 30e2175 to bec5538 Compare September 17, 2025 17:19

BasharRadya reviewed Sep 18, 2025

View reviewed changes

README Show resolved Hide resolved

tomerdbz force-pushed the 4633470_config_gen_docs branch from bec5538 to f782ade Compare September 18, 2025 16:56

tomerdbz force-pushed the 4633470_config_gen_docs branch 4 times, most recently from 99d2e06 to 6215792 Compare September 22, 2025 13:11

tomerdbz force-pushed the 4633470_config_gen_docs branch from 6215792 to 48072b4 Compare September 25, 2025 08:07

BasharRadya reviewed Sep 25, 2025

View reviewed changes

BasharRadya reviewed Sep 28, 2025

View reviewed changes

tomerdbz force-pushed the 4633470_config_gen_docs branch from e751519 to 4dc59b4 Compare September 28, 2025 12:20

tomerdbz force-pushed the 4633470_config_gen_docs branch from 4dc59b4 to 789d41c Compare September 28, 2025 17:54

tomerdbz force-pushed the 4633470_config_gen_docs branch 2 times, most recently from b4c9dad to 5d7331e Compare September 29, 2025 06:36

BasharRadya reviewed Sep 29, 2025

View reviewed changes

tomerdbz force-pushed the 4633470_config_gen_docs branch from 5d7331e to 8ee1a65 Compare September 29, 2025 10:04

tomerdbz removed the request for review from AlexanderGrissik September 29, 2025 10:08

tomerdbz force-pushed the 4633470_config_gen_docs branch from 8ee1a65 to c58b13c Compare September 29, 2025 10:20

BasharRadya approved these changes Sep 30, 2025

View reviewed changes

galnoam merged commit 31f3c5b into Mellanox:vNext Sep 30, 2025
1 check passed

issue: 4633470 fix generate_docs.py +update README #440

issue: 4633470 fix generate_docs.py +update README #440

Uh oh!

Conversation

tomerdbz commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What

Why ?

How ?

Change type

Check list

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

tomerdbz commented Sep 16, 2025

Uh oh!

Uh oh!

Uh oh!

BasharRadya commented Sep 17, 2025

Uh oh!

BasharRadya commented Sep 18, 2025

Uh oh!

Uh oh!

tomerdbz commented Sep 18, 2025

Uh oh!

galnoam commented Sep 18, 2025

Uh oh!

galnoam commented Sep 18, 2025

Uh oh!

tomerdbz commented Sep 24, 2025

Uh oh!

galnoam commented Sep 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BasharRadya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomerdbz commented Sep 16, 2025 •

edited

Loading