Skip to content

Commit

Permalink
Merge branch 'master' into platform-vpp
Browse files Browse the repository at this point in the history
  • Loading branch information
lguohan authored Oct 30, 2024
2 parents f3765e8 + e394ced commit 58af271
Show file tree
Hide file tree
Showing 30 changed files with 708 additions and 35 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:
branches:
- 'master'
- '202[0-9][0-9][0-9]'
pull_request_target:
pull_request:
branches:
- 'master'
- '202[0-9][0-9][0-9]'
Expand Down Expand Up @@ -54,7 +54,7 @@ jobs:
libyang-dev \
libzmq3-dev \
libzmq5 \
swig3.0 \
swig4.0 \
libpython2.7-dev \
libgtest-dev \
libgmock-dev \
Expand Down
6 changes: 5 additions & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ AX_ADD_AM_MACRO_STATIC([])

AM_CONDITIONAL(SONIC_ASIC_PLATFORM_BAREFOOT, test x$CONFIGURED_PLATFORM = xbarefoot)
AM_CONDITIONAL(SONIC_ASIC_PLATFORM_BROADCOM, test x$CONFIGURED_PLATFORM = xbroadcom)
AM_CONDITIONAL(SONIC_ASIC_PLATFORM_MELLANOX, test x$CONFIGURED_PLATFORM = xmellanox)

AM_COND_IF([SONIC_ASIC_PLATFORM_MELLANOX],
AC_DEFINE([MELLANOX], [1], [Define to 1 on Mellanox Platform]))

AC_ARG_ENABLE(debug,
[ --enable-debug turn on debugging],
Expand Down Expand Up @@ -144,7 +148,7 @@ fi
AM_CONDITIONAL(ASAN_ENABLED, test x$asan_enabled = xtrue)
AM_CONDITIONAL(PYTHON2, test x$python2 = xtrue)

AC_PATH_PROGS(SWIG, [swig3.0 swig])
AC_PATH_PROGS(SWIG, [swig4.0 swig3.0 swig])

CXXFLAGS_COMMON=""
CXXFLAGS_COMMON+=" -ansi"
Expand Down
15 changes: 15 additions & 0 deletions lib/RedisRemoteSaiInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2049,6 +2049,21 @@ sai_switch_notifications_t RedisRemoteSaiInterface::syncProcessNotification(
return { };
}

bool RedisRemoteSaiInterface::containsSwitch(
_In_ sai_object_id_t switchId) const
{
SWSS_LOG_ENTER();

if (!m_switchContainer->contains(switchId))
{
SWSS_LOG_INFO("context %s failed to find switch %s",
m_contextConfig->m_name.c_str(), sai_serialize_object_id(switchId).c_str());
return false;
}

return true;
}

const std::map<sai_object_id_t, swss::TableDump>& RedisRemoteSaiInterface::getTableDump() const
{
SWSS_LOG_ENTER();
Expand Down
3 changes: 3 additions & 0 deletions lib/RedisRemoteSaiInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ namespace sairedis

const std::map<sai_object_id_t, swss::TableDump>& getTableDump() const;

bool containsSwitch(
_In_ sai_object_id_t switchId) const;

private: // QUAD API helpers

sai_status_t create(
Expand Down
12 changes: 9 additions & 3 deletions lib/Sai.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,19 @@ sai_status_t Sai::set(

// skip metadata if attribute is redis extension attribute

// TODO this is setting on all contexts, but maybe we want one specific?
// and do set on all if objectId == NULL

bool success = true;

// Setting on all contexts if objectType != SAI_OBJECT_TYPE_SWITCH or objectId == NULL
for (auto& kvp: m_contextMap)
{
if (objectType == SAI_OBJECT_TYPE_SWITCH && objectId != SAI_NULL_OBJECT_ID)
{
if (!kvp.second->m_redisSai->containsSwitch(objectId))
{
continue;
}
}

sai_status_t status = kvp.second->m_redisSai->set(objectType, objectId, attr);

success &= (status == SAI_STATUS_SUCCESS);
Expand Down
19 changes: 17 additions & 2 deletions proxylib/Proxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "meta/ZeroMQSelectableChannel.h"

#include "syncd/ZeroMQNotificationProducer.h"
#include "syncd/Workaround.h"

#include <inttypes.h>

Expand All @@ -35,7 +36,8 @@ Proxy::Proxy(
m_vendorSai(vendorSai),
m_options(options),
m_apiInitialized(false),
m_notificationsSentCount(0)
m_notificationsSentCount(0),
m_apiVersion(SAI_VERSION(0,0,0))
{
SWSS_LOG_ENTER();

Expand Down Expand Up @@ -79,6 +81,17 @@ Proxy::Proxy(

SWSS_LOG_NOTICE("api initialized success");
}

auto st = m_vendorSai->queryApiVersion(&m_apiVersion);

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_WARN("failed to obtain libsai api version: %s", sai_serialize_status(st).c_str());
}
else
{
SWSS_LOG_NOTICE("libsai api version: %lu", m_apiVersion);
}
}

Proxy::~Proxy()
Expand Down Expand Up @@ -1113,7 +1126,9 @@ void Proxy::onPortStateChange(
{
SWSS_LOG_ENTER();

auto s = sai_serialize_port_oper_status_ntf(count, data);
auto ntfdata = syncd::Workaround::convertPortOperStatusNotification(count, data, m_apiVersion);

auto s = sai_serialize_port_oper_status_ntf((uint32_t)ntfdata.size(), ntfdata.data());

sendNotification(SAI_SWITCH_NOTIFICATION_NAME_PORT_STATE_CHANGE, s);
}
Expand Down
2 changes: 2 additions & 0 deletions proxylib/Proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,5 +206,7 @@ namespace saiproxy
* notifications.
*/
uint64_t m_notificationsSentCount;

sai_api_version_t m_apiVersion;
};
}
4 changes: 4 additions & 0 deletions pyext/pysairedis.i
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
%include "cpointer.i"
%include "carrays.i"

// These objects cause issues on Buster because of the function pointers
%ignore _sai_struct_member_info_t;
%ignore _sai_object_type_info_t;

%{
#pragma GCC optimize("no-var-tracking-assignments")

Expand Down
15 changes: 15 additions & 0 deletions syncd/ComparisonLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3129,6 +3129,21 @@ void ComparisonLogic::applyViewTransition(
}
}

for (auto &obj: temp.m_soAll)
{
/*
* Make sure we will create all neighbor entries before next hop that
* have the same IP address as neighbor entry. In Broadcom platform
* neighbor entry needs to be created before next hop with the same ip
* address otherwise create next hop will fail (hardware limitation?).
*/

if (obj.second->getObjectType() == SAI_OBJECT_TYPE_NEIGHBOR_ENTRY)
{
processObjectForViewTransition(current, temp, obj.second);
}
}

for (auto &obj: temp.m_soAll)
{
if (obj.second->getObjectType() != SAI_OBJECT_TYPE_ROUTE_ENTRY)
Expand Down
26 changes: 23 additions & 3 deletions syncd/NotificationHandler.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "NotificationHandler.h"
#include "Workaround.h"
#include "sairediscommon.h"

#include "swss/logger.h"
Expand All @@ -10,8 +11,10 @@
using namespace syncd;

NotificationHandler::NotificationHandler(
_In_ std::shared_ptr<NotificationProcessor> processor):
m_processor(processor)
_In_ std::shared_ptr<NotificationProcessor> processor,
_In_ sai_api_version_t apiVersion):
m_processor(processor),
m_apiVersion(apiVersion)
{
SWSS_LOG_ENTER();

Expand All @@ -27,6 +30,21 @@ NotificationHandler::~NotificationHandler()
// empty
}

void NotificationHandler::setApiVersion(
_In_ sai_api_version_t apiVersion)
{
SWSS_LOG_ENTER();

m_apiVersion = apiVersion;
}

sai_api_version_t NotificationHandler::getApiVersion() const
{
SWSS_LOG_ENTER();

return m_apiVersion;
}

void NotificationHandler::setSwitchNotifications(
_In_ const sai_switch_notifications_t& switchNotifications)
{
Expand Down Expand Up @@ -93,7 +111,9 @@ void NotificationHandler::onPortStateChange(
{
SWSS_LOG_ENTER();

auto s = sai_serialize_port_oper_status_ntf(count, data);
auto ntfdata = Workaround::convertPortOperStatusNotification(count, data, m_apiVersion);

auto s = sai_serialize_port_oper_status_ntf((uint32_t)ntfdata.size(), ntfdata.data());

enqueueNotification(SAI_SWITCH_NOTIFICATION_NAME_PORT_STATE_CHANGE, s);
}
Expand Down
10 changes: 9 additions & 1 deletion syncd/NotificationHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ namespace syncd
public:

NotificationHandler(
_In_ std::shared_ptr<NotificationProcessor> processor);
_In_ std::shared_ptr<NotificationProcessor> processor,
_In_ sai_api_version_t apiVersion = SAI_VERSION(0,0,0));

virtual ~NotificationHandler();

Expand All @@ -35,6 +36,11 @@ namespace syncd
_In_ uint32_t attr_count,
_In_ sai_attribute_t *attr_list) const;

void setApiVersion(
_In_ sai_api_version_t apiVersion);

sai_api_version_t getApiVersion() const;

public: // members reflecting SAI callbacks

void onFdbEvent(
Expand Down Expand Up @@ -99,5 +105,7 @@ namespace syncd
std::shared_ptr<NotificationQueue> m_notificationQueue;

std::shared_ptr<NotificationProcessor> m_processor;

sai_api_version_t m_apiVersion;
};
}
47 changes: 39 additions & 8 deletions syncd/NotificationQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ NotificationQueue::NotificationQueue(
{
SWSS_LOG_ENTER();

// empty;
m_queue = std::make_shared<std::queue<swss::KeyOpFieldsValuesTuple>>();
}

NotificationQueue::~NotificationQueue()
Expand All @@ -34,7 +34,9 @@ bool NotificationQueue::enqueue(
MUTEX;

SWSS_LOG_ENTER();

bool candidateToDrop = false;

std::string currentEvent;

/*
Expand All @@ -49,8 +51,10 @@ bool NotificationQueue::enqueue(
* will also be dropped regardless of its event type to protect the device from crashing due to
* running out of memory
*/
auto queueSize = m_queue.size();
auto queueSize = m_queue->size();

currentEvent = kfvKey(item);

if (currentEvent == m_lastEvent)
{
m_lastEventCount++;
Expand All @@ -60,12 +64,15 @@ bool NotificationQueue::enqueue(
m_lastEventCount = 1;
m_lastEvent = currentEvent;
}

if (queueSize >= m_queueSizeLimit)
{
/* Too many queued up already check if notification fits condition to e dropped
/*
* Too many queued up already check if notification fits condition to e dropped
* 1. All FDB events should be dropped at this point.
* 2. All other notification events will start to drop if it reached the consecutive threshold limit
*/

if (currentEvent == SAI_SWITCH_NOTIFICATION_NAME_FDB_EVENT)
{
candidateToDrop = true;
Expand All @@ -81,7 +88,7 @@ bool NotificationQueue::enqueue(

if (!candidateToDrop)
{
m_queue.push(item);
m_queue->push(item);

return true;
}
Expand All @@ -106,14 +113,38 @@ bool NotificationQueue::tryDequeue(

SWSS_LOG_ENTER();

if (m_queue.empty())
if (m_queue->empty())
{
return false;
}

item = m_queue.front();
item = m_queue->front();

m_queue.pop();
m_queue->pop();

if (m_queue->empty())
{
/*
* Since there could be burst of notifications, that allocated memory
* can be over 2GB, but when queue will be drained that memory will not
* be automatically released. Underlying deque container contains
* function shrink_to_fit but that is just a request, and usually this
* function does nothing.
*
* Make sure we will destroy queue and allocate new one. Assignment
* operator is not enough here, since internal deque container will not
* release memory under assignment. While making sure queue is deleted
* all memory will be released.
*
* Downside of this approach is that even if we will have steady stream
* of single notifications, each time we will allocate new queue.
* Partial solution for this could allocating new queue only when
* previous queue exceeded some size limit, for example 128 items.
*/
m_queue = nullptr;

m_queue = std::make_shared<std::queue<swss::KeyOpFieldsValuesTuple>>();
}

return true;
}
Expand All @@ -124,5 +155,5 @@ size_t NotificationQueue::getQueueSize()

SWSS_LOG_ENTER();

return m_queue.size();
return m_queue->size();
}
5 changes: 3 additions & 2 deletions syncd/NotificationQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

extern "C" {
#include <sai.h>
#include<saimetadata.h>
#include <saimetadata.h>
}

#include "swss/table.h"

#include <queue>
#include <mutex>
#include <memory>

/**
* @brief Default notification queue size limit.
Expand Down Expand Up @@ -54,7 +55,7 @@ namespace syncd

std::mutex m_mutex;

std::queue<swss::KeyOpFieldsValuesTuple> m_queue;
std::shared_ptr<std::queue<swss::KeyOpFieldsValuesTuple>> m_queue;

size_t m_queueSizeLimit;

Expand Down
Loading

0 comments on commit 58af271

Please sign in to comment.