All of lore.kernel.org
 help / color / mirror / Atom feed
* libnl netfilter log
@ 2007-12-08  5:20 Patrick McHardy
  2007-12-08 18:16 ` Pablo Neira Ayuso
  2007-12-10  0:53 ` Philip Craig
  0 siblings, 2 replies; 6+ messages in thread
From: Patrick McHardy @ 2007-12-08  5:20 UTC (permalink / raw)
  To: Philip Craig, Thomas Graf; +Cc: Netfilter Development Mailinglist

I've added support for netfilter queueing to libnl based on the
logging support and would like to suggest a few changes.

One of the problems with the netfilter libraries has been adding
support for new features, since that often required changing
function signatures, for example:

extern int nfq_set_verdict(struct nfq_q_handle *qh,
                              u_int32_t id,
                              u_int32_t verdict,
                              u_int32_t data_len,
                              unsigned char *buf);

extern int nfq_set_verdict_mark(struct nfq_q_handle *qh,
                                   u_int32_t id,
                                   u_int32_t verdict, 

                                   u_int32_t mark,
                                   u_int32_t datalen,
                                   unsigned char *buf);

libnl always took an object-centric approach, so this would
look something like this:

nfnl_queue_msg_set_verdict(pkt, verdict);
nfnl_queue_msg_set_mark(pkt, mark);
nfnl_queue_msg_send_verdict(pkt);

which avoids this problem. The libnl logging support deviates
from this scheme, for example with this function:

int nfnl_log_set_mode(struct nl_handle *nlh, uint16_t queuenum,
                       uint8_t copy_mode, uint32_t copy_range)

What I would like to change is make struct nfnl_log the logging
instance, so you would have:

nfnl_log_set_mode(log, mode);
nfnl_log_set_copy_range(log, range);
nfnl_log_send_config(log);

and add a new struct nfnl_log_message, which represents the
actual messages. Similar for queueing, we would have a
struct nfnl_queue and nfnl_queue_message.

Any objections or suggestions to this API change?

Somewhat related, there currently is a problem with message
reception since libnl uses a page-sized buffer for recvmsg(),
but netfilter can send messages up to 64k. I recall there
was some recvmsg buffer size probing some time ago, but it
seems to be gone. Should I reintroduce it, increase the size
to 64k or make it configurable?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: libnl netfilter log
  2007-12-08  5:20 libnl netfilter log Patrick McHardy
@ 2007-12-08 18:16 ` Pablo Neira Ayuso
  2007-12-08 18:24   ` Pablo Neira Ayuso
  2007-12-09 16:09   ` Patrick McHardy
  2007-12-10  0:53 ` Philip Craig
  1 sibling, 2 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2007-12-08 18:16 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Philip Craig, Thomas Graf, Netfilter Development Mailinglist

Patrick McHardy wrote:
> I've added support for netfilter queueing to libnl based on the
> logging support and would like to suggest a few changes.
> 
> One of the problems with the netfilter libraries has been adding
> support for new features, since that often required changing
> function signatures, for example:
> 
> extern int nfq_set_verdict(struct nfq_q_handle *qh,
>                              u_int32_t id,
>                              u_int32_t verdict,
>                              u_int32_t data_len,
>                              unsigned char *buf);
> 
> extern int nfq_set_verdict_mark(struct nfq_q_handle *qh,
>                                   u_int32_t id,
>                                   u_int32_t verdict,
>                                   u_int32_t mark,
>                                   u_int32_t datalen,
>                                   unsigned char *buf);
> 
> libnl always took an object-centric approach, so this would
> look something like this:
> 
> nfnl_queue_msg_set_verdict(pkt, verdict);
> nfnl_queue_msg_set_mark(pkt, mark);
> nfnl_queue_msg_send_verdict(pkt);
> 
> which avoids this problem. The libnl logging support deviates
> from this scheme, for example with this function:
> 
> int nfnl_log_set_mode(struct nl_handle *nlh, uint16_t queuenum,
>                       uint8_t copy_mode, uint32_t copy_range)
> 
> What I would like to change is make struct nfnl_log the logging
> instance, so you would have:
> 
> nfnl_log_set_mode(log, mode);
> nfnl_log_set_copy_range(log, range);
> nfnl_log_send_config(log);
> 
> and add a new struct nfnl_log_message, which represents the
> actual messages. Similar for queueing, we would have a
> struct nfnl_queue and nfnl_queue_message.
> 
> Any objections or suggestions to this API change?

Looks fine to me. Actually, much better than the current API :)

> Somewhat related, there currently is a problem with message
> reception since libnl uses a page-sized buffer for recvmsg(),
> but netfilter can send messages up to 64k. I recall there
> was some recvmsg buffer size probing some time ago, but it
> seems to be gone. Should I reintroduce it, increase the size
> to 64k or make it configurable?

If you reintroduce the probing I think that something similar to the
libnfnetlink behaviour would be fine. But, indeed, I prefer to make it
configurable.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: libnl netfilter log
  2007-12-08 18:16 ` Pablo Neira Ayuso
@ 2007-12-08 18:24   ` Pablo Neira Ayuso
  2007-12-09 16:08     ` Patrick McHardy
  2007-12-09 16:09   ` Patrick McHardy
  1 sibling, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2007-12-08 18:24 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Philip Craig, Thomas Graf, Netfilter Development Mailinglist

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> I've added support for netfilter queueing to libnl based on the
>> logging support and would like to suggest a few changes.
>>
>> One of the problems with the netfilter libraries has been adding
>> support for new features, since that often required changing
>> function signatures, for example:
>>
>> extern int nfq_set_verdict(struct nfq_q_handle *qh,
>>                              u_int32_t id,
>>                              u_int32_t verdict,
>>                              u_int32_t data_len,
>>                              unsigned char *buf);
>>
>> extern int nfq_set_verdict_mark(struct nfq_q_handle *qh,
>>                                   u_int32_t id,
>>                                   u_int32_t verdict,
>>                                   u_int32_t mark,
>>                                   u_int32_t datalen,
>>                                   unsigned char *buf);
>>
>> libnl always took an object-centric approach, so this would
>> look something like this:
>>
>> nfnl_queue_msg_set_verdict(pkt, verdict);
>> nfnl_queue_msg_set_mark(pkt, mark);
>> nfnl_queue_msg_send_verdict(pkt);
>>
>> which avoids this problem. The libnl logging support deviates
>> from this scheme, for example with this function:
>>
>> int nfnl_log_set_mode(struct nl_handle *nlh, uint16_t queuenum,
>>                       uint8_t copy_mode, uint32_t copy_range)
>>
>> What I would like to change is make struct nfnl_log the logging
>> instance, so you would have:
>>
>> nfnl_log_set_mode(log, mode);
>> nfnl_log_set_copy_range(log, range);
>> nfnl_log_send_config(log);
>>
>> and add a new struct nfnl_log_message, which represents the
>> actual messages. Similar for queueing, we would have a
>> struct nfnl_queue and nfnl_queue_message.
>>
>> Any objections or suggestions to this API change?
> 
> Looks fine to me. Actually, much better than the current API :)

Hm, wait. Did libnl adopt an API similar to libnetfilter_log? Uh bad :)
Then, instead of changing the API that is something that could break any
existing application, I think that you can introduce the new API that
you propose and put the old one with the __attribute__((deprecated)) thing.

BTW, is libnl website broken? Any CVS or something I can get a working
copy to have a look at current netfilter support?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: libnl netfilter log
  2007-12-08 18:24   ` Pablo Neira Ayuso
@ 2007-12-09 16:08     ` Patrick McHardy
  0 siblings, 0 replies; 6+ messages in thread
From: Patrick McHardy @ 2007-12-09 16:08 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Philip Craig, Thomas Graf, Netfilter Development Mailinglist

Pablo Neira Ayuso wrote:
> Hm, wait. Did libnl adopt an API similar to libnetfilter_log? Uh bad :)
> Then, instead of changing the API that is something that could break any
> existing application, I think that you can introduce the new API that
> you propose and put the old one with the __attribute__((deprecated)) thing.

Its quite new, I guess there are no applications yet.

> BTW, is libnl website broken? Any CVS or something I can get a working
> copy to have a look at current netfilter support?

Its hosted on http://git.kernel.org


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: libnl netfilter log
  2007-12-08 18:16 ` Pablo Neira Ayuso
  2007-12-08 18:24   ` Pablo Neira Ayuso
@ 2007-12-09 16:09   ` Patrick McHardy
  1 sibling, 0 replies; 6+ messages in thread
From: Patrick McHardy @ 2007-12-09 16:09 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Philip Craig, Thomas Graf, Netfilter Development Mailinglist

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:

>> Somewhat related, there currently is a problem with message
>> reception since libnl uses a page-sized buffer for recvmsg(),
>> but netfilter can send messages up to 64k. I recall there
>> was some recvmsg buffer size probing some time ago, but it
>> seems to be gone. Should I reintroduce it, increase the size
>> to 64k or make it configurable?
> 
> If you reintroduce the probing I think that something similar to the
> libnfnetlink behaviour would be fine. But, indeed, I prefer to make it
> configurable.


Me too, and maybe have it switch to 64k automatically once a
nfnetlink queue or log socket is opened.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: libnl netfilter log
  2007-12-08  5:20 libnl netfilter log Patrick McHardy
  2007-12-08 18:16 ` Pablo Neira Ayuso
@ 2007-12-10  0:53 ` Philip Craig
  1 sibling, 0 replies; 6+ messages in thread
From: Philip Craig @ 2007-12-10  0:53 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Thomas Graf, Netfilter Development Mailinglist

Patrick McHardy wrote:
> I've added support for netfilter queueing to libnl based on the
> logging support and would like to suggest a few changes.
> 
> One of the problems with the netfilter libraries has been adding
> support for new features, since that often required changing
> function signatures, for example:
> 
> extern int nfq_set_verdict(struct nfq_q_handle *qh,
>                               u_int32_t id,
>                               u_int32_t verdict,
>                               u_int32_t data_len,
>                               unsigned char *buf);
> 
> extern int nfq_set_verdict_mark(struct nfq_q_handle *qh,
>                                    u_int32_t id,
>                                    u_int32_t verdict, 
> 
>                                    u_int32_t mark,
>                                    u_int32_t datalen,
>                                    unsigned char *buf);
> 
> libnl always took an object-centric approach, so this would
> look something like this:
> 
> nfnl_queue_msg_set_verdict(pkt, verdict);
> nfnl_queue_msg_set_mark(pkt, mark);
> nfnl_queue_msg_send_verdict(pkt);
> 
> which avoids this problem. The libnl logging support deviates
> from this scheme, for example with this function:
> 
> int nfnl_log_set_mode(struct nl_handle *nlh, uint16_t queuenum,
>                        uint8_t copy_mode, uint32_t copy_range)
> 
> What I would like to change is make struct nfnl_log the logging
> instance, so you would have:
> 
> nfnl_log_set_mode(log, mode);
> nfnl_log_set_copy_range(log, range);
> nfnl_log_send_config(log);
> 
> and add a new struct nfnl_log_message, which represents the
> actual messages. Similar for queueing, we would have a
> struct nfnl_queue and nfnl_queue_message.
> 
> Any objections or suggestions to this API change?

That makes a lot of sense to me. And don't worry about breaking the
API for this or other netfilter stuff.  I doubt anyone besides me has
used it. There's no publicly available programs besides the test ones
in libnl.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-12-10  0:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-08  5:20 libnl netfilter log Patrick McHardy
2007-12-08 18:16 ` Pablo Neira Ayuso
2007-12-08 18:24   ` Pablo Neira Ayuso
2007-12-09 16:08     ` Patrick McHardy
2007-12-09 16:09   ` Patrick McHardy
2007-12-10  0:53 ` Philip Craig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.