Put the value(s) of a parameter into the top-level item for that
[obnox/wireshark/wip.git] / packet-arp.c
index 20ed13d1ea83ecf163f29430050b7ac0bfdca4d4..a49404fe5ee2a436a62021d761363ed112b11d66 100644 (file)
@@ -1,23 +1,22 @@
 /* packet-arp.c
  * Routines for ARP packet disassembly
  *
- * $Id: packet-arp.c,v 1.36 2000/11/19 01:00:20 guy Exp $
+ * $Id: packet-arp.c,v 1.58 2004/02/28 22:56:35 guy Exp $
  *
  * Ethereal - Network traffic analyzer
- * By Gerald Combs <gerald@zing.org>
+ * By Gerald Combs <gerald@ethereal.com>
  * Copyright 1998 Gerald Combs
  *
- * 
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version 2
  * of the License, or (at your option) any later version.
- * 
+ *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- * 
+ *
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 # include "config.h"
 #endif
 
-#ifdef HAVE_SYS_TYPES_H
-# include <sys/types.h>
-#endif
-
 #include <string.h>
 #include <glib.h>
-#include "packet.h"
-#include "strutil.h"
-#include "resolv.h"
+#include <epan/packet.h>
+#include <epan/strutil.h>
+#include <epan/resolv.h>
 #include "packet-arp.h"
 #include "etypes.h"
+#include "arcnet_pids.h"
 
 static int proto_arp = -1;
 static int hf_arp_hard_type = -1;
 static int hf_arp_proto_type = -1;
 static int hf_arp_hard_size = -1;
-static int hf_atmarp_shtl = -1;
+static int hf_atmarp_sht = -1;
+static int hf_atmarp_shl = -1;
+static int hf_atmarp_sst = -1;
 static int hf_atmarp_ssl = -1;
 static int hf_arp_proto_size = -1;
 static int hf_arp_opcode = -1;
 static int hf_atmarp_spln = -1;
-static int hf_atmarp_thtl = -1;
+static int hf_atmarp_tht = -1;
+static int hf_atmarp_thl = -1;
+static int hf_atmarp_tst = -1;
 static int hf_atmarp_tsl = -1;
 static int hf_atmarp_tpln = -1;
-static int hf_arp_src_ether = -1;
+static int hf_arp_src_hw = -1;
+static int hf_arp_src_hw_mac = -1;
 static int hf_arp_src_proto = -1;
-static int hf_arp_dst_ether = -1;
+static int hf_arp_src_proto_ipv4 = -1;
+static int hf_arp_dst_hw = -1;
+static int hf_arp_dst_hw_mac = -1;
 static int hf_arp_dst_proto = -1;
+static int hf_arp_dst_proto_ipv4 = -1;
 static int hf_atmarp_src_atm_num_e164 = -1;
 static int hf_atmarp_src_atm_num_nsap = -1;
 static int hf_atmarp_src_atm_subaddr = -1;
@@ -64,10 +68,13 @@ static int hf_atmarp_dst_atm_subaddr = -1;
 
 static gint ett_arp = -1;
 static gint ett_atmarp_nsap = -1;
+static gint ett_atmarp_tl = -1;
+
+static dissector_handle_t atmarp_handle;
 
 /* Definitions taken from Linux "linux/if_arp.h" header file, and from
 
-       http://www.isi.edu/in-notes/iana/assignments/arp-parameters
+       http://www.iana.org/assignments/arp-parameters
 
  */
 
@@ -142,29 +149,45 @@ static const value_string atmop_vals[] = {
   {ATMARPOP_NAK,   "nak"  },
   {0,              NULL          } };
 
-#define        ATMARP_IS_E164  0x40    /* bit in shtl/thtl for E.164 format */
-#define        ATMARP_LEN_MASK 0x3F    /* length of address in shtl/thtl */
+#define        ATMARP_IS_E164  0x40    /* bit in type/length for E.164 format */
+#define        ATMARP_LEN_MASK 0x3F    /* length of {sub}address in type/length */
+
+/*
+ * Given the hardware address type and length, check whether an address
+ * is an Ethernet address - the address must be of type "Ethernet" or
+ * "IEEE 802.x", and the length must be 6 bytes.
+ */
+#define ARP_HW_IS_ETHER(ar_hrd, ar_hln) \
+       (((ar_hrd) == ARPHRD_ETHER || (ar_hrd) == ARPHRD_IEEE802) \
+                               && (ar_hln) == 6)
+
+/*
+ * Given the protocol address type and length, check whether an address
+ * is an IPv4 address - the address must be of type "IP", and the length
+ * must be 4 bytes.
+ */
+#define ARP_PRO_IS_IPv4(ar_pro, ar_pln) \
+       ((ar_pro) == ETHERTYPE_IP && (ar_pln) == 4)
 
 gchar *
-arphrdaddr_to_str(guint8 *ad, int ad_len, guint16 type)
+arphrdaddr_to_str(const guint8 *ad, int ad_len, guint16 type)
 {
   if (ad_len == 0)
     return "<No address>";
-  if ((type == ARPHRD_ETHER || type == ARPHRD_EETHER || type == ARPHRD_IEEE802)
-                               && ad_len == 6) {
-    /* Ethernet address (or Experimental 3Mb Ethernet, or IEEE 802.x
-       address, which are the same type of address). */
+  if (ARP_HW_IS_ETHER(type, ad_len)) {
+    /* Ethernet address (or IEEE 802.x address, which is the same type of
+       address). */
     return ether_to_str(ad);
   }
   return bytes_to_str(ad, ad_len);
 }
 
 static gchar *
-arpproaddr_to_str(guint8 *ad, int ad_len, guint16 type)
+arpproaddr_to_str(const guint8 *ad, int ad_len, guint16 type)
 {
   if (ad_len == 0)
     return "<No address>";
-  if (type == ETHERTYPE_IP && ad_len == 4) {
+  if (ARP_PRO_IS_IPv4(type, ad_len)) {
     /* IPv4 address.  */
     return ip_to_str(ad);
   }
@@ -173,8 +196,9 @@ arpproaddr_to_str(guint8 *ad, int ad_len, guint16 type)
 
 #define        N_ATMARPNUM_TO_STR_STRINGS      2
 #define        MAX_E164_STR_LEN                20
+
 static gchar *
-atmarpnum_to_str(guint8 *ad, int ad_tl)
+atmarpnum_to_str(const guint8 *ad, int ad_tl)
 {
   int           ad_len = ad_tl & ATMARP_LEN_MASK;
   static gchar  str[N_ATMARPNUM_TO_STR_STRINGS][MAX_E164_STR_LEN+3+1];
@@ -212,12 +236,20 @@ atmarpnum_to_str(guint8 *ad, int ad_tl)
 }
 
 static gchar *
-atmarpsubaddr_to_str(guint8 *ad, int ad_len)
+atmarpsubaddr_to_str(const guint8 *ad, int ad_tl)
 {
+  int           ad_len = ad_tl & ATMARP_LEN_MASK;
+
   if (ad_len == 0)
     return "<No address>";
 
   /*
+   * E.164 isn't considered legal in subaddresses (RFC 2225 says that
+   * a null or unknown ATM address is indicated by setting the length
+   * to 0, in which case the type must be ignored; we've seen some
+   * captures in which the length of a subaddress is 0 and the type
+   * is E.164).
+   *
    * XXX - break down into subcomponents?
    */
   return bytes_to_str(ad, ad_len);
@@ -271,11 +303,11 @@ arphrdtype_to_str(guint16 hwtype, const char *fmt) {
 #define        ATM_AR_HRD      0
 #define        ATM_AR_PRO      2
 #define        ATM_AR_SHTL     4
-#define        ATM_AR_SS     5
+#define        ATM_AR_SSTL     5
 #define        ATM_AR_OP       6
 #define        ATM_AR_SPLN     8
 #define        ATM_AR_THTL     9
-#define        ATM_AR_TS     10
+#define        ATM_AR_TSTL     10
 #define        ATM_AR_TPLN     11
 #define MIN_ATMARP_HEADER_SIZE 12
 
@@ -373,14 +405,14 @@ dissect_atmarp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
   guint16     ar_hrd;
   guint16     ar_pro;
   guint8      ar_shtl;
-  guint8      ar_sht;
   guint8      ar_shl;
+  guint8      ar_sstl;
   guint8      ar_ssl;
   guint16     ar_op;
   guint8      ar_spln;
   guint8      ar_thtl;
-  guint8      ar_tht;
   guint8      ar_thl;
+  guint8      ar_tstl;
   guint8      ar_tsl;
   guint8      ar_tpln;
   int         tot_len;
@@ -389,32 +421,33 @@ dissect_atmarp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
   gchar       *op_str;
   int         sha_offset, ssa_offset, spa_offset;
   int         tha_offset, tsa_offset, tpa_offset;
-  guint8      *sha_val, *ssa_val, *spa_val;
-  guint8      *tha_val, *tsa_val, *tpa_val;
+  const guint8      *sha_val, *ssa_val, *spa_val;
+  const guint8      *tha_val, *tsa_val, *tpa_val;
   gchar       *sha_str, *ssa_str, *spa_str;
   gchar       *tha_str, *tsa_str, *tpa_str;
+  proto_tree  *tl_tree;
+  proto_item  *tl;
 
-  CHECK_DISPLAY_AS_DATA(proto_arp, tvb, pinfo, tree);
-
+  /* Override the setting to "ARP/RARP". */
   pinfo->current_proto = "ATMARP";
 
   ar_hrd = tvb_get_ntohs(tvb, ATM_AR_HRD);
   ar_pro = tvb_get_ntohs(tvb, ATM_AR_PRO);
   ar_shtl = tvb_get_guint8(tvb, ATM_AR_SHTL);
-  ar_sht = ar_shtl & ATMARP_IS_E164;
   ar_shl = ar_shtl & ATMARP_LEN_MASK;
-  ar_ssl = tvb_get_guint8(tvb, ATM_AR_SSL);
+  ar_sstl = tvb_get_guint8(tvb, ATM_AR_SSTL);
+  ar_ssl = ar_sstl & ATMARP_LEN_MASK;
   ar_op  = tvb_get_ntohs(tvb, AR_OP);
   ar_spln = tvb_get_guint8(tvb, ATM_AR_SPLN);
   ar_thtl = tvb_get_guint8(tvb, ATM_AR_THTL);
-  ar_tht = ar_thtl & ATMARP_IS_E164;
   ar_thl = ar_thtl & ATMARP_LEN_MASK;
-  ar_tsl = tvb_get_guint8(tvb, ATM_AR_TSL);
+  ar_tstl = tvb_get_guint8(tvb, ATM_AR_TSTL);
+  ar_tsl = ar_tstl & ATMARP_LEN_MASK;
   ar_tpln = tvb_get_guint8(tvb, ATM_AR_TPLN);
 
-  tot_len = MIN_ATMARP_HEADER_SIZE + ar_shtl + ar_ssl + ar_spln +
-                               ar_thtl + ar_tsl + ar_tpln;
-  
+  tot_len = MIN_ATMARP_HEADER_SIZE + ar_shl + ar_ssl + ar_spln +
+                               ar_thl + ar_tsl + ar_tpln;
+
   /* Adjust the length of this tvbuff to include only the ARP datagram.
      Our caller may use that to determine how much of its packet
      was padding. */
@@ -433,7 +466,7 @@ dissect_atmarp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
   ssa_offset = sha_offset + ar_shl;
   if (ar_ssl != 0) {
     ssa_val = tvb_get_ptr(tvb, ssa_offset, ar_ssl);
-    ssa_str = atmarpsubaddr_to_str(ssa_val, ar_ssl);
+    ssa_str = atmarpsubaddr_to_str(ssa_val, ar_sstl);
   } else {
     ssa_val = NULL;
     ssa_str = NULL;
@@ -455,7 +488,7 @@ dissect_atmarp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
   tsa_offset = tha_offset + ar_thl;
   if (ar_tsl != 0) {
     tsa_val = tvb_get_ptr(tvb, tsa_offset, ar_tsl);
-    tsa_str = atmarpsubaddr_to_str(tsa_val, ar_tsl);
+    tsa_str = atmarpsubaddr_to_str(tsa_val, ar_tstl);
   } else {
     tsa_val = NULL;
     tsa_str = NULL;
@@ -465,41 +498,41 @@ dissect_atmarp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
   tpa_val = tvb_get_ptr(tvb, tpa_offset, ar_tpln);
   tpa_str = arpproaddr_to_str(tpa_val, ar_tpln, ar_pro);
 
-  if (check_col(pinfo->fd, COL_PROTOCOL)) {
+  if (check_col(pinfo->cinfo, COL_PROTOCOL)) {
     switch (ar_op) {
 
     case ARPOP_REQUEST:
     case ARPOP_REPLY:
     case ATMARPOP_NAK:
     default:
-      col_add_str(pinfo->fd, COL_PROTOCOL, "ATMARP");
+      col_set_str(pinfo->cinfo, COL_PROTOCOL, "ATMARP");
       break;
 
     case ARPOP_RREQUEST:
     case ARPOP_RREPLY:
-      col_add_str(pinfo->fd, COL_PROTOCOL, "ATMRARP");
+      col_set_str(pinfo->cinfo, COL_PROTOCOL, "ATMRARP");
       break;
 
     case ARPOP_IREQUEST:
     case ARPOP_IREPLY:
-      col_add_str(pinfo->fd, COL_PROTOCOL, "Inverse ATMARP");
+      col_set_str(pinfo->cinfo, COL_PROTOCOL, "Inverse ATMARP");
       break;
     }
   }
 
-  if (check_col(pinfo->fd, COL_INFO)) {
+  if (check_col(pinfo->cinfo, COL_INFO)) {
     switch (ar_op) {
       case ARPOP_REQUEST:
-        col_add_fstr(pinfo->fd, COL_INFO, "Who has %s?  Tell %s",
+        col_add_fstr(pinfo->cinfo, COL_INFO, "Who has %s?  Tell %s",
                tpa_str, spa_str);
         break;
       case ARPOP_REPLY:
-        col_add_fstr(pinfo->fd, COL_INFO, "%s is at %s%s%s", spa_str, sha_str,
+        col_add_fstr(pinfo->cinfo, COL_INFO, "%s is at %s%s%s", spa_str, sha_str,
                ((ssa_str != NULL) ? "," : ""),
                ((ssa_str != NULL) ? ssa_str : ""));
         break;
       case ARPOP_IREQUEST:
-        col_add_fstr(pinfo->fd, COL_INFO, "Who is %s%s%s?  Tell %s%s%s",
+        col_add_fstr(pinfo->cinfo, COL_INFO, "Who is %s%s%s?  Tell %s%s%s",
                tha_str,
                ((tsa_str != NULL) ? "," : ""),
                ((tsa_str != NULL) ? tsa_str : ""),
@@ -508,17 +541,17 @@ dissect_atmarp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                ((ssa_str != NULL) ? ssa_str : ""));
         break;
       case ARPOP_IREPLY:
-        col_add_fstr(pinfo->fd, COL_INFO, "%s%s%s is at %s",
+        col_add_fstr(pinfo->cinfo, COL_INFO, "%s%s%s is at %s",
                sha_str,
                ((ssa_str != NULL) ? "," : ""),
                ((ssa_str != NULL) ? ssa_str : ""),
                spa_str);
         break;
       case ATMARPOP_NAK:
-        col_add_fstr(pinfo->fd, COL_INFO, "I don't know where %s is", spa_str);
+        col_add_fstr(pinfo->cinfo, COL_INFO, "I don't know where %s is", spa_str);
         break;
       default:
-        col_add_fstr(pinfo->fd, COL_INFO, "Unknown ATMARP opcode 0x%04x", ar_op);
+        col_add_fstr(pinfo->cinfo, COL_INFO, "Unknown ATMARP opcode 0x%04x", ar_op);
         break;
     }
   }
@@ -526,48 +559,102 @@ dissect_atmarp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
   if (tree) {
     if ((op_str = match_strval(ar_op, atmop_vals)))
       ti = proto_tree_add_protocol_format(tree, proto_arp, tvb, 0, tot_len,
-                                       "ATM Address Resolution Protocol (%s)", 
+                                       "ATM Address Resolution Protocol (%s)",
                                        op_str);
     else
       ti = proto_tree_add_protocol_format(tree, proto_arp, tvb, 0, tot_len,
                                      "ATM Address Resolution Protocol (opcode 0x%04x)", ar_op);
     arp_tree = proto_item_add_subtree(ti, ett_arp);
+
     proto_tree_add_uint(arp_tree, hf_arp_hard_type, tvb, ATM_AR_HRD, 2, ar_hrd);
+
     proto_tree_add_uint(arp_tree, hf_arp_proto_type, tvb, ATM_AR_PRO, 2,ar_pro);
-    proto_tree_add_uint(arp_tree, hf_atmarp_shtl, tvb, ATM_AR_SHTL, 1, ar_shtl);
-    proto_tree_add_uint(arp_tree, hf_atmarp_ssl, tvb, ATM_AR_SSL, 1, ar_ssl);
+
+    tl = proto_tree_add_text(arp_tree, tvb, ATM_AR_SHTL, 1,
+                              "Sender ATM number type/length: %s/%u",
+                              (ar_shtl & ATMARP_IS_E164 ?
+                                 "E.164" :
+                                 "ATM Forum NSAPA"),
+                              ar_shl);
+    tl_tree = proto_item_add_subtree(tl, ett_atmarp_tl);
+    proto_tree_add_boolean(tl_tree, hf_atmarp_sht, tvb, ATM_AR_SHTL, 1, ar_shtl);
+    proto_tree_add_uint(tl_tree, hf_atmarp_shl, tvb, ATM_AR_SHTL, 1, ar_shtl);
+
+    tl = proto_tree_add_text(arp_tree, tvb, ATM_AR_SSTL, 1,
+                              "Sender ATM subaddress type/length: %s/%u",
+                              (ar_sstl & ATMARP_IS_E164 ?
+                                 "E.164" :
+                                 "ATM Forum NSAPA"),
+                              ar_ssl);
+    tl_tree = proto_item_add_subtree(tl, ett_atmarp_tl);
+    proto_tree_add_boolean(tl_tree, hf_atmarp_sst, tvb, ATM_AR_SSTL, 1, ar_sstl);
+    proto_tree_add_uint(tl_tree, hf_atmarp_ssl, tvb, ATM_AR_SSTL, 1, ar_sstl);
+
     proto_tree_add_uint(arp_tree, hf_arp_opcode, tvb, AR_OP,  2, ar_op);
+
     proto_tree_add_uint(arp_tree, hf_atmarp_spln, tvb, ATM_AR_SPLN, 1, ar_spln);
-    proto_tree_add_uint(arp_tree, hf_atmarp_thtl, tvb, ATM_AR_THTL, 1, ar_thtl);
-    proto_tree_add_uint(arp_tree, hf_atmarp_tsl, tvb, ATM_AR_TSL, 1, ar_tsl);
+
+    tl = proto_tree_add_text(arp_tree, tvb, ATM_AR_THTL, 1,
+                              "Target ATM number type/length: %s/%u",
+                              (ar_thtl & ATMARP_IS_E164 ?
+                                 "E.164" :
+                                 "ATM Forum NSAPA"),
+                              ar_thl);
+    tl_tree = proto_item_add_subtree(tl, ett_atmarp_tl);
+    proto_tree_add_boolean(tl_tree, hf_atmarp_tht, tvb, ATM_AR_THTL, 1, ar_thtl);
+    proto_tree_add_uint(tl_tree, hf_atmarp_thl, tvb, ATM_AR_THTL, 1, ar_thtl);
+
+    tl = proto_tree_add_text(arp_tree, tvb, ATM_AR_TSTL, 1,
+                              "Target ATM subaddress type/length: %s/%u",
+                              (ar_tstl & ATMARP_IS_E164 ?
+                                 "E.164" :
+                                 "ATM Forum NSAPA"),
+                              ar_tsl);
+    tl_tree = proto_item_add_subtree(tl, ett_atmarp_tl);
+    proto_tree_add_boolean(tl_tree, hf_atmarp_tst, tvb, ATM_AR_TSTL, 1, ar_tstl);
+    proto_tree_add_uint(tl_tree, hf_atmarp_tsl, tvb, ATM_AR_TSTL, 1, ar_tstl);
+
     proto_tree_add_uint(arp_tree, hf_atmarp_tpln, tvb, ATM_AR_TPLN, 1, ar_tpln);
+
     if (ar_shl != 0)
       dissect_atm_number(tvb, sha_offset, ar_shtl, hf_atmarp_src_atm_num_e164,
                               hf_atmarp_src_atm_num_nsap, arp_tree);
+
     if (ar_ssl != 0)
       proto_tree_add_bytes_format(arp_tree, hf_atmarp_src_atm_subaddr, tvb, ssa_offset,
                               ar_ssl,
                               ssa_val,
                               "Sender ATM subaddress: %s", ssa_str);
-    if (ar_spln != 0)
-      proto_tree_add_bytes_format(arp_tree, hf_arp_src_proto, tvb, spa_offset, ar_spln,
-                              spa_val,
-                              "Sender protocol address: %s", spa_str);
+
+    if (ar_spln != 0) {
+      proto_tree_add_item(arp_tree,
+       ARP_PRO_IS_IPv4(ar_pro, ar_spln) ? hf_arp_src_proto_ipv4
+                                       : hf_arp_src_proto,
+       tvb, spa_offset, ar_spln, FALSE);
+    }
+
     if (ar_thl != 0)
       dissect_atm_number(tvb, tha_offset, ar_thtl, hf_atmarp_dst_atm_num_e164,
                               hf_atmarp_dst_atm_num_nsap, arp_tree);
+
     if (ar_tsl != 0)
       proto_tree_add_bytes_format(arp_tree, hf_atmarp_dst_atm_subaddr, tvb, tsa_offset,
                               ar_tsl,
                               tsa_val,
                               "Target ATM subaddress: %s", tsa_str);
-    if (ar_tpln != 0)
-      proto_tree_add_bytes_format(arp_tree, hf_arp_dst_proto, tvb, tpa_offset, ar_tpln,
-                              tpa_val,
-                              "Target protocol address: %s", tpa_str);
+
+    if (ar_tpln != 0) {
+      proto_tree_add_item(arp_tree,
+       ARP_PRO_IS_IPv4(ar_pro, ar_tpln) ? hf_arp_dst_proto_ipv4
+                                       : hf_arp_dst_proto,
+       tvb, tpa_offset, ar_tpln, FALSE);
+    }
   }
 }
 
+static const guint8 mac_broadcast[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
+static const guint8 mac_allzero[6] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
+
 static void
 dissect_arp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
@@ -581,16 +668,23 @@ dissect_arp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
   proto_item  *ti;
   gchar       *op_str;
   int         sha_offset, spa_offset, tha_offset, tpa_offset;
-  guint8      *sha_val, *spa_val, *tha_val, *tpa_val;
+  const guint8      *sha_val, *spa_val, *tha_val, *tpa_val;
   gchar       *sha_str, *spa_str, *tha_str, *tpa_str;
 
-  CHECK_DISPLAY_AS_DATA(proto_arp, tvb, pinfo, tree);
+  /* Call it ARP, for now, so that if we throw an exception before
+     we decide whether it's ARP or RARP or IARP or ATMARP, it shows
+     up in the packet list as ARP.
 
-  pinfo->current_proto = "ARP";
+     Clear the Info column so that, if we throw an exception, it
+     shows up as a short or malformed ARP frame. */
+  if (check_col(pinfo->cinfo, COL_PROTOCOL))
+      col_set_str(pinfo->cinfo, COL_PROTOCOL, "ARP");
+  if (check_col(pinfo->cinfo, COL_INFO))
+      col_clear(pinfo->cinfo, COL_INFO);
 
   ar_hrd = tvb_get_ntohs(tvb, AR_HRD);
   if (ar_hrd == ARPHRD_ATM2225) {
-    dissect_atmarp(tvb, pinfo, tree);
+    call_dissector(atmarp_handle, tvb, pinfo, tree);
     return;
   }
   ar_pro = tvb_get_ntohs(tvb, AR_PRO);
@@ -599,89 +693,98 @@ dissect_arp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
   ar_op  = tvb_get_ntohs(tvb, AR_OP);
 
   tot_len = MIN_ARP_HEADER_SIZE + ar_hln*2 + ar_pln*2;
-  
+
   /* Adjust the length of this tvbuff to include only the ARP datagram.
      Our caller may use that to determine how much of its packet
      was padding. */
   tvb_set_reported_length(tvb, tot_len);
 
-  /* Extract the addresses.  */
+  /* Get the offsets of the addresses. */
   sha_offset = MIN_ARP_HEADER_SIZE;
-  sha_val = tvb_get_ptr(tvb, sha_offset, ar_hln);
-  sha_str = arphrdaddr_to_str(sha_val, ar_hln, ar_hrd);
-
   spa_offset = sha_offset + ar_hln;
-  spa_val = tvb_get_ptr(tvb, spa_offset, ar_pln);
-  spa_str = arpproaddr_to_str(spa_val, ar_pln, ar_pro);
-
   tha_offset = spa_offset + ar_pln;
-  tha_val = tvb_get_ptr(tvb, tha_offset, ar_hln);
-  tha_str = arphrdaddr_to_str(tha_val, ar_hln, ar_hrd);
-
   tpa_offset = tha_offset + ar_hln;
-  tpa_val = tvb_get_ptr(tvb, tpa_offset, ar_pln);
-  tpa_str = arpproaddr_to_str(tpa_val, ar_pln, ar_pro);
-  
-  if (check_col(pinfo->fd, COL_PROTOCOL)) {
+
+  if (check_col(pinfo->cinfo, COL_PROTOCOL)) {
     switch (ar_op) {
 
     case ARPOP_REQUEST:
     case ARPOP_REPLY:
     default:
-      col_add_str(pinfo->fd, COL_PROTOCOL, "ARP");
+      col_set_str(pinfo->cinfo, COL_PROTOCOL, "ARP");
       break;
 
     case ARPOP_RREQUEST:
     case ARPOP_RREPLY:
-      col_add_str(pinfo->fd, COL_PROTOCOL, "RARP");
+      col_set_str(pinfo->cinfo, COL_PROTOCOL, "RARP");
       break;
 
     case ARPOP_IREQUEST:
     case ARPOP_IREPLY:
-      col_add_str(pinfo->fd, COL_PROTOCOL, "Inverse ARP");
+      col_set_str(pinfo->cinfo, COL_PROTOCOL, "Inverse ARP");
       break;
     }
   }
 
-  if (check_col(pinfo->fd, COL_INFO)) {
+  if (check_col(pinfo->cinfo, COL_INFO)) {
+    sha_val = tvb_get_ptr(tvb, sha_offset, ar_hln);
+    sha_str = arphrdaddr_to_str(sha_val, ar_hln, ar_hrd);
+
+    spa_val = tvb_get_ptr(tvb, spa_offset, ar_pln);
+    spa_str = arpproaddr_to_str(spa_val, ar_pln, ar_pro);
+
+    tha_val = tvb_get_ptr(tvb, tha_offset, ar_hln);
+    tha_str = arphrdaddr_to_str(tha_val, ar_hln, ar_hrd);
+
+    tpa_val = tvb_get_ptr(tvb, tpa_offset, ar_pln);
+    tpa_str = arpproaddr_to_str(tpa_val, ar_pln, ar_pro);
     switch (ar_op) {
       case ARPOP_REQUEST:
-        col_add_fstr(pinfo->fd, COL_INFO, "Who has %s?  Tell %s", tpa_str, spa_str);
+        col_add_fstr(pinfo->cinfo, COL_INFO, "Who has %s?  Tell %s", tpa_str, spa_str);
         break;
       case ARPOP_REPLY:
-        col_add_fstr(pinfo->fd, COL_INFO, "%s is at %s", spa_str, sha_str);
+        col_add_fstr(pinfo->cinfo, COL_INFO, "%s is at %s", spa_str, sha_str);
         break;
       case ARPOP_RREQUEST:
       case ARPOP_IREQUEST:
-        col_add_fstr(pinfo->fd, COL_INFO, "Who is %s?  Tell %s", tha_str, sha_str);
+        col_add_fstr(pinfo->cinfo, COL_INFO, "Who is %s?  Tell %s", tha_str, sha_str);
         break;
       case ARPOP_RREPLY:
+        col_add_fstr(pinfo->cinfo, COL_INFO, "%s is at %s", tha_str, tpa_str);
+        break;
       case ARPOP_IREPLY:
-        col_add_fstr(pinfo->fd, COL_INFO, "%s is at %s", sha_str, spa_str);
+        col_add_fstr(pinfo->cinfo, COL_INFO, "%s is at %s", sha_str, spa_str);
         break;
       default:
-        col_add_fstr(pinfo->fd, COL_INFO, "Unknown ARP opcode 0x%04x", ar_op);
+        col_add_fstr(pinfo->cinfo, COL_INFO, "Unknown ARP opcode 0x%04x", ar_op);
         break;
     }
   }
 
   if ((ar_op == ARPOP_REPLY || ar_op == ARPOP_REQUEST) &&
-      ar_hln == 6 && ar_pln == 4) {
+      ARP_HW_IS_ETHER(ar_hrd, ar_hln) &&
+      ARP_PRO_IS_IPv4(ar_pro, ar_pln)) {
 
     /* inform resolv.c module of the new discovered addresses */
 
-    u_int ip;
-
-    /* add sender address in all cases */
+    guint ip;
+    const guint8 *mac;
 
+    /* Add sender address if sender MAC address is neither a broadcast/
+       multicast address nor an all-zero address and if sender IP address
+       isn't all zeroes. */
     tvb_memcpy(tvb, (guint8 *)&ip, spa_offset, sizeof(ip));
-    add_ether_byip(ip, tvb_get_ptr(tvb, sha_offset, 6));
-    
-    if (ar_op == ARPOP_REQUEST) {
-      /* add destination address */
-      tvb_memcpy(tvb, (guint8 *)&ip, tpa_offset, sizeof(ip));
-      add_ether_byip(ip, tvb_get_ptr(tvb, tha_offset, 6));
-    }
+    mac = tvb_get_ptr(tvb, sha_offset, 6);
+    if ((mac[0] & 0x01) == 0 && memcmp(mac, mac_allzero, 6) != 0 && ip != 0)
+      add_ether_byip(ip, mac);
+
+    /* Add target address if target MAC address is neither a broadcast/
+       multicast address nor an all-zero address and if target IP address
+       isn't all zeroes. */
+    tvb_memcpy(tvb, (guint8 *)&ip, tpa_offset, sizeof(ip));
+    mac = tvb_get_ptr(tvb, tha_offset, 6);
+    if ((mac[0] & 0x01) == 0 && memcmp(mac, mac_allzero, 6) != 0 && ip != 0)
+      add_ether_byip(ip, mac);
   }
 
   if (tree) {
@@ -697,147 +800,209 @@ dissect_arp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
     proto_tree_add_uint(arp_tree, hf_arp_hard_size, tvb, AR_HLN, 1, ar_hln);
     proto_tree_add_uint(arp_tree, hf_arp_proto_size, tvb, AR_PLN, 1, ar_pln);
     proto_tree_add_uint(arp_tree, hf_arp_opcode, tvb, AR_OP,  2, ar_op);
-    if (ar_hln != 0)
-      proto_tree_add_bytes_format(arp_tree, hf_arp_src_ether, tvb, sha_offset, ar_hln,
-                              sha_val,
-                              "Sender hardware address: %s", sha_str);
-    if (ar_pln != 0)
-      proto_tree_add_bytes_format(arp_tree, hf_arp_src_proto, tvb, spa_offset, ar_pln,
-                              spa_val,
-                              "Sender protocol address: %s", spa_str);
-    if (ar_hln != 0)
-      proto_tree_add_bytes_format(arp_tree, hf_arp_dst_ether, tvb, tha_offset, ar_hln,
-                              tha_val,
-                              "Target hardware address: %s", tha_str);
-    if (ar_pln != 0)
-      proto_tree_add_bytes_format(arp_tree, hf_arp_dst_proto, tvb, tpa_offset, ar_pln,
-                              tpa_val,
-                              "Target protocol address: %s", tpa_str);
+    if (ar_hln != 0) {
+      proto_tree_add_item(arp_tree,
+       ARP_HW_IS_ETHER(ar_hrd, ar_hln) ? hf_arp_src_hw_mac : hf_arp_src_hw,
+       tvb, sha_offset, ar_hln, FALSE);
+    }
+    if (ar_pln != 0) {
+      proto_tree_add_item(arp_tree,
+       ARP_PRO_IS_IPv4(ar_pro, ar_pln) ? hf_arp_src_proto_ipv4
+                                       : hf_arp_src_proto,
+       tvb, spa_offset, ar_pln, FALSE);
+    }
+    if (ar_hln != 0) {
+      proto_tree_add_item(arp_tree,
+       ARP_HW_IS_ETHER(ar_hrd, ar_hln) ? hf_arp_dst_hw_mac : hf_arp_dst_hw,
+       tvb, tha_offset, ar_hln, FALSE);
+    }
+    if (ar_pln != 0) {
+      proto_tree_add_item(arp_tree,
+       ARP_PRO_IS_IPv4(ar_pro, ar_pln) ? hf_arp_dst_proto_ipv4
+                                       : hf_arp_dst_proto,
+       tvb, tpa_offset, ar_pln, FALSE);
+    }
   }
 }
 
 void
 proto_register_arp(void)
 {
+  static struct true_false_string type_bit = { "E.164", "ATM Forum NSAPA" };
+
   static hf_register_info hf[] = {
     { &hf_arp_hard_type,
-      { "Hardware type",               "arp.hw.type",   
+      { "Hardware type",               "arp.hw.type",
        FT_UINT16,      BASE_HEX,       VALS(hrd_vals), 0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_arp_proto_type,
       { "Protocol type",               "arp.proto.type",
        FT_UINT16,      BASE_HEX,       VALS(etype_vals),       0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_arp_hard_size,
       { "Hardware size",               "arp.hw.size",
        FT_UINT8,       BASE_DEC,       NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
-    { &hf_atmarp_shtl,
-      { "Sender ATM number type and length",   "arp.src.htl",  
-       FT_UINT8,       BASE_DEC,       NULL,   0x0,
-       "" }},
+    { &hf_atmarp_sht,
+      { "Sender ATM number type",      "arp.src.htype",
+       FT_BOOLEAN,     8,              &type_bit,      ATMARP_IS_E164,
+       "", HFILL }},
+
+    { &hf_atmarp_shl,
+      { "Sender ATM number length",    "arp.src.hlen",
+       FT_UINT8,       BASE_DEC,       NULL,           ATMARP_LEN_MASK,
+       "", HFILL }},
+
+    { &hf_atmarp_sst,
+      { "Sender ATM subaddress type",  "arp.src.stype",
+       FT_BOOLEAN,     8,              &type_bit,      ATMARP_IS_E164,
+       "", HFILL }},
 
     { &hf_atmarp_ssl,
       { "Sender ATM subaddress length",        "arp.src.slen",
-       FT_UINT8,       BASE_DEC,       NULL,   0x0,
-       "" }},
+       FT_UINT8,       BASE_DEC,       NULL,           ATMARP_LEN_MASK,
+       "", HFILL }},
 
     { &hf_arp_proto_size,
       { "Protocol size",               "arp.proto.size",
        FT_UINT8,       BASE_DEC,       NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_arp_opcode,
       { "Opcode",                      "arp.opcode",
        FT_UINT16,      BASE_HEX,       VALS(op_vals),  0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_atmarp_spln,
       { "Sender protocol size",                "arp.src.pln",
        FT_UINT8,       BASE_DEC,       NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
-    { &hf_atmarp_thtl,
-      { "Target ATM number type and length",   "arp.dst.htl",
-       FT_UINT8,       BASE_DEC,       NULL,   0x0,
-       "" }},
+    { &hf_atmarp_tht,
+      { "Target ATM number type",      "arp.dst.htype",
+       FT_BOOLEAN,     8,              &type_bit,      ATMARP_IS_E164,
+       "", HFILL }},
+
+    { &hf_atmarp_thl,
+      { "Target ATM number length",    "arp.dst.hlen",
+       FT_UINT8,       BASE_DEC,       NULL,           ATMARP_LEN_MASK,
+       "", HFILL }},
+
+    { &hf_atmarp_tst,
+      { "Target ATM subaddress type",  "arp.dst.stype",
+       FT_BOOLEAN,     8,              &type_bit,      ATMARP_IS_E164,
+       "", HFILL }},
 
     { &hf_atmarp_tsl,
       { "Target ATM subaddress length",        "arp.dst.slen",
-       FT_UINT8,       BASE_DEC,       NULL,   0x0,
-       "" }},
+       FT_UINT8,       BASE_DEC,       NULL,           ATMARP_LEN_MASK,
+       "", HFILL }},
 
     { &hf_atmarp_tpln,
       { "Target protocol size",                "arp.dst.pln",
        FT_UINT8,       BASE_DEC,       NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
-    { &hf_arp_src_ether,
+    { &hf_arp_src_hw,
       { "Sender hardware address",     "arp.src.hw",
        FT_BYTES,       BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
+
+    { &hf_arp_src_hw_mac,
+      { "Sender MAC address",          "arp.src.hw_mac",
+       FT_ETHER,       BASE_NONE,      NULL,   0x0,
+       "", HFILL }},
 
     { &hf_atmarp_src_atm_num_e164,
       { "Sender ATM number (E.164)",   "arp.src.atm_num_e164",
        FT_STRING,      BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_atmarp_src_atm_num_nsap,
       { "Sender ATM number (NSAP)",    "arp.src.atm_num_nsap",
        FT_BYTES,       BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_atmarp_src_atm_subaddr,
       { "Sender ATM subaddress",       "arp.src.atm_subaddr",
        FT_BYTES,       BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_arp_src_proto,
-      { "Sender protocol address",     "arp.src.proto", 
+      { "Sender protocol address",     "arp.src.proto",
        FT_BYTES,       BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
-    { &hf_arp_dst_ether,
+    { &hf_arp_src_proto_ipv4,
+      { "Sender IP address",           "arp.src.proto_ipv4",
+       FT_IPv4,        BASE_NONE,      NULL,   0x0,
+       "", HFILL }},
+
+    { &hf_arp_dst_hw,
       { "Target hardware address",     "arp.dst.hw",
        FT_BYTES,       BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
+
+    { &hf_arp_dst_hw_mac,
+      { "Target MAC address",          "arp.dst.hw_mac",
+       FT_ETHER,       BASE_NONE,      NULL,   0x0,
+       "", HFILL }},
 
     { &hf_atmarp_dst_atm_num_e164,
       { "Target ATM number (E.164)",   "arp.dst.atm_num_e164",
        FT_STRING,      BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_atmarp_dst_atm_num_nsap,
       { "Target ATM number (NSAP)",    "arp.dst.atm_num_nsap",
        FT_BYTES,       BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_atmarp_dst_atm_subaddr,
       { "Target ATM subaddress",       "arp.dst.atm_subaddr",
        FT_BYTES,       BASE_NONE,      NULL,   0x0,
-       "" }},
+       "", HFILL }},
 
     { &hf_arp_dst_proto,
-      { "Target protocol address",     "arp.dst.proto", 
+      { "Target protocol address",     "arp.dst.proto",
        FT_BYTES,       BASE_NONE,      NULL,   0x0,
-      "" }}
+      "", HFILL }},
+
+    { &hf_arp_dst_proto_ipv4,
+      { "Target IP address",           "arp.dst.proto_ipv4",
+       FT_IPv4,        BASE_NONE,      NULL,   0x0,
+      "", HFILL }}
   };
   static gint *ett[] = {
     &ett_arp,
     &ett_atmarp_nsap,
+    &ett_atmarp_tl,
   };
 
-  proto_arp = proto_register_protocol("Address Resolution Protocol", "arp");
+  proto_arp = proto_register_protocol("Address Resolution Protocol",
+                                     "ARP/RARP", "arp");
   proto_register_field_array(proto_arp, hf, array_length(hf));
   proto_register_subtree_array(ett, array_length(ett));
+
+  atmarp_handle = create_dissector_handle(dissect_atmarp, proto_arp);
+
+  register_dissector( "arp" , dissect_arp, proto_arp );
 }
 
 void
 proto_reg_handoff_arp(void)
 {
-  dissector_add("ethertype", ETHERTYPE_ARP, dissect_arp);
-  dissector_add("ethertype", ETHERTYPE_REVARP, dissect_arp);
+  dissector_handle_t arp_handle;
+
+  arp_handle = find_dissector("arp");
+
+  dissector_add("ethertype", ETHERTYPE_ARP, arp_handle);
+  dissector_add("ethertype", ETHERTYPE_REVARP, arp_handle);
+  dissector_add("arcnet.protocol_id", ARCNET_PROTO_ARP_1051, arp_handle);
+  dissector_add("arcnet.protocol_id", ARCNET_PROTO_ARP_1201, arp_handle);
+  dissector_add("arcnet.protocol_id", ARCNET_PROTO_RARP_1201, arp_handle);
+
 }