Skip to content

Delete the IPv6 link-local Neighbor when ipv6 link-local mode is disabled #1897

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

Merged
merged 6 commits into from
Dec 1, 2021
Merged

Delete the IPv6 link-local Neighbor when ipv6 link-local mode is disabled #1897

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3a63fff
IPv6 link-local Neighor deletion when ipv6 link-local mode is disabled.
AkhileshSamineni Sep 3, 2021
08a4c8d
Added vs test case.
AkhileshSamineni Oct 22, 2021
75ac751
Added few changes to vs test case
AkhileshSamineni Nov 23, 2021
1b88e1a
Updated vs test case
AkhileshSamineni Nov 24, 2021
0ac9869
Merge branch 'master' into ipv6_ll_neigh_delete
AkhileshSamineni Nov 24, 2021
a77af5e
Added changes to delete the neigh entry from kernel
AkhileshSamineni Nov 30, 2021
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
47 changes: 46 additions & 1 deletion cfgmgr/intfmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
m_stateVlanTable(stateDb, STATE_VLAN_TABLE_NAME),
m_stateVrfTable(stateDb, STATE_VRF_TABLE_NAME),
m_stateIntfTable(stateDb, STATE_INTERFACE_TABLE_NAME),
m_appIntfTableProducer(appDb, APP_INTF_TABLE_NAME)
m_appIntfTableProducer(appDb, APP_INTF_TABLE_NAME),
m_neighTable(appDb, APP_NEIGH_TABLE_NAME),
m_neighTableProducer(appDb, APP_NEIGH_TABLE_NAME)
{
if (!WarmStart::isWarmStart())
{
Expand Down Expand Up @@ -449,6 +451,31 @@ bool IntfMgr::isIntfStateOk(const string &alias)
return false;
}

void IntfMgr::delIpv6LinkLocalNeigh(const string &alias)
{
vector<string> neighEntries;

SWSS_LOG_INFO("Deleting ipv6 link local neighbors for %s", alias.c_str());

m_neighTable.getKeys(neighEntries);
for (auto neighKey : neighEntries)
{
if (!neighKey.compare(0, alias.size(), alias.c_str()))
{
vector<string> keys = tokenize(neighKey, ':', 1);
if (keys.size() == 2)
{
IpAddress ipAddress(keys[1]);
if (ipAddress.getAddrScope() == IpAddress::AddrScope::LINK_SCOPE)
{
m_neighTableProducer.del(neighKey);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not a good approach as to have multiple producers for the same table APP_NEIGH_TABLE_NAME (Owned by neighsyncd). I think, intfmgr should delete this from kernel directly. Is that feasible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prsunny I do agree about multiple producers to the same table.
If intfmgr deletes the kernel entry, then neighsync have to delete that entry from APPL_DB even though the link-local mode is disabled right. Added those changes, please continue the review.

SWSS_LOG_INFO("Deleted ipv6 link local neighbor - %s", keys[1].c_str());
}
}
}
}
}

bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
vector<FieldValueTuple> data,
const string& op)
Expand Down Expand Up @@ -571,6 +598,17 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
/* Set ipv6 mode */
if (!ipv6_link_local_mode.empty())
{
if ((ipv6_link_local_mode == "enable") && (m_ipv6LinkLocalModeList.find(alias) == m_ipv6LinkLocalModeList.end()))
{
m_ipv6LinkLocalModeList.insert(alias);
SWSS_LOG_INFO("Inserted ipv6 link local mode list for %s", alias.c_str());
}
else if ((ipv6_link_local_mode == "disable") && (m_ipv6LinkLocalModeList.find(alias) != m_ipv6LinkLocalModeList.end()))
{
m_ipv6LinkLocalModeList.erase(alias);
delIpv6LinkLocalNeigh(alias);
SWSS_LOG_INFO("Erased ipv6 link local mode list for %s", alias.c_str());
}
FieldValueTuple fvTuple("ipv6_use_link_local_only", ipv6_link_local_mode);
data.push_back(fvTuple);
}
Expand Down Expand Up @@ -705,6 +743,13 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
removeSubIntfState(alias);
}

if (m_ipv6LinkLocalModeList.find(alias) != m_ipv6LinkLocalModeList.end())
{
m_ipv6LinkLocalModeList.erase(alias);
delIpv6LinkLocalNeigh(alias);
SWSS_LOG_INFO("Erased ipv6 link local mode list for %s", alias.c_str());
}

m_appIntfTableProducer.del(alias);
m_stateIntfTable.del(alias);
}
Expand Down
5 changes: 4 additions & 1 deletion cfgmgr/intfmgr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ class IntfMgr : public Orch
using Orch::doTask;

private:
ProducerStateTable m_appIntfTableProducer;
ProducerStateTable m_appIntfTableProducer, m_neighTableProducer;
Table m_cfgIntfTable, m_cfgVlanIntfTable, m_cfgLagIntfTable, m_cfgLoopbackIntfTable;
Table m_statePortTable, m_stateLagTable, m_stateVlanTable, m_stateVrfTable, m_stateIntfTable;
Table m_neighTable;

std::set<std::string> m_subIntfList;
std::set<std::string> m_loopbackIntfList;
std::set<std::string> m_pendingReplayIntfList;
std::set<std::string> m_ipv6LinkLocalModeList;

void setIntfIp(const std::string &alias, const std::string &opCmd, const IpPrefix &ipPrefix);
void setIntfVrf(const std::string &alias, const std::string &vrfName);
Expand Down Expand Up @@ -52,6 +54,7 @@ class IntfMgr : public Orch
void removeHostSubIntf(const std::string &subIntf);
void setSubIntfStateOk(const std::string &alias);
void removeSubIntfState(const std::string &alias);
void delIpv6LinkLocalNeigh(const std::string &alias);

bool setIntfProxyArp(const std::string &alias, const std::string &proxy_arp);
bool setIntfGratArp(const std::string &alias, const std::string &grat_arp);
Expand Down
114 changes: 114 additions & 0 deletions tests/test_ipv6_link_local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import time
import json
import pytest

from swsscommon import swsscommon

class TestIPv6LinkLocal(object):
def setup_db(self, dvs):
self.pdb = dvs.get_app_db()
self.adb = dvs.get_asic_db()
self.cdb = dvs.get_config_db()

def set_admin_status(self, interface, status):
self.cdb.update_entry("PORT", interface, {"admin_status": status})

def add_ip_address(self, interface, ip):
self.cdb.create_entry("INTERFACE", interface + "|" + ip, {"NULL": "NULL"})
time.sleep(2)

def remove_ip_address(self, interface, ip):
self.cdb.delete_entry("INTERFACE", interface + "|" + ip)
time.sleep(2)

def create_ipv6_link_local_intf(self, interface):
self.cdb.create_entry("INTERFACE", interface, {"ipv6_use_link_local_only": "enable"})
time.sleep(2)

def remove_ipv6_link_local_intf(self, interface):
self.cdb.delete_entry("INTERFACE", interface)
time.sleep(2)

def test_NeighborAddRemoveIpv6LinkLocal(self, dvs, testlog):
self.setup_db(dvs)

# create ipv6 link local intf
self.create_ipv6_link_local_intf("Ethernet0")
self.create_ipv6_link_local_intf("Ethernet4")

# bring up interface
self.set_admin_status("Ethernet0", "up")
self.set_admin_status("Ethernet4", "up")

# set ip address
self.add_ip_address("Ethernet0", "2000::1/64")
self.add_ip_address("Ethernet4", "2001::1/64")
dvs.runcmd("sysctl -w net.ipv6.conf.all.forwarding=1")
dvs.runcmd("sysctl -w net.ipv6.conf.default.forwarding=1")

# set ip address and default route
dvs.servers[0].runcmd("ip -6 address add 2000::2/64 dev eth0")
dvs.servers[0].runcmd("ip -6 route add default via 2000::1")

dvs.servers[1].runcmd("ip -6 address add 2001::2/64 dev eth0")
dvs.servers[1].runcmd("ip -6 route add default via 2001::1")
time.sleep(2)

neigh_before_entries = self.pdb.get_keys("NEIGH_TABLE")
assert (len(neigh_before_entries) == 0)

# get neighbor entry
dvs.servers[0].runcmd("ping -6 -c 1 2001::2")
time.sleep(2)

# Neigh entries should contain Ipv6-link-local neighbors, should be 4
neigh_entries = self.pdb.get_keys("NEIGH_TABLE")
assert (len(neigh_entries) == 4)

found_entry = False
for key in neigh_entries:
if (key.find("Ethernet4:2001::2") or key.find("Ethernet0:2000::2")):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this an 'or' check? Shouldn't it be mandatory that both entries exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both entries should exist, but here moving ahead by checking 1 entry only. Anyways checking number of entries as 4 right.

found_entry = True

assert found_entry

# remove ip address
self.remove_ip_address("Ethernet0", "2000::1/64")
self.remove_ip_address("Ethernet4", "2001::1/64")

# remove ipv6 link local intf
self.remove_ipv6_link_local_intf("Ethernet0")
self.remove_ipv6_link_local_intf("Ethernet4")

# Neigh entries should not contain Ipv6-link-local neighbors, should be 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't removing the global IPv6 remove both IP link local as well as global neighbor entries? Can you please clarify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Global ipv6 remove will not remove the link local neighbor entries because ipv6 link local is always enable in kernel. The ipv6 link local remove command will remove the neighbors from APPL_DB only.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If it removes the neighbors from APPL_DB and remove_ip_address remove the global neighbors shouldn't the number of entries be zero? My earlier question was why 2 entries are present since the interface now has no IPv6 address (global) or has link local enabled.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@AkhileshSamineni Can you please let me know if my understanding is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dgsudharsan Global neighbor entries gets deleted when interface is set to down.
When interface set to up/down, then only the netlink message are generated (https://github.com/Azure/sonic-swss/blob/master/tests/test_neighbor.py#L79).

neigh_after_entries = self.pdb.get_keys("NEIGH_TABLE")
assert (len(neigh_after_entries) == 2)

found_existing_entry = False
for key in neigh_entries:
if (key.find("Ethernet4:2001::2") or key.find("Ethernet0:2000::2")):
found_existing_entry = True

assert found_existing_entry

self.set_admin_status("Ethernet0", "down")
self.set_admin_status("Ethernet4", "down")

# remove ip address and default route
dvs.servers[0].runcmd("ip -6 route del default dev eth0")
dvs.servers[0].runcmd("ip -6 address del 2000::2/64 dev eth0")

dvs.servers[1].runcmd("ip -6 route del default dev eth0")
dvs.servers[1].runcmd("ip -6 address del 2001::2/64 dev eth0")

dvs.runcmd("sysctl -w net.ipv6.conf.all.forwarding=0")
dvs.runcmd("sysctl -w net.ipv6.conf.default.forwarding=0")

neigh_entries = self.pdb.get_keys("NEIGH_TABLE")
assert (len(neigh_entries) == 0)

# Add Dummy always-pass test at end as workaroud
# for issue when Flaky fail on final test it invokes module tear-down before retrying
def test_nonflaky_dummy():
pass