Skip to content

[Orchagent] Enable ZMQ on orchagent route table.#23028

Merged
qiluo-msft merged 10 commits intosonic-net:masterfrom
liuh-80:dev/liuh/enable_zmq_route
Jul 17, 2025
Merged

[Orchagent] Enable ZMQ on orchagent route table.#23028
qiluo-msft merged 10 commits intosonic-net:masterfrom
liuh-80:dev/liuh/enable_zmq_route

Conversation

@liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Jun 19, 2025

Enable ZMQ on orchagent route table.

Why I did it

To enhance route performance, sonic-swss introduces a new feature enabling ZMQ communication between fpmsyncd and orchagent. With this change, the orchagent.sh script must be updated to always pass the ZMQ address parameter.

Additionally, the feature flag changed during code review of sonic-net/sonic-swss#3619, so yang model and config also need update in this PR.

Work item tracking
  • Microsoft ADO (number only): 32582830

How I did it

Update orchagent.sh and yang module.

How to verify it

Pass all test case
Manually verified with latest image.

Which release branch to backport (provide reason below if selected)

  • 202205
  • 202211
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505

Tested branch (Please provide the tested image version)

Description for the changelog

Enable ZMQ on orchagent route table.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@liuh-80 liuh-80 changed the title [POC] [Orchagent] Enable ZMQ on orchagent route table. [Orchagent] Enable ZMQ on orchagent route table. Jul 7, 2025
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80 liuh-80 marked this pull request as ready for review July 9, 2025 02:31
@liuh-80 liuh-80 requested review from kperumalbfn and prsunny July 9, 2025 05:14
qiluo-msft
qiluo-msft previously approved these changes Jul 9, 2025
@liuh-80 liuh-80 requested a review from prabhataravind July 14, 2025 08:55
@prsunny
Copy link
Contributor

prsunny commented Jul 14, 2025

@prabhataravind , would you review and check if its needed for 202506 internal?

mgmt_ip=$( ip -json -4 addr show eth0 | jq -r ".[0].addr_info[0].local" )
if [[ $midplane_mgmt_state == "UP" ]]; then
# Enable ZMQ with eth0-midplane interface name
ORCHAGENT_ARGS+=" -q tcp://eth0-midplane:8100"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why ORCH_ZMQ_PORT is being removed. Could you clarify please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change in sonic-net/sonic-swss#3619 modifies how orchagent handles ZMQ addresses for support multiple namespaces:

If the address includes a port, then create ZMQ client with the address.

If not, the ZMQ client will be created using the default ZMQ port along with the namespace ID. For the default namespace, the port will still be set to 8100.

code: https://github.com/sonic-net/sonic-swss/blob/27391fcf1785d6e5d9a765219f5c0f2dfcfe751c/lib/orch_zmq_config.cpp#L56

I couldn’t find the code for the 202506 branch. However, if PR #3619 and PR #3632 are not included in that branch, this change should not be cherry-picked into 202506.

Copy link
Contributor

@prabhataravind prabhataravind Jul 15, 2025

Choose a reason for hiding this comment

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

Okay, that makes sense. The 202506 branch is here: https://github.com/Azure/sonic-buildimage-msft/tree/202506. We will have the labels for this branch soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the code of 202506branch, this PR should cherry-pick to it.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -1,4 +1,4 @@
{% if DEVICE_METADATA.localhost.orch_dash_zmq_enabled == "false" %}
{% if DEVICE_METADATA.localhost.orch_northbond_dash_zmq_enabled != "false" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be checked against "true"? If the config is disabled, we don't want all these tables in orch_zmq_tables.conf.

Please check all 4 combinations of dash_zmq_disabled/route_zmq_disabled, dash_zmq_enabled/route_zmq_disabled, dash_zmq_disabled/route_zmq_enabled, dash_zmq_enabled/route_zmq_enabled to make sure orch_zmq_tables.conf looks correct in all cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Verified all combinations with deploy mini graph, this file can be rendered correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. This looks good to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw @liuh-80 it looks like there are some new tables that are not in this list. @zjswhhh and @theasianpianist could you pls double check if all recent tables added by you are here? It looks like some are missing to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zjswhhh , @theasianpianist , if your new tables want to have swssconfig and swsssplayer command support when ZMQ enabled, please also update those tables to orch_zmq_tables.conf.j2

@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 15, 2025

PR validation failed, seems relate with this change:
sonic-net/sonic-mgmt#19263

Waiting for fix PR merge:
#23346

@StormLiangMS
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 16, 2025

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 16, 2025

Fix PR merged, trigger buildimage again

@qiluo-msft qiluo-msft merged commit aec9a66 into sonic-net:master Jul 17, 2025
20 checks passed
stephenxs added a commit to stephenxs/sonic-buildimage that referenced this pull request Aug 11, 2025
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.

6 participants