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

First discussion #1

Closed
technicalpyro opened this issue Feb 3, 2017 · 84 comments
Closed

First discussion #1

technicalpyro opened this issue Feb 3, 2017 · 84 comments

Comments

@technicalpyro
Copy link
Contributor

based on the pihole script installing under the user "pihole" would it fit into the code that at the end of what im guessing will be the eventual bash/whiptail install script to have it run the following commands

sudo touch /etc/pihole/FTL.log
sudo touch /etc/pihole/FTL.pid
sudo touch /etc/pihole/FTL.lport
sudo chown pihole:pihole /etc/pihole/FTL.*

i would guess that is the eventual plan but dont want to assume

so far despite not running with a web interface as it is not agreeing at the moment every operation i have run has been successful and damn fast

@dschaper
Copy link
Member

dschaper commented Feb 4, 2017

Yeah, the permissions and location of the files are most likely going to be updated as the engine takes shape. It's in preview release now for functionality, the details for other items will be tuned as we progress along. Web interface interaction is something that is a bit off, but the CLI directions should cover most of what you can do for now.

There is a log file and a script to generate a massive log file to see how performant the code is when accessing data that would crush an existing install, and the results are indeed fast!

Thanks for the information and comments and please post any other observations or ideas that you have and we'll handle them as we can.

@technicalpyro
Copy link
Contributor Author

My current install is blocking just over 1.24 million domains and a command like >getallqueries which using the current web interface i really can't run due to the ajax error runs in seconds or less

couple features I would love to see:

an ability to view more than just the last blocked address ie >recentBlocked-x where x is the number of lines you want to view

the same could be said for getallquieries-x kind of an idea the time ability s nice but if you dont understand the timecode the database is using its difficult

knowing what the codes mean in the getallqueries results the numbers at the end of the lines would be nice as well or show it as a formatted table but that is definitely just a feature not a requirement

i understand right now this is alpha with me being guinea pig extraordinaire and everything that should be working is working
just me thinking out loud to be honest.

you guys rock and im happy to be allowed to play i really am :)

@DL6ER
Copy link
Member

DL6ER commented Feb 4, 2017

I like the idea >recentBlocked-x and >getallquieries-x and will add them.

in the getallqueries results the numbers at the end of the lines would be nice as well

I don't understand that?

The web interface implementation is finished but kept back for a while since we have to sort out how we do the licensing on that as it will integrate into the already published AdminLTE repo code

@technicalpyro
Copy link
Contributor Author

technicalpyro commented Feb 4, 2017

in the getallqueries results the numbers at the end of the lines would be nice as well
I don't understand that?

1486179259 IPv4 api-v2launch.trakt.tv 172.16.1.107 2

the number two at the end of the line ( in this case) sometimes its a 1 or a 3 as well i just dont understand what it refers to

@PromoFaux
Copy link
Member

just dont understand what it refers to

I have't actually read the FTL code properly yet, but I'm going to take a guess at the number being the pi-holed/OK status. I'm going to guess... 1=OK, 2=Pi-holed(exact), 3=Pi-holed(willdcard)

image

@technicalpyro
Copy link
Contributor Author

That would make sense i had looked through but got lost in the code a few times

@DL6ER
Copy link
Member

DL6ER commented Feb 4, 2017

The statuses are assigned here:

  • 0 = unknown, that should not happen (and has not happened to me so far), but could potentially happen - these lines will be written to the FTL log for possible subsequent investigation
  • 1 = blocked by gravity.list (i.e. exact blacklisting)
  • 2 = forwarded to upstream server
  • 3 = answered by local cache
  • 4 = wildcard blocking

@DL6ER
Copy link
Member

DL6ER commented Feb 4, 2017

or show it as a formatted table but that is definitely just a feature not a requirement

That is a difficult one since I don't know how long the domains and client host names can get, i.e. two unknown widths. If there would have been only one unknown width (like the domain name), I could have moved that to the end and could it have stay out of a preformatted table but I don't know how to do it as it is right now.

@technicalpyro
Copy link
Contributor Author

technicalpyro commented Feb 4, 2017

yeah all the research I've tried to do tells me the limit is 255 bytes but that doesn't necessarily translate to x amount of characters

perhaps attempt local host names limit of 25 characters and see where it goes or just ignore it because it was a want not a need

@DL6ER
Copy link
Member

DL6ER commented Feb 4, 2017

yeah all the research ive tried to do tells e its 255 bytes but that doesnt necessarily translate to x amount of characters

Well, up to 255 characters

perhaps attempt local hostnames limit of 25 characters and see where it goes or just ignore it because it was a want not a need

I know about at least one user that has host names which are much longer than that because they reflect quite some hierarchy in the network, like:

device.room.building.field.company.tld

that can easily become > 25 characters

@technicalpyro
Copy link
Contributor Author

Yeah for sure. i forgot about those who implement on the larger basis. My bad.

@technicalpyro
Copy link
Contributor Author

technicalpyro commented Feb 6, 2017

installed the latest commits
change of filename structure to include pihole-FTL caused some issues all good now

@dschaper
Copy link
Member

dschaper commented Feb 6, 2017

We've got to keep you on your toes! :)

@technicalpyro
Copy link
Contributor Author

technicalpyro commented Feb 6, 2017

yeah and i learn from it too :)
Edit: tested >recentBlocked 10 works great

@DL6ER
Copy link
Member

DL6ER commented Feb 6, 2017

Oh, I wrote some text and somehow that seems to have gotten lost.

Yeah, you already discovered two changes:

  • Filestructure (will change again to some more Linux-typical places like `/var/log/, etc. soon)
  • >recentBlocked XY
  • There is also >getallqueries (15). The ( ) have the nice effect that I was able to extend the scope to all already available filters, e.g.
    • >getallqueries (2) -> show only the recent two queries
    • >getallqueries-client 127.0.0.1 (10) show only the queries from localhost within the recent 10 queries (might return nothing)
    • >getallqueries-domain www.google.com (50)
    • etc.

Maybe I should put in the ( ) syntax everywhere for consistency?

@DL6ER DL6ER changed the title not an issue just not sure where else to put this First discussion Feb 6, 2017
@DL6ER
Copy link
Member

DL6ER commented Feb 6, 2017

The file locations have been changed to

/var/log/pihole-FTL.log
/var/run/pihole-FTL.pid
/var/run/pihole-FTL.port

You can use make && sudo make install to have the files created with proper permissions.

@technicalpyro
Copy link
Contributor Author

Done this morning before work. will be back in approx three hours to reap the benefits. definitely like being able to control very easily the number of results returned very clean code:)

@DL6ER
Copy link
Member

DL6ER commented Feb 6, 2017

Yeah, but I'm not entirely satisfied by the new code, since e.g. >getallqueries-client 127.0.0.1 (10) will show only the queries from localhost within the recent 10 queries (might return nothing) instead of (what I'd expect to see) 10 queries from localhost.

The problem is that we print out the latest query as the end of the results, i.e. I don't know where to start printing.

@technicalpyro
Copy link
Contributor Author

oh OK so it doent know to filter the last ten result containing that client ... you guys are t==he coding masters but will think about it while doing my shift today maybe i can think of something inelegant you guys can turn to gold

@technicalpyro
Copy link
Contributor Author

technicalpyro commented Feb 6, 2017

might have to be a silent command kind of idea and i dont know how to code this but the workflow for me would look something like

  • query database filtering for "domain" or "host" depending on what is specified
  • print results to a temporary file
  • query temporary file for number of results requested
  • Print result to window
  • delete temporary file

@dschaper
Copy link
Member

dschaper commented Feb 7, 2017

Sounds like a good use case for a hashtable. (Coding a hash in C may not be trivial, I don't know that much yet...)

@DL6ER
Copy link
Member

DL6ER commented Feb 7, 2017

To be honest, I'm not sure if that is worth the effort. I'd have to do all the fancy filtering once, extract only what I need and loop over everything a second time, printing only what I need. This will not only slow down FTL but also (considerable) increase the memory needed for the request routines and make the code more complicated.

That is because I don't collect the data somewhere and print it only afterwards, but immediately push every matching data set out to the requester. This is one of the reasons why FTL is so fast.

@technicalpyro
Copy link
Contributor Author

Will download and install release and report back when I get home ... Got called in for a stupidly long shift today

@technicalpyro
Copy link
Contributor Author

release seem to install fine although despite running >kill to terminate before update when i went to start the service it echoed already running .. didnt realize it was started by the make && make install
not sure if it was implemented but >getallqueries 10 shows all not just the ten i thought it was limited to

@DL6ER
Copy link
Member

DL6ER commented Feb 8, 2017

didnt realize it was started by the make && make install

That does not happen. Either you had another instance of pihole-FTL running somewhere or we have a bug. Have you used sudo make install?

not sure if it was implemented but >getallqueries 10 shows all not just the ten i thought it was limited to

Use >getallqueries (10)

@technicalpyro
Copy link
Contributor Author

ok did a clean install following same steps i did last night and i must not have killed the process like i thought everything is working well and installed as expected

also using the () with getallqueries worked perfectly

@technicalpyro
Copy link
Contributor Author

technicalpyro commented Feb 12, 2017

god im slow some days got it sorted see other issue for info for some reason not letting me log in via CLI but got the right info where it needs to be

@technicalpyro
Copy link
Contributor Author

thinking i might put osmc on a new card and see if we run into the same issues we are seeing on the master thoughts?
or is there a different OS i should be testing ?

@DL6ER
Copy link
Member

DL6ER commented Feb 12, 2017

Oh, I might have missed that - which issue?

@technicalpyro
Copy link
Contributor Author

ps -aux has dnsmasq at 43% of memory. also probably due to something i missed when setting up the vps i dont show temp or FTL status in the dashboard and lastly and i will be submitting a bug report on this on the main repo the package dialog if it is missing breaks the install script the os provided by my host didnt include that so might be a good idea to include it in required packages that are loaded or looked for before installation

@DL6ER
Copy link
Member

DL6ER commented Feb 26, 2017

ps -aux has dnsmasq at 43% of memory

Okay, we can attribute that to the insane number of domains

dont show temp or FTL status

Temperature is not displayed if it cannot be detected - they does not work on most VMs (virtual machines have nothing like a "physical" temperature). Remember that there is actually no FTL status display except if FTL is offline.

@technicalpyro
Copy link
Contributor Author

thats what i figured

@DL6ER
Copy link
Member

DL6ER commented Mar 3, 2017

FTL is now merged to Pi-hole development. Closing this very long issue now. Thanks for the very nice discussion. Feel free to open another discussion at any point!

@DL6ER DL6ER closed this as completed Mar 3, 2017
@Felisse Felisse mentioned this issue Jun 21, 2017
3 tasks
@tschuchort tschuchort mentioned this issue Feb 12, 2019
3 tasks
@kunago kunago mentioned this issue Jul 1, 2019
3 tasks
@TC1977 TC1977 mentioned this issue Sep 28, 2019
3 tasks
DL6ER pushed a commit that referenced this issue Oct 14, 2019
@jens1205 jens1205 mentioned this issue Dec 21, 2020
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

No branches or pull requests

4 participants