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

[frr]: change frr as default sonic routing stack #2863

Merged
merged 4 commits into from
May 8, 2019

Conversation

lguohan
Copy link
Collaborator

@lguohan lguohan commented May 3, 2019

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@lguohan
Copy link
Collaborator Author

lguohan commented May 4, 2019

retest this please

Copy link
Collaborator

@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.

it is good we are moving to FRR. A few issues which may need some attention after this change.

<1> no supervisord in frr BGP docker as all other dockers. It is using docker entry point.

<2> zebra/bgpd will be restarted automatically by watchfrr. it will affect warm reboot function.

<3> bgp graceful-restart preserve-fw-state is off by default. It is hardcoded as enable in current sonic Quagga.

<4> Need to evaluate BGP “update-delay MAX-DELAY ESTABLISH-WAIT” configuration.

@jipanyang
Copy link
Collaborator

It looks VS docker change is needed to use frr? supervisord.conf is hardcoded with quagga path.

@lguohan
Copy link
Collaborator Author

lguohan commented May 7, 2019

current pr won't work in case the routing stack is quagga, need to figure out a way to support both quagga and frr.

@lguohan
Copy link
Collaborator Author

lguohan commented May 7, 2019

@jipanyang , for first 2, check #2870

Signed-off-by: Guohan Lu <gulv@microsoft.com>
@lguohan lguohan merged commit f35daa7 into sonic-net:master May 8, 2019
@zhenggen-xu
Copy link
Collaborator

@jipanyang Can you elaborate this "zebra/bgpd will be restarted automatically by watchfrr. it will affect warm reboot function."? What exactly the impact?
There were reasons the daemons were restarted: crashed or hang etc. And I assume we want the daemons to be restarted to recover. They are agnostic to warm-restart/reboot.

In case BGP restart, zebra is supposed to remove all routes from kernel and SONiC. This is BGP feature gap that if we need support unplanned warm restart.

In case zebra crash/restart, routes were not removed and the fpmsyncd will do the reconciliation later once zebra is back.

@jipanyang
Copy link
Collaborator

@zhenggen-xu Currently bgpd and zebra are killed explicitly for warm reboot. If watchfrr is there, change may be needed to take watchfrr into account.

# Kill bgpd to start the bgp graceful restart procedure
debug "Stopping bgp ..."
docker exec -i bgp pkill -9 zebra
docker exec -i bgp pkill -9 bgpd || [ $? == 1 ]
debug "Stopped  bgp ..."

MichelMoriniaux pushed a commit to criteo-forks/sonic-buildimage that referenced this pull request May 28, 2019
* [frr]: change frr as default sonic routing stack

* fix quagga configuration

* [vstest]: fix bgp test for frr

* [vstest]: skip bgp/test_invalid_nexthop.py for frr

Signed-off-by: Guohan Lu <gulv@microsoft.com>
mssonicbld added a commit that referenced this pull request Jun 13, 2023
…atically (#15422)

#### Why I did it
src/sonic-utilities
```
* 1246bc81 - (HEAD -> 202211, origin/202211) [config reload]Config Reload Enhancement (#2693) (#2863) (2 days ago) [Sudharsan Dhamal Gopalarathnam]
* d69aae4d - [vlan][dhcp_relay] Clear dhcpv6 relay counter while deleting vlan (#2852) (2 days ago) [Yaqiang Zhu]
* 0f6bf8ac - [config]: Dynamically start and stop ndppd (#2814) (2 days ago) [Lawrence Lee]
* 48a63ff1 - Fix issue: out of range sflow polling interval is accepted and stored in config_db (#2847) (2 days ago) [Junchao-Mellanox]
```
#### How I did it
#### How to verify it
#### Description for the changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants