HostDev: Return smac/dmac resolve failure via RPC

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<OstProto::Ack*>(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<OstProto::Ack*>(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<OstProto::Ack*>(controller->response());
+    if (ack->status())
+        logError(id(), QString::fromStdString(ack->notes()));
     delete controller;
 }
 

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 {

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",
-    qDebug("PacketListAttrib = %x", (int)packetListAttrib.errorFlags);
+            static_cast<int>(packetListAttrib.errorFlags));
+    return static_cast<int>(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",
-    qDebug("PacketListAttrib = %x", (int)packetListAttrib.errorFlags);
+            static_cast<int>(packetListAttrib.errorFlags));
+    return static_cast<int>(packetListAttrib.errorFlags);
 }
 
 void AbstractPort::stats(PortStats *stats)

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();
+    int updatePacketListSequential();
-    void updatePacketListInterleaved();
+    int updatePacketListInterleaved();
 
     bool isUsable_;
     OstProto::Port          data_;

server/myservice.cpp

@@ -31,6 +31,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 #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<FrameValueAttrib::ErrorFlags>(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

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!