ipvs: convert dests to rcu

In previous commits the schedulers started to access svc->destinations with _rcu list traversal primitives because the IP_VS_WAIT_WHILE macro still plays the role of grace period. Now it is time to finish the updating part, i.e. adding and deleting of dests with _rcu suffix before removing the IP_VS_WAIT_WHILE in next commit. We use the same rule for conns as for the schedulers: dests can be searched in RCU read-side critical section where ip_vs_dest_hold can be called by ip_vs_bind_dest. Some things are not perfect, for example, calling functions like ip_vs_lookup_dest from updating code under RCU, just because we use some function both from reader and from updater. Signed-off-by: Julian Anastasov <ja@ssi.bg> Signed-off-by: Simon Horman <horms@verge.net.au>

ipvs: convert dests to rcu
In previous commits the schedulers started to access svc->destinations with _rcu list traversal primitives because the IP_VS_WAIT_WHILE macro still plays the role of grace period. Now it is time to finish the updating part, i.e. adding and deleting of dests with _rcu suffix before removing the IP_VS_WAIT_WHILE in next commit. We use the same rule for conns as for the schedulers: dests can be searched in RCU read-side critical section where ip_vs_dest_hold can be called by ip_vs_bind_dest. Some things are not perfect, for example, calling functions like ip_vs_lookup_dest from updating code under RCU, just because we use some function both from reader and from updater. Signed-off-by: Julian Anastasov <ja@ssi.bg> Signed-off-by: Simon Horman <horms@verge.net.au>
Julian Anastasov · Pablo Neira Ayuso
1 parent ba3a3ce14e
Showing 4 changed files with 27 additions and 26 deletions Side-by-side Diff
include/net/ip_vs.h
net/netfilter/ipvs/ip_vs_conn.c
net/netfilter/ipvs/ip_vs_ctl.c
net/netfilter/ipvs/ip_vs_sync.c
@@ -1434,7 +1434,7 @@
 ip_vs_find_dest(struct net *net, int af, const union nf_inet_addr *daddr,
 		__be16 dport, const union nf_inet_addr *vaddr, __be16 vport,
 		__u16 protocol, __u32 fwmark, __u32 flags);
-extern struct ip_vs_dest *ip_vs_try_bind_dest(struct ip_vs_conn *cp);
+extern void ip_vs_try_bind_dest(struct ip_vs_conn *cp);
  
 static inline void ip_vs_dest_hold(struct ip_vs_dest *dest)
 {
@@ -611,10 +611,11 @@
  * Check if there is a destination for the connection, if so
  * bind the connection to the destination.
  */
-struct ip_vs_dest *ip_vs_try_bind_dest(struct ip_vs_conn *cp)
+void ip_vs_try_bind_dest(struct ip_vs_conn *cp)
 {
 	struct ip_vs_dest *dest;
  
+	rcu_read_lock();
 	dest = ip_vs_find_dest(ip_vs_conn_net(cp), cp->af, &cp->daddr,
 			       cp->dport, &cp->vaddr, cp->vport,
 			       cp->protocol, cp->fwmark, cp->flags);
@@ -624,7 +625,8 @@
 		spin_lock(&cp->lock);
 		if (cp->dest) {
 			spin_unlock(&cp->lock);
-			return dest;
+			rcu_read_unlock();
+			return;
 		}
  
 		/* Applications work depending on the forwarding method
@@ -648,7 +650,7 @@
 		if (pd && atomic_read(&pd->appcnt))
 			ip_vs_bind_app(cp, pd->pp);
 	}
-	return dest;
+	rcu_read_unlock();
 }
  
  
@@ -565,8 +565,8 @@
 	return false;
 }
  
-/*
- *	Lookup destination by {addr,port} in the given service
+/* Lookup destination by {addr,port} in the given service
+ * Called under RCU lock.
  */
 static struct ip_vs_dest *
 ip_vs_lookup_dest(struct ip_vs_service *svc, const union nf_inet_addr *daddr,
@@ -577,7 +577,7 @@
 	/*
 	 * Find the destination for the given service
 	 */
-	list_for_each_entry(dest, &svc->destinations, n_list) {
+	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
 		if ((dest->af == svc->af)
 		    && ip_vs_addr_equal(svc->af, &dest->addr, daddr)
 		    && (dest->port == dport)) {
  
@@ -591,10 +591,11 @@
  
 /*
  * Find destination by {daddr,dport,vaddr,protocol}
- * Cretaed to be used in ip_vs_process_message() in
+ * Created to be used in ip_vs_process_message() in
  * the backup synchronization daemon. It finds the
  * destination to be bound to the received connection
  * on the backup.
+ * Called under RCU lock, no refcnt is returned.
  */
 struct ip_vs_dest *ip_vs_find_dest(struct net  *net, int af,
 				   const union nf_inet_addr *daddr,
@@ -615,8 +616,6 @@
 	dest = ip_vs_lookup_dest(svc, daddr, port);
 	if (!dest)
 		dest = ip_vs_lookup_dest(svc, daddr, port ^ dport);
-	if (dest)
-		ip_vs_dest_hold(dest);
 	ip_vs_service_put(svc);
 	return dest;
 }
@@ -826,7 +825,7 @@
 	IP_VS_WAIT_WHILE(atomic_read(&svc->usecnt) > 0);
  
 	if (add) {
-		list_add(&dest->n_list, &svc->destinations);
+		list_add_rcu(&dest->n_list, &svc->destinations);
 		svc->num_dests++;
 		if (svc->scheduler->add_dest)
 			svc->scheduler->add_dest(svc, dest);
  
@@ -933,10 +932,10 @@
  
 	ip_vs_addr_copy(svc->af, &daddr, &udest->addr);
  
-	/*
-	 * Check if the dest already exists in the list
-	 */
+	/* We use function that requires RCU lock */
+	rcu_read_lock();
 	dest = ip_vs_lookup_dest(svc, &daddr, dport);
+	rcu_read_unlock();
  
 	if (dest != NULL) {
 		IP_VS_DBG(1, "%s(): dest already exists\n", __func__);
  
@@ -997,10 +996,10 @@
  
 	ip_vs_addr_copy(svc->af, &daddr, &udest->addr);
  
-	/*
-	 *  Lookup the destination list
-	 */
+	/* We use function that requires RCU lock */
+	rcu_read_lock();
 	dest = ip_vs_lookup_dest(svc, &daddr, dport);
+	rcu_read_unlock();
  
 	if (dest == NULL) {
 		IP_VS_DBG(1, "%s(): dest doesn't exist\n", __func__);
@@ -1069,7 +1068,7 @@
 	/*
 	 *  Remove it from the d-linked destination list.
 	 */
-	list_del(&dest->n_list);
+	list_del_rcu(&dest->n_list);
 	svc->num_dests--;
  
 	if (svcupd && svc->scheduler->del_dest)
  
@@ -1094,7 +1093,10 @@
  
 	EnterFunction(2);
  
+	/* We use function that requires RCU lock */
+	rcu_read_lock();
 	dest = ip_vs_lookup_dest(svc, &udest->addr, dport);
+	rcu_read_unlock();
  
 	if (dest == NULL) {
 		IP_VS_DBG(1, "%s(): destination not found!\n", __func__);
@@ -2104,7 +2106,7 @@
 		else
 			seq_putc(seq, '\n');
  
-		list_for_each_entry(dest, &svc->destinations, n_list) {
+		list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
 #ifdef CONFIG_IP_VS_IPV6
 			if (dest->af == AF_INET6)
 				seq_printf(seq,
@@ -858,23 +858,20 @@
 		flags |= cp->flags & ~IP_VS_CONN_F_BACKUP_UPD_MASK;
 		cp->flags = flags;
 		spin_unlock(&cp->lock);
-		if (!dest) {
-			dest = ip_vs_try_bind_dest(cp);
-			if (dest)
-				ip_vs_dest_put(dest);
-		}
+		if (!dest)
+			ip_vs_try_bind_dest(cp);
 	} else {
 		/*
 		 * Find the appropriate destination for the connection.
 		 * If it is not found the connection will remain unbound
 		 * but still handled.
 		 */
+		rcu_read_lock();
 		dest = ip_vs_find_dest(net, type, daddr, dport, param->vaddr,
 				       param->vport, protocol, fwmark, flags);
  
 		cp = ip_vs_conn_new(param, daddr, dport, flags, dest, fwmark);
-		if (dest)
-			ip_vs_dest_put(dest);
+		rcu_read_unlock();
 		if (!cp) {
 			if (param->pe_data)
 				kfree(param->pe_data);
...	...	@@ -1434,7 +1434,7 @@
1434	1434	ip_vs_find_dest(struct net net, int af, const union nf_inet_addr daddr,
1435	1435	__be16 dport, const union nf_inet_addr *vaddr, __be16 vport,
1436	1436	__u16 protocol, __u32 fwmark, __u32 flags);
1437		-extern struct ip_vs_dest ip_vs_try_bind_dest(struct ip_vs_conn cp);
	1437	+extern void ip_vs_try_bind_dest(struct ip_vs_conn *cp);
1438	1438
1439	1439	static inline void ip_vs_dest_hold(struct ip_vs_dest *dest)
1440	1440	{
...	...	@@ -611,10 +611,11 @@
611	611	* Check if there is a destination for the connection, if so
612	612	* bind the connection to the destination.
613	613	*/
614		-struct ip_vs_dest ip_vs_try_bind_dest(struct ip_vs_conn cp)
	614	+void ip_vs_try_bind_dest(struct ip_vs_conn *cp)
615	615	{
616	616	struct ip_vs_dest *dest;
617	617
	618	+ rcu_read_lock();
618	619	dest = ip_vs_find_dest(ip_vs_conn_net(cp), cp->af, &cp->daddr,
619	620	cp->dport, &cp->vaddr, cp->vport,
620	621	cp->protocol, cp->fwmark, cp->flags);
...	...	@@ -624,7 +625,8 @@
624	625	spin_lock(&cp->lock);
625	626	if (cp->dest) {
626	627	spin_unlock(&cp->lock);
627		- return dest;
	628	+ rcu_read_unlock();
	629	+ return;
628	630	}
629	631
630	632	/* Applications work depending on the forwarding method
...	...	@@ -648,7 +650,7 @@
648	650	if (pd && atomic_read(&pd->appcnt))
649	651	ip_vs_bind_app(cp, pd->pp);
650	652	}
651		- return dest;
	653	+ rcu_read_unlock();
652	654	}
653	655
654	656
...	...	@@ -565,8 +565,8 @@
565	565	return false;
566	566	}
567	567
568		-/*
569		- * Lookup destination by {addr,port} in the given service
	568	+/* Lookup destination by {addr,port} in the given service
	569	+ * Called under RCU lock.
570	570	*/
571	571	static struct ip_vs_dest *
572	572	ip_vs_lookup_dest(struct ip_vs_service svc, const union nf_inet_addr daddr,
...	...	@@ -577,7 +577,7 @@
577	577	/*
578	578	* Find the destination for the given service
579	579	*/
580		- list_for_each_entry(dest, &svc->destinations, n_list) {
	580	+ list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
581	581	if ((dest->af == svc->af)
582	582	&& ip_vs_addr_equal(svc->af, &dest->addr, daddr)
583	583	&& (dest->port == dport)) {
584	584
...	...	@@ -591,10 +591,11 @@
591	591
592	592	/*
593	593	* Find destination by {daddr,dport,vaddr,protocol}
594		- * Cretaed to be used in ip_vs_process_message() in
	594	+ * Created to be used in ip_vs_process_message() in
595	595	* the backup synchronization daemon. It finds the
596	596	* destination to be bound to the received connection
597	597	* on the backup.
	598	+ * Called under RCU lock, no refcnt is returned.
598	599	*/
599	600	struct ip_vs_dest ip_vs_find_dest(struct net net, int af,
600	601	const union nf_inet_addr *daddr,
...	...	@@ -615,8 +616,6 @@
615	616	dest = ip_vs_lookup_dest(svc, daddr, port);
616	617	if (!dest)
617	618	dest = ip_vs_lookup_dest(svc, daddr, port ^ dport);
618		- if (dest)
619		- ip_vs_dest_hold(dest);
620	619	ip_vs_service_put(svc);
621	620	return dest;
622	621	}
...	...	@@ -826,7 +825,7 @@
826	825	IP_VS_WAIT_WHILE(atomic_read(&svc->usecnt) > 0);
827	826
828	827	if (add) {
829		- list_add(&dest->n_list, &svc->destinations);
	828	+ list_add_rcu(&dest->n_list, &svc->destinations);
830	829	svc->num_dests++;
831	830	if (svc->scheduler->add_dest)
832	831	svc->scheduler->add_dest(svc, dest);
833	832
...	...	@@ -933,10 +932,10 @@
933	932
934	933	ip_vs_addr_copy(svc->af, &daddr, &udest->addr);
935	934
936		- /*
937		- * Check if the dest already exists in the list
938		- */
	935	+ /* We use function that requires RCU lock */
	936	+ rcu_read_lock();
939	937	dest = ip_vs_lookup_dest(svc, &daddr, dport);
	938	+ rcu_read_unlock();
940	939
941	940	if (dest != NULL) {
942	941	IP_VS_DBG(1, "%s(): dest already exists\n", __func__);
943	942
...	...	@@ -997,10 +996,10 @@
997	996
998	997	ip_vs_addr_copy(svc->af, &daddr, &udest->addr);
999	998
1000		- /*
1001		- * Lookup the destination list
1002		- */
	999	+ /* We use function that requires RCU lock */
	1000	+ rcu_read_lock();
1003	1001	dest = ip_vs_lookup_dest(svc, &daddr, dport);
	1002	+ rcu_read_unlock();
1004	1003
1005	1004	if (dest == NULL) {
1006	1005	IP_VS_DBG(1, "%s(): dest doesn't exist\n", __func__);
...	...	@@ -1069,7 +1068,7 @@
1069	1068	/*
1070	1069	* Remove it from the d-linked destination list.
1071	1070	*/
1072		- list_del(&dest->n_list);
	1071	+ list_del_rcu(&dest->n_list);
1073	1072	svc->num_dests--;
1074	1073
1075	1074	if (svcupd && svc->scheduler->del_dest)
1076	1075
...	...	@@ -1094,7 +1093,10 @@
1094	1093
1095	1094	EnterFunction(2);
1096	1095
	1096	+ /* We use function that requires RCU lock */
	1097	+ rcu_read_lock();
1097	1098	dest = ip_vs_lookup_dest(svc, &udest->addr, dport);
	1099	+ rcu_read_unlock();
1098	1100
1099	1101	if (dest == NULL) {
1100	1102	IP_VS_DBG(1, "%s(): destination not found!\n", __func__);
...	...	@@ -2104,7 +2106,7 @@
2104	2106	else
2105	2107	seq_putc(seq, '\n');
2106	2108
2107		- list_for_each_entry(dest, &svc->destinations, n_list) {
	2109	+ list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
2108	2110	#ifdef CONFIG_IP_VS_IPV6
2109	2111	if (dest->af == AF_INET6)
2110	2112	seq_printf(seq,
...	...	@@ -858,23 +858,20 @@
858	858	flags \|= cp->flags & ~IP_VS_CONN_F_BACKUP_UPD_MASK;
859	859	cp->flags = flags;
860	860	spin_unlock(&cp->lock);
861		- if (!dest) {
862		- dest = ip_vs_try_bind_dest(cp);
863		- if (dest)
864		- ip_vs_dest_put(dest);
865		- }
	861	+ if (!dest)
	862	+ ip_vs_try_bind_dest(cp);
866	863	} else {
867	864	/*
868	865	* Find the appropriate destination for the connection.
869	866	* If it is not found the connection will remain unbound
870	867	* but still handled.
871	868	*/
	869	+ rcu_read_lock();
872	870	dest = ip_vs_find_dest(net, type, daddr, dport, param->vaddr,
873	871	param->vport, protocol, fwmark, flags);
874	872
875	873	cp = ip_vs_conn_new(param, daddr, dport, flags, dest, fwmark);
876		- if (dest)
877		- ip_vs_dest_put(dest);
	874	+ rcu_read_unlock();
878	875	if (!cp) {
879	876	if (param->pe_data)
880	877	kfree(param->pe_data);