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

Make changes to support compiling on Bookworm (with GCC 12) #1344

Merged
merged 4 commits into from
Feb 2, 2024
Merged

Make changes to support compiling on Bookworm (with GCC 12) #1344

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e3f50e8
Update SAI submodule.
saiarcot895 Jan 26, 2024
2a286e3
Update for Bookworm (GCC 12) compilation support
saiarcot895 Jan 26, 2024
9bce29e
Add tests
saiarcot895 Jan 26, 2024
dad0ccd
Merge branch 'master' into fix-bookworm-compilation
saiarcot895 Jan 29, 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
2 changes: 1 addition & 1 deletion SAI
Open in desktop
Submodule SAI updated 16 files
+70 −0 doc/SAI-Proposal-L3-Rewrite-disable.md
+208 −0 doc/TAM/Granular-Counter-Subscription.md
+279 −0 doc/cable_diag/cable_diag.md
+ − doc/cable_diag/images/cable_diag.png
+ − doc/cable_diag/images/ocp.png
+ − doc/figures/sai-l3-pipeline.png
+1 −0 experimental/saiextensions.h
+61 −0 experimental/saiportextensions.h
+27 −0 inc/sainexthop.h
+92 −2 inc/saiport.h
+222 −56 inc/saitam.h
+1 −0 inc/saitypes.h
+2 −2 meta/Doxyfile
+1 −0 meta/acronyms.txt
+7 −1 meta/parse.pl
+38 −0 meta/test.pm
2 changes: 2 additions & 0 deletions lib/sai_redis_tam.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ REDIS_GENERIC_QUAD(TAM_TELEMETRY,tam_telemetry);
REDIS_GENERIC_QUAD(TAM_COLLECTOR,tam_collector);
REDIS_GENERIC_QUAD(TAM_EVENT_ACTION,tam_event_action);
REDIS_GENERIC_QUAD(TAM_EVENT,tam_event);
REDIS_GENERIC_QUAD(TAM_COUNTER_SUBSCRIPTION,tam_counter_subscription);

const sai_tam_api_t redis_tam_api = {

Expand All @@ -37,4 +38,5 @@ const sai_tam_api_t redis_tam_api = {
REDIS_GENERIC_QUAD_API(tam_collector)
REDIS_GENERIC_QUAD_API(tam_event_action)
REDIS_GENERIC_QUAD_API(tam_event)
REDIS_GENERIC_QUAD_API(tam_counter_subscription)
};
1 change: 1 addition & 0 deletions syncd/ServiceMethodTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "swss/logger.h"

#include <array>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this is added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GCC 12 complained that std::array was not completely defined, but is instantiated on line 74.

Snippet of error log:

ServiceMethodTable.cpp: In instantiation of 'constexpr auto declare_static(std::index_sequence<i ...>) [with B = syncd::ServiceMethodTable::SlotBase; D = syncd::ServiceMethodTable::Slot; long unsigned int ...i = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; std::index_sequence<i ...> = std::integer_sequence<long unsigned int, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9>]':
ServiceMethodTable.cpp:81:35:   required from 'constexpr auto declare_static() [with B = syncd::ServiceMethodTable::SlotBase; D = syncd::ServiceMethodTable::Slot; long unsigned int size = 10]'
ServiceMethodTable.cpp:86:79:   required from here
ServiceMethodTable.cpp:74:56: error: invalid use of incomplete type 'struct std::array<syncd::ServiceMethodTable::SlotBase*, 10>'
   74 |     return std::array<B*, sizeof...(i)>{{new D<i>()...}};
      |                                                        ^
In file included from /usr/include/c++/12/bits/stl_map.h:63,
                 from /usr/include/c++/12/map:61,
                 from /usr/include/swss/logger.h:6,
                 from ServiceMethodTable.h:7,
                 from ServiceMethodTable.cpp:1:
/usr/include/c++/12/tuple:1595:45: note: declaration of 'struct std::array<syncd::ServiceMethodTable::SlotBase*, 10>'
 1595 |   template<typename _Tp, size_t _Nm> struct array;
      |                                             ^~~~~
ServiceMethodTable.cpp: In instantiation of 'constexpr auto declare_static() [with B = syncd::ServiceMethodTable::SlotBase; D = syncd::ServiceMethodTable::Slot; long unsigned int size = 10]':
ServiceMethodTable.cpp:86:79:   required from here
ServiceMethodTable.cpp:82:48: error: no matching function for call to 'std::vector<syncd::ServiceMethodTable::SlotBase*>::vector(<brace-enclosed initializer list>)'
   82 |     return std::vector<B*>{arr.begin(), arr.end()};
      |                                         ~~~~~~~^~

Copy link
Collaborator

Choose a reason for hiding this comment

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

3hivh version cimpiler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GCC 12.2, specifically (also using libstdc++ 12.2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you want, I think I can modify the pipeline to build this on Bookworm as well, so that is covered.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Your title suggest tat this is the change so support bookworm so what other changes would be needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure at this point. At this point, only compilation (and the tests that get run as part of the build) have been done; runtime testing has not been done yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then please update title to reflect changes, like GCC latest support or update headers etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

#include <utility>

using namespace syncd;
Expand Down
1 change: 1 addition & 0 deletions syncd/SwitchNotifications.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "SwitchNotifications.h"

#include "swss/logger.h"
#include <array>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this isnadded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GCC 12 complained that std::array was not completely defined, but is instantiated on line 144.

Snippet of error log:

SwitchNotifications.cpp: In instantiation of 'constexpr auto declare_static(std::index_sequence<i ...>) [with B = syncd::SwitchNotifications::SlotBase; D = syncd::SwitchNotifications::Slot; long unsigned int ...i = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}; std::index_sequence<i ...> = std::integer_sequence<long unsigned int, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15>]':
SwitchNotifications.cpp:151:35:   required from 'constexpr auto declare_static() [with B = syncd::SwitchNotifications::SlotBase; D = syncd::SwitchNotifications::Slot; long unsigned int size = 16]'
SwitchNotifications.cpp:156:83:   required from here
SwitchNotifications.cpp:144:56: error: invalid use of incomplete type 'struct std::array<syncd::SwitchNotifications::SlotBase*, 16>'
  144 |     return std::array<B*, sizeof...(i)>{{new D<i>()...}};
      |                                                        ^
In file included from /usr/include/c++/12/bits/stl_map.h:63,
                 from /usr/include/c++/12/map:61,
                 from /usr/include/swss/logger.h:6,
                 from SwitchNotifications.h:7,
                 from SwitchNotifications.cpp:1:
/usr/include/c++/12/tuple:1595:45: note: declaration of 'struct std::array<syncd::SwitchNotifications::SlotBase*, 16>'
...


using namespace syncd;

Expand Down
5 changes: 5 additions & 0 deletions unittest/lib/test_sai_redis_tam.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ TEST(libsairedis, tam)
EXPECT_NE(SAI_STATUS_SUCCESS, api->remove_tam_event(0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->set_tam_event_attribute(0,0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->get_tam_event_attribute(0,0,0));

EXPECT_NE(SAI_STATUS_SUCCESS, api->create_tam_counter_subscription(&id,0,0,0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->remove_tam_counter_subscription(0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->set_tam_counter_subscription_attribute(0,0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->get_tam_counter_subscription_attribute(0,0,0));
}

TEST(libsairedis, sai_tam_telemetry_get_data)
Expand Down
5 changes: 5 additions & 0 deletions unittest/vslib/test_sai_vs_tam.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ TEST(libsaivs, tam)
EXPECT_NE(SAI_STATUS_SUCCESS, api->remove_tam_event(0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->set_tam_event_attribute(0,0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->get_tam_event_attribute(0,0,0));

EXPECT_NE(SAI_STATUS_SUCCESS, api->create_tam_counter_subscription(&id,0,0,0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->remove_tam_counter_subscription(0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->set_tam_counter_subscription_attribute(0,0));
EXPECT_NE(SAI_STATUS_SUCCESS, api->get_tam_counter_subscription_attribute(0,0,0));
}

TEST(libsairedis, sai_tam_telemetry_get_data)
Expand Down
2 changes: 2 additions & 0 deletions vslib/sai_vs_tam.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ VS_GENERIC_QUAD(TAM_TELEMETRY,tam_telemetry);
VS_GENERIC_QUAD(TAM_COLLECTOR,tam_collector);
VS_GENERIC_QUAD(TAM_EVENT_ACTION,tam_event_action);
VS_GENERIC_QUAD(TAM_EVENT,tam_event);
VS_GENERIC_QUAD(TAM_COUNTER_SUBSCRIPTION,tam_counter_subscription);

const sai_tam_api_t vs_tam_api = {

Expand All @@ -37,5 +38,6 @@ const sai_tam_api_t vs_tam_api = {
VS_GENERIC_QUAD_API(tam_collector)
VS_GENERIC_QUAD_API(tam_event_action)
VS_GENERIC_QUAD_API(tam_event)
VS_GENERIC_QUAD_API(tam_counter_subscription)

};
Loading