Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add firewall support to LKL #431

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pmembrey
Copy link

@pmembrey pmembrey commented Apr 23, 2018

Hi guys,

I know this is a (very) early PR, but I wanted to start off the conversation. I'm looking to test firewall rulesets and the like and LKL seems to be a really good way to do that - except it doesn't have firewall support.

The change I've made compiles in all of the netfilter / iptables modules. This is sufficient to allow iptables to show and list any rules. I wrote a hacky bit of code to inject a rule as part of the hijack lib, and was able to see it with iptables -L. A simple set of tests with ping also confirmed that the rules were working as expected. liblkl.so is about 6MB larger after this change.

Of course, this set up is not going to be very useful without being able to load a proper rule set. My current plan is to compile iptables as a library and modify hijack to load the provided ruleset as part of initialisation.

I'm keen to contribute back to the project and I'm completely happy to change or implement this differently if that would be preferred.

As before, any guidance would be gratefully received :)


This change is Reviewable

@thehajime
Copy link
Member

Since the addition makes bigger liblkl (and potentially slow down something), I don't think it's fine to merge to lkl.

I wrote a hacky bit of code to inject a rule as part of the hijack lib,

I think if the patchset includes some utility functions (maybe generalized one of your hacky code) in tools/lkl/lib/net.c to configure iptable rules, it would be nicer to accept this patch. The following PR might be an example of those addition.

#386

@tavip
Copy link
Member

tavip commented Jun 12, 2018

Agree with @thehajime , maybe we can have a different defconfig for this? See #440

@lkl-jenkins
Copy link

Can one of the admins verify this patch?

@yu-re-ka yu-re-ka mentioned this pull request Sep 21, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants