svn commit: r225586 - in head/sys: modules/netgraph/ipfw netgraph

[Gleb Smirnoff]

  Alexander, Andrey,

  see a couple of comments below please.

On Thu, Sep 15, 2011 at 12:28:17PM +0000, Andrey V. Elsukov wrote:
A> Author: ae
A> Date: Thu Sep 15 12:28:17 2011
A> New Revision: 225586
A> URL: http://svn.freebsd.org/changeset/base/225586
A> 
A> Log:
A>   Add IPv6 support to the ng_ipfw(4) [1]. Also add ifdefs to be able
A>   build it with and without INET/INET6 support.
A>   
A>   Submitted by:	Alexander V. Chernikov <melifaro at yandex-team.ru> [1]
A>   Tested by:	Alexander V. Chernikov <melifaro at yandex-team.ru> [1]
A>   Approved by:	re (bz)
A>   MFC after:	2 weeks
A> 
A> Modified:
A>   head/sys/modules/netgraph/ipfw/Makefile
A>   head/sys/netgraph/ng_ipfw.c
A> 
A> Modified: head/sys/modules/netgraph/ipfw/Makefile
A> ==============================================================================
A> --- head/sys/modules/netgraph/ipfw/Makefile	Thu Sep 15 12:27:26 2011	(r225585)
A> +++ head/sys/modules/netgraph/ipfw/Makefile	Thu Sep 15 12:28:17 2011	(r225586)
A> @@ -1,6 +1,20 @@
A>  # $FreeBSD$
A>  
A> +.include <bsd.own.mk>
A> +
A>  KMOD=	ng_ipfw
A> -SRCS= 	ng_ipfw.c
A> +SRCS= 	ng_ipfw.c opt_inet.h opt_inet6.h
A> +
A> +.if !defined(KERNBUILDDIR)
A> +
A> +.if ${MK_INET_SUPPORT} != "no"
A> +opt_inet.h:
A> +	echo "#define INET 1" > ${.TARGET}
A> +.endif
A> +.if ${MK_INET6_SUPPORT} != "no"
A> +opt_inet6.h:
A> +	echo "#define INET6 1" > ${.TARGET}
A> +.endif
A> +.endif
A>  
A>  .include <bsd.kmod.mk>
A> 
A> Modified: head/sys/netgraph/ng_ipfw.c
A> ==============================================================================
A> --- head/sys/netgraph/ng_ipfw.c	Thu Sep 15 12:27:26 2011	(r225585)
A> +++ head/sys/netgraph/ng_ipfw.c	Thu Sep 15 12:28:17 2011	(r225586)
A> @@ -26,6 +26,9 @@
A>   * $FreeBSD$
A>   */
A>  
A> +#include "opt_inet.h"
A> +#include "opt_inet6.h"
A> +
A>  #include <sys/param.h>
A>  #include <sys/systm.h>
A>  #include <sys/kernel.h>
A> @@ -47,6 +50,8 @@
A>  #include <netinet/ip_fw.h>
A>  #include <netinet/ipfw/ip_fw_private.h>
A>  #include <netinet/ip.h>
A> +#include <netinet/ip6.h>
A> +#include <netinet6/ip6_var.h>
A>  
A>  #include <netgraph/ng_message.h>
A>  #include <netgraph/ng_parse.h>
A> @@ -224,6 +229,7 @@ ng_ipfw_rcvdata(hook_p hook, item_p item
A>  	struct m_tag *tag;
A>  	struct ipfw_rule_ref *r;
A>  	struct mbuf *m;
A> +	struct ip *ip;
A>  
A>  	NGI_GET_M(item, m);
A>  	NG_FREE_ITEM(item);
A> @@ -234,23 +240,47 @@ ng_ipfw_rcvdata(hook_p hook, item_p item
A>  		return (EINVAL);	/* XXX: find smth better */
A>  	};
A>  
A> +	if (m->m_len < sizeof(struct ip) &&
A> +	    (m = m_pullup(m, sizeof(struct ip))) == NULL)
A> +		return (EINVAL);

In most cases we return ENOBUFS in case if m_pullup() failure. Lesser amount
of code uses ENOMEM and EINVAL. I personally hate EINVAL, since it is usually
used as one-for-all error, and thus is meaningless for user.

A> +	ip = mtod(m, struct ip *);
A> +
A>  	r = (struct ipfw_rule_ref *)(tag + 1);
A>  	if (r->info & IPFW_INFO_IN) {
A> -		ip_input(m);
A> +		switch (ip->ip_v) {
A> +#ifdef INET
A> +		case IPVERSION:
A> +			ip_input(m);
A> +			break;
A> +#endif
A> +#ifdef INET6
A> +		case IPV6_VERSION >> 4:
A> +			ip6_input(m);
A> +			break;
A> +#endif
A> +		default:
A> +			NG_FREE_M(m);
A> +			return (EINVAL);
A> +		}
A>  		return (0);
A>  	} else {
A> -		struct ip *ip;
A> -
A> -		if (m->m_len < sizeof(struct ip) &&
A> -		    (m = m_pullup(m, sizeof(struct ip))) == NULL)
A> +		switch (ip->ip_v) {
A> +#ifdef INET
A> +		case IPVERSION:
A> +			SET_HOST_IPLEN(ip);
A> +			return (ip_output(m, NULL, NULL, IP_FORWARDING,
A> +			    NULL, NULL));
A> +#endif
A> +#ifdef INET6
A> +		case IPV6_VERSION >> 4:
A> +			return (ip6_output(m, NULL, NULL, 0, NULL,
A> +			    NULL, NULL));
A> +#endif
A> +		default:
A>  			return (EINVAL);

Shouldn't you free the mbuf before returning?

A> -
A> -		ip = mtod(m, struct ip *);
A> -
A> -		SET_HOST_IPLEN(ip);
A> -
A> -		return ip_output(m, NULL, NULL, IP_FORWARDING, NULL, NULL);
A> -	}	
A> +		}
A> +	}
A>  }
A>  
A>  static int

-- 
Totus tuus, Glebius.