[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
Patrick McHardy wrote: Alexey Dobriyan wrote: On Thu, Sep 04, 2008 at 06:54:16PM +0200, Patrick McHardy wrote: [EMAIL PROTECTED] wrote: Make untracked conntrack per-netns. Compare conntracks with relevant untracked one. The following code you'll start laughing at this code: if (ct == ct-ct_net-ct.untracked) ... let me remind you that -ct_net is set in only one place, and never overwritten later. All of this requires some surgery with headers, otherwise horrible circular dependencies. And we lost nf_ct_is_untracked() as function, it became macro. I think you could avoid this mess by using a struct nf_conntrack for the untracked conntrack instead of struct nf_conn. It shouldn't make any difference since its ignored anyways. Ewww, can I? I hope so :) A different possiblity suggest by Pablo some time ago would be to mark untracked packets in skb-nfctinfo and not attach a conntrack at all. Indeed, I remember that :). I left that patch of the table time ago [1]. There's a nf_reset call missing as Patrick said at that time. I can recover it if you like the idea. [1] http://lists.netfilter.org/pipermail/netfilter-devel/2005-June/020171.html -- Los honestos son inadaptados sociales -- Les Luthiers ___ Containers mailing list [EMAIL PROTECTED] https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
Alexey Dobriyan wrote: On Thu, Sep 04, 2008 at 06:54:16PM +0200, Patrick McHardy wrote: [EMAIL PROTECTED] wrote: Make untracked conntrack per-netns. Compare conntracks with relevant untracked one. The following code you'll start laughing at this code: if (ct == ct-ct_net-ct.untracked) ... let me remind you that -ct_net is set in only one place, and never overwritten later. All of this requires some surgery with headers, otherwise horrible circular dependencies. And we lost nf_ct_is_untracked() as function, it became macro. I think you could avoid this mess by using a struct nf_conntrack for the untracked conntrack instead of struct nf_conn. It shouldn't make any difference since its ignored anyways. Ewww, can I? I hope so :) A different possiblity suggest by Pablo some time ago would be to mark untracked packets in skb-nfctinfo and not attach a conntrack at all. Regardless of netns, switching to struct nf_conntrack nf_conntrack_untracked; means we must be absolutely sure that every place which uses, say, ct-status won't get untracked conntrack. For example, does setting IPS_NAT_DONE_MASK and IPS_CONFIRMED_BIT on untracked conntracked really necessary? I don't think so, untracked conntracks are skipped early in the NAT table. In conntrack_mt_v0() ct-status can be used even for untracked connection, is this right? It looks that way, but its not right. I think it should return false for every match except on (untracked) state. ___ Containers mailing list [EMAIL PROTECTED] https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
[EMAIL PROTECTED] wrote:
Make untracked conntrack per-netns. Compare conntracks with relevant
untracked one.
The following code you'll start laughing at this code:
if (ct == ct-ct_net-ct.untracked)
...
let me remind you that -ct_net is set in only one place, and never
overwritten later.
All of this requires some surgery with headers, otherwise horrible circular
dependencies. And we lost nf_ct_is_untracked() as function, it became macro.
I think you could avoid this mess by using a struct nf_conntrack
for the untracked conntrack instead of struct nf_conn. It shouldn't
make any difference since its ignored anyways.
struct netns_ct {
atomic_tcount;
@@ -12,5 +13,7 @@ struct netns_ct {
struct hlist_head *expect_hash;
int expect_vmalloc;
struct hlist_head unconfirmed;
+ /* Fake conntrack entry for untracked connections */
+ struct nf_conn untracked;
};
___
Containers mailing list
[EMAIL PROTECTED]
https://lists.linux-foundation.org/mailman/listinfo/containers
___
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
On Thu, Sep 04, 2008 at 06:54:16PM +0200, Patrick McHardy wrote:
[EMAIL PROTECTED] wrote:
Make untracked conntrack per-netns. Compare conntracks with relevant
untracked one.
The following code you'll start laughing at this code:
if (ct == ct-ct_net-ct.untracked)
...
let me remind you that -ct_net is set in only one place, and never
overwritten later.
All of this requires some surgery with headers, otherwise horrible circular
dependencies. And we lost nf_ct_is_untracked() as function, it became macro.
I think you could avoid this mess by using a struct nf_conntrack
for the untracked conntrack instead of struct nf_conn. It shouldn't
make any difference since its ignored anyways.
Ewww, can I?
Regardless of netns, switching to
struct nf_conntrack nf_conntrack_untracked;
means we must be absolutely sure that every place which uses, say,
ct-status won't get untracked conntrack.
For example, does setting IPS_NAT_DONE_MASK and IPS_CONFIRMED_BIT on
untracked conntracked really necessary?
In conntrack_mt_v0() ct-status can be used even for untracked connection,
is this right?
struct netns_ct {
atomic_tcount;
@@ -12,5 +13,7 @@ struct netns_ct {
struct hlist_head *expect_hash;
int expect_vmalloc;
struct hlist_head unconfirmed;
+/* Fake conntrack entry for untracked connections */
+struct nf_conn untracked;
};
___
Containers mailing list
[EMAIL PROTECTED]
https://lists.linux-foundation.org/mailman/listinfo/containers
___
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
On Thursday 2008-09-04 22:58, Alexey Dobriyan wrote: In conntrack_mt_v0() ct-status can be used even for untracked connection, is this right? Yes. For example, does setting IPS_NAT_DONE_MASK and IPS_CONFIRMED_BIT on untracked conntracked really necessary? Does it even happen? Something smells afoul if ct-status is anything but zero/unset for the untracked entry. ___ Containers mailing list [EMAIL PROTECTED] https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
On Thursday 2008-08-21 18:04, [EMAIL PROTECTED] wrote: Make untracked conntrack per-netns. Why? It does not store any useful information per se, it is merely used to add a third type of ct, iow: (a) ct==NULL (b) ct!=NULL (c) ct==untracked mmap(2)'s return value for example has something similar: (a) mmap(...)==NULL (b) mmap(...)==MMAP_FAILED (c) otherwise The untracked ct is a singleton, and should stay one, unless there are further reasons not to do so. ___ Containers mailing list [EMAIL PROTECTED] https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
On Friday 2008-08-22 07:30, [EMAIL PROTECTED] wrote: We wait for untracked ct refcount to drop to 1 back: /* wait until all references to nf_conntrack_untracked are dropped */ while (atomic_read(nf_conntrack_untracked.ct_general.use) 1) schedule(); Consequently it should be one per netns, otherwise netns A can prevent netns B from stopping. But nf_conntrack_cleanup is not per netns, is it? At least I do not think it should be. ___ Containers mailing list [EMAIL PROTECTED] https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
On Sat, Aug 23, 2008 at 08:35:07PM -0400, Jan Engelhardt wrote: On Friday 2008-08-22 07:30, [EMAIL PROTECTED] wrote: We wait for untracked ct refcount to drop to 1 back: /* wait until all references to nf_conntrack_untracked are dropped */ while (atomic_read(nf_conntrack_untracked.ct_general.use) 1) schedule(); Consequently it should be one per netns, otherwise netns A can prevent netns B from stopping. But nf_conntrack_cleanup is not per netns, is it? That's because nf_conntrack_cleanup() is _code_. If netns A actively uses NOTRACK, untracked ct refcount will be bumped. And netns B which haven't used NOTRACK at all will wait for netns A to stop using NOTRACK potentially indefinitely. At least I do not think it should be. ___ Containers mailing list [EMAIL PROTECTED] https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 20/38] netns ct: NOTRACK in netns
On Thu, Aug 21, 2008 at 07:06:37PM -0400, Jan Engelhardt wrote: On Thursday 2008-08-21 18:04, [EMAIL PROTECTED] wrote: Make untracked conntrack per-netns. Why? It does not store any useful information per se, it is merely used to add a third type of ct, iow: (a) ct==NULL (b) ct!=NULL (c) ct==untracked mmap(2)'s return value for example has something similar: (a) mmap(...)==NULL (b) mmap(...)==MMAP_FAILED (c) otherwise The untracked ct is a singleton, and should stay one, unless there are further reasons not to do so. We wait for untracked ct refcount to drop to 1 back: /* wait until all references to nf_conntrack_untracked are dropped */ while (atomic_read(nf_conntrack_untracked.ct_general.use) 1) schedule(); Consequently it should be one per netns, otherwise netns A can prevent netns B from stopping. ___ Containers mailing list [EMAIL PROTECTED] https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
