On Wed, Nov 08, 2000 at 08:08:15AM +1100, Darren Reed wrote:

 > Hmmm, it would make sense for all packet information going through
 > pfil to either be in network byte order or host byte order.  Since
 > it is both impracticle and awkward to do the latter for all places
 > where you might want to add filtering hooks, changing it to always
 > pass packets in network byte order does seem better.

So, here is my proposed patch.

-- 
        -- Jason R. Thorpe <thorpej@zembu.com>

Index: fil.c
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/fil.c,v
retrieving revision 1.40
diff -c -r1.40 fil.c
*** fil.c	2000/10/08 13:01:30	1.40
--- fil.c	2000/11/08 01:14:24
***************
*** 713,718 ****
--- 713,744 ----
  	return pass;
  }
  
+ #if defined(__NetBSD__) && defined(_KERNEL)
+ int
+ fr_check_wrapper(ip_t *ip, int hlen, void *ifp, int out, mb_t **mp)
+ {
+ 	int rv;
+ 
+ 	/*
+ 	 * We get the packet with all fields in network byte
+ 	 * order.  We expect ip_len and ip_off to be in host
+ 	 * order.  We frob them, call the filter, then frob
+ 	 * them back.
+ 	 *
+ 	 * Note, we don't need to update the checksum, because
+ 	 * it has already been verified.
+ 	 */
+ 	NTOHS(ip->ip_len);
+ 	NTOHS(ip->ip_off);
+ 
+ 	rv = fr_check(ip, hlen, ifp, out, mp);
+ 
+ 	HTONS(ip->ip_len);
+ 	HTONS(ip->ip_off);
+ 
+ 	return (rv);
+ }
+ #endif /* __NetBSD__ && _KERNEL */
  
  /*
   * frcheck - filter check
Index: ip_fil.c
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/ip_fil.c,v
retrieving revision 1.59
diff -c -r1.59 ip_fil.c
*** ip_fil.c	2000/08/22 16:02:16	1.59
--- ip_fil.c	2000/11/08 01:14:25
***************
*** 265,271 ****
  
  # ifdef NETBSD_PF
  #  if __NetBSD_Version__ >= 104200000
! 	error = pfil_add_hook((void *)fr_check, PFIL_IN|PFIL_OUT,
  			      &inetsw[ip_protox[IPPROTO_IP]].pr_pfh);
  	if (error) {
  #   ifdef USE_INET6
--- 265,271 ----
  
  # ifdef NETBSD_PF
  #  if __NetBSD_Version__ >= 104200000
! 	error = pfil_add_hook((void *)fr_check_wrapper, PFIL_IN|PFIL_OUT,
  			      &inetsw[ip_protox[IPPROTO_IP]].pr_pfh);
  	if (error) {
  #   ifdef USE_INET6
***************
*** 284,290 ****
  	error = pfil_add_hook((void *)fr_check, PFIL_IN|PFIL_OUT,
  			      &inetsw[ip_protox[IPPROTO_IPV6]].pr_pfh);
  	if (error) {
! 		pfil_remove_hook((void *)fr_check, PFIL_IN|PFIL_OUT,
  				 &inetsw[ip_protox[IPPROTO_IP]].pr_pfh);
  pfil_error:
  		appr_unload();
--- 284,290 ----
  	error = pfil_add_hook((void *)fr_check, PFIL_IN|PFIL_OUT,
  			      &inetsw[ip_protox[IPPROTO_IPV6]].pr_pfh);
  	if (error) {
! 		pfil_remove_hook((void *)fr_check_wrapper, PFIL_IN|PFIL_OUT,
  				 &inetsw[ip_protox[IPPROTO_IP]].pr_pfh);
  pfil_error:
  		appr_unload();
***************
*** 388,394 ****
  
  # ifdef NETBSD_PF
  #  if __NetBSD_Version__ >= 104200000
! 	error = pfil_remove_hook((void *)fr_check, PFIL_IN|PFIL_OUT,
  				 &inetsw[ip_protox[IPPROTO_IP]].pr_pfh);
  	if (error)
  		return error;
--- 388,394 ----
  
  # ifdef NETBSD_PF
  #  if __NetBSD_Version__ >= 104200000
! 	error = pfil_remove_hook((void *)fr_check_wrapper, PFIL_IN|PFIL_OUT,
  				 &inetsw[ip_protox[IPPROTO_IP]].pr_pfh);
  	if (error)
  		return error;
***************
*** 396,402 ****
  	pfil_remove_hook((void *)fr_check, PFIL_IN|PFIL_OUT);
  #  endif
  #  ifdef USE_INET6
! 	error = pfil_remove_hook((void *)fr_check, PFIL_IN|PFIL_OUT,
  				 &inetsw[ip_protox[IPPROTO_IPV6]].pr_pfh);
  	if (error)
  		return error;
--- 396,402 ----
  	pfil_remove_hook((void *)fr_check, PFIL_IN|PFIL_OUT);
  #  endif
  #  ifdef USE_INET6
! 	error = pfil_remove_hook((void *)fr_check_wrapper, PFIL_IN|PFIL_OUT,
  				 &inetsw[ip_protox[IPPROTO_IPV6]].pr_pfh);
  	if (error)
  		return error;
Index: ip_fil.h
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/ip_fil.h,v
retrieving revision 1.37
diff -c -r1.37 ip_fil.h
*** ip_fil.h	2000/06/12 10:28:21	1.37
--- ip_fil.h	2000/11/08 01:14:25
***************
*** 531,536 ****
--- 531,539 ----
  extern	int	fr_qout __P((queue_t *, mblk_t *));
  extern	int	iplread __P((dev_t, struct uio *, cred_t *));
  # else /* SOLARIS */
+ #if defined(__NetBSD__)
+ extern	int	fr_check_wrapper __P((ip_t *, int, void *, int, mb_t **));
+ #endif
  extern	int	fr_check __P((ip_t *, int, void *, int, mb_t **));
  extern	int	(*fr_checkp) __P((ip_t *, int, void *, int, mb_t **));
  extern	int	ipfr_fastroute __P((mb_t *, fr_info_t *, frdest_t *));
Index: ip_input.c
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/ip_input.c,v
retrieving revision 1.119
diff -c -r1.119 ip_input.c
*** ip_input.c	2000/10/13 01:50:04	1.119
--- ip_input.c	2000/11/08 01:14:26
***************
*** 414,425 ****
  		goto bad;
  	}
  
! 	/*
! 	 * Convert fields to host representation.
! 	 */
! 	NTOHS(ip->ip_len);
! 	NTOHS(ip->ip_off);
! 	len = ip->ip_len;
  
  	/*
  	 * Check for additional length bogosity
--- 414,421 ----
  		goto bad;
  	}
  
! 	/* Retrieve the packet length. */
! 	len = ntohs(ip->ip_len);
  
  	/*
  	 * Check for additional length bogosity
***************
*** 480,485 ****
--- 476,487 ----
  			ip = mtod(m, struct ip *);
  		}
  #endif /* PFIL_HOOKS */
+ 
+ 	/*
+ 	 * Convert fields to host representation.
+ 	 */
+ 	NTOHS(ip->ip_len);
+ 	NTOHS(ip->ip_off);
  
  	/*
  	 * Process options and, if not destined for us,
Index: ip_output.c
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/ip_output.c,v
retrieving revision 1.77
diff -c -r1.77 ip_output.c
*** ip_output.c	2000/10/23 03:42:18	1.77
--- ip_output.c	2000/11/08 01:14:27
***************
*** 185,190 ****
--- 185,191 ----
  	struct socket *so;
  	struct secpolicy *sp = NULL;
  #endif /*IPSEC*/
+ 	u_int16_t ip_len, ip_off;
  
  	va_start(ap, m0);
  	opt = va_arg(ap, struct mbuf *);
***************
*** 420,425 ****
--- 421,436 ----
  	    (ro->ro_rt->rt_rmx.rmx_locks & RTV_MTU) == 0)
  		ip->ip_off |= IP_DF;
  
+ 	/*
+ 	 * Remember the current ip_len and ip_off, and swap them into
+ 	 * network order.
+ 	 */
+ 	ip_len = ip->ip_len;
+ 	ip_off = ip->ip_off;
+ 
+ 	HTONS(ip->ip_len);
+ 	HTONS(ip->ip_off);
+ 
  #ifdef PFIL_HOOKS
  	/*
  	 * Run through list of hooks for output packets.
***************
*** 482,490 ****
  		printf("ip_output: Invalid policy found. %d\n", sp->policy);
  	}
  
! 	ip->ip_len = htons((u_short)ip->ip_len);
! 	ip->ip_off = htons((u_short)ip->ip_off);
! 	ip->ip_sum = 0;
  
      {
  	struct ipsec_output_state state;
--- 493,502 ----
  		printf("ip_output: Invalid policy found. %d\n", sp->policy);
  	}
  
! 	/*
! 	 * ipsec4_output() expects ip_len and ip_off in network
! 	 * order.  They have been set to network order above.
! 	 */
  
      {
  	struct ipsec_output_state state;
***************
*** 541,546 ****
--- 553,561 ----
  #else
  	hlen = ip->ip_hl << 2;
  #endif
+ 	ip_len = ntohs(ip->ip_len);
+ 	ip_off = ntohs(ip->ip_off);
+ 
  	if (ro->ro_rt == NULL) {
  		if ((flags & IP_ROUTETOIF) == 0) {
  			printf("ip_output: "
***************
*** 553,568 ****
  		ifp = ro->ro_rt->rt_ifp;
  	}
  
- 	/* make it flipped, again. */
- 	ip->ip_len = ntohs((u_short)ip->ip_len);
- 	ip->ip_off = ntohs((u_short)ip->ip_off);
  skip_ipsec:
  #endif /*IPSEC*/
  
  	/*
  	 * If small enough for mtu of path, can just send directly.
  	 */
! 	if ((u_int16_t)ip->ip_len <= mtu) {
  #if IFA_STATS
  		/*
  		 * search for the source address structure to
--- 568,580 ----
  		ifp = ro->ro_rt->rt_ifp;
  	}
  
  skip_ipsec:
  #endif /*IPSEC*/
  
  	/*
  	 * If small enough for mtu of path, can just send directly.
  	 */
! 	if (ip_len <= mtu) {
  #if IFA_STATS
  		/*
  		 * search for the source address structure to
***************
*** 570,579 ****
  		 */
  		INADDR_TO_IA(ip->ip_src, ia);
  		if (ia)
! 			ia->ia_ifa.ifa_data.ifad_outbytes += ip->ip_len;
  #endif
- 		HTONS(ip->ip_len);
- 		HTONS(ip->ip_off);
  		ip->ip_sum = 0;
  		ip->ip_sum = in_cksum(m, hlen);
  		error = (*ifp->if_output)(ifp, m, sintosa(dst), ro->ro_rt);
--- 582,589 ----
  		 */
  		INADDR_TO_IA(ip->ip_src, ia);
  		if (ia)
! 			ia->ia_ifa.ifa_data.ifad_outbytes += ip_len;
  #endif
  		ip->ip_sum = 0;
  		ip->ip_sum = in_cksum(m, hlen);
  		error = (*ifp->if_output)(ifp, m, sintosa(dst), ro->ro_rt);
***************
*** 583,589 ****
--- 593,606 ----
  	/*
  	 * Too large for interface; fragment if possible.
  	 * Must be able to put at least 8 bytes per fragment.
+ 	 *
+ 	 * Note we swap ip_len and ip_off into host order to make
+ 	 * the logic below a little simpler.
  	 */
+ 
+ 	NTOHS(ip->ip_len);
+ 	NTOHS(ip->ip_off);
+ 
  #if 0
  	/*
  	 * If IPsec packet is too big for the interface, try fragment it.