-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[Orchagent] Enable ZMQ on orchagent route table. #23028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
bde5ae2
be174d3
fad4005
b65b15f
83f8508
4f6eced
cd3e72f
5016f0b
0b31932
8ddfb50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,20 +93,23 @@ else | |
| ORCHAGENT_ARGS+="-m $MAC_ADDRESS" | ||
| fi | ||
|
|
||
| # Enable ZMQ for SmartSwitch | ||
| # Enable ZMQ | ||
| LOCALHOST_SUBTYPE=`sonic-db-cli CONFIG_DB hget "DEVICE_METADATA|localhost" "subtype"` | ||
| if [[ x"${LOCALHOST_SUBTYPE}" == x"SmartSwitch" ]]; then | ||
| midplane_mgmt_state=$( ip -json -4 addr show eth0-midplane | jq -r ".[0].operstate" ) | ||
| 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" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| ORCHAGENT_ARGS+=" -q tcp://eth0-midplane" | ||
| elif [[ $mgmt_ip != "" ]] && [[ $mgmt_ip != "null" ]]; then | ||
| # If eth0-midplane interface does not up, enable ZMQ with eth0 address | ||
| ORCHAGENT_ARGS+=" -q tcp://${mgmt_ip}:8100" | ||
| ORCHAGENT_ARGS+=" -q tcp://${mgmt_ip}" | ||
| else | ||
| ORCHAGENT_ARGS+=" -q tcp://127.0.0.1:8100" | ||
| ORCHAGENT_ARGS+=" -q tcp://127.0.0.1" | ||
| fi | ||
| else | ||
| # For other platforms, use the default ZMQ address | ||
| ORCHAGENT_ARGS+=" -q tcp://127.0.0.1" | ||
| fi | ||
|
|
||
| # Add VRF parameter when mgmt-vrf enabled | ||
|
|
||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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