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.

Sign up for GitHub

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

add vrrpmgrd to support FRR-VRRP configuration #3106

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

add vrrpmgrd to support FRR-VRRP configuration #3106

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d58a143
add vrrpmgrd to support FRR-VRRP configuration
philo-micas Apr 8, 2024
72a3fae
Merge branch 'master' into master-Vrrp
philo-micas Jul 23, 2024
4debad9
Merge branch 'master' into master-Vrrp
philo-micas Jul 26, 2024
007657f
Change the name of vrrpmgrd to macvlanmgrd
philo-micas Jul 30, 2024
30d27c3
Merge branch 'master' into master-Vrrp
philo-micas Oct 9, 2024
fead7c7
Merge branch 'master' into master-Vrrp
philo-micas Oct 11, 2024
e719be1
Merge branch 'master' into master-Vrrp
philo-micas Oct 21, 2024
3b15de8
Merge branch 'master' into master-Vrrp
philo-micas Oct 23, 2024
8c61423
Revert "Change the name of vrrpmgrd to macvlanmgrd"
philo-micas Oct 23, 2024
7bbd64a
Update .gitignore
philo-micas Oct 25, 2024
2dcfe3f
Merge branch 'master' into master-Vrrp
philo-micas Oct 29, 2024
e435f24
Merge branch 'master' into master-Vrrp
philo-micas Oct 31, 2024
8e1b602
Merge branch 'master' into master-Vrrp
philo-micas Nov 5, 2024
fb3a6a1
Merge branch 'master' into master-Vrrp
philo-micas Nov 21, 2024
e938f6a
Merge branch 'master' into master-Vrrp
philo-micas Nov 25, 2024
9e17275
Update test_vrrp.py
philo-micas Nov 25, 2024
2e16b74
Update test_vrrp.py
philo-micas Nov 26, 2024
25476ec
Update test_vrrp.py
philo-micas Nov 26, 2024
582a9e6
Update test_vrrp.py
philo-micas Nov 26, 2024
61b2f4e
Merge branch 'master' into master-Vrrp
philo-micas Nov 26, 2024
d73ab54
Update test_vrrp.py
philo-micas Nov 26, 2024
792d1ae
Merge branch 'master' into master-Vrrp
philo-micas Nov 27, 2024
31d2eb8
Merge branch 'master' into master-Vrrp
philo-micas Nov 27, 2024
e7e9aac
Merge branch 'master' into master-Vrrp
philo-micas Dec 3, 2024
6710321
support vrf
philo-micas Dec 26, 2024
5f0cfe5
Merge branch 'master' into master-Vrrp
philo-micas Dec 26, 2024
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ cfgmgr/sflowmgrd
cfgmgr/macsecmgrd
cfgmgr/coppmgrd
cfgmgr/tunnelmgrd
cfgmgr/macvlanmgrd
fpmsyncd/fpmsyncd
gearsyncd/gearsyncd
mclagsyncd/mclagsyncd
Expand Down
9 changes: 8 additions & 1 deletion cfgmgr/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LIBNL_LIBS = -lnl-genl-3 -lnl-route-3 -lnl-3
SAIMETA_LIBS = -lsaimeta -lsaimetadata -lzmq
COMMON_LIBS = -lswsscommon -lpthread

bin_PROGRAMS = vlanmgrd teammgrd portmgrd intfmgrd buffermgrd vrfmgrd nbrmgrd vxlanmgrd sflowmgrd natmgrd coppmgrd tunnelmgrd macsecmgrd fabricmgrd
bin_PROGRAMS = vlanmgrd teammgrd portmgrd intfmgrd buffermgrd vrfmgrd nbrmgrd vxlanmgrd sflowmgrd natmgrd coppmgrd tunnelmgrd macsecmgrd fabricmgrd macvlanmgrd
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer vrrpmgrd, looks like we used macvlanmgrd in the HLD, please make the changes if possible.

Copy link
Author

Choose a reason for hiding this comment

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

sonic-net/SONiC#1446 (comment)

As suggested by @nser77, renaming vrrpmgrd to macvlanmgrd would enhance its generality within the whole SONiC architecture, making it more convenient for future functional components to use the macvlan interface type.

Copy link
Contributor

Choose a reason for hiding this comment

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

What future functional components you are referring to here? the whole code is written for VRRP, wondering how you would use this macvlanmgr for other features.

Copy link
Author

Choose a reason for hiding this comment

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

In VRRP, macvlanmgrd manages the configurations of adding, deleting, and addressing for macvlan devices based on VRRP configurations. Although at present only vrrp is using macvlan devices, as long as there is a similar function like VRRP which needs to use macvlan devices, you can subscribe to the corresponding table entries in macvlanmagrd, and then a simple adaptation can realize the unified management of macvlan devices.

Copy link
Contributor

@venkatmahalingam venkatmahalingam Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not convinced with your answer, I don't see any mention of future functional components you are referring to here. You are trying to make the code changes with the assumption that some feature will make use of this macvlanmgrd.
If you really want to have a generic macvlanmgrd, remove VRRP name usage in the files and use it as a generic library so that any future module can use it, today VRRP feature can make use of generic MACVLAN mgr.

All I'm trying to say here is, either have the generic MACVLAN mgr and use it for the VRRP for now or specific VRRP mgr but not mixed.

Copy link
Author

Choose a reason for hiding this comment

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

Yes，you are right. We will use vrrpmgrd and have also submitted the PR for HLD sonic-net/SONiC#1834. Could you please review it. Thanks.

Copy link

Choose a reason for hiding this comment

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

Mmm, I agree with both of you.


cfgmgrdir = $(datadir)/swss

Expand Down Expand Up @@ -101,6 +101,11 @@ macsecmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CF
macsecmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN)
macsecmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS)

macvlanmgrd_SOURCES = macvlanmgrd.cpp macvlanmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h $(top_srcdir)/lib/vrrpintf.cpp
macvlanmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN)
macvlanmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN)
macvlanmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS)

if GCOV_ENABLED
vlanmgrd_SOURCES += ../gcovpreload/gcovpreload.cpp
teammgrd_SOURCES += ../gcovpreload/gcovpreload.cpp
Expand All @@ -116,6 +121,7 @@ natmgrd_SOURCES += ../gcovpreload/gcovpreload.cpp
coppmgrd_SOURCES += ../gcovpreload/gcovpreload.cpp
tunnelmgrd_SOURCES += ../gcovpreload/gcovpreload.cpp
macsecmgrd_SOURCES += ../gcovpreload/gcovpreload.cpp
macvlanmgrd_LDADD += ../gcovpreload/gcovpreload.cpp
endif

if ASAN_ENABLED
Expand All @@ -133,5 +139,6 @@ coppmgrd_SOURCES += $(top_srcdir)/lib/asan.cpp
tunnelmgrd_SOURCES += $(top_srcdir)/lib/asan.cpp
macsecmgrd_SOURCES += $(top_srcdir)/lib/asan.cpp
fabricmgrd_SOURCES += $(top_srcdir)/lib/asan.cpp
macvlanmgrd_SOURCES += $(top_srcdir)/lib/asan.cpp
endif

Loading
Loading