Subject: Re: multiple copies of arp_lock_try()
To: Atsushi Onoe <onoe@sm.sony.co.jp>
From: Jun-ichiro itojun Hagino <itojun@iijlab.net>
List: tech-net
Date: 06/24/2002 15:37:52
>> another question - why do we need if_ieee1394arp.c? i took a diff
>> between if_arp.c and i saw almost no difference...
>Hmm. I can't recall the reason but it seems simply because 1394ARP is
>defined differently from normal ARP. It doesn't have target hardware
>address, and the length of hardware address is defined as 16 rather
>than 8 (IEEE1394_ADDR_LEN).
>But as you see, we designed 1394 interface with address length 16 which is
>the same format as 1394ARP. The difference in code is minimal as a result.
>So we may merge it into if_arp.c, though my knowledge about 1394 is almost
>gone...
if the only difference is the lack of target hardware address, does the
following diff look sane? i really need an environment to test it...
itojun
Index: netinet/if_ieee1394arp.c - remove
Index: netinet/if_ieee1394arp.h - remove
Index: conf/files
===================================================================
RCS file: /cvsroot/syssrc/sys/conf/files,v
retrieving revision 1.535
diff -u -r1.535 files
--- conf/files 2002/06/18 00:33:57 1.535
+++ conf/files 2002/06/24 06:34:13
@@ -1156,7 +1156,6 @@
file netccitt/pk_usrreq.c ccitt
file netinet/fil.c ipfilter
file netinet/if_arp.c arp | netatalk needs-flag
-file netinet/if_ieee1394arp.c arp & ieee1394 & inet
file netinet/if_atm.c atm
file netinet/igmp.c inet
file netinet/in.c inet
Index: net/if_arp.h
===================================================================
RCS file: /cvsroot/syssrc/sys/net/if_arp.h,v
retrieving revision 1.22
diff -u -r1.22 if_arp.h
--- net/if_arp.h 2001/06/12 15:17:27 1.22
+++ net/if_arp.h 2002/06/24 06:34:13
@@ -76,9 +76,14 @@
u_int8_t ar_tpa[]; /* target protocol address */
#endif
#define ar_sha(ap) (((caddr_t)((ap)+1))+0)
-#define ar_spa(ap) (((caddr_t)((ap)+1))+ (ap)->ar_hln)
-#define ar_tha(ap) (((caddr_t)((ap)+1))+ (ap)->ar_hln+(ap)->ar_pln)
-#define ar_tpa(ap) (((caddr_t)((ap)+1))+2*(ap)->ar_hln+(ap)->ar_pln)
+#define ar_spa(ap) (((caddr_t)((ap)+1))+(ap)->ar_hln)
+#define ar_tha(ap) \
+ (ntohs((ap)->ar_hrd) == ARPHRD_IEEE1394 \
+ ? NULL : (((caddr_t)((ap)+1))+(ap)->ar_hln+(ap)->ar_pln))
+#define ar_tpa(ap) \
+ (ntohs((ap)->ar_hrd) == ARPHRD_IEEE1394 \
+ ? (((caddr_t)((ap)+1))+(ap)->ar_hln+(ap)->ar_pln) \
+ : (((caddr_t)((ap)+1))+(ap)->ar_hln+(ap)->ar_pln+(ap)->ar_hln))
} __attribute__((__packed__));
Index: net/if_ieee1394subr.c
===================================================================
RCS file: /cvsroot/syssrc/sys/net/if_ieee1394subr.c,v
retrieving revision 1.15
diff -u -r1.15 if_ieee1394subr.c
--- net/if_ieee1394subr.c 2002/05/16 09:08:33 1.15
+++ net/if_ieee1394subr.c 2002/06/24 06:34:13
@@ -66,7 +66,7 @@
#ifdef INET
#include <netinet/in.h>
#include <netinet/in_var.h>
-#include <netinet/if_ieee1394arp.h>
+#include <netinet/if_inarp.h>
#endif /* INET */
#ifdef INET6
#include <netinet/in.h>
@@ -93,9 +93,6 @@
struct rtentry *rt;
struct mbuf *mcopy = NULL;
struct ieee1394_hwaddr hwdst, *myaddr;
-#ifdef INET
- struct ieee1394_arphdr *ah;
-#endif /* INET */
ALTQ_DECL(struct altq_pktattr pktattr;)
if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING))
@@ -146,7 +143,7 @@
case AF_INET:
if (m0->m_flags & (M_BCAST | M_MCAST))
memcpy(&hwdst, ifp->if_broadcastaddr, sizeof(hwdst));
- else if (!ieee1394arpresolve(ifp, rt, m0, dst, &hwdst))
+ else if (!arpresolve(ifp, rt, m0, dst, (u_char *)&hwdst))
return 0; /* if not yet resolved */
/* if broadcasting on a simplex interface, loopback a copy */
if ((m0->m_flags & M_BCAST) && (ifp->if_flags & IFF_SIMPLEX))
@@ -154,7 +151,6 @@
etype = htons(ETHERTYPE_IP);
break;
case AF_ARP:
- ah = mtod(m0, struct ieee1394_arphdr *);
memcpy(&hwdst, ifp->if_broadcastaddr, sizeof(hwdst));
etype = htons(ETHERTYPE_ARP);
break;
@@ -388,7 +384,8 @@
break;
case ETHERTYPE_ARP:
- in_ieee1394arpinput(m);
+ schednetisr(NETISR_ARP);
+ inq = &arpintrq;
return;
#endif /* INET */
@@ -703,7 +700,7 @@
if ((error = fw_init(ifp)) != 0)
#endif
break;
- ieee1394arp_ifinit(ifp, ifa);
+ arp_ifinit(ifp, ifa);
break;
#endif /* INET */
default:
Index: netinet/if_arp.c
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/if_arp.c,v
retrieving revision 1.81
diff -u -r1.81 if_arp.c
--- netinet/if_arp.c 2002/06/09 16:33:37 1.81
+++ netinet/if_arp.c 2002/06/24 06:34:14
@@ -110,7 +110,6 @@
#include <net/if_types.h>
#include <net/route.h>
-
#include <netinet/in.h>
#include <netinet/in_systm.h>
#include <netinet/in_var.h>
@@ -247,9 +246,8 @@
* protection of the ARP_LOCK; if arp_drain() or arptimer is called
* while the arp table is locked, we punt and try again later.
*/
-
-int arp_locked;
+static int arp_locked;
static __inline int arp_lock_try __P((int));
static __inline void arp_unlock __P((void));
@@ -1014,14 +1012,18 @@
arpstat.as_rcvrequest++;
if (in_hosteq(itaddr, myaddr)) {
/* I am the target */
- bcopy((caddr_t)ar_sha(ah), (caddr_t)ar_tha(ah), ah->ar_hln);
+ if (ar_tha(ah))
+ bcopy((caddr_t)ar_sha(ah), (caddr_t)ar_tha(ah),
+ ah->ar_hln);
bcopy(LLADDR(ifp->if_sadl), (caddr_t)ar_sha(ah), ah->ar_hln);
} else {
la = arplookup(m, &itaddr, 0, SIN_PROXY);
if (la == 0)
goto out;
rt = la->la_rt;
- bcopy((caddr_t)ar_sha(ah), (caddr_t)ar_tha(ah), ah->ar_hln);
+ if (ar_tha(ah))
+ bcopy((caddr_t)ar_sha(ah), (caddr_t)ar_tha(ah),
+ ah->ar_hln);
sdl = SDL(rt->rt_gateway);
bcopy(LLADDR(sdl), (caddr_t)ar_sha(ah), ah->ar_hln);
}
@@ -1193,6 +1195,11 @@
ah = mtod(m, struct arphdr *);
op = ntohs(ah->ar_op);
+
+ /* ARP without target hardware address (IEEE1394) is not supported */
+ if (!ar_tha(ah))
+ goto out;
+
switch (op) {
case ARPOP_REQUEST:
case ARPOP_REPLY: /* per RFC */
@@ -1410,4 +1417,3 @@
}
#endif
#endif /* INET */
-