From 8b9cceb7ed065e2aaa95cecdc77dbfc5dbf6ab43 Mon Sep 17 00:00:00 2001 From: Srivats P Date: Sat, 9 Feb 2019 17:16:31 +0530 Subject: [PATCH] HostDev: Return smac/dmac resolve failure via RPC --- client/portgroup.cpp | 13 +++++++ common/protocol.proto | 7 +++- server/abstractport.cpp | 23 ++++++------ server/abstractport.h | 6 ++-- server/myservice.cpp | 78 ++++++++++++++++++++++++++++++++++------- server/myservice.h | 2 ++ 6 files changed, 102 insertions(+), 27 deletions(-) diff --git a/client/portgroup.cpp b/client/portgroup.cpp index b8df277..4b77c3f 100644 --- a/client/portgroup.cpp +++ b/client/portgroup.cpp @@ -667,6 +667,11 @@ void PortGroup::processModifyStreamAck(int portIndex, qDebug("apply completed"); logInfo(id(), mPorts[portIndex]->id(), QString("All port changes applied")); + + OstProto::Ack *ack = static_cast(controller->response()); + if (ack->status()) + logError(id(), mPorts[portIndex]->id(), + QString::fromStdString(ack->notes())); mPorts[portIndex]->when_syncComplete(); mainWindow->setEnabled(true); @@ -803,6 +808,7 @@ void PortGroup::modifyPort(int portIndex, OstProto::Port portConfig) void PortGroup::processModifyPortAck(bool restoreUi,PbRpcController *controller) { + qDebug("In %s", __FUNCTION__); if (controller->Failed()) @@ -813,6 +819,10 @@ void PortGroup::processModifyPortAck(bool restoreUi,PbRpcController *controller) .arg(controller->ErrorString())); } + OstProto::Ack *ack = static_cast(controller->response()); + if (ack->status()) + logError(id(), QString::fromStdString(ack->notes())); + if (restoreUi) { mainWindow->setEnabled(true); QApplication::restoreOverrideCursor(); @@ -1339,6 +1349,9 @@ void PortGroup::processStartTxAck(PbRpcController *controller) { qDebug("In %s", __FUNCTION__); + OstProto::Ack *ack = static_cast(controller->response()); + if (ack->status()) + logError(id(), QString::fromStdString(ack->notes())); delete controller; } diff --git a/common/protocol.proto b/common/protocol.proto index 86d0bfc..f3f3640 100644 --- a/common/protocol.proto +++ b/common/protocol.proto @@ -175,7 +175,12 @@ message Void { } message Ack { - //! \todo (LOW) do we need any fields in 'Ack' + enum RpcStatus { + kRpcSuccess = 0; + kRpcFail = 1; + } + optional RpcStatus status = 1; // FIXME: make required + optional string notes = 2; } message PortId { diff --git a/server/abstractport.cpp b/server/abstractport.cpp index c237765..ab1cba7 100644 --- a/server/abstractport.cpp +++ b/server/abstractport.cpp @@ -202,23 +202,24 @@ bool AbstractPort::setRateAccuracy(Accuracy accuracy) return true; } -void AbstractPort::updatePacketList() +int AbstractPort::updatePacketList() { switch(data_.transmit_mode()) { case OstProto::kSequentialTransmit: - updatePacketListSequential(); + return updatePacketListSequential(); break; case OstProto::kInterleavedTransmit: - updatePacketListInterleaved(); + return updatePacketListInterleaved(); break; default: Q_ASSERT(false); // Unreachable!!! break; } + return 0; } -void AbstractPort::updatePacketListSequential() +int AbstractPort::updatePacketListSequential() { FrameValueAttrib packetListAttrib; long sec = 0; @@ -397,11 +398,12 @@ void AbstractPort::updatePacketListSequential() _stop_no_more_pkts: isSendQueueDirty_ = false; - // FIXME: send attrib back in Ack - qDebug("PacketListAttrib = %x", (int)packetListAttrib.errorFlags); + qDebug("PacketListAttrib = %x", + static_cast(packetListAttrib.errorFlags)); + return static_cast(packetListAttrib.errorFlags); } -void AbstractPort::updatePacketListInterleaved() +int AbstractPort::updatePacketListInterleaved() { FrameValueAttrib packetListAttrib; int numStreams = 0; @@ -432,7 +434,7 @@ void AbstractPort::updatePacketListInterleaved() if (activeStreamCount == 0) { isSendQueueDirty_ = false; - return; + return 0; } // First sort the streams by ordinalValue @@ -643,8 +645,9 @@ void AbstractPort::updatePacketListInterleaved() setPacketListLoopMode(true, delaySec, delayNsec); isSendQueueDirty_ = false; - // FIXME: send attrib back in Ack - qDebug("PacketListAttrib = %x", (int)packetListAttrib.errorFlags); + qDebug("PacketListAttrib = %x", + static_cast(packetListAttrib.errorFlags)); + return static_cast(packetListAttrib.errorFlags); } void AbstractPort::stats(PortStats *stats) diff --git a/server/abstractport.h b/server/abstractport.h index 0d1e964..91d3ff4 100644 --- a/server/abstractport.h +++ b/server/abstractport.h @@ -105,7 +105,7 @@ public: int length) = 0; virtual void setPacketListLoopMode(bool loop, quint64 secDelay, quint64 nsecDelay) = 0; - void updatePacketList(); + int updatePacketList(); virtual void startTransmit() = 0; virtual void stopTransmit() = 0; @@ -140,8 +140,8 @@ protected: void addNote(QString note); - void updatePacketListSequential(); - void updatePacketListInterleaved(); + int updatePacketListSequential(); + int updatePacketListInterleaved(); bool isUsable_; OstProto::Port data_; diff --git a/server/myservice.cpp b/server/myservice.cpp index 1599189..4588637 100644 --- a/server/myservice.cpp +++ b/server/myservice.cpp @@ -31,6 +31,7 @@ along with this program. If not, see #include "../common/abstractprotocol.h" #endif +#include "../common/framevalueattrib.h" #include "../common/streambase.h" #include "../rpc/pbrpccontroller.h" #include "device.h" @@ -116,9 +117,11 @@ void MyService::getPortConfig(::google::protobuf::RpcController* /*controller*/, void MyService::modifyPort(::google::protobuf::RpcController* /*controller*/, const ::OstProto::PortConfigList* request, - ::OstProto::Ack* /*response*/, + ::OstProto::Ack *response, ::google::protobuf::Closure* done) { + bool fail = false; + QString notes; // notification needs to be on heap because signal/slot is across threads! OstProto::Notification *notif = new OstProto::Notification; @@ -127,7 +130,7 @@ void MyService::modifyPort(::google::protobuf::RpcController* /*controller*/, for (int i = 0; i < request->port_size(); i++) { OstProto::Port port; - int id; + int id, err = 0; port = request->port(i); id = port.port_id().id(); @@ -143,15 +146,22 @@ void MyService::modifyPort(::google::protobuf::RpcController* /*controller*/, portLock[id]->lockForWrite(); portInfo[id]->modify(port); if (dirty) - portInfo[id]->updatePacketList(); + err = portInfo[id]->updatePacketList(); portLock[id]->unlock(); - - + if (err) { + notes += frameValueErrorNotes(id, err); + fail = true; + } notif->mutable_port_id_list()->add_port_id()->set_id(id); } } - //! \todo (LOW): fill-in response "Ack"???? + if (fail) { + response->set_status(OstProto::Ack::kRpcFail); + response->set_notes(notes.toStdString()); + } + else + response->set_status(OstProto::Ack::kRpcSuccess); done->Run(); if (notif->port_id_list().port_id_size()) { @@ -318,10 +328,11 @@ _exit: void MyService::modifyStream(::google::protobuf::RpcController* controller, const ::OstProto::StreamConfigList* request, - ::OstProto::Ack* /*response*/, + ::OstProto::Ack* response, ::google::protobuf::Closure* done) { int portId; + int err = 0; qDebug("In %s", __PRETTY_FUNCTION__); @@ -346,11 +357,16 @@ void MyService::modifyStream(::google::protobuf::RpcController* controller, } if (portInfo[portId]->isDirty()) - portInfo[portId]->updatePacketList(); + err = portInfo[portId]->updatePacketList(); portLock[portId]->unlock(); - //! \todo(LOW): fill-in response "Ack"???? - + if (err) { + QString notes = frameValueErrorNotes(portId, err); + response->set_status(OstProto::Ack::kRpcFail); + response->set_notes(notes.toStdString()); + } + else + response->set_status(OstProto::Ack::kRpcSuccess); done->Run(); return; @@ -365,14 +381,18 @@ _exit: void MyService::startTransmit(::google::protobuf::RpcController* /*controller*/, const ::OstProto::PortIdList* request, - ::OstProto::Ack* /*response*/, + ::OstProto::Ack* response, ::google::protobuf::Closure* done) { + bool fail = false; + QString notes; + qDebug("In %s", __PRETTY_FUNCTION__); for (int i = 0; i < request->port_id_size(); i++) { int portId; + int err = 0; portId = request->port_id(i).id(); if ((portId < 0) || (portId >= portInfo.size())) @@ -380,12 +400,21 @@ void MyService::startTransmit(::google::protobuf::RpcController* /*controller*/, portLock[portId]->lockForWrite(); if (portInfo[portId]->isDirty()) - portInfo[portId]->updatePacketList(); + err = portInfo[portId]->updatePacketList(); portInfo[portId]->startTransmit(); portLock[portId]->unlock(); + if (err) { + notes += frameValueErrorNotes(portId, err); + fail = true; + } } - //! \todo (LOW): fill-in response "Ack"???? + if (fail) { + response->set_status(OstProto::Ack::kRpcFail); + response->set_notes(notes.toStdString()); + } + else + response->set_status(OstProto::Ack::kRpcSuccess); done->Run(); } @@ -1000,6 +1029,29 @@ _invalid_port: done->Run(); } +QString MyService::frameValueErrorNotes(int portId, int error) +{ + if (!error) + return QString(); + + QString pfx = QString("Port %1: ").arg(portId); + auto errorFlags = static_cast(error); + + // If smac resolve fails, dmac will always fail - so check that first + // and report only that so as not to confuse users (they may not realize + // that without a source device, we have no ARP table to lookup for dmac) + + if (errorFlags & FrameValueAttrib::UnresolvedSrcMacError) + return pfx + "Source mac resolve failed for one or more " + "streams - Device matching stream's source ip not found\n"; + + if (errorFlags & FrameValueAttrib::UnresolvedDstMacError) + return pfx + "Destination mac resolve failed for one or more " + "streams - possible ARP/ND failure\n"; + + return QString(); +} + /* * =================================================================== * Friends diff --git a/server/myservice.h b/server/myservice.h index d706f40..eda8f3b 100644 --- a/server/myservice.h +++ b/server/myservice.h @@ -172,6 +172,8 @@ signals: void notification(int notifType, SharedProtobufMessage notifData); private: + QString frameValueErrorNotes(int portId, int error); + /* * NOTES: * - AbstractPort::id() and index into portInfo[] are same!