IGMP/MLD msg types have been moved into the IGMP/MLD subclasses from GMP. Packet is now correctly padded with zero rather than garbage. Implemented AbstractProtocol::protocolHasPayload() method to force payload to None for IGMP/MLD/ARP etc.

client/streamconfigdialog.cpp

@@ -113,6 +113,15 @@ StreamConfigDialog::StreamConfigDialog(Port &port, uint streamIndex,
                 if (validProtocolCount == 0)
                     connect(btn1, SIGNAL(clicked(bool)),
                         bgProto[i+1]->button(ButtonIdNone), SLOT(click()));
+
+                // If the id1 protocol doesn't have a payload (e.g. IGMP)
+                // force payload protocol to 'None'
+                if (!OstProtocolManager->protocolHasPayload(id1))
+                {
+                    connect(btn1, SIGNAL(clicked(bool)),
+                        bgProto[ProtoPayload]->button(ButtonIdNone), 
+                        SLOT(click()));
+                }
             }
         }
     }

common/abstractprotocol.cpp

@@ -79,6 +79,7 @@ AbstractProtocol::AbstractProtocol(StreamBase *stream, AbstractProtocol *parent)
     _metaFieldCount = -1;
     _frameFieldCount = -1;
     protoSize = -1;
+    _hasPayload = true;
 }
 
 /*!
@@ -614,6 +615,18 @@ bool AbstractProtocol::isProtocolFramePayloadSizeVariable() const
     return false;
 }
 
+/*!
+  Returns true if the protocol typically contains a payload or other protocols
+  following it e.g. TCP, UDP have payloads, while ARP, IGMP do not 
+
+  The default implementation returns true. If a subclass does not have a
+  payload, it should set the _hasPayload data member to false
+*/
+bool AbstractProtocol::protocolHasPayload() const
+{
+    return _hasPayload;
+}
+
 /*!
   Returns the checksum (of the requested type) of the protocol's contents
 

common/abstractprotocol.h

@@ -59,6 +59,9 @@ protected:
     AbstractProtocol    *prev;     //!< Protocol preceding this protocol
     AbstractProtocol    *next;     //!< Protocol succeeding this protocol 
 
+    //! Is protocol typically followed by payload or another protocol
+    bool _hasPayload;
+
 public:
     //! Properties of a field, can be OR'd
     enum FieldFlag {
@@ -139,6 +142,8 @@ public:
     bool isProtocolFramePayloadValueVariable() const;
     bool isProtocolFramePayloadSizeVariable() const;
 
+    bool protocolHasPayload() const;
+
     virtual quint32 protocolFrameCksum(int streamIndex = 0,
         CksumType cksumType = CksumIp) const;
     quint32 protocolFrameHeaderCksum(int streamIndex = 0,

common/arp.cpp

@@ -88,6 +88,7 @@ void ArpConfigForm::on_targetProtoAddrMode_currentIndexChanged(int index)
 ArpProtocol::ArpProtocol(StreamBase *stream, AbstractProtocol *parent)
     : AbstractProtocol(stream, parent)
 {
+    _hasPayload = false;
     configForm = NULL;
 }
 

common/gmp.cpp

@@ -29,16 +29,6 @@ GmpConfigForm::GmpConfigForm(QWidget *parent)
 {
     setupUi(this);
 
-    // TODO: this should be in subclass
-    msgTypeCombo->setValueMask(0xFF);
-    msgTypeCombo->addItem(kIgmpV1Query,  "IGMPv1 Query");
-    msgTypeCombo->addItem(kIgmpV1Report, "IGMPv1 Report");
-    msgTypeCombo->addItem(kIgmpV2Query,  "IGMPv2 Query");
-    msgTypeCombo->addItem(kIgmpV2Report, "IGMPv2 Report");
-    msgTypeCombo->addItem(kIgmpV2Leave,  "IGMPv2 Leave");
-    msgTypeCombo->addItem(kIgmpV3Query,  "IGMPv3 Query");
-    msgTypeCombo->addItem(kIgmpV3Report, "IGMPv3 Report");
-
     auxData->setValidator(new QRegExpValidator(
                 QRegExp("[0-9A-Fa-f]*"), this));
 }
@@ -54,43 +44,6 @@ void GmpConfigForm::update()
             groupList->currentItem());
 }
 
-void GmpConfigForm::on_msgTypeCombo_currentIndexChanged(int /*index*/)
-{
-    switch(msgTypeCombo->currentValue())
-    {
-    case kIgmpV1Query:
-    case kIgmpV1Report:
-    case kIgmpV2Query:
-    case kIgmpV2Report:
-    case kIgmpV2Leave:
-    case kMldV1Query:
-    case kMldV1Report:
-    case kMldV1Done:
-        asmGroup->show();
-        ssmWidget->hide();
-        break;
-
-    case kIgmpV3Query:
-    case kMldV2Query:
-        asmGroup->hide();
-        ssmWidget->setCurrentIndex(kSsmQueryPage);
-        ssmWidget->show();
-        break;
-
-    case kIgmpV3Report:
-    case kMldV2Report:
-        asmGroup->hide();
-        ssmWidget->setCurrentIndex(kSsmReportPage);
-        ssmWidget->show();
-        break;
-
-    default:
-        asmGroup->hide();
-        ssmWidget->hide();
-        break;
-    }
-}
-
 void GmpConfigForm::on_groupMode_currentIndexChanged(int index)
 {
     bool disabled = (index == 0);

common/gmp.h

@@ -27,31 +27,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 
 #include <QHash>
 
-// Both IGMP and MLD use the same msg type value for 'Query' message 
-// across versions despite the fields being different. To distinguish 
-// Query messages of different versions, we use an additional upper byte
-enum GmpMsgType
-{
-    // IGMP
-    kIgmpV1Query = 0x11,
-    kIgmpV1Report = 0x12,
-
-    kIgmpV2Query = 0xFF11,
-    kIgmpV2Report = 0x16,
-    kIgmpV2Leave = 0x17,
-
-    kIgmpV3Query = 0xFE11,
-    kIgmpV3Report = 0x22,
-
-    // MLD
-    kMldV1Query = 0x82,
-    kMldV1Report = 0x83,
-    kMldV1Done = 0x84,
-
-    kMldV2Query = 0xFF82,
-    kMldV2Report = 0x8F
-};
-
 /* 
 Gmp Protocol Frame Format - TODO: for now see the respective RFCs
 */
@@ -67,13 +42,11 @@ public:
 protected:
     QString _defaultGroupIp;
     QString _defaultSourceIp;
-private:
     enum {
         kSsmQueryPage = 0,
         kSsmReportPage = 1
     };
 private slots:
-    void on_msgTypeCombo_currentIndexChanged(int index);
     void on_groupMode_currentIndexChanged(int index);
     void on_addSource_clicked();
     void on_deleteSource_clicked();
@@ -163,11 +136,11 @@ protected:
     OstProto::Gmp    data;
     GmpConfigForm    *configForm;
 
-    GmpMsgType msgType() const;
+    int msgType() const;
 
-    virtual bool isSsmReport() const;
+    virtual bool isSsmReport() const = 0;
-    virtual bool isQuery() const;
+    virtual bool isQuery() const = 0;
-    virtual bool isSsmQuery() const;
+    virtual bool isSsmQuery() const = 0;
 
     int qqic(int value) const;
 
@@ -176,30 +149,9 @@ private:
     static QHash<int, int> frameFieldCountMap;
 };
 
-inline GmpMsgType GmpProtocol::msgType() const 
+inline int GmpProtocol::msgType() const 
 { 
-    return GmpMsgType(fieldData(kType, FieldValue).toUInt()); 
+    return fieldData(kType, FieldValue).toInt(); 
-}
-
-inline bool GmpProtocol::isSsmReport() const
-{
-    return ((msgType() == kIgmpV3Report) 
-         || (msgType() == kMldV2Report ));
-}
-
-inline bool GmpProtocol::isQuery() const
-{
-    return ((msgType() == kIgmpV1Query) 
-         || (msgType() == kIgmpV2Query)
-         || (msgType() == kIgmpV3Query)
-         || (msgType() == kMldV1Query )
-         || (msgType() == kMldV2Query ));
-}
-
-inline bool GmpProtocol::isSsmQuery() const
-{
-    return ((msgType() == kIgmpV3Query) 
-         || (msgType() == kMldV2Query ));
 }
 
 inline int GmpProtocol::qqic(int value) const

common/igmp.cpp

@@ -28,6 +28,18 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 IgmpConfigForm::IgmpConfigForm(QWidget *parent)
     : GmpConfigForm(parent)
 {
+    connect(msgTypeCombo, SIGNAL(currentIndexChanged(int)),
+            SLOT(on_msgTypeCombo_currentIndexChanged(int)));
+
+    msgTypeCombo->setValueMask(0xFF);
+    msgTypeCombo->addItem(kIgmpV1Query,  "IGMPv1 Query");
+    msgTypeCombo->addItem(kIgmpV1Report, "IGMPv1 Report");
+    msgTypeCombo->addItem(kIgmpV2Query,  "IGMPv2 Query");
+    msgTypeCombo->addItem(kIgmpV2Report, "IGMPv2 Report");
+    msgTypeCombo->addItem(kIgmpV2Leave,  "IGMPv2 Leave");
+    msgTypeCombo->addItem(kIgmpV3Query,  "IGMPv3 Query");
+    msgTypeCombo->addItem(kIgmpV3Report, "IGMPv3 Report");
+
     _defaultGroupIp = "0.0.0.0";
     _defaultSourceIp = "0.0.0.0";
 
@@ -37,9 +49,43 @@ IgmpConfigForm::IgmpConfigForm(QWidget *parent)
     groupRecordSourceList->setItemDelegate(new IPv4AddressDelegate(this));
 }
 
+void IgmpConfigForm::on_msgTypeCombo_currentIndexChanged(int /*index*/)
+{
+    switch(msgTypeCombo->currentValue())
+    {
+    case kIgmpV1Query:
+    case kIgmpV1Report:
+    case kIgmpV2Query:
+    case kIgmpV2Report:
+    case kIgmpV2Leave:
+        asmGroup->show();
+        ssmWidget->hide();
+        break;
+
+    case kIgmpV3Query:
+        asmGroup->hide();
+        ssmWidget->setCurrentIndex(kSsmQueryPage);
+        ssmWidget->show();
+        break;
+
+    case kIgmpV3Report:
+        asmGroup->hide();
+        ssmWidget->setCurrentIndex(kSsmReportPage);
+        ssmWidget->show();
+        break;
+
+    default:
+        asmGroup->hide();
+        ssmWidget->hide();
+        break;
+    }
+}
+
 IgmpProtocol::IgmpProtocol(StreamBase *stream, AbstractProtocol *parent)
     : GmpProtocol(stream, parent)
 {
+    _hasPayload = false;
+
     data.set_type(kIgmpV2Query);
 }
 
@@ -388,7 +434,7 @@ quint16 IgmpProtocol::checksum(int streamIndex) const
 {
     quint16 cks;
     quint32 sum = 0;
-#if 1 // FIXME
+
     // TODO: add as a new CksumType (CksumIgmp?) and implement in AbsProto 
     cks = protocolFrameCksum(streamIndex, CksumIp);
     sum += (quint16) ~cks;
@@ -398,6 +444,6 @@ quint16 IgmpProtocol::checksum(int streamIndex) const
         sum = (sum & 0xFFFF) + (sum >> 16);
 
     cks = (~sum) & 0xFFFF;
-#endif
+
     return cks;
 }

common/igmp.h

@@ -22,11 +22,30 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 #include "igmp.pb.h"
 #include "gmp.h"
 
+// IGMP uses the same msg type value for 'Query' messages across
+// versions despite the fields being different. To distinguish 
+// Query messages of different versions, we use an additional 
+// upper byte
+enum IgmpMsgType
+{
+    kIgmpV1Query = 0x11,
+    kIgmpV1Report = 0x12,
+
+    kIgmpV2Query = 0xFF11,
+    kIgmpV2Report = 0x16,
+    kIgmpV2Leave = 0x17,
+
+    kIgmpV3Query = 0xFE11,
+    kIgmpV3Report = 0x22,
+};
+
 class IgmpConfigForm : public GmpConfigForm 
 {
+    Q_OBJECT
 public:
     IgmpConfigForm(QWidget *parent = 0);
 private slots:
+    void on_msgTypeCombo_currentIndexChanged(int index);
 };
 
 class IgmpProtocol : public GmpProtocol
@@ -57,11 +76,32 @@ public:
     virtual void storeConfigWidget();
 
 protected:
+    virtual bool isSsmReport() const;
+    virtual bool isQuery() const;
+    virtual bool isSsmQuery() const;
+
     virtual quint16 checksum(int streamIndex) const;
 private:
     int mrc(int value) const;
 };
 
+inline bool IgmpProtocol::isSsmReport() const
+{
+    return (msgType() == kIgmpV3Report);
+}
+
+inline bool IgmpProtocol::isQuery() const
+{
+    return ((msgType() == kIgmpV1Query) 
+         || (msgType() == kIgmpV2Query)
+         || (msgType() == kIgmpV3Query));
+}
+
+inline bool IgmpProtocol::isSsmQuery() const
+{
+    return (msgType() == kIgmpV3Query); 
+}
+
 inline int IgmpProtocol::mrc(int value) const
 {
     return quint8(value); // TODO: if value > 128, convert to mantissa/exp form

common/mld.cpp

@@ -49,11 +49,44 @@ public:
 MldConfigForm::MldConfigForm(QWidget *parent)
     : GmpConfigForm(parent)
 {
+    connect(msgTypeCombo, SIGNAL(currentIndexChanged(int)),
+            SLOT(on_msgTypeCombo_currentIndexChanged(int)));
+
     _defaultSourceIp = "::";
     sourceList->setItemDelegate(new IpAddressDelegate(this));
     groupRecordSourceList->setItemDelegate(new IpAddressDelegate(this));
 }
 
+void MldConfigForm::on_msgTypeCombo_currentIndexChanged(int /*index*/)
+{
+    switch(msgTypeCombo->currentValue())
+    {
+    case kMldV1Query:
+    case kMldV1Report:
+    case kMldV1Done:
+        asmGroup->show();
+        ssmWidget->hide();
+        break;
+
+    case kMldV2Query:
+        asmGroup->hide();
+        ssmWidget->setCurrentIndex(kSsmQueryPage);
+        ssmWidget->show();
+        break;
+
+    case kMldV2Report:
+        asmGroup->hide();
+        ssmWidget->setCurrentIndex(kSsmReportPage);
+        ssmWidget->show();
+        break;
+
+    default:
+        asmGroup->hide();
+        ssmWidget->hide();
+        break;
+    }
+}
+
 MldProtocol::MldProtocol(StreamBase *stream, AbstractProtocol *parent)
     : GmpProtocol(stream, parent)
 {

common/mld.h

@@ -25,11 +25,27 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 
 #include "abstractprotocol.h"
 
+// MLD uses the same msg type value for 'Query' messages across
+// versions despite the fields being different. To distinguish 
+// Query messages of different versions, we use an additional 
+// upper byte
+enum MldMsgType
+{
+    kMldV1Query = 0x82,
+    kMldV1Report = 0x83,
+    kMldV1Done = 0x84,
+
+    kMldV2Query = 0xFF82,
+    kMldV2Report = 0x8F
+};
+
 class MldConfigForm : public GmpConfigForm 
 {
+    Q_OBJECT
 public:
     MldConfigForm(QWidget *parent = 0);
 private slots:
+    void on_msgTypeCombo_currentIndexChanged(int index);
 };
 
 class MldProtocol : public GmpProtocol
@@ -60,7 +76,28 @@ public:
     virtual void loadConfigWidget();
     virtual void storeConfigWidget();
 protected:
+    virtual bool isSsmReport() const;
+    virtual bool isQuery() const;
+    virtual bool isSsmQuery() const;
+
     virtual quint16 checksum(int streamIndex) const;
 };
 
+inline bool MldProtocol::isSsmReport() const
+{
+    return (msgType() == kMldV2Report);
+}
+
+inline bool MldProtocol::isQuery() const
+{
+    return ((msgType() == kMldV1Query)
+         || (msgType() == kMldV2Query));
+}
+
+inline bool MldProtocol::isSsmQuery() const
+{
+    return (msgType() == kMldV2Query);
+}
+
+
 #endif

common/protocolmanager.cpp

@@ -122,8 +122,9 @@ ProtocolManager::~ProtocolManager()
     nameToNumberMap.clear();
     neighbourProtocols.clear();
     factory.clear();
-    while (!protocolList.isEmpty())
+    QList<AbstractProtocol*> pl = protocolList.values();
-        delete protocolList.takeFirst();
+    while (!pl.isEmpty())
+        delete pl.takeFirst();
 }
 
 void ProtocolManager::registerProtocol(int protoNumber,
@@ -136,7 +137,7 @@ void ProtocolManager::registerProtocol(int protoNumber,
     factory.insert(protoNumber, protoInstanceCreator);
 
     p = createProtocol(protoNumber, NULL);
-    protocolList.append(p);
+    protocolList.insert(protoNumber, p);
 
     numberToNameMap.insert(protoNumber, p->shortName());
     nameToNumberMap.insert(p->shortName(), protoNumber);
@@ -190,6 +191,13 @@ bool ProtocolManager::isValidNeighbour(int protoPrefix, int protoSuffix)
         return false;
 }
 
+bool ProtocolManager::protocolHasPayload(int protoNumber)
+{
+    Q_ASSERT(protocolList.contains(protoNumber));
+
+    return protocolList.value(protoNumber)->protocolHasPayload();
+}
+
 QStringList ProtocolManager::protocolDatabase()
 {
     return numberToNameMap.values();

common/protocolmanager.h

@@ -32,7 +32,7 @@ class ProtocolManager
     QMap<QString, int>    nameToNumberMap;
     QMultiMap<int, int> neighbourProtocols;
     QMap<int, void*>    factory;
-    QList<AbstractProtocol*>    protocolList;
+    QMap<int, AbstractProtocol*>    protocolList;
 
     void populateNeighbourProtocols();
 
@@ -48,6 +48,7 @@ public:
         AbstractProtocol *parent = 0);
 
     bool isValidNeighbour(int protoPrefix, int protoSuffix);
+    bool protocolHasPayload(int protoNumber);
 
     QStringList protocolDatabase();
 };

common/streambase.cpp

@@ -444,6 +444,10 @@ int StreamBase::frameValue(uchar *buf, int bufMaxSize, int frameIndex) const
     }
     delete iter;
 
+    // Pad with zero, if required
+    if (len < pktLen)
+        memset(buf+len, 0, pktLen-len);
+
     return pktLen;
 }