ipvs: Stop traffic to a destination when setting its weight to -1

PATCH In review Yannick Brosseau 2 participants 2 messages Apr 28, 2015 – Apr 29, 2015

Subsystems (3)

Networking netfilter IPVS

lore

Share & tools

lore

Yannick Brosseau

author

2015-04-28 15:10 UTC #0 lore raw

PATCH 1/1 ipvs: Stop traffic to a destination when setting its weight to -1

In some situation, we want to reap all the active connections to a host that is failing. Setting its weight to zero only stop new connections to the destination while still forwarding packets part of an active one.

We need a finer grained version of the sysctl_expire_nodest_conn because in most case we want to kept the behavior of only blocking new connexion. But if one host destination is failing we want to cut the traffic to that one immediately. Sending a RST is preferable than blackholling packets in that case.

Signed-off-by Yannick Brosseau <[email protected]>

---
 net/netfilter/ipvs/ip_vs_core.c |  3 ++-
 net/netfilter/ipvs/ip_vs_ctl.c  | 14 +++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

net/netfilter/ipvs/ip_vs_core.c +2 -1

index 5d2b806..b24791f 100644
@@ -1718,7 +1718,8 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
 	if (cp->dest && !(cp->dest->flags & IP_VS_DEST_F_AVAILABLE)) {
 		/* the destination server is not available */
 
-		if (sysctl_expire_nodest_conn(ipvs)) {
+		if (sysctl_expire_nodest_conn(ipvs) ||
+			atomic_read(&cp->dest->weight) == -1) {
 			/* try to expire the connection immediately */
 			ip_vs_conn_expire_now(cp);
 		}

net/netfilter/ipvs/ip_vs_ctl.c +9 -5

index 4953267..e1a8c39 100644
@@ -829,7 +829,11 @@ __ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
 	}
 
 	/* set the dest status flags */
-	dest->flags |= IP_VS_DEST_F_AVAILABLE;
+	if (atomic_read(&dest->weight) >= 0) {
+		dest->flags |= IP_VS_DEST_F_AVAILABLE;
+	} else {
+		dest->flags &= ~IP_VS_DEST_F_AVAILABLE;
+	}
 
 	if (udest->u_threshold == 0 || udest->u_threshold > dest->u_threshold)
 		dest->flags &= ~IP_VS_DEST_F_OVERLOAD;

@@ -939,8 +943,8 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 
 	EnterFunction(2);
 
-	if (udest->weight < 0) {
-		pr_err("%s(): server weight less than zero\n", __func__);
+	if (udest->weight < -1) {
+		pr_err("%s(): server weight less than -1\n", __func__);
 		return -ERANGE;
 	}

@@ -1003,8 +1007,8 @@ ip_vs_edit_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 
 	EnterFunction(2);
 
-	if (udest->weight < 0) {
-		pr_err("%s(): server weight less than zero\n", __func__);
+	if (udest->weight < -1) {
+		pr_err("%s(): server weight less than -1\n", __func__);
 		return -ERANGE;
 	}

Signature · 1 line

2.1.4

Julian Anastasov

maintainer

2015-04-29 19:15 UTC #1 lore raw

Hello,

Quoted 10 lines · Yannick Brosseau

In some situation, we want to reap all the active connections to a host that is failing. Setting its weight to zero only stop new connections to the destination while still forwarding packets part of an active one.
We need a finer grained version of the sysctl_expire_nodest_conn because in most case we want to kept the behavior of only blocking new connexion. But if one host destination is failing we want to cut the traffic to that one immediately. Sending a RST is preferable than blackholling packets in that case.

IIRC, we send RST only for this case:
"ip_vs_in: packet continues traversal as normal", i.e. when no destination server is available. The place you change uses NF_DROP.

Here is the current situation:

- expire_nodest_conn=1 && weight=0 && cp != NULL => create new cp to any available dest, expire old cp, forward packet

- cp != NULL && any weight && dest present (IP_VS_DEST_F_AVAILABLE is set) => forward, even if weight=0

- cp != NULL && dest removed (IP_VS_DEST_F_AVAILABLE not set) => NF_DROP and expire old cp depending on expire_nodest_conn

Can you confirm that the only goal of this change is
to allow per-destination expire_nodest_conn flag, i.e. the benefit is when expire_nodest_conn=0 to use weight=-1, so that traffic to be dropped? That is all I see in this change, right? So, the same is achieved if destination is removed and expire_nodest_conn=1 is used?

net/netfilter/ipvs/ip_vs_ctl.c __ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest, · line 836

Quoted 7 lines

net/netfilter/ipvs/ip_vs_ctl.c +5 -1

 	/* set the dest status flags */
-	dest->flags |= IP_VS_DEST_F_AVAILABLE;
+	if (atomic_read(&dest->weight) >= 0) {
+		dest->flags |= IP_VS_DEST_F_AVAILABLE;
+	} else {
+		dest->flags &= ~IP_VS_DEST_F_AVAILABLE;
+	}

Note that IP_VS_DEST_F_AVAILABLE is not checked
by all schedulers, only by ones that can hold dest after it is removed (LBLC, LBLCR). IIRC, the check for DH scheduler is not needed. As result, some schedulers need to be fixed to check properly the negative weight, for example, LC has atomic_read(&dest->weight) == 0 check. May be all places that use dest->weight should be checked for possible problem.

Regards

--
Julian Anastasov <[email protected]>