* [patch 0/4] ip_vs_ftp cleanups
@ 2006-09-01 10:10 Horms
2006-09-01 10:10 ` [patch 1/4] Document the ports option to ip_vs_ftp in kernel-parameters.txt Horms
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Horms @ 2006-09-01 10:10 UTC (permalink / raw)
To: netdev; +Cc: Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T
Hi,
This series of four patches has several minor cleanups for the options to
the ip_vs_ftp modules.
--
Horms
H: http://www.vergenet.net/~horms/
W: http://www.valinux.co.jp/en/
^ permalink raw reply [flat|nested] 10+ messages in thread* [patch 1/4] Document the ports option to ip_vs_ftp in kernel-parameters.txt 2006-09-01 10:10 [patch 0/4] ip_vs_ftp cleanups Horms @ 2006-09-01 10:10 ` Horms 2006-09-01 10:10 ` [patch 2/4] auto-help for ip_vs_ftp Horms ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Horms @ 2006-09-01 10:10 UTC (permalink / raw) To: netdev; +Cc: Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T [-- Attachment #1: ip_vs_ftp-doc.patch --] [-- Type: text/plain, Size: 934 bytes --] I'm not sure if documenting this here is appropriate, but if it is, here is some text to put there. Signed-Off-By: Simon Horman <horms@verge.net.au> Index: linux-2.6/Documentation/kernel-parameters.txt =================================================================== --- linux-2.6.orig/Documentation/kernel-parameters.txt 2006-09-01 17:02:10.000000000 +0900 +++ linux-2.6/Documentation/kernel-parameters.txt 2006-09-01 17:02:49.000000000 +0900 @@ -697,6 +697,12 @@ ips= [HW,SCSI] Adaptec / IBM ServeRAID controller See header of drivers/scsi/ips.c. + ports= [IP_VS_FTP] IPVS ftp helper module + Default is 21. + Up to 8 (IP_VS_APP_MAX_PORTS) ports + may be specified. + Format: <port>,<port>.... + irqfixup [HW] When an interrupt is not handled search all handlers for it. Intended to get systems with badly broken -- -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch 2/4] auto-help for ip_vs_ftp 2006-09-01 10:10 [patch 0/4] ip_vs_ftp cleanups Horms 2006-09-01 10:10 ` [patch 1/4] Document the ports option to ip_vs_ftp in kernel-parameters.txt Horms @ 2006-09-01 10:10 ` Horms 2006-09-01 10:10 ` [patch 3/4] Make sure ip_vs_ftp ports are valid Horms 2006-09-01 10:10 ` [patch 4/4] remove the debug option go ip_vs_ftp Horms 3 siblings, 0 replies; 10+ messages in thread From: Horms @ 2006-09-01 10:10 UTC (permalink / raw) To: netdev; +Cc: Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T [-- Attachment #1: ip_vs_ftp-auto-doc.patch --] [-- Type: text/plain, Size: 670 bytes --] Fill in a help message for the ports option to ip_vs_ftp Signed-Off-By: Simon Horman <horms@verge.net.au> Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c =================================================================== --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:02:38.000000000 +0900 +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:06:42.000000000 +0900 @@ -46,6 +46,7 @@ */ static int ports[IP_VS_APP_MAX_PORTS] = {21, 0}; module_param_array(ports, int, NULL, 0); +MODULE_PARM_DESC(ports, "Ports to monitor for FTP control commands"); /* * Debug level -- -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch 3/4] Make sure ip_vs_ftp ports are valid 2006-09-01 10:10 [patch 0/4] ip_vs_ftp cleanups Horms 2006-09-01 10:10 ` [patch 1/4] Document the ports option to ip_vs_ftp in kernel-parameters.txt Horms 2006-09-01 10:10 ` [patch 2/4] auto-help for ip_vs_ftp Horms @ 2006-09-01 10:10 ` Horms 2006-09-03 23:09 ` Patrick McHardy 2006-09-01 10:10 ` [patch 4/4] remove the debug option go ip_vs_ftp Horms 3 siblings, 1 reply; 10+ messages in thread From: Horms @ 2006-09-01 10:10 UTC (permalink / raw) To: netdev; +Cc: Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T [-- Attachment #1: ip_vs_ftp-ignore-invalid.patch --] [-- Type: text/plain, Size: 915 bytes --] I'm not entirely sure what happens in the case of a valid port, at best it'll be silently ignored. This patch ignores them a little more verbosely. Signed-Off-By: Simon Horman <horms@verge.net.au> Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c =================================================================== --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:06:42.000000000 +0900 +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:08:19.000000000 +0900 @@ -373,6 +373,12 @@ for (i=0; i<IP_VS_APP_MAX_PORTS; i++) { if (!ports[i]) continue; + if (ports[i] < 0 || ports[i] > 0xffff) { + IP_VS_WARNING("ip_vs_ftp: Ignoring invalid " + "configuration port[%d] = %d\n", + i, ports[i]); + continue; + } ret = register_ip_vs_app_inc(app, app->protocol, ports[i]); if (ret) break; -- -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 3/4] Make sure ip_vs_ftp ports are valid 2006-09-01 10:10 ` [patch 3/4] Make sure ip_vs_ftp ports are valid Horms @ 2006-09-03 23:09 ` Patrick McHardy 2006-09-04 0:44 ` Horms 0 siblings, 1 reply; 10+ messages in thread From: Patrick McHardy @ 2006-09-03 23:09 UTC (permalink / raw) To: Horms Cc: netdev, Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T Horms wrote: > I'm not entirely sure what happens in the case of a valid port, > at best it'll be silently ignored. This patch ignores them a little > more verbosely. > > Signed-Off-By: Simon Horman <horms@verge.net.au> > Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c > =================================================================== > --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:06:42.000000000 +0900 > +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:08:19.000000000 +0900 > @@ -373,6 +373,12 @@ > for (i=0; i<IP_VS_APP_MAX_PORTS; i++) { > if (!ports[i]) > continue; > + if (ports[i] < 0 || ports[i] > 0xffff) { > + IP_VS_WARNING("ip_vs_ftp: Ignoring invalid " > + "configuration port[%d] = %d\n", > + i, ports[i]); > + continue; > + } How about just changing the module parameter type to ushort, similar to what ip_conntrack_ftp does? # modprobe ip_conntrack_ftp ports=999392 ip_conntrack_ftp: `999392' invalid for parameter `ports' -- VGER BF report: H 0.41558 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 3/4] Make sure ip_vs_ftp ports are valid 2006-09-03 23:09 ` Patrick McHardy @ 2006-09-04 0:44 ` Horms 2006-09-04 2:02 ` Horms 0 siblings, 1 reply; 10+ messages in thread From: Horms @ 2006-09-04 0:44 UTC (permalink / raw) To: Patrick McHardy Cc: netdev, Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T On Mon, Sep 04, 2006 at 01:09:59AM +0200, Patrick McHardy wrote: > Horms wrote: > > I'm not entirely sure what happens in the case of a valid port, > > at best it'll be silently ignored. This patch ignores them a little > > more verbosely. > > > > Signed-Off-By: Simon Horman <horms@verge.net.au> > > Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c > > =================================================================== > > --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:06:42.000000000 +0900 > > +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:08:19.000000000 +0900 > > @@ -373,6 +373,12 @@ > > for (i=0; i<IP_VS_APP_MAX_PORTS; i++) { > > if (!ports[i]) > > continue; > > + if (ports[i] < 0 || ports[i] > 0xffff) { > > + IP_VS_WARNING("ip_vs_ftp: Ignoring invalid " > > + "configuration port[%d] = %d\n", > > + i, ports[i]); > > + continue; > > + } > > How about just changing the module parameter type to ushort, similar to > what ip_conntrack_ftp does? Sure. I wasn't sure if that was possible or not. But as it is, I will make it so. -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ -- VGER BF report: U 0.575956 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 3/4] Make sure ip_vs_ftp ports are valid 2006-09-04 0:44 ` Horms @ 2006-09-04 2:02 ` Horms 2006-09-20 10:29 ` Patrick McHardy 0 siblings, 1 reply; 10+ messages in thread From: Horms @ 2006-09-04 2:02 UTC (permalink / raw) To: Patrick McHardy Cc: netdev, Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T On Mon, Sep 04, 2006 at 09:44:02AM +0900, Horms wrote: > On Mon, Sep 04, 2006 at 01:09:59AM +0200, Patrick McHardy wrote: > > Horms wrote: > > > I'm not entirely sure what happens in the case of a valid port, > > > at best it'll be silently ignored. This patch ignores them a little > > > more verbosely. > > > > > > Signed-Off-By: Simon Horman <horms@verge.net.au> > > > Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c > > > =================================================================== > > > --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:06:42.000000000 +0900 > > > +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:08:19.000000000 +0900 > > > @@ -373,6 +373,12 @@ > > > for (i=0; i<IP_VS_APP_MAX_PORTS; i++) { > > > if (!ports[i]) > > > continue; > > > + if (ports[i] < 0 || ports[i] > 0xffff) { > > > + IP_VS_WARNING("ip_vs_ftp: Ignoring invalid " > > > + "configuration port[%d] = %d\n", > > > + i, ports[i]); > > > + continue; > > > + } > > > > How about just changing the module parameter type to ushort, similar to > > what ip_conntrack_ftp does? > > Sure. I wasn't sure if that was possible or not. > But as it is, I will make it so. Here is the revised patch. -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ [IPVS] Make sure ip_vs_ftp ports are valid I'm not entirely sure what happens in the case of a valid port, at best it'll be silently ignored. This patch ensures that the port values are unsigned short values, and thus always valid. Cc: Patrick McHardy <kaber@trash.net> Signed-Off-By: Simon Horman <horms@verge.net.au> Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c =================================================================== --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-04 10:47:09.000000000 +0900 +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-04 10:59:30.000000000 +0900 @@ -44,8 +44,8 @@ * List of ports (up to IP_VS_APP_MAX_PORTS) to be handled by helper * First port is set to the default port. */ -static int ports[IP_VS_APP_MAX_PORTS] = {21, 0}; -module_param_array(ports, int, NULL, 0); +static unsigned short ports[IP_VS_APP_MAX_PORTS] = {21, 0}; +module_param_array(ports, ushort, NULL, 0); MODULE_PARM_DESC(ports, "Ports to monitor for FTP control commands"); /* -- VGER BF report: U 0.832414 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 3/4] Make sure ip_vs_ftp ports are valid 2006-09-04 2:02 ` Horms @ 2006-09-20 10:29 ` Patrick McHardy 2006-09-20 14:49 ` Horms 0 siblings, 1 reply; 10+ messages in thread From: Patrick McHardy @ 2006-09-20 10:29 UTC (permalink / raw) To: Horms Cc: netdev, Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T Horms wrote: > Here is the revised patch. > > > [IPVS] Make sure ip_vs_ftp ports are valid > > I'm not entirely sure what happens in the case of a valid port, > at best it'll be silently ignored. This patch ensures that > the port values are unsigned short values, and thus always valid. > > Cc: Patrick McHardy <kaber@trash.net> > Signed-Off-By: Simon Horman <horms@verge.net.au> > > Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c > =================================================================== > --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-04 10:47:09.000000000 +0900 > +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-04 10:59:30.000000000 +0900 > @@ -44,8 +44,8 @@ > * List of ports (up to IP_VS_APP_MAX_PORTS) to be handled by helper > * First port is set to the default port. > */ > -static int ports[IP_VS_APP_MAX_PORTS] = {21, 0}; > -module_param_array(ports, int, NULL, 0); > +static unsigned short ports[IP_VS_APP_MAX_PORTS] = {21, 0}; > +module_param_array(ports, ushort, NULL, 0); > MODULE_PARM_DESC(ports, "Ports to monitor for FTP control commands"); > > /* It looks like the wrong patch went in: http://marc.theaimsgroup.com/?l=git-commits-head&m=115862407021941&w=2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 3/4] Make sure ip_vs_ftp ports are valid 2006-09-20 10:29 ` Patrick McHardy @ 2006-09-20 14:49 ` Horms 0 siblings, 0 replies; 10+ messages in thread From: Horms @ 2006-09-20 14:49 UTC (permalink / raw) To: Patrick McHardy Cc: netdev, Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T On Wed, Sep 20, 2006 at 12:29:45PM +0200, Patrick McHardy wrote: > Horms wrote: > > Here is the revised patch. > > > > > > [IPVS] Make sure ip_vs_ftp ports are valid > > > > I'm not entirely sure what happens in the case of a valid port, > > at best it'll be silently ignored. This patch ensures that > > the port values are unsigned short values, and thus always valid. > > > > Cc: Patrick McHardy <kaber@trash.net> > > Signed-Off-By: Simon Horman <horms@verge.net.au> > > > > Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c > > =================================================================== > > --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-04 > 10:47:09.000000000 +0900 > > +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-04 10:59:30.000000000 > +0900 > > @@ -44,8 +44,8 @@ > > * List of ports (up to IP_VS_APP_MAX_PORTS) to be handled by helper > > * First port is set to the default port. > > */ > > -static int ports[IP_VS_APP_MAX_PORTS] = {21, 0}; > > -module_param_array(ports, int, NULL, 0); > > +static unsigned short ports[IP_VS_APP_MAX_PORTS] = {21, 0}; > > +module_param_array(ports, ushort, NULL, 0); > > MODULE_PARM_DESC(ports, "Ports to monitor for FTP control commands"); > > > > /* > > It looks like the wrong patch went in: > > http://marc.theaimsgroup.com/?l=git-commits-head&m=115862407021941&w=2 Thanks for pointing that out. I'll send out patches to reverse the committed change, and add the newer incarntation. -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch 4/4] remove the debug option go ip_vs_ftp 2006-09-01 10:10 [patch 0/4] ip_vs_ftp cleanups Horms ` (2 preceding siblings ...) 2006-09-01 10:10 ` [patch 3/4] Make sure ip_vs_ftp ports are valid Horms @ 2006-09-01 10:10 ` Horms 3 siblings, 0 replies; 10+ messages in thread From: Horms @ 2006-09-01 10:10 UTC (permalink / raw) To: netdev; +Cc: Wensong Zhang, Julian Anastasov, David Miller, Joseph Mack NA3T [-- Attachment #1: ip_vs_ftp-debug-parameter.patch --] [-- Type: text/plain, Size: 2225 bytes --] This patch makes the debuging behaviour of this code more consistent with the rest of IPVS. Signed-Off-By: Simon Horman <horms@verge.net.au> Index: linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c =================================================================== --- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:06:49.000000000 +0900 +++ linux-2.6/net/ipv4/ipvs/ip_vs_ftp.c 2006-09-01 19:06:50.000000000 +0900 @@ -48,14 +48,6 @@ module_param_array(ports, int, NULL, 0); MODULE_PARM_DESC(ports, "Ports to monitor for FTP control commands"); -/* - * Debug level - */ -#ifdef CONFIG_IP_VS_DEBUG -static int debug=0; -module_param(debug, int, 0); -#endif - /* Dummy variable */ static int ip_vs_ftp_pasv; @@ -178,7 +170,7 @@ &start, &end) != 1) return 1; - IP_VS_DBG(1-debug, "PASV response (%u.%u.%u.%u:%d) -> " + IP_VS_DBG(7, "PASV response (%u.%u.%u.%u:%d) -> " "%u.%u.%u.%u:%d detected\n", NIPQUAD(from), ntohs(port), NIPQUAD(cp->caddr), 0); @@ -281,7 +273,7 @@ while (data <= data_limit - 6) { if (strnicmp(data, "PASV\r\n", 6) == 0) { /* Passive mode on */ - IP_VS_DBG(1-debug, "got PASV at %zd of %zd\n", + IP_VS_DBG(7, "got PASV at %zd of %zd\n", data - data_start, data_limit - data_start); cp->app_data = &ip_vs_ftp_pasv; @@ -303,7 +295,7 @@ &start, &end) != 1) return 1; - IP_VS_DBG(1-debug, "PORT %u.%u.%u.%u:%d detected\n", + IP_VS_DBG(7, "PORT %u.%u.%u.%u:%d detected\n", NIPQUAD(to), ntohs(port)); /* Passive mode off */ @@ -312,7 +304,7 @@ /* * Now update or create a connection entry for it */ - IP_VS_DBG(1-debug, "protocol %s %u.%u.%u.%u:%d %u.%u.%u.%u:%d\n", + IP_VS_DBG(7, "protocol %s %u.%u.%u.%u:%d %u.%u.%u.%u:%d\n", ip_vs_proto_name(iph->protocol), NIPQUAD(to), ntohs(port), NIPQUAD(cp->vaddr), 0); @@ -382,8 +374,8 @@ ret = register_ip_vs_app_inc(app, app->protocol, ports[i]); if (ret) break; - IP_VS_DBG(1-debug, "%s: loaded support on port[%d] = %d\n", - app->name, i, ports[i]); + IP_VS_INFO("%s: loaded support on port[%d] = %d\n", + app->name, i, ports[i]); } if (ret) -- -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2006-09-20 14:50 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-09-01 10:10 [patch 0/4] ip_vs_ftp cleanups Horms 2006-09-01 10:10 ` [patch 1/4] Document the ports option to ip_vs_ftp in kernel-parameters.txt Horms 2006-09-01 10:10 ` [patch 2/4] auto-help for ip_vs_ftp Horms 2006-09-01 10:10 ` [patch 3/4] Make sure ip_vs_ftp ports are valid Horms 2006-09-03 23:09 ` Patrick McHardy 2006-09-04 0:44 ` Horms 2006-09-04 2:02 ` Horms 2006-09-20 10:29 ` Patrick McHardy 2006-09-20 14:49 ` Horms 2006-09-01 10:10 ` [patch 4/4] remove the debug option go ip_vs_ftp Horms
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).