Re: [patch v2.3 3/4] IPVS: make FTP work with full NAT support

[Patrick McHardy <kaber@trash.net>]

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.