[patch 0/1] [libnetfilter-conntrack] Add subsystem accessors

[patch 0/1] [libnetfilter-conntrack] Add subsystem accessors

[heitzenberger]

Hi,

please consider the following patch.  It is required for the
soon to be send ulogd v2 RFC patches.

Thanks.

  /holger

[patch 1/1] Add subsystem accessors

[heitzenberger]

[-- Attachment #1: nfct-add-subsys-accessors.diff --]
[-- Type: text/plain, Size: 1496 bytes --]

Need by newer ulogd v2.

Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>

Index: libnetfilter-conntrack/include/libnetfilter_conntrack/libnetfilter_conntrack.h
===================================================================
--- libnetfilter-conntrack.orig/include/libnetfilter_conntrack/libnetfilter_conntrack.h
+++ libnetfilter-conntrack/include/libnetfilter_conntrack/libnetfilter_conntrack.h
@@ -44,6 +44,12 @@ extern int nfct_close(struct nfct_handle
 extern int nfct_fd(struct nfct_handle *cth);
 extern const struct nfnl_handle *nfct_nfnlh(struct nfct_handle *cth);
 
+/*
+ * Subsys accessors
+ */
+extern struct nfnl_subsys_handle *nfct_subsys_ct(const struct nfct_handle *);
+extern struct nfnl_subsys_handle *nfct_subsys_exp(const struct nfct_handle *);
+
 /* 
  * NEW libnetfilter_conntrack API 
  */
Index: libnetfilter-conntrack/src/main.c
===================================================================
--- libnetfilter-conntrack.orig/src/main.c
+++ libnetfilter-conntrack/src/main.c
@@ -109,3 +109,18 @@ const struct nfnl_handle *nfct_nfnlh(str
 {
 	return cth->nfnlh;
 }
+
+/*
+ * Subsystem accessors
+ */
+struct nfnl_subsys_handle *
+nfct_subsys_ct(const struct nfct_handle *cth)
+{
+	return cth->nfnlssh_ct;
+}
+
+struct nfnl_subsys_handle *
+nfct_subsys_exp(const struct nfct_handle *cth)
+{
+	return cth->nfnlssh_exp;
+}

-- 
Holger Eitzenberger <heitzenberger@astaro.com> | Kernel Developer
Astaro AG | www.astaro.com | Phone +49-721-490069-33 | Fax -55

Re: [patch 1/1] Add subsystem accessors

[Pablo Neira Ayuso]

heitzenberger@astaro.com wrote:
> Need by newer ulogd v2.

Why do you need this? If I apply this, I'll have to provide support for
people mixing calls to libnfnetlink and libnetfilter_conntrack which can
be a problematic. The subsys handles are encapsulated for that reason.

If you really need them, I suggest you to use libnfnetlink together with
the low level functions libnetfilter_conntrack functions:
nfct_[build|parse]_conntrack and nfct_build_query.

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

Re: [patch 1/1] Add subsystem accessors

[Holger Eitzenberger]

Pablo Neira Ayuso wrote:

>> Need by newer ulogd v2.
> 
> Why do you need this? If I apply this, I'll have to provide support for
> people mixing calls to libnfnetlink and libnetfilter_conntrack which can
> be a problematic. The subsys handles are encapsulated for that reason.
> 
> If you really need them, I suggest you to use libnfnetlink together with
> the low level functions libnetfilter_conntrack functions:
> nfct_[build|parse]_conntrack and nfct_build_query.

Which would nullify the use of libnetfilter-conntrack with ulogd.

BTW, after using the new libnetfilter-conntrack I get this:

  ulogd_inpflow_NFCT.c:455: warning: passing argument 1 of 'nfnl_send'
   discards qualifiers from pointer target type
ulogd_inpflow_NFCT.c: In function 'read_cb_nfct':
  ulogd_inpflow_NFCT.c:1043: warning: passing argument 1 of 
'nfnl_recv_msgs' discards qualifiers from pointer target type
  ulogd_inpflow_NFCT.c: In function 'nfct_start':
ulogd_inpflow_NFCT.c:1156: warning: passing argument 1 of
  'nfnl_rcvbufsiz' discards qualifiers from pointer target type
ulogd_inpflow_NFCT.c: In function 'read_cb_nfct':
  ulogd_inpflow_NFCT.c:1043: warning: passing argument 1 of 
'nfnl_recv_msgs' discards qualifiers from pointer target type
...

Do you accept patches for that or should I just cast-away the wrong 
const there?

  /holger

Re: [patch 1/1] Add subsystem accessors

[Pablo Neira Ayuso]

Holger Eitzenberger wrote:
> Pablo Neira Ayuso wrote:
> 
>>> Need by newer ulogd v2.
>>
>> Why do you need this? If I apply this, I'll have to provide support for
>> people mixing calls to libnfnetlink and libnetfilter_conntrack which can
>> be a problematic. The subsys handles are encapsulated for that reason.
>>
>> If you really need them, I suggest you to use libnfnetlink together with
>> the low level functions libnetfilter_conntrack functions:
>> nfct_[build|parse]_conntrack and nfct_build_query.
> 
> Which would nullify the use of libnetfilter-conntrack with ulogd.

No way. You're abusing the library APIs. The proper way for what you
want to do is the following:

1) open a nfnl handle
2) open a nfnl_subsys_handle
3) register the callback
4) use nfct_parse_conntrack inside the callback to get the conntrack
object while you can still access netlink details (such as the sequence
number).
5) use nfct_build_query plus nfnl_query if you want to send request to
ctnetlink.

This is the way to work in "low level" with libnfnetlink and
libnetfilter_conntrack. As said, with "low level" I mean if you require
netlink details. This is pretty similar to libnl.

If you require "high level", ie. no netlink details, you only have to
use the libnetfilter_conntrack API. Sorry, your patch violates layering
since you're doing some in "the wrong way(tm)", your want to mix low and
high level.

> BTW, after using the new libnetfilter-conntrack I get this:
> 
>  ulogd_inpflow_NFCT.c:455: warning: passing argument 1 of 'nfnl_send'
>   discards qualifiers from pointer target type
> ulogd_inpflow_NFCT.c: In function 'read_cb_nfct':
>  ulogd_inpflow_NFCT.c:1043: warning: passing argument 1 of
> 'nfnl_recv_msgs' discards qualifiers from pointer target type
>  ulogd_inpflow_NFCT.c: In function 'nfct_start':
> ulogd_inpflow_NFCT.c:1156: warning: passing argument 1 of
>  'nfnl_rcvbufsiz' discards qualifiers from pointer target type
> ulogd_inpflow_NFCT.c: In function 'read_cb_nfct':
>  ulogd_inpflow_NFCT.c:1043: warning: passing argument 1 of
> 'nfnl_recv_msgs' discards qualifiers from pointer target type
> ...
> 
> Do you accept patches for that or should I just cast-away the wrong
> const there?

I'm going to apply a patch to constify nfnl_send. The const in
nfct_nfnlh() is fine, it's there to warn people about improper use of
the API, ie. those that want to violate encapsulation.

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

Re: [patch 1/1] Add subsystem accessors

[Pablo Neira Ayuso]

Pablo Neira Ayuso wrote:
> Holger Eitzenberger wrote:
>> Pablo Neira Ayuso wrote:
>>
>>>> Need by newer ulogd v2.
>>> Why do you need this? If I apply this, I'll have to provide support for
>>> people mixing calls to libnfnetlink and libnetfilter_conntrack which can
>>> be a problematic. The subsys handles are encapsulated for that reason.
>>>
>>> If you really need them, I suggest you to use libnfnetlink together with
>>> the low level functions libnetfilter_conntrack functions:
>>> nfct_[build|parse]_conntrack and nfct_build_query.
>> Which would nullify the use of libnetfilter-conntrack with ulogd.
> 
> No way. You're abusing the library APIs. The proper way for what you
> want to do is the following:
> 
> 1) open a nfnl handle
> 2) open a nfnl_subsys_handle
> 3) register the callback
> 4) use nfct_parse_conntrack inside the callback to get the conntrack
> object while you can still access netlink details (such as the sequence
> number).
> 5) use nfct_build_query plus nfnl_query if you want to send request to
> ctnetlink.

I have found an example of this for you in old conntrack-tools release
such as 0.9.3. See netlink.c file. I needed something similar at that
time. Recent releases and current SVN only use libnetfilter_conntrack.

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