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.

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

Support ConfigDB neighbor configuration, introduce nbrmgr daemon #693

Merged
merged 6 commits into from
Nov 28, 2018

Conversation

prsunny
Copy link
Collaborator

@prsunny prsunny commented Nov 16, 2018

What I did
Initial commit to support ConfigDB neighbor table configurations

Why I did it
Requirement to resolve mac address for a configured neighbor IP. Also support static neighbor configuration

How I verified it
Logs:

Resolve case:

{
    "NEIGH": {
        "Vlan100|100.1.1.3": { 
            "family": "IPv4" 
        }
    }
}

19:01:02.216254 ARP, Request who-has 100.1.1.3 tell 100.1.1.1, length 28

Static Nbr:

{
    "NEIGH": {
        "Vlan100|100.1.1.5": { 
            "neigh": "00:02:02:03:04:05",
            "family": "IPv4" 
        }
    }
}

[admin@str-a7170-acs-1 ~ 08:10 PM]$ ip neigh show 
100.1.1.3 dev Vlan100  FAILED
100.1.1.5 dev Vlan100 lladdr 00:02:02:03:04:05 PERMANENT

Details if related
This is the initial commit for phase 1.
For phase 2:

  1. Combine nbrmgrd and neighsync.
  2. Address any priority items from Fix potential blackholing/looping traffic when link-local was used and refresh ipv6 neighbor to avoid CPU hit sonic-buildimage#1904 (comment)

cfgmgr/nbrmgr.cpp Outdated Show resolved Hide resolved
cfgmgr/nbrmgr.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@pavel-shirshov pavel-shirshov left a comment

Choose a reason for hiding this comment

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

Is it possible to use libnl3 for netlink programming?

Copy link
Contributor

@pavel-shirshov pavel-shirshov left a comment

Choose a reason for hiding this comment

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

as comments

cfgmgr/nbrmgr.cpp Outdated Show resolved Hide resolved
cfgmgr/nbrmgr.cpp Outdated Show resolved Hide resolved
cfgmgr/nbrmgr.cpp Outdated Show resolved Hide resolved
cfgmgr/nbrmgr.cpp Show resolved Hide resolved
cfgmgr/nbrmgr.cpp Show resolved Hide resolved
cfgmgr/nbrmgr.cpp Show resolved Hide resolved
Copy link
Contributor

@jipanyang jipanyang left a comment

Choose a reason for hiding this comment

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

In the case of mac address of the neighbor is empty, how would neighmgr handle warm restart/warm reboot scenarios? Some cooperation with neighsyncd seems necessary.

cfgmgr/nbrmgr.cpp Show resolved Hide resolved
cfgmgr/nbrmgr.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@pavel-shirshov pavel-shirshov left a comment

Choose a reason for hiding this comment

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

Approve from my side. Please check Jipans' comments

cfgmgr/nbrmgr.cpp Show resolved Hide resolved
@prsunny
Copy link
Collaborator Author

prsunny commented Nov 21, 2018

@jipanyang, as mentioned in description, plan is to merge neighsyncd and neighbor manager in next phase. I'll consider the warm-restart scenarios along with that. For the current use-case, we can skip the warm-restart requirements.

@prsunny prsunny merged commit 705b092 into sonic-net:master Nov 28, 2018
@prsunny prsunny deleted the nbrmgr branch November 28, 2018 06:25
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
* Per-switching silicon Common config for Broadcom Supported Platforms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants