Skip to content

[vnetorch]: Bitmap VNet implementation#773

Merged
prsunny merged 5 commits intosonic-net:masterfrom
marian-pritsak:vnetorch
Feb 21, 2019
Merged

[vnetorch]: Bitmap VNet implementation#773
prsunny merged 5 commits intosonic-net:masterfrom
marian-pritsak:vnetorch

Conversation

@marian-pritsak
Copy link
Copy Markdown
Collaborator

Signed-off-by: Marian Pritsak marianp@mellanox.com

Signed-off-by: Marian Pritsak <marianp@mellanox.com>
Signed-off-by: Marian Pritsak <marianp@mellanox.com>
Conflicts:
	orchagent/intfsorch.cpp
	orchagent/vnetorch.cpp
	orchagent/vnetorch.h
@marian-pritsak
Copy link
Copy Markdown
Collaborator Author

retest this please

static map<tuple<MacAddress, sai_object_id_t>, sai_neighbor_entry_t> neighMap_;

uint32_t vnet_id_;
uint32_t vni_;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

vni_ is already in VnetObject. Can we use that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

};

typedef map<IpAddress, sai_object_id_t> NextHopMap;
typedef map<string, NextHopMap> NextHopTunnels;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see these being used anywhere. Please delete if not required

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Bad merge
Done

}
else
{
if (!isVnetExists(name))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we move this outside the if for both cases?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done


uint32_t VNetBitmapObject::getFreeNeighbor(void)
{
const uint32_t neighborRangeStart = 0xa9fe0000;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is the magic number? Typically we avoid using hardcoded values

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Obsolete. Removed


for (uint32_t i = 0; i < 256; i++)
{
if (tunnelOffsets_.count(i) == 0)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My preference would be to use a simple Boolean array of 256/512 for this logic. Just a suggestion.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done


if (bridgeInfoMap_.find(vni) != bridgeInfoMap_.end())
{
return std::move(bridgeInfoMap_.at(vni));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need to use move here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Because struct is returned

throw std::runtime_error("vni creation failed");
}

SWSS_LOG_NOTICE("Created RIF");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since it is notice, please add more info like vni, tunnel name. If not relevant, suggest to move to INFO.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Just a debug comment. Removed

const uint32_t neighborRangeStart = 0xa9fe0000;
static set<uint32_t> neighbors;

for (uint32_t i = 0; i < 0xffff; i++)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here for the magic number

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

Signed-off-by: Marian Pritsak <marianp@mellanox.com>
Signed-off-by: Marian Pritsak <marianp@mellanox.com>
@prsunny prsunny merged commit dd6bdd9 into sonic-net:master Feb 21, 2019
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
It it hard to checkout code to non-case sensitive system.
Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025
* [vnetorch]: Bitmap VNet implementation
* Adress review comments

Signed-off-by: Marian Pritsak <marianp@mellanox.com>
jianyuewu pushed a commit to jianyuewu/sonic-swss that referenced this pull request Dec 24, 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.

2 participants