Refer to a PID of 0x0002 as the PID for the HP Teaming heartbeat

protocol.

Point to an HP document o the protocol, and discuss the two ways to hook
up the dissector.

Don't bother checking the destination address - either the OUI/PID
suffices, or the MAC address is both necessary and, presumably,
sufficient and we need to introduce a heuristic dissector table for SNAP
frames.  What's more, there's no guarantee that the destination address
is a MAC address - it might be absent, e.g. because you're capturing on
the Linux "any" device and are getting the "Linux cooked" header, with
only a destination address.

Don't put the source address into the Info column - it's already in the
source column *if* it exists (which it might not, for packets captured
from the "any" device and sent by the machine doing the capturing).

svn path=/trunk/; revision=40195

1 files changed, 48 insertions, 24 deletions

diff --git a/epan/dissectors/packet-hpteam.c b/epan/dissectors/packet-hpteam.c
index 465d9fa975..d514e43992 100644
--- a/epan/dissectors/packet-hpteam.c
+++ b/epan/dissectors/packet-hpteam.c
@@ -42,7 +42,7 @@ static dissector_handle_t data_handle;
 
 /* Known HP NIC teaming PID values */
 static const value_string hpteam_pid_vals[] = {
-	{ 0x0002,	"Hewlett-Packard" },
+	{ 0x0002,	"HP Teaming heartbeat" },
 	{ 0,		NULL }
 };
 
@@ -52,38 +52,62 @@ static gint hf_llc_hpteam_pid = -1;
 /* These are the ids of the subtrees that we may be creating */
 static gint ett_hpteam = -1;
 
+/*
+ * According to the HP document at
+ *
+ *	http://www.hp.com/sbso/bus_protect/teaming.pdf
+ *
+ * the heartbeats are sent to 03-00-C7-00-00-EE in SNAP frames
+ * in unnumbered TEST frames.  It says that the LLC header is
+ * followed by 63 bytes of "Insignificant data" and the FCS.
+ * This means that the SNAP header is part of the "Insignificant
+ * data".
+ *
+ * The SNAP specification (section 10.3 "Subnetwork Access Protocol"
+ * of IEEE Std 802-2001) says that *all* SNAP PDUs have an LLC
+ * payload that starts with the 5-octet Protocol Identification
+ * field, i.e. the OUI and PID.
+ *
+ * At least some Teaming heartbeat packets have an OUI of 00-80-5F,
+ * which belongs to HP, and a protocol ID of 0x0002.
+ *
+ * If all heartbeat packets have that OUI/PID combination, and no other
+ * packets have it, the right way to recognize them is by registering
+ * the PID of 0x0002 in the dissector table for that OUI; there is no
+ * need to check the destination MAC address.
+ *
+ * If not all heartbeat packets have that OUI/PID combination and/or other
+ * packets have it, the only way to recognize them would be to add
+ * support for heuristic dissectors to the SNAP dissector, register this
+ * as a heuristic dissector for that table, and have it compare pinfo->dl_dst
+ * against an address structure with a type of AT_ETHER, a length of 6,
+ * and data of 03-00-C7-00-00-EE.  It is *not* sufficient to just check
+ * pinfo->dl_dst.data, as there is no guarantee that it will be a MAC
+ * address - SNAP frames can also be captured with "Linux cooked mode"
+ * headers, e.g. on the "any" device, and those only have a destination
+ * address for packets sent by the machine capturing the traffic, not for
+ * packets received by the machine.
+ */
+
 static void
 dissect_hpteam(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
 	proto_item *hpteam_item;
 	proto_tree *hpteam_tree;
 	guint32 offset = 0;
-	const char   *strPtr, *HP_Mac;
-	const guint8 *mac_addr;
 
-	mac_addr = pinfo->dl_dst.data;
-	strPtr = ether_to_str(mac_addr);
-	HP_Mac = "03:00:c7:00:00:ee";
 	/*
-	 * Check to see if SNAP frame is a HP Teaming frame or
-	 * if it is really just SNAP
+	 * No need to show the source MAC address; there's no 100%
+	 * guarantee that it's there, and it should show up in
+	 * the source address column by default.
 	 */
-	if (memcmp(strPtr, HP_Mac, 17) == 0) {
-		mac_addr = pinfo->dl_src.data;
-		strPtr = ether_to_str(mac_addr);
-		col_set_str(pinfo->cinfo, COL_PROTOCOL, "HP NIC Team");
-		/* Clear out stuff in the info column */
-		col_set_str(pinfo->cinfo, COL_INFO, "HP NIC Teaming Heartbeat; ");
-		col_append_fstr(pinfo->cinfo, COL_INFO, "Port MAC = %s ", strPtr);
-
-		if (tree) { /* we are being asked for details */
-			hpteam_item = proto_tree_add_item(tree, proto_hpteam, tvb, 0, -1, ENC_NA);
-			hpteam_tree = proto_item_add_subtree(hpteam_item, ett_hpteam);
-			proto_tree_add_item(hpteam_tree, hf_hpteam, tvb, offset, 58, ENC_NA);
-		}
-	}
-	else {
-		call_dissector(data_handle, tvb, pinfo, tree);
+	col_set_str(pinfo->cinfo, COL_PROTOCOL, "HP NIC Team");
+	col_set_str(pinfo->cinfo, COL_INFO, "HP NIC Teaming Heartbeat");
+
+	if (tree) { /* we are being asked for details */
+		hpteam_item = proto_tree_add_item(tree, proto_hpteam, tvb, 0, -1, ENC_NA);
+		hpteam_tree = proto_item_add_subtree(hpteam_item, ett_hpteam);
+		proto_tree_add_item(hpteam_tree, hf_hpteam, tvb, offset, -1, ENC_NA);
 	}
 }