Subject: Re: [review please] tcp syn cache cleanup code for sc->sc_so
To: Matt Thomas <matt@3am-software.com>
From: None <itojun@iijlab.net>
List: tech-net
Date: 08/20/1999 17:34:58
------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4825.935138074.1@coconut.itojun.org>
Content-Transfer-Encoding: 7bit

>>Instead of pointing to socket in the syn cache, point to an intermediate
>>structure which contains a reference counter and a pointer the socket.
>>the [listening] socket should also contain a pointer to this structure
>>so that it can nuke the socket pointer inside of it and decrement the
>>reference counter when either itself is deleted or its security policy 
>>is changed.  As each syn cache is freed, the reference counter is decremented
>>and when it reaches 0, the structure is freed. 
>	Hmm, this will save us from searching.  Thanks.
>	Is there any use for this extra structure other than in tcp?
>	(I guess not)
>	If not, I'd point it from syn cache and tcpcb, since to add
>	pointer to struct socket the pointer must be of universal use.

	So here's an update; give me your comments on structure naming
	(syn_cache_link) or some logic.  sc_refcnt is signed int but
	it should be enough as we only have 16bit port number space.

itojun

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4825.935138074.2@coconut.itojun.org>
Content-Transfer-Encoding: 7bit

Index: tcp_input.c
===================================================================
RCS file: /cvsroot/kame/kame/netbsd/sys/netinet/tcp_input.c,v
retrieving revision 1.4
retrieving revision 1.7
diff -c -r1.4 -r1.7
*** tcp_input.c	1999/08/11 05:54:48	1.4
--- tcp_input.c	1999/08/20 08:26:24	1.7
***************
*** 2341,2350 ****
--- 2341,2358 ----
  		(void) m_free((sc)->sc_ipopts);				\
  	if ((sc)->sc_route4.ro_rt != NULL)				\
  		RTFREE((sc)->sc_route4.ro_rt);				\
+ 	if ((sc)->sc_scl) {						\
+ 		(sc)->sc_scl->scl_refcnt--;				\
+ 		if ((sc)->sc_scl->scl_refcnt < 0)			\
+ 			printf("%s %d: scl_refcnt < 0\n", __FILE__, __LINE__); \
+ 		if ((sc)->sc_scl->scl_refcnt <= 0)			\
+ 			pool_put(&syn_cache_link_pool, (sc)->sc_scl);	\
+ 	}								\
  	pool_put(&syn_cache_pool, (sc));				\
  } while (0)
  
  struct pool syn_cache_pool;
+ struct pool syn_cache_link_pool;
  
  /*
   * We don't estimate RTT with SYNs, so each packet starts with the default
***************
*** 2377,2382 ****
--- 2385,2394 ----
  	/* Initialize the syn cache pool. */
  	pool_init(&syn_cache_pool, sizeof(struct syn_cache), 0, 0, 0,
  	    "synpl", 0, NULL, NULL, M_PCB);
+ 
+ 	/* Initialize the syn cache link pool. */
+ 	pool_init(&syn_cache_link_pool, sizeof(struct syn_cache_link), 0, 0, 0,
+ 	    "synlpl", 0, NULL, NULL, M_PCB);
  }
  
  void
***************
*** 2994,3002 ****
  	struct syn_cache *sc;
  	struct syn_cache_head *scp;
  	struct mbuf *ipopts;
- #ifdef IPSEC
  	size_t ipsechdrsiz;
- #endif
  
  	tp = sototcpcb(so);
  
--- 3006,3012 ----
***************
*** 3121,3127 ****
  		sc->sc_requested_s_scale = 15;
  		sc->sc_request_r_scale = 15;
  	}
! 	sc->sc_so = so;
  	if (syn_cache_respond(sc, m) == 0) {
  		syn_cache_insert(sc);
  		tcpstat.tcps_sndacks++;
--- 3131,3151 ----
  		sc->sc_requested_s_scale = 15;
  		sc->sc_request_r_scale = 15;
  	}
! 
! 	/* fill syn_cache_link for tcpcb <-> syn_cache relationship */
! 	if (tp->t_scl == NULL) {
! 		tp->t_scl = pool_get(&syn_cache_link_pool, PR_NOWAIT);
! 		if (tp->t_scl) {
! 			bzero(tp->t_scl, sizeof(*tp->t_scl));
! 			tp->t_scl->scl_tp = tp;
! 			tp->t_scl->scl_refcnt = 1;
! 		}
! 	}
! 	if (tp->t_scl != NULL) {
! 		sc->sc_scl = tp->t_scl;
! 		sc->sc_scl->scl_refcnt++;
! 	}
! 
  	if (syn_cache_respond(sc, m) == 0) {
  		syn_cache_insert(sc);
  		tcpstat.tcps_sndacks++;
***************
*** 3189,3196 ****
  	m->m_data += max_linkhdr;
  	m->m_len = m->m_pkthdr.len = tlen;
  #ifdef IPSEC
! 	/* use IPsec policy on listening socket, on SYN ACK */
! 	m->m_pkthdr.rcvif = (struct ifnet *)sc->sc_so;
  #else
  	m->m_pkthdr.rcvif = NULL;
  #endif
--- 3213,3234 ----
  	m->m_data += max_linkhdr;
  	m->m_len = m->m_pkthdr.len = tlen;
  #ifdef IPSEC
! 	if (sc->sc_scl && sc->sc_scl->scl_tp) {
! 		struct tcpcb *tp;
! 		struct socket *so;
! 
! 		tp = sc->sc_scl->scl_tp;
! 		if (tp->t_inpcb)
! 			so = tp->t_inpcb->inp_socket;
! #ifdef INET6
! 		else if (tp->t_in6pcb)
! 			so = tp->t_in6pcb->in6p_socket;
! #endif
! 		else
! 			so = NULL;
! 		/* use IPsec policy on listening socket, on SYN ACK */
! 		m->m_pkthdr.rcvif = (struct ifnet *)so;
! 	}
  #else
  	m->m_pkthdr.rcvif = NULL;
  #endif
Index: tcp_subr.c
===================================================================
RCS file: /cvsroot/kame/kame/netbsd/sys/netinet/tcp_subr.c,v
retrieving revision 1.5
retrieving revision 1.7
diff -c -r1.5 -r1.7
*** tcp_subr.c	1999/08/11 06:04:32	1.5
--- tcp_subr.c	1999/08/20 08:26:24	1.7
***************
*** 181,186 ****
--- 181,187 ----
  int	tcp_freeq __P((struct tcpcb *));
  
  struct pool tcpcb_pool;
+ extern struct pool syn_cache_link_pool;
  
  /*
   * Tcp initialization
***************
*** 901,906 ****
--- 902,915 ----
  	if (tp->t_template) {
  		m_free(tp->t_template);
  		tp->t_template = NULL;
+ 	}
+ 	if (tp->t_scl) {
+ 		tp->t_scl->scl_refcnt--;
+ 		tp->t_scl->scl_tp = NULL;
+ 		if (tp->t_scl->scl_refcnt < 0)
+ 			printf("%s %d: scl_refcnt < 0\n", __FILE__, __LINE__);
+ 		if (tp->t_scl->scl_refcnt <= 0)
+ 			pool_put(&syn_cache_link_pool, tp->t_scl);
  	}
  	pool_put(&tcpcb_pool, tp);
  	if (inp) {
Index: tcp_var.h
===================================================================
RCS file: /cvsroot/kame/kame/netbsd/sys/netinet/tcp_var.h,v
retrieving revision 1.4
retrieving revision 1.6
diff -c -r1.4 -r1.6
*** tcp_var.h	1999/08/12 13:58:16	1.4
--- tcp_var.h	1999/08/20 08:26:24	1.6
***************
*** 128,133 ****
--- 128,135 ----
  /*
   * Tcp control block, one per tcp; fields:
   */
+ struct syn_cache_link;
+ 
  struct tcpcb {
  	int	t_family;		/* address family on the wire */
  	struct ipqehead segq;		/* sequencing queue */
***************
*** 224,229 ****
--- 226,234 ----
  
  /* SACK stuff */
  	struct ipqehead timeq;		/* time sequenced queue (for SACK) */
+ 
+ /* back pointer from syn cache */
+ 	struct syn_cache_link *t_scl;
  };
  
  #ifdef _KERNEL
***************
*** 322,327 ****
--- 327,340 ----
  };
  
  /*
+  * syn_cache <-> tcpcb linkage structure with refcnt'ing.
+  */
+ struct syn_cache_link {
+ 	int scl_refcnt;
+ 	struct tcpcb *scl_tp;
+ };
+ 
+ /*
   * Data for the TCP compressed state engine.
   */
  union syn_cache_sa {
***************
*** 367,373 ****
  	u_int16_t sc_ourmaxseg;
  	u_int8_t sc_request_r_scale	: 4,
  		 sc_requested_s_scale	: 4;
! 	struct socket *sc_so;			/* listening socket */
  };
  
  struct syn_cache_head {
--- 380,386 ----
  	u_int16_t sc_ourmaxseg;
  	u_int8_t sc_request_r_scale	: 4,
  		 sc_requested_s_scale	: 4;
! 	struct syn_cache_link *sc_scl;
  };
  
  struct syn_cache_head {

------- =_aaaaaaaaaa0--