On Tue, Sep 18, 2012 at 12:23 PM, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Thu, Sep 13, 2012 at 12:37:18AM -0700, Ansis Atteka wrote:
> [...]
>> I know that this would be a massive change, so I might miss
>> something in the proposal. Anyway feel free to correct me or ask for
>> more details where necessary. Here is a list of necessary changes:
>>
>> 1. Quite a lot of refactoring in configuration parser.
>>
>> I would suggest to split ct_conf struct into three logical pieces (i.e. smaller
>> structs):
>> a. channel config (instance per remote conntrackd instance)
>> b. conntrack-state config (instance per namespace)
>> c. static/global config (single instance; Would contain path to log
>> file, unix socket e.t.c.)
>
> I always wanted to clean up ct_conf. See patch attached, I didn't
> commit it yet since I want to give it some test (Please, not need to
> rebase the patch we're currently discussing upon it).
I don't see any patches attached.
>
>> This decoupling would allow much more easily to maintain relation
>> between conntrack-states and channels (for example, when multiple
>> namespaces are using the same channel).
>
> I'm not familiar with the channel definition you're using.
By "channel" I meant channel & channel_ops structures.
>
> Note that conntrackd already uses the name `channel' for the state-sync
> link abstraction (ie. TCP / UDP / MCAST / TIPC ...).
I guess we are actually talking about the same thing.
>
>> Also, currently CONFIG(X) macro allows to reference only a single ct_conf
>> instance. This will need to be changed so that each namespace has its own
>> ct_conf_sync instance. And each channel has its own ct_conf_channel instance.
>>
>> Also, I am afraid that, for this to make more sense, I would have to extend
>> conntrackd.conf syntax, For example,introduce following top-level sections:
>> channel {}, sync {} and global {} respectively.
>>
>> 2. Allow to add, remove and list configuration dynamically without
>> restarting conntrackd. This would require ability to start conntrackd
>> with minimal global/static configuration. After that add namespaces and
>> channel configuration by using Unix Domain socket.
>>
>> For example, instead of starting conntrackd with following command:
>> conntrackd -C /etc/conntrackd/conntrackd.conf
>>
>> One would have to use something like this:
>> conntrackd --global-config /etc/conntrackd/conntrackd_global.conf #
>> This config file would just specify Unix socket, log file e.t.c.
>> and then on-the-fly add channel and namespace configuration with:
>> conntrackd -U /var/run/conntrackd.ctl --add
>> /etc/conntrackd/conntrackd_namespace1.conf
>> conntrackd -U /var/run/conntrackd.ctl --add
>> /etc/conntrackd/conntrackd_channel1.conf
>> conntrackd -U /var/run/conntrackd.ctl --add
>> /etc/conntrackd/conntrackd_namespace2.conf
>> conntrackd -U /var/run/conntrackd.ctl --add
>> /etc/conntrackd/conntrackd_channel2.conf
>>
>> We could glue namespaces together with channels by using some IDs.
>>
>> Another question is, whether over the Unix domain socket we would prefer to
>> transfer binary (already parsed) or textual (yet unparsed) configuration?
>>
>> Also, I would need to figure out if/how to maintain backward compatibility with
>> already existing commands, when there are multiple namespaces (e.g. dump
>> internal cache, commit external cache ...).
>>
>> 3. Wire protocol format improvements, so that namespace ID would be encapsulated
>> into the protocol too. This is required, when same channel is being
>> used by multiple namespaces.
>
> This only requires adding one new TLV attribute to identify this. So
> we don't need to bloat the header with one field that is not use.
Here is how I am currently doing that:
struct nethdr {
#if __BYTE_ORDER == __LITTLE_ENDIAN
uint8_t type:4,
version:4;
#elif __BYTE_ORDER == __BIG_ENDIAN
uint8_t version:4,
type:4;
#else
#error "Unknown system endianess!"
#endif
uint8_t flags;
uint16_t len;
uint32_t seq;
uint32_t nsid; < -present only if nethdr.flag &
};
nsid is the namespace id. This field would be present only
if nethdr.flags & NET_F_NAMESPACE != 0.
Could you please elaborate more on your suggested approach? If I
understand it correctly, then you want to add the namespace id
inside the data section that follows immediately after the nethdr
structure as TLV variable?
Also, if I understand correctly, then after nethdr conntrackd
simply transfers nf_conntrack struct (or the contents of cache
object). I have had imagined that we should have cache table
per namespace, so storing namespace id inside nf_conntrackd
would become redundant.
What is your take on this?
>
>> 4. Similarly as CONFIG(x) was broken down into 3 logical pieces, the
>> same thing would
>> need to be done for STATE(x) macros.
>
> This seems to be a huge changeset what you're proposing.
>
> I need some convincing architecture example that describes how this
> can be used before you submit such a big patch. I need to understand
> it to know if there is a different way to make this.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
