Prev: netfilter: xt_ipvs (netfilter matcher for IPVS)
Next: [PATCH] cifs: remove unused ip_address field in struct TCP_Server_Info
From: Patrick McHardy on 6 Jul 2010 07:50 Simon Horman wrote: > @@ -219,19 +358,23 @@ static int ip_vs_ftp_out(struct ip_vs_ap > > buf_len = strlen(buf); > > + ct = nf_ct_get(skb, &ctinfo); > + ret = nf_nat_mangle_tcp_packet(skb, > + ct, > + ctinfo, > + start-data, > + end-start, > + buf, > + buf_len); > + > + if (ct && ct != &nf_conntrack_untracked) > This does not make sense, you're already using the conntrack above in the call to nf_nat_mangle_tcp_packet(), so the check should probably happen before that. You also should be checking the return value of nf_nat_mangle_tcp_packet() before setting up the expectation. > + ip_vs_expect_related(skb, ct, n_cp, > + IPPROTO_TCP, NULL, 0); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Simon Horman on 7 Jul 2010 03:00 On Tue, Jul 06, 2010 at 01:43:44PM +0200, Patrick McHardy wrote: > Simon Horman wrote: > >@@ -219,19 +358,23 @@ static int ip_vs_ftp_out(struct ip_vs_ap > > buf_len = strlen(buf); > >+ ct = nf_ct_get(skb, &ctinfo); > >+ ret = nf_nat_mangle_tcp_packet(skb, > >+ ct, > >+ ctinfo, > >+ start-data, > >+ end-start, > >+ buf, > >+ buf_len); > >+ > >+ if (ct && ct != &nf_conntrack_untracked) > This does not make sense, you're already using the conntrack above > in the call to nf_nat_mangle_tcp_packet(), so the check should > probably happen before that. You also should be checking the > return value of nf_nat_mangle_tcp_packet() before setting up the > expectation. > > >+ ip_vs_expect_related(skb, ct, n_cp, > >+ IPPROTO_TCP, NULL, 0); Good point. Is this better? ct = nf_ct_get(skb, &ctinfo); if (ct && !nf_ct_is_untracked()) { ret = nf_nat_mangle_tcp_packet(skb, ct, ctinfo, start-data, end-start, buf, buf_len); if (ret) ip_vs_expect_related(skb, ct, n_cp, IPPROTO_TCP, NULL, 0); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Patrick McHardy on 9 Jul 2010 11:30 Am 07.07.2010 08:53, schrieb Simon Horman: > On Tue, Jul 06, 2010 at 01:43:44PM +0200, Patrick McHardy wrote: >> Simon Horman wrote: >>> @@ -219,19 +358,23 @@ static int ip_vs_ftp_out(struct ip_vs_ap >>> buf_len = strlen(buf); >>> + ct = nf_ct_get(skb, &ctinfo); >>> + ret = nf_nat_mangle_tcp_packet(skb, >>> + ct, >>> + ctinfo, >>> + start-data, >>> + end-start, >>> + buf, >>> + buf_len); >>> + >>> + if (ct && ct != &nf_conntrack_untracked) >> This does not make sense, you're already using the conntrack above >> in the call to nf_nat_mangle_tcp_packet(), so the check should >> probably happen before that. You also should be checking the >> return value of nf_nat_mangle_tcp_packet() before setting up the >> expectation. >> >>> + ip_vs_expect_related(skb, ct, n_cp, >>> + IPPROTO_TCP, NULL, 0); > > Good point. Is this better? > > ct = nf_ct_get(skb, &ctinfo); > if (ct && !nf_ct_is_untracked()) { > ret = nf_nat_mangle_tcp_packet(skb, ct, ctinfo, > start-data, end-start, > buf, buf_len); > if (ret) > ip_vs_expect_related(skb, ct, n_cp, > IPPROTO_TCP, NULL, 0); Yes, that's better, although we're usually dropping packets when mangling fails. This can only happen under memory pressure, the assumption is that we might be able to properly mangle the packet when it is retransmitted. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Simon Horman on 9 Jul 2010 22:00 On Fri, Jul 09, 2010 at 05:24:56PM +0200, Patrick McHardy wrote: > Am 07.07.2010 08:53, schrieb Simon Horman: > > On Tue, Jul 06, 2010 at 01:43:44PM +0200, Patrick McHardy wrote: > >> Simon Horman wrote: > >>> @@ -219,19 +358,23 @@ static int ip_vs_ftp_out(struct ip_vs_ap > >>> buf_len = strlen(buf); > >>> + ct = nf_ct_get(skb, &ctinfo); > >>> + ret = nf_nat_mangle_tcp_packet(skb, > >>> + ct, > >>> + ctinfo, > >>> + start-data, > >>> + end-start, > >>> + buf, > >>> + buf_len); > >>> + > >>> + if (ct && ct != &nf_conntrack_untracked) > >> This does not make sense, you're already using the conntrack above > >> in the call to nf_nat_mangle_tcp_packet(), so the check should > >> probably happen before that. You also should be checking the > >> return value of nf_nat_mangle_tcp_packet() before setting up the > >> expectation. > >> > >>> + ip_vs_expect_related(skb, ct, n_cp, > >>> + IPPROTO_TCP, NULL, 0); > > > > Good point. Is this better? > > > > ct = nf_ct_get(skb, &ctinfo); > > if (ct && !nf_ct_is_untracked()) { > > ret = nf_nat_mangle_tcp_packet(skb, ct, ctinfo, > > start-data, end-start, > > buf, buf_len); > > if (ret) > > ip_vs_expect_related(skb, ct, n_cp, > > IPPROTO_TCP, NULL, 0); > > Yes, that's better, although we're usually dropping packets > when mangling fails. This can only happen under memory pressure, > the assumption is that we might be able to properly mangle > the packet when it is retransmitted. I didn't notice this either, but ret will be end up being the return value of ip_vs_ftp_out(), and if that is zero the packet will be dropped. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Simon Horman on 11 Jul 2010 05:10
On Sat, Jul 10, 2010 at 12:01:00PM +0900, Simon Horman wrote: > From: Hannes Eder <heder(a)google.com> > > Use nf_conntrack/nf_nat code to do the packet mangling and the TCP > sequence adjusting. The function 'ip_vs_skb_replace' is now dead > code, so it is removed. > > To SNAT FTP, use something like: > > % iptables -t nat -A POSTROUTING -m ipvs --vaddr 192.168.100.30/32 \ > > --vport 21 -j SNAT --to-source 192.168.10.10 > > and for the data connections in passive mode: > > % iptables -t nat -A POSTROUTING -m ipvs --vaddr 192.168.100.30/32 \ > > --vportctl 21 -j SNAT --to-source 192.168.10.10 > > using '-m state --state RELATED' would also works. > > Make sure the kernel modules ip_vs_ftp, nf_conntrack_ftp, and > nf_nat_ftp are loaded. [snip] > Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_ftp.c > =================================================================== > --- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_ftp.c 2010-07-10 11:48:54.000000000 +0900 > +++ nf-next-2.6/net/netfilter/ipvs/ip_vs_ftp.c 2010-07-10 11:59:19.000000000 +0900 [snip] > @@ -43,6 +57,16 @@ > #define SERVER_STRING "227 Entering Passive Mode (" > #define CLIENT_STRING "PORT " > > +#define FMT_TUPLE "%pI4:%u->%pI4:%u/%u" > +#define ARG_TUPLE(T) (T)->src.u3.ip, ntohs((T)->src.u.all), \ > + (T)->dst.u3.ip, ntohs((T)->dst.u.all), \ > + (T)->dst.protonum > + > +#define FMT_CONN "%pI4:%u->%pI4:%u->%pI4:%u/%u:%u" > +#define ARG_CONN(C) (C)->caddr, ntohs((C)->cport), \ > + (C)->vaddr, ntohs((C)->vport), \ > + (C)->daddr, ntohs((C)->dport), \ > + (C)->protocol, (C)->state > > /* > * List of ports (up to IP_VS_APP_MAX_PORTS) to be handled by helper The argument to the %pI4 needs to be a pointer so (T)->src.u3.ip should be &(T)->src.u3.ip and (C)->caddr should be &(C)->caddr.ip. I'm not sure how this slipped through the cracks so far. I will repost. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |