Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 28 Feb 2012 23:22:52 +0100
From:      Luigi Rizzo <rizzo@iet.unipi.it>
To:        Oleg Bulyzhin <oleg@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r232272 - head/sys/netinet/ipfw
Message-ID:  <20120228222252.GA801@onelab2.iet.unipi.it>
In-Reply-To: <201202282153.q1SLrdfQ093936@svn.freebsd.org>
References:  <201202282153.q1SLrdfQ093936@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Feb 28, 2012 at 09:53:39PM +0000, Oleg Bulyzhin wrote:
> Author: oleg
> Date: Tue Feb 28 21:53:39 2012
> New Revision: 232272
> URL: http://svn.freebsd.org/changeset/base/232272
> 
> Log:
>   lookup_dyn_rule_locked(): style(9) cleanup
>   
>   MFC after:	1 month

is there a reason for delaying the MFC, such as this is
preparatory work for further changes ?
Otherwise i'd suggest that whitespace changes like this
are quickly pushed to the stable branch so that the code
versions do not diverge.

cheers
luigi


> Modified:
>   head/sys/netinet/ipfw/ip_fw_dynamic.c
> 
> Modified: head/sys/netinet/ipfw/ip_fw_dynamic.c
> ==============================================================================
> --- head/sys/netinet/ipfw/ip_fw_dynamic.c	Tue Feb 28 21:45:21 2012	(r232271)
> +++ head/sys/netinet/ipfw/ip_fw_dynamic.c	Tue Feb 28 21:53:39 2012	(r232272)
> @@ -390,72 +390,68 @@ ipfw_remove_dyn_children(struct ip_fw *r
>  	IPFW_DYN_UNLOCK();
>  }
>  
> -/**
> - * lookup a dynamic rule, locked version
> +/*
> + * Lookup a dynamic rule, locked version.
>   */
>  static ipfw_dyn_rule *
>  lookup_dyn_rule_locked(struct ipfw_flow_id *pkt, int *match_direction,
>      struct tcphdr *tcp)
>  {
>  	/*
> -	 * stateful ipfw extensions.
> -	 * Lookup into dynamic session queue
> +	 * Stateful ipfw extensions.
> +	 * Lookup into dynamic session queue.
>  	 */
>  #define MATCH_REVERSE	0
>  #define MATCH_FORWARD	1
>  #define MATCH_NONE	2
>  #define MATCH_UNKNOWN	3
>  	int i, dir = MATCH_NONE;
> -	ipfw_dyn_rule *prev, *q=NULL;
> +	ipfw_dyn_rule *prev, *q = NULL;
>  
>  	IPFW_DYN_LOCK_ASSERT();
>  
>  	if (V_ipfw_dyn_v == NULL)
> -		goto done;	/* not found */
> -	i = hash_packet( pkt );
> -	for (prev=NULL, q = V_ipfw_dyn_v[i] ; q != NULL ; ) {
> +		goto done;				/* not found */
> +	i = hash_packet(pkt);
> +	for (prev = NULL, q = V_ipfw_dyn_v[i]; q != NULL;) {
>  		if (q->dyn_type == O_LIMIT_PARENT && q->count)
>  			goto next;
> -		if (TIME_LEQ( q->expire, time_uptime)) { /* expire entry */
> +		if (TIME_LEQ(q->expire, time_uptime)) {	/* expire entry */
>  			UNLINK_DYN_RULE(prev, V_ipfw_dyn_v[i], q);
>  			continue;
>  		}
> -		if (pkt->proto == q->id.proto &&
> -		    q->dyn_type != O_LIMIT_PARENT) {
> -			if (IS_IP6_FLOW_ID(pkt)) {
> -			    if (IN6_ARE_ADDR_EQUAL(&(pkt->src_ip6),
> -				&(q->id.src_ip6)) &&
> -			    IN6_ARE_ADDR_EQUAL(&(pkt->dst_ip6),
> -				&(q->id.dst_ip6)) &&
> +		if (pkt->proto != q->id.proto || q->dyn_type == O_LIMIT_PARENT)
> +			goto next;
> +
> +		if (IS_IP6_FLOW_ID(pkt)) {
> +			if (IN6_ARE_ADDR_EQUAL(&pkt->src_ip6, &q->id.src_ip6) &&
> +			    IN6_ARE_ADDR_EQUAL(&pkt->dst_ip6, &q->id.dst_ip6) &&
>  			    pkt->src_port == q->id.src_port &&
> -			    pkt->dst_port == q->id.dst_port ) {
> +			    pkt->dst_port == q->id.dst_port) {
>  				dir = MATCH_FORWARD;
>  				break;
> -			    }
> -			    if (IN6_ARE_ADDR_EQUAL(&(pkt->src_ip6),
> -				    &(q->id.dst_ip6)) &&
> -				IN6_ARE_ADDR_EQUAL(&(pkt->dst_ip6),
> -				    &(q->id.src_ip6)) &&
> -				pkt->src_port == q->id.dst_port &&
> -				pkt->dst_port == q->id.src_port ) {
> -				    dir = MATCH_REVERSE;
> -				    break;
> -			    }
> -			} else {
> -			    if (pkt->src_ip == q->id.src_ip &&
> -				pkt->dst_ip == q->id.dst_ip &&
> -				pkt->src_port == q->id.src_port &&
> -				pkt->dst_port == q->id.dst_port ) {
> -				    dir = MATCH_FORWARD;
> -				    break;
> -			    }
> -			    if (pkt->src_ip == q->id.dst_ip &&
> -				pkt->dst_ip == q->id.src_ip &&
> -				pkt->src_port == q->id.dst_port &&
> -				pkt->dst_port == q->id.src_port ) {
> -				    dir = MATCH_REVERSE;
> -				    break;
> -			    }
> +			}
> +			if (IN6_ARE_ADDR_EQUAL(&pkt->src_ip6, &q->id.dst_ip6) &&
> +			    IN6_ARE_ADDR_EQUAL(&pkt->dst_ip6, &q->id.src_ip6) &&
> +			    pkt->src_port == q->id.dst_port &&
> +			    pkt->dst_port == q->id.src_port) {
> +				dir = MATCH_REVERSE;
> +				break;
> +			}
> +		} else {
> +			if (pkt->src_ip == q->id.src_ip &&
> +			    pkt->dst_ip == q->id.dst_ip &&
> +			    pkt->src_port == q->id.src_port &&
> +			    pkt->dst_port == q->id.dst_port) {
> +				dir = MATCH_FORWARD;
> +				break;
> +			}
> +			if (pkt->src_ip == q->id.dst_ip &&
> +			    pkt->dst_ip == q->id.src_ip &&
> +			    pkt->src_port == q->id.dst_port &&
> +			    pkt->dst_port == q->id.src_port) {
> +				dir = MATCH_REVERSE;
> +				break;
>  			}
>  		}
>  next:
> @@ -463,43 +459,45 @@ next:
>  		q = q->next;
>  	}
>  	if (q == NULL)
> -		goto done; /* q = NULL, not found */
> +		goto done;	/* q = NULL, not found */
>  
> -	if ( prev != NULL) { /* found and not in front */
> +	if (prev != NULL) {	/* found and not in front */
>  		prev->next = q->next;
>  		q->next = V_ipfw_dyn_v[i];
>  		V_ipfw_dyn_v[i] = q;
>  	}
>  	if (pkt->proto == IPPROTO_TCP) { /* update state according to flags */
> -		u_char flags = pkt->_flags & (TH_FIN|TH_SYN|TH_RST);
> +		uint32_t ack;
> +		u_char flags = pkt->_flags & (TH_FIN | TH_SYN | TH_RST);
>  
>  #define BOTH_SYN	(TH_SYN | (TH_SYN << 8))
>  #define BOTH_FIN	(TH_FIN | (TH_FIN << 8))
> -		q->state |= (dir == MATCH_FORWARD ) ? flags : (flags << 8);
> +		q->state |= (dir == MATCH_FORWARD) ? flags : (flags << 8);
>  		switch (q->state) {
> -		case TH_SYN:				/* opening */
> +		case TH_SYN:			/* opening */
>  			q->expire = time_uptime + V_dyn_syn_lifetime;
>  			break;
>  
>  		case BOTH_SYN:			/* move to established */
> -		case BOTH_SYN | TH_FIN :	/* one side tries to close */
> -		case BOTH_SYN | (TH_FIN << 8) :
> - 			if (tcp) {
> +		case BOTH_SYN | TH_FIN:		/* one side tries to close */
> +		case BOTH_SYN | (TH_FIN << 8):
>  #define _SEQ_GE(a,b) ((int)(a) - (int)(b) >= 0)
> -			    u_int32_t ack = ntohl(tcp->th_ack);
> -			    if (dir == MATCH_FORWARD) {
> +			if (tcp == NULL) {
> +				q->expire = time_uptime + V_dyn_ack_lifetime;
> +				break;
> +			}
> +
> +			ack = ntohl(tcp->th_ack);
> +			if (dir == MATCH_FORWARD) {
>  				if (q->ack_fwd == 0 || _SEQ_GE(ack, q->ack_fwd))
> -				    q->ack_fwd = ack;
> -				else { /* ignore out-of-sequence */
> -				    break;
> -				}
> -			    } else {
> +					q->ack_fwd = ack;
> +				else	/* ignore out-of-sequence */
> +					break;
> +			} else {
>  				if (q->ack_rev == 0 || _SEQ_GE(ack, q->ack_rev))
> -				    q->ack_rev = ack;
> -				else { /* ignore out-of-sequence */
> -				    break;
> -				}
> -			    }
> +					q->ack_rev = ack;
> +				else	/* ignore out-of-sequence */
> +					break;
>  			}
>  			q->expire = time_uptime + V_dyn_ack_lifetime;
>  			break;
> @@ -531,9 +529,9 @@ next:
>  		q->expire = time_uptime + V_dyn_short_lifetime;
>  	}
>  done:
> -	if (match_direction)
> +	if (match_direction != NULL)
>  		*match_direction = dir;
> -	return q;
> +	return (q);
>  }
>  
>  ipfw_dyn_rule *



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120228222252.GA801>