Skip to content

Add option to exclude IPs #341

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.

Sign up for GitHub

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

Open
ilyes-ced wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add option to exclude IPs #341

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
af43365
added ability to exlude ip_v4 and ip_v4:port
ilyes-ced Dec 4, 2023
148fae0
fixed clippy issues
ilyes-ced Dec 5, 2023
2aa6de4
made ip exclutions better
ilyes-ced Dec 7, 2023
f4cad33
formating
ilyes-ced Dec 7, 2023
798c662
Merge branch 'main' into add-excluded-ips
ilyes-ced Dec 8, 2023
b37571a
fix fmt
ilyes-ced Dec 8, 2023
3ba1f9c
added changelog
ilyes-ced Dec 8, 2023
b2dde6f
added hostname filtering
ilyes-ced Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::{net::Ipv4Addr, path::PathBuf};
use std::{
net::{Ipv4Addr, SocketAddrV4},
path::PathBuf,
};

use clap::{Args, Parser};
use clap_verbosity_flag::{InfoLevel, Verbosity};
Expand Down Expand Up @@ -38,6 +41,16 @@ pub struct Opt {
#[derivative(Default(value = "Verbosity::new(0, 0)"))]
pub verbosity: Verbosity<InfoLevel>,

#[arg(short, long)]
/// exclude ip addres with <-e x.x.x.x>
/// exclude multiple ip addresses with <-e x.x.x.x -e y.y.y.y>
pub excluded_ipv4: Option<Vec<Ipv4Addr>>,

#[arg(short = 'E', long)]
/// exclude ip addres with <-e x.x.x.x:zzzz>
/// exclude multiple ip addresses and port with <-e x.x.x.x:zzzz -e y.y.y.y:zzzz>
pub excluded_ipv4_port: Option<Vec<SocketAddrV4>>,

Copy link
Collaborator

@cyqsimon cyqsimon Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty rigid and non-extensible. For example, it's pretty reasonable for a user to want to exclude by hostname. Or by CIDR. Do we need an option for each? And then double that if we want to support IPv6 (which we definitely do; it's not 2003).

Much better would be to declare a struct/enum HostFilter (or something like that) and implement FromStr and Display for it so that it can be used as a clap option parameter. Then all these different kinds of possible filters can be united under a single option.

#[command(flatten)]
pub render_opts: RenderOpts,
}
Expand Down
8 changes: 7 additions & 1 deletion src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,13 @@ where
move || {
while running.load(Ordering::Acquire) {
let render_start_time = Instant::now();
let utilization = { network_utilization.lock().unwrap().clone_and_reset() };
let mut utilization = { network_utilization.lock().unwrap().clone_and_reset() };
if let Some(ref ex) = opts.excluded_ipv4 {
utilization.remove_ip(ex)
}
if let Some(ref ex) = opts.excluded_ipv4_port {
utilization.remove_ip_port(ex)
}
Copy link
Collaborator

@cyqsimon cyqsimon Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to do this here. IMO there are two ways to achieve this logic and this is neither:

  1. Apply the filtering in the sniffer threads, so that the filtered traffic is never accounted.
  • This is actually not my preferred option. Sniffers are not "hostname-aware", and therefore cannot apply hostname filters if we want to support those.
  1. Apply the filtering while rendering. But then this logic belongs in UIState::update instead.

let OpenSockets { sockets_to_procs } = get_open_sockets();
let mut ip_to_host = IpTable::new();
if let Some(dns_client) = dns_client.as_mut() {
Expand Down
37 changes: 36 additions & 1 deletion src/network/utilization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::collections::HashMap;
use std::{
collections::HashMap,
net::{Ipv4Addr, SocketAddrV4},
};

use crate::network::{Connection, Direction, Segment};

Expand Down Expand Up @@ -42,4 +45,36 @@ impl Utilization {
}
}
}
pub fn remove_ip(&mut self, ips: &[Ipv4Addr]) {
// might be possible to refactor this part better
// i still don't understand the whole borrow/own system very well yet
let placeholder = self.connections.clone();
Copy link
Collaborator

@cyqsimon cyqsimon Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see why you did this. Probably got yelled at by the borrow checker (cannot borrow as mutable because it is also borrowed as immutable)? If so, this is because you tried to remove from the HashMap (there's your mutable borrow) while holding a reference to one of its items (there's your immutable borrow).

The API that will allow you to do this without cloning is HashMap::entry.

Copy link
Collaborator

@cyqsimon cyqsimon Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this being said, if you are doing this filtering in UIState::update (as previously mentioned), you likely won't need any of this. On a high level, this is because you are already copying data (from memory to the screen), so all you need is to tell it what to copy (i.e. a filter). Then there's no mutability issues involved at all.

Personally I think reducing mutability as much as possible is the best practise regardless of language.

for util in placeholder {
match util.0.remote_socket.ip {
std::net::IpAddr::V4(ip) => {
if ips.contains(&ip) {
self.connections.remove_entry(&util.0);
}
}
std::net::IpAddr::V6(..) => { /* nothing here yet (maybe implement it for ipV6 too) */
}
}
}
}
pub fn remove_ip_port(&mut self, ips: &[SocketAddrV4]) {
// might be possible to refactor this part better
// i still don't understand the whole borrow/own system very well yet
let placeholder = self.connections.clone();
for util in placeholder {
match util.0.remote_socket.ip {
std::net::IpAddr::V4(ip) => {
if ips.contains(&SocketAddrV4::new(ip, util.0.remote_socket.port)) {
self.connections.remove_entry(&util.0);
}
}
std::net::IpAddr::V6(..) => { /* nothing here yet (maybe implement it for ipV6 too) */
}
}
}
}
}