[PATCH]: drop packet without verdict from nfqueue after timeout

[PATCH]: drop packet without verdict from nfqueue after timeout

[Kuzin Andrey]

This is patch for problem with stucked packets in nf_queue if
something going wrong in userspace program. Automatically drop packets
without any verdict after timeout defined by NFQNL_TIMEOUT_ENTRY_DROP.

Who may create patch for menu config for this feature ?


diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 8c86011..74fc322 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -169,17 +169,29 @@ __enqueue_entry(struct nfqnl_instance *queue, struct nf_queue_entry *entry)
        queue->queue_total++;
 }

+#define NFQNL_TIMEOUT_ENTRY_DROP 30
+
 static struct nf_queue_entry *
 find_dequeue_entry(struct nfqnl_instance *queue, unsigned int id)
 {
-       struct nf_queue_entry *entry = NULL, *i;
+       struct nf_queue_entry *entry = NULL, *next, *i;
+       ktime_t kt = ktime_get_real();

        spin_lock_bh(&queue->lock);

-       list_for_each_entry(i, &queue->queue_list, list) {
+       list_for_each_entry_safe(i, next, &queue->queue_list, list) {
                if (i->id == id) {
                        entry = i;
                        break;
+                } else {
+                       struct timeval tv = ktime_to_timeval(ktime_sub(kt, i->skb->tstamp));
+                       if (tv.tv_sec > NFQNL_TIMEOUT_ENTRY_DROP) {
+                               printk(KERN_ERR "nf_queue: drop timeouted packet "
+                                       "(queue_num=%u seq_id=%u)\n", queue->queue_num, i->id);
+                               list_del(&i->list);
+                               queue->queue_total--;
+                               nf_reinject(i, NF_DROP);
+                       }
                }
        }

Re: [PATCH]: drop packet without verdict from nfqueue after timeout

[Patrick McHardy]

Kuzin Andrey wrote:
> This is patch for problem with stucked packets in nf_queue if
> something going wrong in userspace program. Automatically drop packets
> without any verdict after timeout defined by NFQNL_TIMEOUT_ENTRY_DROP.

I don't want to add per-packet timeouts. The number one problem cause
I've seen in userspace programs so far has been "missed" packets by
incorrect application logic/error handling. These applications usually
continue to send verdicts, they just miss some packets, which accumulate
in the queue until it is full.

There's a very easy and cheap way to handle this. The packets have
sequence numbers and userspace should issues verdicts in ascending
order anyways to avoid reordering. Just add something that will drop
everything in the queue up to the sequence number contained in the
netlink message. And if you want to make it seem like something that
isn't just meant to work around buggy application behaviour, you can
use the same mechanism to add verdict batching :)

Re: [PATCH]: drop packet without verdict from nfqueue after timeout

[Eric Leblond]

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

Hi,

Le lundi 23 mars 2009 à 20:18 +0100, Patrick McHardy a écrit :
> Kuzin Andrey wrote:
> > This is patch for problem with stucked packets in nf_queue if
> > something going wrong in userspace program. Automatically drop packets
> > without any verdict after timeout defined by NFQNL_TIMEOUT_ENTRY_DROP.
> 
> There's a very easy and cheap way to handle this. The packets have
> sequence numbers and userspace should issues verdicts in ascending
> order anyways to avoid reordering. Just add something that will drop
> everything in the queue up to the sequence number contained in the
> netlink message.

I don't think the described mechanism is generic enough to be a default
behaviour. It should be useful for projects like snort-inline but it
will really a problem for software like NuFW which are asynchronous by
design.

In NuFW, packet authentication is triggered by a user message (signing
of packet is done is userspace). Thus the ordering of the answer depends
of the ordering of user messages. As NuFW authenticate packet at network
scale (there is thus plenty of users), it is not possible to assume that
the answer will be ordered.

Thus, even if it could be useful, this mechanism should only be
activated by an explicit userspace query. 

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: [PATCH]: drop packet without verdict from nfqueue after timeout

[Patrick McHardy]

Eric Leblond wrote:
>> There's a very easy and cheap way to handle this. The packets have
>> sequence numbers and userspace should issues verdicts in ascending
>> order anyways to avoid reordering. Just add something that will drop
>> everything in the queue up to the sequence number contained in the
>> netlink message.
>>     
>
> I don't think the described mechanism is generic enough to be a default
> behaviour. It should be useful for projects like snort-inline but it
> will really a problem for software like NuFW which are asynchronous by
> design.
>
> In NuFW, packet authentication is triggered by a user message (signing
> of packet is done is userspace). Thus the ordering of the answer depends
> of the ordering of user messages. As NuFW authenticate packet at network
> scale (there is thus plenty of users), it is not possible to assume that
> the answer will be ordered.
>
> Thus, even if it could be useful, this mechanism should only be
> activated by an explicit userspace query. 

Good point. The in-sequence handling is also only necessary per flow,
so this definitely would need to be enabled explicitly.

Re[2]: [PATCH]: drop packet without verdict from nfqueue after timeout

[Kuzin Andrey]

EL> I don't think the described mechanism is generic enough to be a default
EL> behaviour. It should be useful for projects like snort-inline but it
EL> will really a problem for software like NuFW which are asynchronous by
EL> design.
EL> In NuFW, packet authentication is triggered by a user message (signing
EL> of packet is done is userspace). Thus the ordering of the answer depends
EL> of the ordering of user messages. As NuFW authenticate packet at network
EL> scale (there is thus plenty of users), it is not possible to assume that
EL> the answer will be ordered.
EL> Thus, even if it could be useful, this mechanism should only be
EL> activated by an explicit userspace query. 

Indeed. I use nfqueue for traffic accounting on network gateway. And
as i describe in previous letters after _several tens of millions_ packets
every time i have one or more such packets without verdict.
I can't find any errors in userspace, and i think that Patrick way
may be don't work for catching problem place, earlier i try to use
nfqnl_test example program (easier can't be imagine) for verdict
sending, and some packets don't get verdicts. May be errors take place
in kernel on high load bandwidths due to some SMP/RCU bugs, skbuf
or hardware drivers bugs (forcedeth for example is not so perfect
driver because write by reverse engineering way).
So this patch for me can automatically erase any delays on gateway
due to trash queue fills. I think this feature need to be realize as
menu config options (for people who really need this).

Re[2]: [PATCH]: drop packet without verdict from nfqueue after timeout

[Kuzin Andrey]

>> Thus, even if it could be useful, this mechanism should only be
>> activated by an explicit userspace query. 

PM> Good point. The in-sequence handling is also only necessary per flow,
PM> so this definitely would need to be enabled explicitly.

I fully agree with you, but do this changes it not so easy for me as for you.
I'am newbie in kernel programming. And i think would be better if this
changes will be made by library/kernel maintainer with his experience, point
of view, programming style and other cool stuff.