Skip to content

Commit bc755b8

Browse files
oleksandrivantsivShu0T1an ChenG
authored and
Shu0T1an ChenG
committed
[intfsorch]: Fix issue with setting IP address in run time (#241)
Issue: sonic-net/SONiC#22 Issue was observed only when setting IP address via ifconfig command. Caused by ifconfig command behavior: first it sets IP address with netmask /8 and then changes netmask to specified in command. Problem is not observed when set IP address with "ip addr" command. Introduced overlaps variable to check if the newly set IP prefix has any overhaps with the current IP prefixes and wait until there is no overlaps at all.
1 parent 476772d commit bc755b8

File tree

2 files changed

+54
-25
lines changed

2 files changed

+54
-25
lines changed

orchagent/intfsorch.cpp

+52-24
Original file line numberDiff line numberDiff line change
@@ -87,33 +87,61 @@ void IntfsOrch::doTask(Consumer &consumer)
8787
}
8888

8989
auto it_intfs = m_syncdIntfses.find(alias);
90-
if (it_intfs == m_syncdIntfses.end() ||
91-
!m_syncdIntfses[alias].ip_addresses.contains(ip_prefix.getIp()))
90+
if (it_intfs == m_syncdIntfses.end())
9291
{
93-
if (it_intfs == m_syncdIntfses.end())
92+
if (addRouterIntfs(port))
9493
{
95-
if (addRouterIntfs(port))
96-
{
97-
IntfsEntry intfs_entry;
98-
intfs_entry.ref_count = 0;
99-
m_syncdIntfses[alias] = intfs_entry;
100-
}
101-
else
102-
{
103-
it++;
104-
continue;
105-
}
94+
IntfsEntry intfs_entry;
95+
intfs_entry.ref_count = 0;
96+
m_syncdIntfses[alias] = intfs_entry;
97+
}
98+
else
99+
{
100+
it++;
101+
continue;
106102
}
107-
108-
addSubnetRoute(port, ip_prefix);
109-
addIp2MeRoute(ip_prefix);
110-
111-
m_syncdIntfses[alias].ip_addresses.add(ip_prefix.getIp());
112-
it = consumer.m_toSync.erase(it);
113103
}
114-
else
104+
105+
if (m_syncdIntfses[alias].ip_addresses.count(ip_prefix))
106+
{
115107
/* Duplicate entry */
116108
it = consumer.m_toSync.erase(it);
109+
continue;
110+
}
111+
112+
/* NOTE: Overlap checking is required to handle ifconfig weird behavior.
113+
* When set IP address using ifconfig command it applies it in two stages.
114+
* On stage one it sets IP address with netmask /8. On stage two it
115+
* changes netmask to specified in command. As DB is async event to
116+
* add IP address with original netmask may come before event to
117+
* delete IP with netmask /8. To handle this we in case of overlap
118+
* we should wait until entry with /8 netmask will be removed.
119+
* Time frame between those event is quite small.*/
120+
bool overlaps = false;
121+
for (const auto &prefixIt: m_syncdIntfses[alias].ip_addresses)
122+
{
123+
if (prefixIt.isAddressInSubnet(ip_prefix.getIp()) ||
124+
ip_prefix.isAddressInSubnet(prefixIt.getIp()))
125+
{
126+
overlaps = true;
127+
SWSS_LOG_NOTICE("Router interface %s IP %s overlaps with %s.", port.m_alias.c_str(),
128+
prefixIt.to_string().c_str(), ip_prefix.to_string().c_str());
129+
break;
130+
}
131+
}
132+
133+
if (overlaps)
134+
{
135+
/* Overlap of IP address network */
136+
++it;
137+
continue;
138+
}
139+
140+
addSubnetRoute(port, ip_prefix);
141+
addIp2MeRoute(ip_prefix);
142+
143+
m_syncdIntfses[alias].ip_addresses.insert(ip_prefix);
144+
it = consumer.m_toSync.erase(it);
117145
}
118146
else if (op == DEL_COMMAND)
119147
{
@@ -134,16 +162,16 @@ void IntfsOrch::doTask(Consumer &consumer)
134162

135163
if (m_syncdIntfses.find(alias) != m_syncdIntfses.end())
136164
{
137-
if (m_syncdIntfses[alias].ip_addresses.contains(ip_prefix.getIp()))
165+
if (m_syncdIntfses[alias].ip_addresses.count(ip_prefix))
138166
{
139167
removeSubnetRoute(port, ip_prefix);
140168
removeIp2MeRoute(ip_prefix);
141169

142-
m_syncdIntfses[alias].ip_addresses.remove(ip_prefix.getIp());
170+
m_syncdIntfses[alias].ip_addresses.erase(ip_prefix);
143171
}
144172

145173
/* Remove router interface that no IP addresses are associated with */
146-
if (m_syncdIntfses[alias].ip_addresses.getSize() == 0)
174+
if (m_syncdIntfses[alias].ip_addresses.size() == 0)
147175
{
148176
if (removeRouterIntfs(port))
149177
{

orchagent/intfsorch.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@
99
#include "macaddress.h"
1010

1111
#include <map>
12+
#include <set>
1213

1314
extern sai_object_id_t gVirtualRouterId;
1415
extern MacAddress gMacAddress;
1516

1617
struct IntfsEntry
1718
{
18-
IpAddresses ip_addresses;
19+
std::set<IpPrefix> ip_addresses;
1920
int ref_count;
2021
};
2122

0 commit comments

Comments
 (0)