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

New IPv4 filter implementation that isn't a memory hog -- fixes rtorrent issue #106 and #71 #109

Closed
wants to merge 1 commit into from

Conversation

sallyswiss
Copy link

New extents.h data structure in libtorrent so that it does not waste memory. it is now an ordered map instead of a crazy sparse hash, much better, still log n access time. it also stores ranges as in p2p file format so it can handle p2p data from bluetack lists as well as cidr notation which is now only thing supported. There is corresponding commit and pull request for rtorrent that makes the necessary changes there too.

@jenkins101
Copy link

so with this added it is possible to use p2p format files?

with comments?

@sallyswiss
Copy link
Author

when parsing file line everything up to colon ':' is ignored. if you mean comments that start with '#' then handling is not ideal as parser will still try to parse line so '# abc def: 1.2.3.4.5-1.2.3.4.10' will still get added to list even though it is commented out. However, it will not crash on comments so if nothing that could be parsed as ip address is in comments they will not prevent program from running.

@jenkins101
Copy link

good!

@jenkins101
Copy link

mm...

seems like you have pulled from your master...
you should branch then add pull from branch instead...

latest commit is unrelated to this issue.
looks interesting though... :-)

@sallyswiss
Copy link
Author

ugh. sorry. I am familiar with git but new to github.

do you know what would be the easiest way to fix it now that pull request is sent and second commit is pushed to the same branch?

@jenkins101
Copy link

yes... did the same thing in the beginning...

you need to do a rebase and then a force push

something like this if you only did one commit i think...
git rebase -i HEAD~1. leave the first line the way it is, change the following line from 'pick' to 'f'. exit the editor and it'll squash them up. force push to your master branch.

but if you want to repull from a branch it is easier to close this delete your repo and refork...
save your work some were though!
create a branch from master add work and then do pull from branch

or you probably could rebase back to beginning...
aa... i think you will solve it... ;-)

@jenkins101
Copy link

the github guides are a little messy...

I have found this one really useful: https://code.djangoproject.com/wiki/CollaborateOnGithub

@jenkins101
Copy link

@sallyswiss
Copy link
Author

doing rebase would just combine commits though. not what I want.

instead [1] checkout last commit in rakshasa master [2] create new branch from there [3] cherry-pick latest commit into new branch [4] revert commit in master. problem solved.

@jenkins101
Copy link

no... you want to do rebase with the f option

f, fixup = like "squash", but discard this commit's log message
it will be removed from your master...
I did it and it worked for me.

then you can branch and add it to a branch but you will still have this commit thats not in rakshasas master yet...
so you probably want to close this and then refork or rebase back to beginning. then branch... and pull again

@jenkins101
Copy link

or if you can rebase in to a new branch... but i dont know hove to do that...

@jenkins101
Copy link

I tested it joust now
git rebase -i HEAD~3 leave first line change other 2 to f save and they are gone...

you probably had to do that revert first yes...

@sallyswiss
Copy link
Author

ok. all fixed now.

@jenkins101
Copy link

but you should do a rebase to fixup all the commit messages...
so its only one again...

else al commit messages will go into rtorrent...

@sallyswiss
Copy link
Author

yes. I just did a rebase. only one commit is left.

@jenkins101
Copy link

a sorry... had to reload the page... <:-)

@Jellyfrog
Copy link

Now the waiting being for the mighty @rakshasa

@DugieHowsa
Copy link

Tried the new version with the ipfilter corrections.

Loaded a 5.6 MB CIDR format list. Memory usage appears to be minimal:

1358359195 I Loaded 338529 unwanted address blocks (3967 kb in-memory) from '~/ipfilter.dat'.

Not sure if it is actually blocking though, as I loaded a torrent and let it run for a few minutes, but I didn't see any messages in the logs about connections being blocked.

I'll continue to run the service with logging over the next few days to see if anything pops up.

@rakshasa
Copy link
Owner

Thanks for the work.

I'll do a more thorough review later, however I'd like to note that C-style comments are not accepted in my codebase.

@sallyswiss
Copy link
Author

sorry for not matching preferred style. I will switch comments to C++ style.

…handle p2p lists from bluetack [3] still has quick log n lookup time for extents data structure.
@sallyswiss
Copy link
Author

all comments are now changed to C++ style in both rtorrent and libtorrent. I also made two minor improvements. I added code to handle comments "#" when parsing filter file lines and added logging when connection to unwanted peer is prevented. this addresses issue pointed out by DugieHowsa above. wihtout logging it is hard to know when filter is working. log level is set to LOG_PEER_INFO.

@rakshasa if you need more information when reviewing my code please ask and I will be happy to help. if you want any further changes beyond c++ comments just ask.

@jenkins101
Copy link

Is it not time to implement this and make a new release son...
ip blocking is not working in current rel without this...

I have been run this in my OpenELEC rTorrent service add-on for a while and it seams to work good.

@Jellyfrog
Copy link

Ping @rakshasa

@rakshasa
Copy link
Owner

Currently not in a position to handle this and other issues that have to be done before a release.

@garmahis
Copy link

garmahis commented Feb 9, 2014

Can you make it compatible with the standard ipfilter.dat files?

@gdlx
Copy link

gdlx commented Sep 5, 2014

@rakshasa I'm using @sallyswiss fork for more than 1 year now and it works like a charm with IP filters, without memory leak.

Now that you have bumped the new release, maybe could you merge it ?

@aruhier
Copy link

aruhier commented Aug 2, 2015

Any news about a possible merge ?

@rakshasa
Copy link
Owner

rakshasa commented Aug 3, 2016

I'll take a proper look later.

@gdlx
Copy link

gdlx commented Dec 10, 2017

Still no merge ? That's a 4 years PR and I'm stuck on the fork. Is the issue fixed on master ?

@chros73
Copy link
Contributor

chros73 commented Dec 10, 2017

@gauthier-delacroix : I'll rebase this on current master branches today, so you can try it out.

@chros73
Copy link
Contributor

chros73 commented Dec 10, 2017

Done, rebased on current master branch in: #671

  • you can try it out
  • or try to compile rtorrent-ps-ch 1.6.1-0.9.7 that is based on master branches and contains this one as well along with other goodies

Enjoy!

@chros73
Copy link
Contributor

chros73 commented Jun 5, 2018

This can be closed since the other one is merged: #671

@rakshasa rakshasa closed this Jun 5, 2018
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.

9 participants