Skip to content

Commit

Permalink
Virtualize System::Layer interfaces (#9366)
Browse files Browse the repository at this point in the history
* Virtualize System::Layer interfaces.

#### Problem

Issue #7715 _Virtualize System and Inet interfaces_

#### Change overview

- Establishes an abstract `System::Layer` class hierarchy, described below.
- Converts the transitional `WatchableEventManager` classes into
  corresponding `LayerImpl` classes.
- Move `LwIPEventHandlerDelegate` into `LayerLwIP`.
- Remove `GetClock()`, which was only used in one file.
  (`System::Clock` is currently fully static, but might benefit from its
  own virtualization, e.g. for test timeouts.)
- For the moment, `CHIP_SYSTEM_CONFIG_USE_DISPATCH` is still included
  by `#if` on `LayerImplSelect`. With some changes to Inet, Dispatch can
  probably have its own much simpler implementation of `LayerSockets`.

The class hierarchy is:
- `Layer`: Core timer methods.
  - `LayerLwIP`: Adds methods specific to builds using
    `CHIP_SYSTEM_CONFIG_USING_LWIP`.
    - `LayerImplLwIP`: Concrete implementation of `LayerLwIP`.
      (This is currently used by all LwIP-based platforms.)
  - `LayerSockets`: Adds I/O event methods specific to builds using
    `CHIP_SYSTEM_CONFIG_USING_SOCKETS`.
    - `LayerSocketsLoop`: Adds methods for event-loop-based
       implementations.
        - `LayerImplSelect`: Concrete implementation of `LayerSocketLoop`,
           using `select()`. (This is currenly used by all sockets-based
           platforms.)
        - `LayerImplLibevent`: Concrete implementation of
          `LayerSocketLoop`, using `libevent`. (This is currenly a
          demonstration of substitutability, not used by any platform.)

#### Testing

CI; no changes to functionality. Tests updated where necessary to create
concrete `LayerImpl` objects.

* remove debug code from SystemPacketBuffer.h
  • Loading branch information
kpschoedel authored and pull[bot] committed Sep 22, 2021
1 parent 35a0f1e commit 1213432
Show file tree
Hide file tree
Showing 44 changed files with 530 additions and 719 deletions.
2 changes: 1 addition & 1 deletion examples/platform/nrfconnect/util/test/TestInetCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
using namespace chip;
using namespace chip::Inet;

System::Layer gSystemLayer;
System::LayerImpl gSystemLayer;

Inet::InetLayer gInet;

Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestCommandInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <protocols/interaction_model/Constants.h>
#include <protocols/secure_channel/MessageCounterManager.h>
#include <protocols/secure_channel/PASESession.h>
#include <system/SystemLayerImpl.h>
#include <system/SystemPacketBuffer.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -48,7 +49,7 @@
#include <nlunit-test.h>

namespace chip {
static System::Layer gSystemLayer;
static System::LayerImpl gSystemLayer;
static SecureSessionMgr gSessionManager;
static Messaging::ExchangeManager gExchangeManager;
static TransportMgr<Transport::UDP> gTransportManager;
Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestEventLogging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <platform/CHIPDeviceLayer.h>
#include <protocols/secure_channel/MessageCounterManager.h>
#include <protocols/secure_channel/PASESession.h>
#include <system/SystemLayerImpl.h>
#include <system/SystemPacketBuffer.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -55,7 +56,7 @@ static const uint32_t kLivenessChangeEvent = 1;
static const chip::EndpointId kTestEndpointId = 2;
static const uint64_t kLivenessDeviceStatus = chip::TLV::ContextTag(1);
static chip::TransportMgr<chip::Transport::UDP> gTransportManager;
static chip::System::Layer gSystemLayer;
static chip::System::LayerImpl gSystemLayer;

static uint8_t gDebugEventBuffer[128];
static uint8_t gInfoEventBuffer[128];
Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestInteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <platform/CHIPDeviceLayer.h>
#include <protocols/secure_channel/MessageCounterManager.h>
#include <protocols/secure_channel/PASESession.h>
#include <system/SystemLayerImpl.h>
#include <system/SystemPacketBuffer.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -43,7 +44,7 @@
#include <nlunit-test.h>

namespace {
static chip::System::Layer gSystemLayer;
static chip::System::LayerImpl gSystemLayer;
static chip::SecureSessionMgr gSessionManager;
static chip::Messaging::ExchangeManager gExchangeManager;
static chip::secure_channel::MessageCounterManager gMessageCounterManager;
Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestReportingEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <platform/CHIPDeviceLayer.h>
#include <protocols/secure_channel/MessageCounterManager.h>
#include <protocols/secure_channel/PASESession.h>
#include <system/SystemLayerImpl.h>
#include <system/SystemPacketBuffer.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -44,7 +45,7 @@
#include <nlunit-test.h>

namespace chip {
static System::Layer gSystemLayer;
static System::LayerImpl gSystemLayer;
static SecureSessionMgr gSessionManager;
static Messaging::ExchangeManager gExchangeManager;
static TransportMgr<Transport::UDP> gTransportManager;
Expand Down
16 changes: 8 additions & 8 deletions src/controller/java/CHIPDeviceController-JNI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include <protocols/Protocols.h>
#include <protocols/temp_zcl/TempZCL.h>
#include <pthread.h>
#include <system/SystemLayerImpl.h>

// Choose an approximation of PTHREAD_NULL if pthread.h doesn't define one.
#ifndef PTHREAD_NULL
Expand Down Expand Up @@ -83,7 +84,7 @@ static CHIP_ERROR N2J_Error(JNIEnv * env, CHIP_ERROR inErr, jthrowable & outEx);
namespace {

JavaVM * sJVM;
System::Layer sSystemLayer;
System::LayerImpl sSystemLayer;
Inet::InetLayer sInetLayer;

#if CONFIG_NETWORK_LAYER_BLE
Expand Down Expand Up @@ -200,7 +201,7 @@ void JNI_OnUnload(JavaVM * jvm, void * reserved)
if (sIOThread != PTHREAD_NULL)
{
sShutdown = true;
sSystemLayer.WatchableEventsManager().Signal();
sSystemLayer.Signal();

StackUnlockGuard unlockGuard(JniReferences::GetInstance().GetStackLock());
pthread_join(sIOThread, NULL);
Expand Down Expand Up @@ -963,18 +964,17 @@ void * IOThreadMain(void * arg)
// Lock the stack to prevent collisions with Java threads.
pthread_mutex_lock(JniReferences::GetInstance().GetStackLock());

System::WatchableEventManager & watchState = sSystemLayer.WatchableEventsManager();
watchState.EventLoopBegins();
sSystemLayer.EventLoopBegins();

// Loop until we are told to exit.
while (!quit.load(std::memory_order_relaxed))
{
watchState.PrepareEvents();
sSystemLayer.PrepareEvents();

// Unlock the stack so that Java threads can make API calls.
pthread_mutex_unlock(JniReferences::GetInstance().GetStackLock());

watchState.WaitForEvents();
sSystemLayer.WaitForEvents();

// Break the loop if requested to shutdown.
// if (sShutdown)
Expand All @@ -983,9 +983,9 @@ void * IOThreadMain(void * arg)
// Re-lock the stack.
pthread_mutex_lock(JniReferences::GetInstance().GetStackLock());

watchState.HandleEvents();
sSystemLayer.HandleEvents();
}
watchState.EventLoopEnds();
sSystemLayer.EventLoopEnds();

// Detach the thread from the JVM.
sJVM->DetachCurrentThread();
Expand Down
3 changes: 2 additions & 1 deletion src/include/platform/CHIPDeviceLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <platform/GeneralUtils.h>
#include <platform/PlatformManager.h>
#include <system/SystemClock.h>
#include <system/SystemLayerImpl.h>
#if CHIP_DEVICE_CONFIG_ENABLE_SOFTWARE_UPDATE_MANAGER
#include <platform/SoftwareUpdateManager.h>
#endif // CHIP_DEVICE_CONFIG_ENABLE_SOFTWARE_UPDATE_MANAGER
Expand All @@ -44,7 +45,7 @@ namespace chip {
namespace DeviceLayer {

struct ChipDeviceEvent;
extern chip::System::Layer SystemLayer;
extern chip::System::LayerImpl SystemLayer;
extern Inet::InetLayer InetLayer;

} // namespace DeviceLayer
Expand Down
6 changes: 3 additions & 3 deletions src/include/platform/internal/GenericPlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ CHIP_ERROR GenericPlatformManagerImpl<ImplClass>::_InitChipStack()
SuccessOrExit(err);

// Initialize the CHIP system layer.
new (&SystemLayer) System::Layer();
new (&SystemLayer) System::LayerImpl();
err = SystemLayer.Init();
if (err != CHIP_NO_ERROR)
{
Expand Down Expand Up @@ -259,8 +259,8 @@ void GenericPlatformManagerImpl<ImplClass>::DispatchEventToSystemLayer(const Chi
CHIP_ERROR err = CHIP_NO_ERROR;

// Invoke the System Layer's event handler function.
err = SystemLayer.WatchableEventsManager().HandleEvent(*event->ChipSystemLayerEvent.Target, event->ChipSystemLayerEvent.Type,
event->ChipSystemLayerEvent.Argument);
err = SystemLayer.HandleEvent(*event->ChipSystemLayerEvent.Target, event->ChipSystemLayerEvent.Type,
event->ChipSystemLayerEvent.Argument);
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Error handling CHIP System Layer event (type %d): %s", event->Type, ErrorStr(err));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ void GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_RunEventLoop(void)

// Call into the system layer to dispatch the callback functions for all timers
// that have expired.
err = SystemLayer.WatchableEventsManager().HandlePlatformTimer();
err = SystemLayer.HandlePlatformTimer();
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Error handling CHIP timers: %s", ErrorStr(err));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ bool GenericPlatformManagerImpl_POSIX<ImplClass>::_IsChipStackLockedByCurrentThr
template <class ImplClass>
CHIP_ERROR GenericPlatformManagerImpl_POSIX<ImplClass>::_StartChipTimer(int64_t aMilliseconds)
{
// Let WatchableEventManager.PrepareEvents() handle timers.
// Let SystemLayer.PrepareEvents() handle timers.
return CHIP_NO_ERROR;
}

Expand All @@ -125,7 +125,7 @@ void GenericPlatformManagerImpl_POSIX<ImplClass>::_PostEvent(const ChipDeviceEve
{
mChipEventQueue.Push(*event);

SystemLayer.WatchableEventsManager().Signal(); // Trigger wake select on CHIP thread
SystemLayer.Signal(); // Trigger wake select on CHIP thread
}

template <class ImplClass>
Expand Down Expand Up @@ -160,21 +160,20 @@ void GenericPlatformManagerImpl_POSIX<ImplClass>::_RunEventLoop()

Impl()->LockChipStack();

System::WatchableEventManager & watchState = SystemLayer.WatchableEventsManager();
watchState.EventLoopBegins();
SystemLayer.EventLoopBegins();
do
{
watchState.PrepareEvents();
SystemLayer.PrepareEvents();

Impl()->UnlockChipStack();
watchState.WaitForEvents();
SystemLayer.WaitForEvents();
Impl()->LockChipStack();

watchState.HandleEvents();
SystemLayer.HandleEvents();

ProcessDeviceEvents();
} while (mShouldRunEventLoop.load(std::memory_order_relaxed));
watchState.EventLoopEnds();
SystemLayer.EventLoopEnds();

Impl()->UnlockChipStack();

Expand Down Expand Up @@ -255,7 +254,7 @@ CHIP_ERROR GenericPlatformManagerImpl_POSIX<ImplClass>::_StopEventLoopTask()
// SystemLayer.
//
Impl()->LockChipStack();
SystemLayer.WatchableEventsManager().Signal();
SystemLayer.Signal();
Impl()->UnlockChipStack();

pthread_mutex_lock(&mStateLock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void GenericPlatformManagerImpl_Zephyr<ImplClass>::_UnlockChipStack(void)
template <class ImplClass>
CHIP_ERROR GenericPlatformManagerImpl_Zephyr<ImplClass>::_StartChipTimer(uint32_t aMilliseconds)
{
// Let WatchableEventManager.PrepareEvents() handle timers.
// Let Systemlayer.PrepareEvents() handle timers.
return CHIP_NO_ERROR;
}

Expand All @@ -106,7 +106,7 @@ void GenericPlatformManagerImpl_Zephyr<ImplClass>::_PostEvent(const ChipDeviceEv
// k_msgq_put takes `void*` instead of `const void*`. Nonetheless, it should be safe to
// const_cast here and there are components in Zephyr itself which do the same.
if (k_msgq_put(&mChipEventQueue, const_cast<ChipDeviceEvent *>(event), K_NO_WAIT) == 0)
SystemLayer.WatchableEventsManager().Signal(); // Trigger wake on CHIP thread
SystemLayer.Signal(); // Trigger wake on CHIP thread
else
ChipLogError(DeviceLayer, "Failed to post event to CHIP Platform event queue");
}
Expand All @@ -125,21 +125,20 @@ void GenericPlatformManagerImpl_Zephyr<ImplClass>::_RunEventLoop(void)
{
Impl()->LockChipStack();

System::WatchableEventManager & watchState = SystemLayer.WatchableEventsManager();
watchState.EventLoopBegins();
SystemLayer.EventLoopBegins();
while (true)
{
watchState.PrepareEvents();
SystemLayer.PrepareEvents();

Impl()->UnlockChipStack();
watchState.WaitForEvents();
SystemLayer.WaitForEvents();
Impl()->LockChipStack();

watchState.HandleEvents();
SystemLayer.HandleEvents();

ProcessDeviceEvents();
}
watchState.EventLoopEnds();
SystemLayer.EventLoopEnds();

Impl()->UnlockChipStack();
}
Expand Down
2 changes: 1 addition & 1 deletion src/inet/EndPointBasis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void EndPointBasis::DeferredFree(chip::System::Object::ReleaseDeferralErrorTacti
{
if (!CHIP_SYSTEM_CONFIG_USE_SOCKETS || IsLWIPEndPoint())
{
DeferredRelease(Layer().SystemLayer(), aTactic);
DeferredRelease(static_cast<System::LayerLwIP *>(Layer().SystemLayer()), aTactic);
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions src/inet/IPEndPointBasis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ IPPacketInfo * IPEndPointBasis::GetPacketInfo(const System::PacketBufferHandle &
return (lPacketInfo);
}

CHIP_ERROR IPEndPointBasis::PostPacketBufferEvent(chip::System::Layer * aLayer, System::Object & aTarget,
CHIP_ERROR IPEndPointBasis::PostPacketBufferEvent(chip::System::LayerLwIP * aLayer, System::Object & aTarget,
System::EventType aEventType, System::PacketBufferHandle && aBuffer)
{
VerifyOrReturnError(aLayer != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
Expand Down Expand Up @@ -943,7 +943,7 @@ CHIP_ERROR IPEndPointBasis::GetSocket(IPAddressType aAddressType, int aType, int
mSocket = ::socket(family, aType, aProtocol);
if (mSocket == -1)
return chip::System::MapErrorPOSIX(errno);
ReturnErrorOnFailure(Layer().SystemLayer()->StartWatchingSocket(mSocket, &mWatch));
ReturnErrorOnFailure(static_cast<System::LayerSockets *>(Layer().SystemLayer())->StartWatchingSocket(mSocket, &mWatch));

mAddrType = aAddressType;

Expand Down
4 changes: 2 additions & 2 deletions src/inet/IPEndPointBasis.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ class DLL_EXPORT IPEndPointBasis : public EndPointBasis
#if CHIP_SYSTEM_CONFIG_USE_LWIP
public:
static struct netif * FindNetifFromInterfaceId(InterfaceId aInterfaceId);
static CHIP_ERROR PostPacketBufferEvent(chip::System::Layer * aLayer, System::Object & aTarget, System::EventType aEventType,
System::PacketBufferHandle && aBuffer);
static CHIP_ERROR PostPacketBufferEvent(chip::System::LayerLwIP * aLayer, System::Object & aTarget,
System::EventType aEventType, System::PacketBufferHandle && aBuffer);

protected:
void HandleDataReceived(chip::System::PacketBufferHandle && aBuffer);
Expand Down
4 changes: 2 additions & 2 deletions src/inet/InetLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ InetLayer::InetLayer()
}

#if CHIP_SYSTEM_CONFIG_USE_LWIP
chip::System::LwIPEventHandlerDelegate InetLayer::sInetEventHandlerDelegate;
chip::System::LayerLwIP::EventHandlerDelegate InetLayer::sInetEventHandlerDelegate;
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

#if INET_CONFIG_MAX_DROPPABLE_EVENTS && CHIP_SYSTEM_CONFIG_USE_LWIP
Expand Down Expand Up @@ -274,7 +274,7 @@ CHIP_ERROR InetLayer::Init(chip::System::Layer & aSystemLayer, void * aContext)
err = InitQueueLimiter();
SuccessOrExit(err);

mSystemLayer->AddEventHandlerDelegate(sInetEventHandlerDelegate);
static_cast<System::LayerLwIP *>(mSystemLayer)->AddEventHandlerDelegate(sInetEventHandlerDelegate);
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

State = kState_Initialized;
Expand Down
2 changes: 1 addition & 1 deletion src/inet/InetLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ class DLL_EXPORT InetLayer
#if CHIP_SYSTEM_CONFIG_USE_LWIP
static CHIP_ERROR HandleInetLayerEvent(chip::System::Object & aTarget, chip::System::EventType aEventType, uintptr_t aArgument);

static chip::System::LwIPEventHandlerDelegate sInetEventHandlerDelegate;
static chip::System::LayerLwIP::EventHandlerDelegate sInetEventHandlerDelegate;

// In some implementations, there may be a shared event / message
// queue for the InetLayer used by other system events / messages.
Expand Down
14 changes: 8 additions & 6 deletions src/inet/RawEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ CHIP_ERROR RawEndPoint::Bind(IPAddressType addrType, const IPAddress & addr, Int
ReturnErrorOnFailure(IPEndPointBasis::Bind(addrType, addr, 0, intfId));

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
dispatch_queue_t dispatchQueue = Layer().SystemLayer()->GetDispatchQueue();
dispatch_queue_t dispatchQueue = static_cast<System::LayerSocketsLoop *>(Layer().SystemLayer())->GetDispatchQueue();
if (dispatchQueue != nullptr)
{
unsigned long fd = static_cast<unsigned long>(mSocket);
Expand Down Expand Up @@ -350,7 +350,7 @@ CHIP_ERROR RawEndPoint::BindIPv6LinkLocal(InterfaceId intfId, const IPAddress &

optfail:
res = chip::System::MapErrorPOSIX(errno);
Layer().SystemLayer()->StopWatchingSocket(&mWatch);
static_cast<System::LayerSockets *>(Layer().SystemLayer())->StopWatchingSocket(&mWatch);
close(mSocket);
mSocket = INET_INVALID_SOCKET_FD;
mAddrType = kIPAddressType_Unknown;
Expand Down Expand Up @@ -424,8 +424,9 @@ CHIP_ERROR RawEndPoint::Listen(IPEndPointBasis::OnMessageReceivedFunct onMessage

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
// Wait for ability to read on this endpoint.
ReturnErrorOnFailure(Layer().SystemLayer()->SetCallback(mWatch, HandlePendingIO, reinterpret_cast<intptr_t>(this)));
ReturnErrorOnFailure(Layer().SystemLayer()->RequestCallbackOnPendingRead(mWatch));
auto layer = static_cast<System::LayerSockets *>(Layer().SystemLayer());
ReturnErrorOnFailure(layer->SetCallback(mWatch, HandlePendingIO, reinterpret_cast<intptr_t>(this)));
ReturnErrorOnFailure(layer->RequestCallbackOnPendingRead(mWatch));
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS

return CHIP_NO_ERROR;
Expand Down Expand Up @@ -467,7 +468,7 @@ void RawEndPoint::Close()
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
if (mSocket != INET_INVALID_SOCKET_FD)
{
Layer().SystemLayer()->StopWatchingSocket(&mWatch);
static_cast<System::LayerSockets *>(Layer().SystemLayer())->StopWatchingSocket(&mWatch);
close(mSocket);
mSocket = INET_INVALID_SOCKET_FD;
}
Expand Down Expand Up @@ -987,7 +988,8 @@ u8_t RawEndPoint::LwIPReceiveRawMessage(void * arg, struct raw_pcb * pcb, struct
pktInfo->DestPort = 0;
}

PostPacketBufferEvent(ep->Layer().SystemLayer(), *ep, kInetEvent_RawDataReceived, std::move(buf));
PostPacketBufferEvent(static_cast<System::LayerLwIP *>(ep->Layer().SystemLayer()), *ep, kInetEvent_RawDataReceived,
std::move(buf));
}

return enqueue;
Expand Down
Loading

0 comments on commit 1213432

Please sign in to comment.