Skip to content

[orchagent] Fix getopt string for -R option (ring_thread_enabled regression)#4207

Merged
prsunny merged 5 commits intosonic-net:masterfrom
Chiranjeevi-U-ML:fix-ring-mode-getopt
Feb 23, 2026
Merged

[orchagent] Fix getopt string for -R option (ring_thread_enabled regression)#4207
prsunny merged 5 commits intosonic-net:masterfrom
Chiranjeevi-U-ML:fix-ring-mode-getopt

Conversation

@Chiranjeevi-U-ML
Copy link
Copy Markdown
Contributor

What I did
Fixed a typo in the getopt option string in orchagent/main.cpp where the -R option was incorrectly defined as requiring a mandatory argument (R:) instead of being a simple flag (R).

The fix changes line 403 from:

while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:v:I:R:D:M")) != -1)

to:

while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:v:I:RD:M")) != -1)

Why I did it

When ring_thread_enabled is set to true in DEVICE_METADATA, orchagent fails to start with the error:
/usr/bin/orchagent: option requires an argument -- 'R'

This bug was introduced in PR #3814 (specifically, commit 5de5922) when the -D option was added.

The getopt string was changed from I:R to I:R:D: instead of the correct I:RD:.

This accidentally added a colon after R, making it require a mandatory argument.

How I verified it

I did a code review and verified that the -R case handler does not use optarg and simply sets gRingMode = true. Also, I have checked the git history and traced the bug introduction to PR #3814 and confirmed the original implementation in PR #3242 was correct.

Check the code lines here:

Original PR link:
https://github.com/sonic-net/sonic-swss/pull/3242/changes#diff-3cba33a216c0ea2b942dc03b2638fa27a40a8b6eb81abf9f83f13e1ee7831ac4R360

Bug Link:
https://github.com/sonic-net/sonic-swss/pull/3814/changes#diff-3cba33a216c0ea2b942dc03b2638fa27a40a8b6eb81abf9f83f13e1ee7831ac4R373

Details if related

Fixes: sonic-net/sonic-buildimage#25366

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D is added for delay. @dgsudharsan , please review

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@dgsudharsan
Copy link
Copy Markdown
Collaborator

@Chiranjeevi-U-ML Don't we have a UT in buildimage or vstest to catch these failures? If possible can we add UT?

Copy link
Copy Markdown
Collaborator

@dgsudharsan dgsudharsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a basic change where we will not even start orchagent if the parameter is wrong, I suggest we need to add coverage through UT .

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Chiranjeevi U <chiranjeevi.uddanti@xoriant.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@Chiranjeevi-U-ML
Copy link
Copy Markdown
Contributor Author

@dgsudharsan Added an integration test test_ring_thread in tests/test_zmq.py to cover the -R flag. It verifies that orchagent starts successfully with the -R flag (no argument required).

@prsunny
Copy link
Copy Markdown
Collaborator

prsunny commented Feb 11, 2026

@dgsudharsan , can you sign off?

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny merged commit ebf8c73 into sonic-net:master Feb 23, 2026
16 checks passed
ksravani-hcl pushed a commit to ksravani-hcl/sonic-swss that referenced this pull request Feb 25, 2026
…ession) (sonic-net#4207)

What I did
Fixed a typo in the getopt option string in orchagent/main.cpp where the -R option was incorrectly defined as requiring a mandatory argument (R:) instead of being a simple flag (R).

The fix changes line 403 from:

while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:v:I:R:D:M")) != -1)
to:

while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:v:I:RD:M")) != -1)
Why I did it

When ring_thread_enabled is set to true in DEVICE_METADATA, orchagent fails to start with the error:
/usr/bin/orchagent: option requires an argument -- 'R'

This bug was introduced in PR sonic-net#3814 (specifically, commit 5de5922) when the -D option was added.

The getopt string was changed from I:R to I:R:D: instead of the correct I:RD:.

This accidentally added a colon after R, making it require a mandatory argument.
ksravani-hcl pushed a commit to ksravani-hcl/sonic-swss that referenced this pull request Feb 26, 2026
…ession) (sonic-net#4207)

What I did
Fixed a typo in the getopt option string in orchagent/main.cpp where the -R option was incorrectly defined as requiring a mandatory argument (R:) instead of being a simple flag (R).

The fix changes line 403 from:

while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:v:I:R:D:M")) != -1)
to:

while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:v:I:RD:M")) != -1)
Why I did it

When ring_thread_enabled is set to true in DEVICE_METADATA, orchagent fails to start with the error:
/usr/bin/orchagent: option requires an argument -- 'R'

This bug was introduced in PR sonic-net#3814 (specifically, commit 5de5922) when the -D option was added.

The getopt string was changed from I:R to I:R:D: instead of the correct I:RD:.

This accidentally added a colon after R, making it require a mandatory argument.
ksravani-hcl pushed a commit to ksravani-hcl/sonic-swss that referenced this pull request Feb 27, 2026
…ession) (sonic-net#4207)

What I did
Fixed a typo in the getopt option string in orchagent/main.cpp where the -R option was incorrectly defined as requiring a mandatory argument (R:) instead of being a simple flag (R).

The fix changes line 403 from:

while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:v:I:R:D:M")) != -1)
to:

while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:q:c:t:v:I:RD:M")) != -1)
Why I did it

When ring_thread_enabled is set to true in DEVICE_METADATA, orchagent fails to start with the error:
/usr/bin/orchagent: option requires an argument -- 'R'

This bug was introduced in PR sonic-net#3814 (specifically, commit 5de5922) when the -D option was added.

The getopt string was changed from I:R to I:R:D: instead of the correct I:RD:.

This accidentally added a colon after R, making it require a mandatory argument.
@Chiranjeevi-U-ML Chiranjeevi-U-ML deleted the fix-ring-mode-getopt branch March 10, 2026 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug:Setting ring_thread_enabled to true in DEVICE_METADATA causes orchagent to fail during startup.

4 participants