New H.323 conntrack & NAT helper module
Jing Min Zhao
zhaojingmin at hotmail.com
Sat Feb 25 18:07:00 CET 2006
----- Original Message -----
From: "Patrick McHardy" <kaber at trash.net>
To: "Jing Min Zhao" <zhaojingmin at hotmail.com>
Cc: <netfilter-devel at lists.netfilter.org>; "Greg Scott"
<GregScott at InfraSupportEtc.com>
Sent: Saturday, February 25, 2006 4:01 AM
Subject: Re: New H.323 conntrack & NAT helper module
> Jing Min Zhao wrote:
>> I think maybe Patrick McHardy is inspecting my code, if I'm lucky,
>> it may go into the kernel tree, and you won't need a separate
>> patch any more. I really hope so.
> I'm almost done reviewing it. It really looks great, it is the IMO
> cleanest conntrack helper so far, which is really an achievement
> for such a complex thing. I've fixed a number of smaller issues
> and prepared patches for that, I'll send the first batch in follow-up
I've seen all your patches. I can't believe this. I spent almost 2 months to
write the code, but it took you only 3 days to find out so many issues!
And I know you were still doing other things while you were reviewing my
code. Now I know not everybody can do the work you guys are doing.
> Besides my patches, I have a few small issues with the patch, but if
> they are resolved I'd be happy to put this helper into 2.6.17.
This is really great news! I'll try my best.
> The issues so far:
> - ASN1 parser: I would prefer the parser to be seperated from the
> H.225/H.245 data.
OK, I'll seperate them. I was just trying to make the code look clean.
> - ASN1 parser: Right now the H.225/H.245 data includes lots of
> forward declarations, probably because it seem to be in the
> same order as in the ASN.1 file. The forward declarations make
> it a lot harder to verify that their is no recursion, so I would
> prefer to have the data ordered in a way that doesn't need them.
I generated the object definitions using a parser. I was too lazy to sort
it seems not a good format for the kernel. I'll modify my parser to sort
> - TPKT handling: I've seen gnomemeeting send nested TPKTs about a year
> ago when I worked on my helper. I can't get it do it anymore, but my
> question is if it nested TPKTs are something that should be supported.
What kind of nested TPKTs do you mean? A packet containing multiple TPKTs or
a TPKT containing another TPKT? I can't imagine the latter situation. But if
it's the first
situation, you are right. I didn't think of this. Thank you.
> - process_rcf uses the stored sig_port to find the expectation and
> adjust it's timeout. The sig_port is only set with NAT however.
> This seems to be a bug.
I'll fix this.
> - RAS tracking: should be made optional IMO. This is the only part
> where foreign IP addresses not belonging to the connection are
> used for expectations, which is potentially dangerous.
I totally agree with you. I'll add a parameter for this.
> I'll describe the other issues in the mails containing the patches.
Thank you so much!
Jing Min Zhao
More information about the netfilter-devel