ulogd2 commit

ulogd2 commit

[Pablo Neira Ayuso]

Hi Eric, Pierre,

Please, could you justify the need for commit
0ff525cb0506b2c043bc9df6d7e7b486c865bc38?

http://git.netfilter.org/cgi-bin/gitweb.cgi?p=ulogd2.git;a=commit;h=0ff525cb0506b2c043bc9df6d7e7b486c865bc38

I don't think that it's a good idea to support external compilation of
ulogd2 modules. We should promote people to push forward things to mainline.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: ulogd2 commit

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]

Hi,

Le vendredi 17 avril 2009 à 12:52 +0200, Pablo Neira Ayuso a écrit :
> Hi Eric, Pierre,
> 
> Please, could you justify the need for commit
> 0ff525cb0506b2c043bc9df6d7e7b486c865bc38?
> 
> http://git.netfilter.org/cgi-bin/gitweb.cgi?p=ulogd2.git;a=commit;h=0ff525cb0506b2c043bc9df6d7e7b486c865bc38
> 
> I don't think that it's a good idea to support external compilation of
> ulogd2 modules. We should promote people to push forward things to mainline.

I clearly agree with the goal of promoting people to push forward things
to mainline but it has to be done by openness and reactivity of
mainteners, not by technical means.

But let looks at the diffstat of this commit:
 include/Makefile.am       |    1 +
 include/ulogd/Makefile.am |    3 +--
 include/ulogd/ulogd.h     |    2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

That's really not a big work for anyone to arrive to the same result. An
other point is that compiling outside of tree require you to maintain
your own build system which is quiet boring.

Thus, using git to maintain you own forked branch is the easiest way to
keep your work hidden and/or non public.

IMHO, this patch does not change anything about external contribution.

BR,
-- 
Eric Leblond <eric@inl.fr>
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: ulogd2 commit

[Pablo Neira Ayuso]

Eric Leblond wrote:
> Hi,
> 
> Le vendredi 17 avril 2009 à 12:52 +0200, Pablo Neira Ayuso a écrit :
>> Hi Eric, Pierre,
>>
>> Please, could you justify the need for commit
>> 0ff525cb0506b2c043bc9df6d7e7b486c865bc38?
>>
>> http://git.netfilter.org/cgi-bin/gitweb.cgi?p=ulogd2.git;a=commit;h=0ff525cb0506b2c043bc9df6d7e7b486c865bc38
>>
>> I don't think that it's a good idea to support external compilation of
>> ulogd2 modules. We should promote people to push forward things to mainline.
> 
> I clearly agree with the goal of promoting people to push forward things
> to mainline but it has to be done by openness and reactivity of
> mainteners, not by technical means.
> 
> But let looks at the diffstat of this commit:
>  include/Makefile.am       |    1 +
>  include/ulogd/Makefile.am |    3 +--
>  include/ulogd/ulogd.h     |    2 ++
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> That's really not a big work for anyone to arrive to the same result. An
> other point is that compiling outside of tree require you to maintain
> your own build system which is quiet boring.
> 
> Thus, using git to maintain you own forked branch is the easiest way to
> keep your work hidden and/or non public.
> 
> IMHO, this patch does not change anything about external contribution.

1) Looking at my ulogd2 installation, this exports a lot of header files
for external use:

$ ls /usr/include/ulogd/
common.h    hash.h            jhash.h      linux_rbtree.h  timer.h
conffile.h  ipfix_protocol.h  linuxlist.h  slist.h         ulogd.h

Sorry, but this is a bit nasty. If this patch aims to allow compiling
external ulogd2 module,  please make public only the basic functions
(register, unregister, ...) in one header public file called ulogd.h or
something like that. But not exporting all those jhash.h, linux_rbtree.h
and so on.

2) Related to the previous point, if we make public some API, we'll have
to get attached to them to ensure backward compatibility. If we support
external modules, we'll have to engage to not changing the exported API,
otherwise people may complain about breakages of their external modules.
All those files that you have exported means quite a lot APIs that we'll
have to get attached to.

3) I particularly don't want people mixing proprietary external modules
with our GPL code. So, if we export some API, it would be nice if we
have some mechanism to make the user of this functions to explicitly
agree that it's external module is GPL, something like the Linux kernel
macros or something.

I have my concerns with regards to this patch from both the technical
approach and the new policy that it introduces.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ulogd2 commit

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]

Hi,

Le samedi 18 avril 2009 à 13:43 +0200, Pablo Neira Ayuso a écrit :
> Eric Leblond wrote:
> > Hi,
> > 
> > Le vendredi 17 avril 2009 à 12:52 +0200, Pablo Neira Ayuso a écrit :
> >> 

...

> 1) Looking at my ulogd2 installation, this exports a lot of header files
> for external use:
> 
> $ ls /usr/include/ulogd/
> common.h    hash.h            jhash.h      linux_rbtree.h  timer.h
> conffile.h  ipfix_protocol.h  linuxlist.h  slist.h         ulogd.h
> 
> Sorry, but this is a bit nasty. If this patch aims to allow compiling
> external ulogd2 module,  please make public only the basic functions
> (register, unregister, ...) in one header public file called ulogd.h or
> something like that. But not exporting all those jhash.h, linux_rbtree.h
> and so on.

Your point is correct...

> 2) Related to the previous point, if we make public some API, we'll have
> to get attached to them to ensure backward compatibility. If we support
> external modules, we'll have to engage to not changing the exported API,
> otherwise people may complain about breakages of their external modules.
> All those files that you have exported means quite a lot APIs that we'll
> have to get attached to.


This is not what I was trying to do. The only goal was compilation
outside of tree. Target was not to provide a complete and fixed plugin
infrastructure. In this case, a clean and stable API is really needed. 

> I have my concerns with regards to this patch from both the technical
> approach and the new policy that it introduces.

I agree for a revert of this commit. I can push it if you want.

BR,
-- 
Eric Leblond <eric@inl.fr>
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: ulogd2 commit

[Pablo Neira Ayuso]

Eric Leblond wrote:
> Hi,
> 
> Le samedi 18 avril 2009 à 13:43 +0200, Pablo Neira Ayuso a écrit :
>> Eric Leblond wrote:
>>> Hi,
>>>
>>> Le vendredi 17 avril 2009 à 12:52 +0200, Pablo Neira Ayuso a écrit :
> 
> ...
> 
>> 1) Looking at my ulogd2 installation, this exports a lot of header files
>> for external use:
>>
>> $ ls /usr/include/ulogd/
>> common.h    hash.h            jhash.h      linux_rbtree.h  timer.h
>> conffile.h  ipfix_protocol.h  linuxlist.h  slist.h         ulogd.h
>>
>> Sorry, but this is a bit nasty. If this patch aims to allow compiling
>> external ulogd2 module,  please make public only the basic functions
>> (register, unregister, ...) in one header public file called ulogd.h or
>> something like that. But not exporting all those jhash.h, linux_rbtree.h
>> and so on.
> 
> Your point is correct...
> 
>> 2) Related to the previous point, if we make public some API, we'll have
>> to get attached to them to ensure backward compatibility. If we support
>> external modules, we'll have to engage to not changing the exported API,
>> otherwise people may complain about breakages of their external modules.
>> All those files that you have exported means quite a lot APIs that we'll
>> have to get attached to.
> 
> 
> This is not what I was trying to do. The only goal was compilation
> outside of tree. Target was not to provide a complete and fixed plugin
> infrastructure. In this case, a clean and stable API is really needed. 

Yes, but that compilation needs those header files to compile because it
will use functions that are implemented in ulogd2, right? Otherwise, I
don't see the point of making them all public.

>> I have my concerns with regards to this patch from both the technical
>> approach and the new policy that it introduces.
> 
> I agree for a revert of this commit. I can push it if you want.

Please, go ahead. Thanks Eric.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html