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.

Sign up for GitHub

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

Jump to bottom

[vxlan decap]: Don't remove tunnel when it has tunnel maps configured #1047

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from
Draft

[vxlan decap]: Don't remove tunnel when it has tunnel maps configured #1047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a5b0a88
Pospone QueueMap initialization until activation of counters
pavel-shirshov Jun 25, 2018
1d54717
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Jun 29, 2018
b767c2e
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Jun 29, 2018
5e96d67
Generate queue maps only for front panel ports
pavel-shirshov Jul 2, 2018
21c072c
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Jul 4, 2018
00192a4
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Aug 22, 2018
1a71c39
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Aug 29, 2018
6bc8091
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Sep 5, 2018
83efbee
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Sep 11, 2018
c2f4fdf
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Sep 15, 2018
c4094b0
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Nov 7, 2018
fc6185a
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Nov 10, 2018
4e5d73d
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Nov 13, 2018
cc57c96
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Dec 4, 2018
3e879f0
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Apr 1, 2019
6429642
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Jun 14, 2019
f60c7d1
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Jun 15, 2019
a286fda
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Jun 19, 2019
3bf6a54
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Jul 27, 2019
8d446d4
Merge branch 'master' of https://github.com/Azure/sonic-swss
pavel-shirshov Sep 5, 2019
c6006a7
Initialize ids with correct values
pavel-shirshov Sep 5, 2019
3c55d0a
Don't remove tunnel when it has tunnel map configured
pavel-shirshov Sep 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions orchagent/vxlanorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,23 @@ void VxlanTunnel::insertMapperEntry(sai_object_id_t encap, sai_object_id_t decap
tunnel_map_entries_[vni] = std::pair<sai_object_id_t, sai_object_id_t>(encap, decap);
}

void VxlanTunnel::removeMapperEntry(uint32_t vni)
{
tunnel_map_entries_.erase(vni);
}

std::vector<uint32_t> VxlanTunnel::getMapperKeys()
{
std::vector<uint32_t> keys;

for (const auto& p: tunnel_map_entries_)
{
keys.push_back(p.first);
}

return keys;
}

std::pair<sai_object_id_t, sai_object_id_t> VxlanTunnel::getMapperEntry(uint32_t vni)
{
if (tunnel_map_entries_.find(vni) != tunnel_map_entries_.end())
Expand Down Expand Up @@ -666,6 +683,8 @@ bool VxlanTunnelOrch::removeVxlanTunnelMap(string tunnelName, uint32_t vni)
remove_tunnel_map_entry(mapper.first);
remove_tunnel_map_entry(mapper.second);

tunnel_obj->removeMapperEntry(vni);

SWSS_LOG_DEBUG("Vxlan tunnel encap entry '%" PRIx64 "' decap entry '0x%" PRIx64 "'", mapper.first, mapper.second);
}
catch(const std::runtime_error& error)
Expand Down Expand Up @@ -731,7 +750,24 @@ bool VxlanTunnelOrch::delOperation(const Request& request)
return true;
}

auto tunnel_term_id = vxlan_tunnel_table_[tunnel_name].get()->getTunnelTermId();
auto tunnel = vxlan_tunnel_table_[tunnel_name].get();

if (!tunnel->isMapperEmpty())
{
std::string vni_list;
bool first = true;
for (auto vni: tunnel->getMapperKeys())
{
if (!first)
vni_list += ", ";
vni_list += std::to_string(vni);
first = false;
}
SWSS_LOG_WARN("Vxlan tunnel '%s' has tunnel maps configured [%s]. Can't remove now.", tunnel_name.c_str(), vni_list.c_str());
return false;
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 do retry here if mapper is not empty?

Copy link
Contributor

Choose a reason for hiding this comment

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

if orchagent gets tunnel remove first and then tunnel map, then we should pend the tunnel remove (retry) later to resolve the dependency issue.

Copy link
Contributor Author

@pavel-shirshov pavel-shirshov Sep 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

'return false' mean don't erase from the queue. So the operation will be retried later

We don't have such infrastructure for pending anything in orchagent.
Our common way to deal with this:
If we see that request tries to remove something which has dependencies, just don't do anything and don't erase it from the queue. Later the request is going to be retried, if the dependency still not removed, it will be postponed again. We don't have counters or anything to limit number of attempts to remove something with dependency.

Copy link
Contributor

Choose a reason for hiding this comment

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

i mean retry.

}

auto tunnel_term_id = tunnel->getTunnelTermId();
try
{
remove_tunnel_termination(tunnel_term_id);
Expand All @@ -742,7 +778,7 @@ bool VxlanTunnelOrch::delOperation(const Request& request)
return false;
}

auto tunnel_id = vxlan_tunnel_table_[tunnel_name].get()->getTunnelId();
auto tunnel_id = tunnel->getTunnelId();
try
{
remove_tunnel(tunnel_id);
Expand Down
9 changes: 8 additions & 1 deletion orchagent/vxlanorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,14 @@ class VxlanTunnel
sai_object_id_t addDecapMapperEntry(sai_object_id_t obj, uint32_t vni);

void insertMapperEntry(sai_object_id_t encap, sai_object_id_t decap, uint32_t vni);
void removeMapperEntry(uint32_t vni);
std::pair<sai_object_id_t, sai_object_id_t> getMapperEntry(uint32_t vni);
std::vector<uint32_t> getMapperKeys();

bool isMapperEmpty() const
{
return tunnel_map_entries_.empty();
}

sai_object_id_t getTunnelId() const
{
Expand Down Expand Up @@ -121,7 +128,7 @@ class VxlanTunnel
string tunnel_name_;
bool active_ = false;

tunnel_ids_t ids_ = {0, 0, 0, 0};
tunnel_ids_t ids_ = { SAI_NULL_OBJECT_ID, SAI_NULL_OBJECT_ID, SAI_NULL_OBJECT_ID, SAI_NULL_OBJECT_ID };
std::pair<MAP_T, MAP_T> tunnel_map_ = { MAP_T::MAP_TO_INVALID, MAP_T::MAP_TO_INVALID };

TunnelMapEntries tunnel_map_entries_;
Expand Down