NFLOG

[dzamlo]

This is a draft implementation of support for NFLOG.

I'm interested in a general feedback, before I finish.

• The example need more work
• Tests are missing
• There is still some todos

Unfortunately, I have limited access to my computer, so I may not respond quickly

[little-dude]

Hey @dzamlo thanks a lot for the PR. I didn't have much time today to review it in depth but overall I think it's a great start. I'll give it a more in-depth review next week-end. Feel free to finish it off in the meantime, I don't think I'm going to ask for big changes before merging this in.

[dzamlo]

I didn't have time to add tests and I'm not very happy with the example.

Unfortunately, I won't have time to do that until next week, at best.

[little-dude]

That's fine. It's more than enough for me to start reviewing :) I also have other PRs waiting for me, so no rush. Thanks a lot!

[dzamlo]

Hi @stbuehler , I just realised you worked on netfilter related stuff by reading comments in #170. Could you take a look at this pr and see if I should rebase my work on what you did, or the opposite or if the two should remain separate?

[stbuehler]

Hi @stbuehler , I just realised you worked on netfilter related stuff by reading comments in #170. Could you take a look at this pr and see if I should rebase my work on what you did, or the opposite or if the two should remain separate?

Hi, I don't think you did any major changes on the existing code, and #170 shouldn't change the API (I think) - so a simple merge should be fine in the end.

I don't think you considered the need for batch requests yet (netlink requires those for changes, at least for what I want to do), which is what #170 is all about - so you might want to rebase once #170 got merged if you want to test something like that.

(I used a very different way in my own code (not published anywhere yet :) ) to serialize/deserialize messages and their attributes (I went for proc_macro derive on structs where attributes represent field values), so I'm not sure I'd PR my own netfilter code to this repo - probably gonna keep it separate, so don't worry about that part.)

[dzamlo]

Hi @stbuehler , I just realised you worked on netfilter related stuff by reading comments in #170. Could you take a look at this pr and see if I should rebase my work on what you did, or the opposite or if the two should remain separate?

Hi, I don't think you did any major changes on the existing code, and #170 shouldn't change the API (I think) - so a simple merge should be fine in the end.

I don't think you considered the need for batch requests yet (netlink requires those for changes, at least for what I want to do), which is what #170 is all about - so you might want to rebase once #170 got merged if you want to test something like that.

(I used a very different way in my own code (not published anywhere yet :) ) to serialize/deserialize messages and their attributes (I went for proc_macro derive on structs where attributes represent field values), so I'm not sure I'd PR my own netfilter code to this repo - probably gonna keep it separate, so don't worry about that part.)

I was interested only in NFLOG and NFQUEUE which don't need the batch functionality, so #170 doesn't impact me. I was worried aboout the unpublished serialize/deserialize part. So everything is fine on this front.

[dzamlo]

The only things missing are automated tests. I don't know when I will have the bandwidth to write any. Not soon unfortunately.

[little-dude]

Tbh I'd be willing to get this in already @dzamlo. Better than letting all this work hanging. I'll need some time to review though, and I'll likely won't have time over Christmas. But let's try to get this in in January :)

[dzamlo]

As much as I like this PR to be be top notch, I have literraly zero bandwidth to finalize it (I have acess to my phone to write answer here, but not my laptop to code). But this PR is at least manually tested, so there is that.

After a review you may want to squash most commit, but It simpler for me and likely simpler to review like it is now.

[astro]

I've given this PR a try. The code works for me.

@dzamlo Are you open to financial or material support?

[dzamlo]

The code should work well. What's missing is automated tests and I was, at the time, not very happy with the example.

I wasn't able to work on that at the time of the PR and I forgot this PR after that. I don't know if I will have any time for this PR soon.

I cannot accept any financial/material help.

[little-dude]

@astro I'm planning to spend some time releasing this, hopefully tomorrow. As I said earlier, I think even without tests, having a basic nflog crate is a net win. I just haven't really had the time/motivation to do it so far :S

[little-dude]

I'm going to close this since I'll merge and release from #235. Thank you again @dzamlo !

[little-dude]

@dzamlo @astro 0.1.0 is available: https://crates.io/crates/netlink-packet-netfilter