Skip to content
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

[switchorch]: Add support of ECMP and LAG hash seed attribute #324

Merged
merged 1 commit into from
Oct 3, 2017
Merged

[switchorch]: Add support of ECMP and LAG hash seed attribute #324

merged 1 commit into from
Oct 3, 2017

Conversation

volodymyrsamotiy
Copy link
Collaborator

Signed-off-by: Volodymyr Samotiy volodymyrs@mellanox.com

Copy link
Contributor

@stcheng stcheng left a comment

Choose a reason for hiding this comment

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

Looks good to me. Left some comments to discuss.


if (switch_attribute_map.find(attribute) == switch_attribute_map.end())
if (fvField(i) == "hash_seed")
Copy link
Contributor

Choose a reason for hiding this comment

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

i wonder if we don't need such logics here. if we would like to set the two values with the same value, it is okay that we feed the swssconfig with the JSON file that contains both attributes with the same value. Thus the for loop below is not necessary.

Copy link
Collaborator Author

@volodymyrsamotiy volodymyrsamotiy Sep 28, 2017

Choose a reason for hiding this comment

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

I agree that it would be better and logical to set hash seed attributes separately. I will update the pull request with appropriate changes.
Since changes were done according to requirements document, I believe it should be updated as well. (https://github.com/Azure/SONiC/wiki/ECMP-and-LAG-Hash-Seed)

@@ -12,7 +13,9 @@ const map<string, sai_switch_attr_t> switch_attribute_map =
{
{"fdb_unicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_UNICAST_MISS_PACKET_ACTION},
{"fdb_broadcast_miss_packet_action", SAI_SWITCH_ATTR_FDB_BROADCAST_MISS_PACKET_ACTION},
{"fdb_multicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_MULTICAST_MISS_PACKET_ACTION}
{"fdb_multicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_MULTICAST_MISS_PACKET_ACTION},
{"ecmp_default_hash_seed", SAI_SWITCH_ATTR_ECMP_DEFAULT_HASH_SEED},
Copy link
Contributor

Choose a reason for hiding this comment

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

-> ecmp_hash_seed

Copy link
Contributor

Choose a reason for hiding this comment

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

agree. default shall not be in the attribute name. default is a value not an attribute. the SAI attribute name is not perfect.

{"fdb_multicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_MULTICAST_MISS_PACKET_ACTION}
{"fdb_multicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_MULTICAST_MISS_PACKET_ACTION},
{"ecmp_default_hash_seed", SAI_SWITCH_ATTR_ECMP_DEFAULT_HASH_SEED},
{"lag_default_hash_seed", SAI_SWITCH_ATTR_LAG_DEFAULT_HASH_SEED}
Copy link
Contributor

Choose a reason for hiding this comment

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

-> lag_hash_seed


case SAI_SWITCH_ATTR_ECMP_DEFAULT_HASH_SEED:
case SAI_SWITCH_ATTR_LAG_DEFAULT_HASH_SEED:
attr.value.u32 = to_uint<uint32_t>(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have test cases for different hash seeds?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, as far as I know, we don't have.

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>
@stcheng stcheng merged commit 70a2866 into sonic-net:master Oct 3, 2017
praveen-li pushed a commit to praveen-li/sonic-swss that referenced this pull request Aug 24, 2020
* msft_github/master:
  [portsorch]: Use sai_serialize api to write to DB (sonic-net#331)
  [portsorch]: Update comments (sonic-net#333)
  [switchorch]: Add support of ECMP and LAG hash seed attribute (sonic-net#324)
  [portsorch]: Add support of cable breakout feature (sonic-net#320)
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
…tem info on device (sonic-net#324)

The information includes:
-- platform string
-- system mac
-- port configuration: alias, lanes, speed, index

This is useful when we first load the configuration from config_db.json generated
outside which does not have the information on the device.

The default behaviour is not changed, so it won't impact the system unless you use this option.

This can be used for ZTP to config the box the first time and avoid any addtional work
which could cause inconsistency.
lukasstockner pushed a commit to genesiscloud/sonic-swss that referenced this pull request Apr 2, 2023
…onic-net#324) (sonic-net#329)

- Description
Add media assignment options to Application Advertisement

- Motivation and Context
Add media assignment options to Application Advertisement

- How Has This Been Tested?
Manual test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants