[PATCH 4/4] ipvs: reduce kernel stack usage

Arnd Bergmann arnd at arndb.de
Fri Jun 28 12:37:49 UTC 2019


With the new CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL option, the stack
usage in the ipvs debug output grows because each instance of
IP_VS_DBG_BUF() now has its own buffer of 160 bytes that add up
rather than reusing the stack slots:

net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_sched_persist':
net/netfilter/ipvs/ip_vs_core.c:427:1: error: the frame size of 1052 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_new_conn_out':
net/netfilter/ipvs/ip_vs_core.c:1231:1: error: the frame size of 1048 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_out':
net/netfilter/ipvs/ip_vs_ftp.c:397:1: error: the frame size of 1104 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_in':
net/netfilter/ipvs/ip_vs_ftp.c:555:1: error: the frame size of 1200 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

Since printk() already has a way to print IPv4/IPv6 addresses using
the %pIS format string, use that instead, combined with a macro that
creates a local sockaddr structure on the stack. These will still
add up, but the stack frames are now under 200 bytes.

Signed-off-by: Arnd Bergmann <arnd at arndb.de>
---
I'm not sure this actually does what I think it does. Someone
needs to verify that we correctly print the addresses here.
I've also only added three files that caused the warning messages
to be reported. There are still a lot of other instances of
IP_VS_DBG_BUF() that could be converted the same way after the
basic idea is confirmed.
---
 include/net/ip_vs.h             | 71 +++++++++++++++++++--------------
 net/netfilter/ipvs/ip_vs_core.c | 44 ++++++++++----------
 net/netfilter/ipvs/ip_vs_ftp.c  | 20 +++++-----
 3 files changed, 72 insertions(+), 63 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 3759167f91f5..3dfbeef67be6 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -227,6 +227,16 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len,
 		       sizeof(ip_vs_dbg_buf), addr,			\
 		       &ip_vs_dbg_idx)
 
+#define IP_VS_DBG_SOCKADDR4(fam, addr, port)				\
+	(struct sockaddr*)&(struct sockaddr_in)				\
+	{ .sin_family = (fam), .sin_addr = (addr)->in, .sin_port = (port) }
+#define IP_VS_DBG_SOCKADDR6(fam, addr, port)				\
+	(struct sockaddr*)&(struct sockaddr_in6) \
+	{ .sin6_family = (fam), .sin6_addr = (addr)->in6, .sin6_port = (port) }
+#define IP_VS_DBG_SOCKADDR(fam, addr, port) (fam == AF_INET ?		\
+			IP_VS_DBG_SOCKADDR4(fam, addr, port) :		\
+			IP_VS_DBG_SOCKADDR6(fam, addr, port))
+
 #define IP_VS_DBG(level, msg, ...)					\
 	do {								\
 		if (level <= ip_vs_get_debug_level())			\
@@ -251,6 +261,7 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len,
 #else	/* NO DEBUGGING at ALL */
 #define IP_VS_DBG_BUF(level, msg...)  do {} while (0)
 #define IP_VS_ERR_BUF(msg...)  do {} while (0)
+#define IP_VS_DBG_SOCKADDR(fam, addr, port) NULL
 #define IP_VS_DBG(level, msg...)  do {} while (0)
 #define IP_VS_DBG_RL(msg...)  do {} while (0)
 #define IP_VS_DBG_PKT(level, af, pp, skb, ofs, msg)	do {} while (0)
@@ -1244,31 +1255,31 @@ static inline void ip_vs_control_del(struct ip_vs_conn *cp)
 {
 	struct ip_vs_conn *ctl_cp = cp->control;
 	if (!ctl_cp) {
-		IP_VS_ERR_BUF("request control DEL for uncontrolled: "
-			      "%s:%d to %s:%d\n",
-			      IP_VS_DBG_ADDR(cp->af, &cp->caddr),
-			      ntohs(cp->cport),
-			      IP_VS_DBG_ADDR(cp->af, &cp->vaddr),
-			      ntohs(cp->vport));
+		pr_err("request control DEL for uncontrolled: "
+		       "%pISp to %pISp\n",
+		       IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr,
+		       ntohs(cp->cport)),
+		       IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr,
+		       ntohs(cp->vport)));
 
 		return;
 	}
 
-	IP_VS_DBG_BUF(7, "DELeting control for: "
-		      "cp.dst=%s:%d ctl_cp.dst=%s:%d\n",
-		      IP_VS_DBG_ADDR(cp->af, &cp->caddr),
-		      ntohs(cp->cport),
-		      IP_VS_DBG_ADDR(cp->af, &ctl_cp->caddr),
-		      ntohs(ctl_cp->cport));
+	IP_VS_DBG(7, "DELeting control for: "
+		  "cp.dst=%pISp ctl_cp.dst=%pISp\n",
+		  IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr,
+		  ntohs(cp->cport)),
+		  IP_VS_DBG_SOCKADDR(cp->af, &ctl_cp->caddr,
+		  ntohs(ctl_cp->cport)));
 
 	cp->control = NULL;
 	if (atomic_read(&ctl_cp->n_control) == 0) {
-		IP_VS_ERR_BUF("BUG control DEL with n=0 : "
-			      "%s:%d to %s:%d\n",
-			      IP_VS_DBG_ADDR(cp->af, &cp->caddr),
-			      ntohs(cp->cport),
-			      IP_VS_DBG_ADDR(cp->af, &cp->vaddr),
-			      ntohs(cp->vport));
+		pr_err("BUG control DEL with n=0 : "
+		       "%pISp to %pISp\n",
+		       IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr,
+		       ntohs(cp->cport)),
+		       IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr,
+		       ntohs(cp->vport)));
 
 		return;
 	}
@@ -1279,22 +1290,22 @@ static inline void
 ip_vs_control_add(struct ip_vs_conn *cp, struct ip_vs_conn *ctl_cp)
 {
 	if (cp->control) {
-		IP_VS_ERR_BUF("request control ADD for already controlled: "
-			      "%s:%d to %s:%d\n",
-			      IP_VS_DBG_ADDR(cp->af, &cp->caddr),
-			      ntohs(cp->cport),
-			      IP_VS_DBG_ADDR(cp->af, &cp->vaddr),
-			      ntohs(cp->vport));
+		pr_err("request control ADD for already controlled: "
+		       "%pISp to %pISp\n",
+		       IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr,
+		     		     ntohs(cp->cport)),
+		       IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr,
+		     		     ntohs(cp->vport)));
 
 		ip_vs_control_del(cp);
 	}
 
-	IP_VS_DBG_BUF(7, "ADDing control for: "
-		      "cp.dst=%s:%d ctl_cp.dst=%s:%d\n",
-		      IP_VS_DBG_ADDR(cp->af, &cp->caddr),
-		      ntohs(cp->cport),
-		      IP_VS_DBG_ADDR(cp->af, &ctl_cp->caddr),
-		      ntohs(ctl_cp->cport));
+	IP_VS_DBG(7, "ADDing control for: "
+		  "cp.dst=%pISp ctl_cp.dst=%pISp\n",
+		  IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr,
+				     ntohs(cp->cport)),
+		  IP_VS_DBG_SOCKADDR(cp->af, &ctl_cp->caddr,
+				     ntohs(ctl_cp->cport)));
 
 	cp->control = ctl_cp;
 	atomic_inc(&ctl_cp->n_control);
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index f662f198b458..0277cd3c5446 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -51,7 +51,6 @@
 #include <net/ip_vs.h>
 #include <linux/indirect_call_wrapper.h>
 
-
 EXPORT_SYMBOL(register_ip_vs_scheduler);
 EXPORT_SYMBOL(unregister_ip_vs_scheduler);
 EXPORT_SYMBOL(ip_vs_proto_name);
@@ -294,11 +293,11 @@ ip_vs_sched_persist(struct ip_vs_service *svc,
 #endif
 		snet.ip = src_addr->ip & svc->netmask;
 
-	IP_VS_DBG_BUF(6, "p-schedule: src %s:%u dest %s:%u "
-		      "mnet %s\n",
-		      IP_VS_DBG_ADDR(svc->af, src_addr), ntohs(src_port),
-		      IP_VS_DBG_ADDR(svc->af, dst_addr), ntohs(dst_port),
-		      IP_VS_DBG_ADDR(svc->af, &snet));
+	IP_VS_DBG(6, "p-schedule: src %pISp dest %pISp "
+		      "mnet %pIS\n",
+		      IP_VS_DBG_SOCKADDR(svc->af, src_addr, ntohs(src_port)),
+		      IP_VS_DBG_SOCKADDR(svc->af, dst_addr, ntohs(dst_port)),
+		      IP_VS_DBG_SOCKADDR(svc->af, &snet, 0));
 
 	/*
 	 * As far as we know, FTP is a very complicated network protocol, and
@@ -566,12 +565,12 @@ ip_vs_schedule(struct ip_vs_service *svc, struct sk_buff *skb,
 		}
 	}
 
-	IP_VS_DBG_BUF(6, "Schedule fwd:%c c:%s:%u v:%s:%u "
-		      "d:%s:%u conn->flags:%X conn->refcnt:%d\n",
+	IP_VS_DBG(6, "Schedule fwd:%c c:%pISp v:%pISp "
+		      "d:%pISp conn->flags:%X conn->refcnt:%d\n",
 		      ip_vs_fwd_tag(cp),
-		      IP_VS_DBG_ADDR(cp->af, &cp->caddr), ntohs(cp->cport),
-		      IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport),
-		      IP_VS_DBG_ADDR(cp->daf, &cp->daddr), ntohs(cp->dport),
+		      IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, ntohs(cp->cport)),
+		      IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, ntohs(cp->vport)),
+		      IP_VS_DBG_SOCKADDR(cp->daf, &cp->daddr, ntohs(cp->dport)),
 		      cp->flags, refcount_read(&cp->refcnt));
 
 	ip_vs_conn_stats(cp, svc);
@@ -885,8 +884,8 @@ static int handle_response_icmp(int af, struct sk_buff *skb,
 	/* Ensure the checksum is correct */
 	if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
 		/* Failed checksum! */
-		IP_VS_DBG_BUF(1, "Forward ICMP: failed checksum from %s!\n",
-			      IP_VS_DBG_ADDR(af, snet));
+		IP_VS_DBG(1, "Forward ICMP: failed checksum from %pISp!\n",
+			      IP_VS_DBG_SOCKADDR(af, snet, 0));
 		goto out;
 	}
 
@@ -1219,13 +1218,13 @@ struct ip_vs_conn *ip_vs_new_conn_out(struct ip_vs_service *svc,
 	ip_vs_conn_stats(cp, svc);
 
 	/* return connection (will be used to handle outgoing packet) */
-	IP_VS_DBG_BUF(6, "New connection RS-initiated:%c c:%s:%u v:%s:%u "
-		      "d:%s:%u conn->flags:%X conn->refcnt:%d\n",
-		      ip_vs_fwd_tag(cp),
-		      IP_VS_DBG_ADDR(cp->af, &cp->caddr), ntohs(cp->cport),
-		      IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport),
-		      IP_VS_DBG_ADDR(cp->af, &cp->daddr), ntohs(cp->dport),
-		      cp->flags, refcount_read(&cp->refcnt));
+	IP_VS_DBG(6, "New connection RS-initiated:%c c:%pISp v:%pISp "
+		  "d:%pISp conn->flags:%X conn->refcnt:%d\n",
+		  ip_vs_fwd_tag(cp),
+		  IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, ntohs(cp->cport)),
+		  IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, ntohs(cp->vport)),
+		  IP_VS_DBG_SOCKADDR(cp->af, &cp->daddr, ntohs(cp->dport)),
+		  cp->flags, refcount_read(&cp->refcnt));
 	LeaveFunction(12);
 	return cp;
 }
@@ -1931,7 +1930,6 @@ static int ip_vs_in_icmp_v6(struct netns_ipvs *ipvs, struct sk_buff *skb,
 }
 #endif
 
-
 /*
  *	Check if it's for virtual services, look it up,
  *	and send it on its way...
@@ -1960,10 +1958,10 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, int
 		      hooknum != NF_INET_LOCAL_OUT) ||
 		     !skb_dst(skb))) {
 		ip_vs_fill_iph_skb(af, skb, false, &iph);
-		IP_VS_DBG_BUF(12, "packet type=%d proto=%d daddr=%s"
+		IP_VS_DBG(12, "packet type=%d proto=%d daddr=%pIS"
 			      " ignored in hook %u\n",
 			      skb->pkt_type, iph.protocol,
-			      IP_VS_DBG_ADDR(af, &iph.daddr), hooknum);
+			      IP_VS_DBG_SOCKADDR(af, &iph.daddr, 0), hooknum);
 		return NF_ACCEPT;
 	}
 	/* ipvs enabled in this netns ? */
diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index cf925906f59b..d57dcc2b4396 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -306,9 +306,9 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 					   &start, &end) != 1)
 			return 1;
 
-		IP_VS_DBG_BUF(7, "EPSV response (%s:%u) -> %s:%u detected\n",
-			      IP_VS_DBG_ADDR(cp->af, &from), ntohs(port),
-			      IP_VS_DBG_ADDR(cp->af, &cp->caddr), 0);
+		IP_VS_DBG(7, "EPSV response (%pISp) -> %pISp detected\n",
+			  IP_VS_DBG_SOCKADDR(cp->af, &from, ntohs(port)),
+			  IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, 0));
 	} else {
 		return 1;
 	}
@@ -510,15 +510,15 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
 					  &to, &port, cp->af,
 					  &start, &end) == 1) {
 
-		IP_VS_DBG_BUF(7, "EPRT %s:%u detected\n",
-			      IP_VS_DBG_ADDR(cp->af, &to), ntohs(port));
+		IP_VS_DBG(7, "EPRT %pISp detected\n",
+			  IP_VS_DBG_SOCKADDR(cp->af, &to, ntohs(port)));
 
 		/* Now update or create a connection entry for it */
-		IP_VS_DBG_BUF(7, "protocol %s %s:%u %s:%u\n",
-			      ip_vs_proto_name(ipvsh->protocol),
-			      IP_VS_DBG_ADDR(cp->af, &to), ntohs(port),
-			      IP_VS_DBG_ADDR(cp->af, &cp->vaddr),
-			      ntohs(cp->vport)-1);
+		IP_VS_DBG(7, "protocol %s %pISp %pISp\n",
+			  ip_vs_proto_name(ipvsh->protocol),
+			  IP_VS_DBG_SOCKADDR(cp->af, &to, ntohs(port)),
+			  IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr,
+			  ntohs(cp->vport)-1));
 	} else {
 		return 1;
 	}
-- 
2.20.0



More information about the devel mailing list