sysprog21 / vwifi

A virtual wireless device driver for Linux
MIT License
205 stars 39 forks source link

Implement userspace tool to display driver status and specify blocklist #53

Closed vax-r closed 10 months ago

vax-r commented 11 months ago

Summary

Enable ability to use blacklist to block packets from specified interface.

Support blacklist in vwifi kernel module, used as interfaces pair such as "owl2 blocks owl1", allow maximum blacklist size to be 1024 bytes now and maintained as global content within struct owl_content.

Using userspace program to dynamically alter the blacklist maintaining in vwifi kernel module.

Testing

step 1

After mounting vwifi as kernel module in kernel, make sure the ping test works as normal as indicated in README.md.

step 2

compile blacklist_user.c and execute it

$ gcc blacklist_user.c -o blacklist_user.o
$ ./blacklist_user.o

After this step, we should have the blacklist loaded into vwifi kernel module.

step 3

Do the ping test again, it should block packets from the interface pair you specefied in ./scripts/blacklist.txt. You can make the content of ./scripts/blacklist.txt empty and execute $ ./blacklist_user.o again, blacklist pairs should be removed then.

I think blacklist ability checking can be added as a tesing stage into verify.sh, I would love to help if needed. Btw I found some type error in verify.sh, and correct "sucess" to "success".

Issue linked

Fix #48

vax-r commented 11 months ago
  1. Use C-style comments. That is, /* comment */.
  2. Change Blacklist To Blocklist.
  3. Minimize the necessary changes. That is, send another pull request(s) for fixing typo.
  4. Avoid the statement such as strlen(owl->blacklist) == 0). Use !*(owl->blacklist) instead.

I've made some changes according to the suggestion above, please take a look when you have time, Thank you!

vax-r commented 11 months ago

@jserv I've adjust the code according to the suggestion, and allow user-specific filename from command line. Please take a look when you are available. Thank you!

vax-r commented 11 months ago

Sorry I've made a mistake on last modification, the unit parameter in netlink_kernel_create shouldn't be MAX_LINKS, MAX_LINKS stands for maximum amount of netlink families , we should choose a netlink family for our netlink socket. In our case I think it's more appropriate to use NETLINK_USERSOCK since we're using netlink for communication between userspace and kernel space.

vax-r commented 10 months ago

@jserv I've made the requested changes and use getopt() to allow user to configure blocklist interface pair in command line options. -d stands for destination interface name -s stands for source interface name -c are set when user wants to unset the blocklist

Please take a look when you're available. Thank you!

vax-r commented 10 months ago

Also I made the maximum blocklist pair allowed in vwifi to be 5. I think it's better to set a maximum bound for blocklist in vwifi currently, maybe we can make it more flexible in future. Feel free to let me know if it's ok or we need further adjustment for it.

jserv commented 10 months ago

Please take a look when you're available. Thank you!

For future reference, avoid using vague phrases such as "Please take a look when you're available. Thank you!" in the context of code reviewing. It is important to focus specifically on the detailed changes and suggestions you have in mind. Being overly polite can detract from the clarity and effectiveness of your feedback. Instead, aim for clear, direct communication that emphasizes the specific aspects of the code that need attention.

vax-r commented 10 months ago

@jserv Request changes are made, including improvements of english writing and modularizing vwifi-tool.c.

jserv commented 10 months ago

@jserv Request changes are made, including improvements of english writing and modularizing vwifi-tool.c.

Instead of writing the sentence above (and mentioning me), you can request a code review through the GitHub web interface by selecting the appropriate option. Aim to keep the code review process streamlined and sophisticated, avoiding unnecessary or inefficient language.

vax-r commented 10 months ago

I just refined git commit message according to https://cbea.ms/git-commit/ I believe the following requirements are met.

jserv commented 10 months ago

I just refined git commit message according to https://cbea.ms/git-commit/ I believe the following requirements are met. [...]

  • [x] Wrap the body around 72 characters
  • [x] Use the body to explain what and why vs. how

No, you didn't. Check again.

vax-r commented 10 months ago

I just refined git commit message according to https://cbea.ms/git-commit/ I believe the following requirements are met. [...]

  • [x] Wrap the body around 72 characters
  • [x] Use the body to explain what and why vs. how

No, you didn't. Check again.

Ok, I've refined it again , each line is at most 72 characters and add more detailed explanation.

jserv commented 10 months ago

Thank @vax-r for contributing. The commit message was amended.