Skip to content

Allow IPv4 link-local nexthops #1903

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

Merged
merged 1 commit into from
Feb 28, 2022
Merged
Changes from all commits
Commits
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
25 changes: 12 additions & 13 deletions orchagent/neighorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -671,19 +671,6 @@ void NeighOrch::doTask(Consumer &consumer)

IpAddress ip_address(key.substr(found+1));

/* Verify Ipv4 LinkLocal and skip neighbor entry added for RFC5549 */
if ((ip_address.getAddrScope() == IpAddress::LINK_SCOPE) && (ip_address.isV4()))
{
/* Check if this prefix is not a configured ip, if so allow */
IpPrefix ipll_prefix(ip_address.getV4Addr(), 16);
if (!m_intfsOrch->isPrefixSubnet (ipll_prefix, alias))
{
SWSS_LOG_NOTICE("Skip IPv4LL neighbor %s, Intf:%s op: %s ", ip_address.to_string().c_str(), alias.c_str(), op.c_str());
it = consumer.m_toSync.erase(it);
continue;
}
}

NeighborEntry neighbor_entry = { ip_address, alias };
Copy link
Contributor

Choose a reason for hiding this comment

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

root@sonic:/home/admin# redis-cli hgetall "NEIGH_TABLE:Ethernet0:169.254.0.1"

  1. "family"
  2. "IPv4"
  3. "neigh"
  4. "00:10:94:00:00:01"

Is this static or dynamic neighbor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For testing, the neighbour/route entries were manually added to APP DB. In typical use-case, this neighbour entry will be static.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, which module would push this static ARP and MAC into the kernel? I believe, we should push static nbr entry add/del code as well to community to support BGP unnumbered use-case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FRR(Zebra) is already adding static(PERMANENT) IPv4 LL neighbours in kernel. Please check the APP DB log in the issue. This change is only to allow such entries in NeighOrch as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

@utpalkantpintoo Can you attach the testing logs of BGP unnumbered use-case with the community image, just to make sure this code is working fine for that use-case?


if (op == SET_COMMAND)
Expand Down Expand Up @@ -793,6 +780,18 @@ bool NeighOrch::addNeighbor(const NeighborEntry &neighborEntry, const MacAddress
memcpy(neighbor_attr.value.mac, macAddress.getMac(), 6);
neighbor_attrs.push_back(neighbor_attr);

if ((ip_address.getAddrScope() == IpAddress::LINK_SCOPE) && (ip_address.isV4()))
{
/* Check if this prefix is a configured ip, if not allow */
IpPrefix ipll_prefix(ip_address.getV4Addr(), 16);
if (!m_intfsOrch->isPrefixSubnet (ipll_prefix, alias))
{
neighbor_attr.id = SAI_NEIGHBOR_ENTRY_ATTR_NO_HOST_ROUTE;
neighbor_attr.value.booldata = 1;
neighbor_attrs.push_back(neighbor_attr);
}
}

MuxOrch* mux_orch = gDirectory.get<MuxOrch*>();
bool hw_config = isHwConfigured(neighborEntry);

Expand Down