-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[build]: Adding support for Free-Range-Routing stack. #510
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments about the files checked in in this pull requests.
dockers/docker-fpm-frr/Dockerfile
Outdated
|
||
ENTRYPOINT /usr/bin/config.sh \ | ||
&& /usr/bin/start.sh \ | ||
&& /bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dockerfile.j2 is already in the commit. The generated Dockerfile is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dockers/docker-fpm-quagga/Dockerfile
Outdated
|
||
ENTRYPOINT /usr/bin/config.sh \ | ||
&& /usr/bin/start.sh \ | ||
&& /bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this file. We could add Dockerfile to .gitignore if Dockerfile.j2 exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
rules/frr.mk
Outdated
FRR_VERSION = 3.1 | ||
export FRR_VERSION | ||
|
||
FRR = frr_$(FRR_VERSION)-dev_amd64.deb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-dev [](start = 24, length = 4)
why the package name has dev? usually dev pacakge is for the library development files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was originally pulling from dev/master branch, but i just changed to 'stable', so just got rid of 'dev' string.
$(DOCKER_FPM_FRR)_PATH = $(DOCKERS_PATH)/docker-fpm-frr | ||
$(DOCKER_FPM_FRR)_DEPENDS += $(FRR) $(SWSS) | ||
$(DOCKER_FPM_FRR)_LOAD_DOCKERS += $(DOCKER_CONFIG_ENGINE) | ||
SONIC_DOCKER_IMAGES += $(DOCKER_FPM_FRR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this as you have remove this in docker-fpm-gobgp.mk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the rules/docker-fpm-gobgp.mk, you have removed "SONIC_DOCKER_IMAGES += $(DOCKER_FPM_GOBGP)", I am wondering why you still keep this in docker-fpm-frr.mk
rules/docker-fpm-frr.mk
Outdated
|
||
$(DOCKER_FPM_FRR)_CONTAINER_NAME = bgp | ||
$(DOCKER_FPM_FRR)_RUN_OPT += --net=host --privileged -t | ||
$(DOCKER_FPM_FRR)_RUN_OPT += --volumes-from database |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need to get volumes-from database now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understood, done
dockers/docker-fpm-frr/start.sh
Outdated
@@ -0,0 +1,5 @@ | |||
#!/bin/bash | |||
|
|||
service rsyslog start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest to add rm -f /var/run/rsyslogd.pid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
! | ||
{% block bgp_init %} | ||
! | ||
! bgp multiple-instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should fpm-frr use same configuration as quagga or should it have a separate configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently config-CLIs for Quagga and FRR are not identical (to say the least) -- there's an increasing feature gap. Overtime, as more features are added to FRR, this gap will become larger, so I would definitely vote for having different 'j2' templates for both routing-stacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok for me.
src/sonic-frr/Makefile
Outdated
$(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% : | ||
# Cloning FRR repo if not already done | ||
if [ -d "~/frr" ]; then \ | ||
git clone https://github.com/FRRouting/frr.git; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to checkout a tag or this master branch, in this case the nightly build will be different as the frr moves forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my latest patch (to be submitted soon), I'm now pulling frr's "stable/3.0" branch.
@@ -0,0 +1,24 @@ | |||
.ONESHELL: | |||
SHELL = /bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to call it sonic-frr, so just frr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my current proposal, 'sonic-frr' is the wrapper folder containing the SONiC specific information required to build frr. Whereas 'frr' subfolder contains the cloned frr repo. Please let me know if you have any other suggestion for the naming convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine by me.
- Extending SONiC building infrastructure to provide users with greater flexibility, by allowing them to elect a routing-stack different than the default one (quagga). The desired routing-stack will be defined in rules/config file. - As part of these changes I'm adding support for Free-Range-Routing (FRR) stack. Quagga will continue to be the default routing-stack. Signed-off-by: Rodny Molina <rodny@linkedin.com>
Thanks for the PR. |
We are just users of SONiC but prefer FRR as the routing stack. What are our options?
Appreciate your thoughts. |
Right, AFAIK we are only building quagga-based images in the official sonic-jenkins page. The easiest way for you to go here is to build your own sonic-image with FRR routing-stack enabled.
Answering your second question: yes, you can switch from one routing-stack to the other at runtime, but be aware that this is not officially supported and that it will definitely have an impact on your service during the time you remove the old bgp container and you re-create the new one. From the host linux-shell in your sonic device do:
$ docker stop bgp
$ docker rm bgp
$ docker load < docker-fpm-frr.gz [ you will find this docker-image in your build-server at sonic-buildimage/target/ folder ]
$ sudo sed -i 's/bgp docker-fpm-quagga/bgp docker-fpm-frr/g' /usr/bin/bgp.sh
$ sudo /usr/bin/bgp.sh start
That will recreate a new bgp container with frr as the routing-stack.
/Rodny
On Mar 28, 2018, at 12:50 PM, Ragsboss <notifications@github.com<mailto:notifications@github.com>> wrote:
We are just users of SONiC but prefer FRR as the routing stack. What are our options?
Build our own SONiC image with FRR routing stack - this is non-ideal because we want to use a stock image, not something we build on our own and who knows what differences our image will have.. plus we need to keep in sync with changes upstream etc - too costly
Switch the routing stack at runtime - is this possible, supported and tested? Is there a documentation for this?
Any other way?
Appreciate your thoughts.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fsonic-buildimage%2Fpull%2F510%23issuecomment-377013674&data=02%7C01%7C%7C95197b3dea754c6fa2b608d594e52b47%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636578634369822362&sdata=xe8KX0YI8bvR%2Fp6xjsxAuS5o5UGMIi3YQgxlxReC9ss%3D&reserved=0>, or mute the thread<https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAQit1b18GE_3qn_2IWAr6t6r05c5HWtLks5ti-mIgaJpZM4M-tX-&data=02%7C01%7C%7C95197b3dea754c6fa2b608d594e52b47%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636578634369978591&sdata=AkEltKS8tRxRL2Y%2B%2FLsht1oFKfCkSPau7%2FcP1HUEI%2FY%3D&reserved=0>.
|
Thank you @rodnymolina. By runtime, I really meant as a one-time setup thing before the switch is put into service. I believe it's fair to say switching routing stacks causes disruption. Its good to know there is a way to do this but can you elaborate on differences between choosing BGP stack at image build time vs. afterwards like you described above. I ask specifically because your note was cautionary - not sure if this is because FRR in general is not as tested as Quagga or due to some differences between binding FRR at image time vs. later. |
Submodule src/sonic-utilities 6aee909..79a0185: > [fast/warm reboot] add some sanity check before warm reboot (sonic-net#510) > In sync with our latest change, where we default failthrough to be False. (sonic-net#507) > [generate_dump] system dump improvements (sonic-net#503) Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Submodule src/sonic-utilities 6ee0aea..b531934: > [db migrator] Introduce the DB migration infrastructure (sonic-net#519) > Skip INTERFACE entries w/o prefix (sonic-net#477) > Bring queue storm status to 'pfcwd show stats' (sonic-net#500) > Align PSU DB count field with the schema Spec. (sonic-net#509) > [scripts] remove duplicate script copying for nbrshow (sonic-net#517) > If fast-reboot-dump gives an error, don't continue with fast-reboot (sonic-net#515) > load_minigraph: restart hostcfgd (sonic-net#511) > [fast/warm reboot] add some sanity check before warm reboot (sonic-net#510) > show BPS, PPS, UTIL rates w/o previous clear (sonic-net#508) > In sync with our latest change, where we default failthrough to be False. (sonic-net#507) > Add warm-boot feature processing for wedge100bf_32x/65x platforms (sonic-net#485) > [generate_dump] system dump improvements (sonic-net#503) > [neighbor advertiser] convert int to string before concatenating (sonic-net#505) Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Submodule src/sonic-utilities 6ee0aea..b531934: > [db migrator] Introduce the DB migration infrastructure (#519) > Skip INTERFACE entries w/o prefix (#477) > Bring queue storm status to 'pfcwd show stats' (#500) > Align PSU DB count field with the schema Spec. (#509) > [scripts] remove duplicate script copying for nbrshow (#517) > If fast-reboot-dump gives an error, don't continue with fast-reboot (#515) > load_minigraph: restart hostcfgd (#511) > [fast/warm reboot] add some sanity check before warm reboot (#510) > show BPS, PPS, UTIL rates w/o previous clear (#508) > In sync with our latest change, where we default failthrough to be False. (#507) > Add warm-boot feature processing for wedge100bf_32x/65x platforms (#485) > [generate_dump] system dump improvements (#503) > [neighbor advertiser] convert int to string before concatenating (#505) Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Submodule src/sonic-utilities 6ee0aea..b531934: > [db migrator] Introduce the DB migration infrastructure (sonic-net#519) > Skip INTERFACE entries w/o prefix (sonic-net#477) > Bring queue storm status to 'pfcwd show stats' (sonic-net#500) > Align PSU DB count field with the schema Spec. (sonic-net#509) > [scripts] remove duplicate script copying for nbrshow (sonic-net#517) > If fast-reboot-dump gives an error, don't continue with fast-reboot (sonic-net#515) > load_minigraph: restart hostcfgd (sonic-net#511) > [fast/warm reboot] add some sanity check before warm reboot (sonic-net#510) > show BPS, PPS, UTIL rates w/o previous clear (sonic-net#508) > In sync with our latest change, where we default failthrough to be False. (sonic-net#507) > Add warm-boot feature processing for wedge100bf_32x/65x platforms (sonic-net#485) > [generate_dump] system dump improvements (sonic-net#503) > [neighbor advertiser] convert int to string before concatenating (sonic-net#505) Signed-off-by: Ying Xie <ying.xie@microsoft.com>
…t#510) * [fast/warm reboot] carve out variable setup code to a function Signed-off-by: Ying Xie <ying.xie@microsoft.com> * [fast/warm reboot] reorder code so that we have a clear main start point - Parse option before checking privilege so all users could get help information. Signed-off-by: Ying Xie <ying.xie@microsoft.com> * [fast/warm reboot] add some reboot pre-checks - Make sure that /host has enough space and write-able. - Make sure the next image is available. Signed-off-by: Ying Xie <ying.xie@microsoft.com> * fix error message
update multiDB changes in sairedis, including: [MultiDB]: repalce old APIs with New APIs incuding testing (#537) [syncd] Move port map and port map parser to proper class (#542) [syncd] Update syncd to expect correct object query message (#548) Support for snat available entry switch attribute in vslib (needed to run vs pytests) (#546) Move apiInitialized flag to Globals namespace (#545) Move command line options and parser to separate classes (#541) Move ntf_queue to proper NotificationQueue class (#539) Move api mutex to global class and add sairedis namespace (#544) Clean sairedis.h header (#543) Fix mlnx.pl script for perl include local directory (#540) Layer 2 Forwarding Enhancements (#510) Signed-off-by: Dong Zhang d.zhang@alibaba-inc.com
Added code changes related to Layer 2 Forwarding Enhancement: As part of L2 enhancements, per port, per vlan and per port-vlan fdb flush support has been added. The fdb flush response from SAI comes as individual mac delete as mentioned in the HLD. The change here is to follow the same way for VS as well. Regarding SAI_FDB_EVENT_FLUSHED, it is handled in sync only for 'flush all' case and handling SAI_FDB_EVENT_FLUSHED for per port and per port-vlan will require hgetall on all fdb keys. So, individual mac delete response is preferred. Another change is to move fdb handling to fdborch. As the fdb reference count is now maintained in fdborch as well, need to move the sai redis fdb handling also there to avoid the reference counts to go out of sync between the two.
> Add a new field for FLEX_COUNTER_TABLE to indicate delay for flex counters (sonic-net#523) > [logger] Make map access thread safe and proper terminate thread (sonic-net#510) > Test with coverage output, publish gcov on Azure pipelines (sonic-net#517) > Update schema.h to include config_db DHCP and state_db counter table (sonic-net#521) > [schema] Add next hop group table to schema (sonic-net#475) > Fix: DBInterface::get() return nullable strings (sonic-net#516) > [netlink]refill netlink cache when failing to get the link object by name. (sonic-net#506) > Fix test_ConfigDBSubscribe timing risk (sonic-net#512) > added missing headers for building with gcc-10.3 (sonic-net#494) > Modify the hardcode separator ":" to variable. (sonic-net#473)
508202b 2021-08-24 Add a new field for FLEX_COUNTER_TABLE to indicate delay for flex counters (#523) 9fd7dbf 2021-08-20 [logger] Make map access thread safe and proper terminate thread (#510) e4c3d0b 2021-08-20 Test with coverage output, publish gcov on Azure pipelines (#517) ef21bec 2021-08-18 Update schema.h to include config_db DHCP and state_db counter table (#521) 4e4eb9d 2021-08-19 [schema] Add next hop group table to schema (#475) Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
… automatically (#20670) #### Why I did it src/sonic-platform-common ``` * 59babf5 - (HEAD -> master, origin/master, origin/HEAD) Add/modify VDM and Status related cmis fields for onboarding xcvr diagnostic features (#510) (3 hours ago) [mihirpat1] ``` #### How I did it #### How to verify it #### Description for the changelog
… automatically (#20669) #### Why I did it src/sonic-platform-common ``` * 075da80 - (HEAD -> 202405, origin/202405) Add/modify VDM and Status related cmis fields for onboarding xcvr diagnostic features (#510) (61 minutes ago) [mihirpat1] ``` #### How I did it #### How to verify it #### Description for the changelog
… automatically (sonic-net#20670) #### Why I did it src/sonic-platform-common ``` * 59babf5 - (HEAD -> master, origin/master, origin/HEAD) Add/modify VDM and Status related cmis fields for onboarding xcvr diagnostic features (sonic-net#510) (3 hours ago) [mihirpat1] ``` #### How I did it #### How to verify it #### Description for the changelog
… automatically (sonic-net#20670) #### Why I did it src/sonic-platform-common ``` * 59babf5 - (HEAD -> master, origin/master, origin/HEAD) Add/modify VDM and Status related cmis fields for onboarding xcvr diagnostic features (sonic-net#510) (3 hours ago) [mihirpat1] ``` #### How I did it #### How to verify it #### Description for the changelog
Signed-off-by: Rodny Molina rodny@linkedin.com