Setting of the verdict

[vorner]

The current API has some drawbacks around the setting of verdict:

• It is possible to set the verdict multiple times on the same message.
• On the other hand, it is not possible to delay setting the verdict outside of the callback. This is well supported by the C library ‒ they even go so far to promise it's OK to verdict packets out of order and inside another thread.

Is it possible to pass owned (not borrowed) message to the callback and consume it during verdicting? And maybe go so far as to panic (or at least warn) if the message is destroyed without getting a verdict?

If the problem is the buffer of data, it could be passed to the callback as two parameters ‒ one Message, that'd contain the metadata and the ability to verdict, and borrowed slice of bytes as the payload.

[sharksforarms]

Hey @vorner and @chifflier, take a look at these changes:

sharksforarms/nfqueue-rs@improvements...sharksforarms:issue-#10

Let me know what you think!

[vorner]

I know it's mostly what I suggested that 1.5 years ago, but I wonder if it was the best idea. The downsides I see:

• I might want to store the message for a while around and set the verdict later. I don't need to keep the content of packet around for that. I guess it's the same problem currently too, so it doesn't make it worse, but maybe it could be taken in one go with the changes? Maybe a way to clear the data off the message and leave it „hollow“?
• The panicking is a nice reminder during normal application run (messages should not be just forgotten, because they are then stuck in the kernel side of nfqueue until the application disconnects), but I wonder if this may make shutting down without panics harder ‒ when I'm tearing everything down, I may want to just throw the messages out without verdicting them and kernel will drop once I terminate. This way the shutdown will explode.

Though I'd be glad to see the changes in either way 😇, these are just some points where it could not be the best solution (it's at least some solution).

[sharksforarms]

1. Yeah that's my use case. Did you try any of the other copy modes?

/// Copy modes
pub enum CopyMode {
    /// Do not copy packet contents nor metadata
    CopyNone,
    /// Copy only packet metadata, not payload
    CopyMeta,
    /// Copy packet metadata and not payload
    CopyPacket,
}

2. Not sure I like having it panic, better to have documentation around it and leave it up to the user (flexibility)

Thanks for the quick response @vorner !

[sharksforarms]

Interesting to note, I tried not setting a verdict and got this on stdout error in nfq_handle_packet(), so I guess there's some sort of warning. This function was returning -1 indicating a failure

[vorner]

Did you try any of the other copy modes?

Well, the use case I have in mind is to first inspect the packet (including the content) and decide that:

• I may let it through.
• I want to drop it/mark it for rejection in iptables.
• A need to see some more packets before I can decide on this one. I've already mined all the info I ever will be able to from this one, so there's little use storing all its data any more, but I'll still want to give it the verdict eventually.