all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [RFC PATCH proxmox-backup] traffic-control: use SocketAddr from 'accept()'
Date: Fri, 28 Jan 2022 15:10:30 +0100	[thread overview]
Message-ID: <20220128141030.1373160-1-d.csapak@proxmox.com> (raw)

instead of getting the 'peer_addr()' from the socket.
The advantage is that we must get this and thus can drop the mapping
from result -> option, and can drop the testing for None and a test case

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
did stumble upon this while debugging. looks cleaner to me and
i see no obvious disadvantage

 src/bin/proxmox-backup-proxy.rs |  5 ++---
 src/cached_traffic_control.rs   | 22 ++++++----------------
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 30b730ef..8d0033de 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -391,7 +391,7 @@ async fn accept_connection(
     let accept_counter = Arc::new(());
 
     loop {
-        let (sock, _addr) = match listener.accept().await {
+        let (sock, peer) = match listener.accept().await {
             Ok(conn) => conn,
             Err(err) =>  {
                 eprintln!("error accepting tcp connection: {}", err);
@@ -402,7 +402,6 @@ async fn accept_connection(
         sock.set_nodelay(true).unwrap();
         let _ = set_tcp_keepalive(sock.as_raw_fd(), PROXMOX_BACKUP_TCP_KEEPALIVE_TIME);
 
-        let peer = sock.peer_addr().ok();
         let sock = RateLimitedStream::with_limiter_update_cb(sock, move || lookup_rate_limiter(peer));
 
         let ssl = { // limit acceptor_guard scope
@@ -1144,7 +1143,7 @@ async fn run_traffic_control_updater() {
 }
 
 fn lookup_rate_limiter(
-    peer: Option<std::net::SocketAddr>,
+    peer: std::net::SocketAddr,
 ) -> (Option<Arc<dyn ShareableRateLimit>>, Option<Arc<dyn ShareableRateLimit>>) {
     let mut cache = TRAFFIC_CONTROL_CACHE.lock().unwrap();
 
diff --git a/src/cached_traffic_control.rs b/src/cached_traffic_control.rs
index ba552215..2f077d36 100644
--- a/src/cached_traffic_control.rs
+++ b/src/cached_traffic_control.rs
@@ -305,15 +305,10 @@ impl TrafficControlCache {
 
     pub fn lookup_rate_limiter(
         &self,
-        peer: Option<SocketAddr>,
+        peer: SocketAddr,
         now: i64,
     ) -> (&str, Option<Arc<dyn ShareableRateLimit>>, Option<Arc<dyn ShareableRateLimit>>) {
 
-        let peer = match peer {
-            None => return ("", None, None),
-            Some(peer) => peer,
-        };
-
         let peer_ip = cannonical_ip(peer.ip());
 
         log::debug!("lookup_rate_limiter: {:?}", peer_ip);
@@ -427,32 +422,27 @@ rule: somewhere
         let private = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(192, 168, 2, 35)), 1234);
         let somewhere = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4)), 1234);
 
-        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(None, THURSDAY_80_00);
-        assert_eq!(rule, "");
-        assert!(read_limiter.is_none());
-        assert!(write_limiter.is_none());
-
-        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(Some(somewhere), THURSDAY_80_00);
+        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(somewhere, THURSDAY_80_00);
         assert_eq!(rule, "somewhere");
         assert!(read_limiter.is_some());
         assert!(write_limiter.is_some());
 
-         let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(Some(local), THURSDAY_19_00);
+         let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(local, THURSDAY_19_00);
         assert_eq!(rule, "rule2");
         assert!(read_limiter.is_some());
         assert!(write_limiter.is_some());
 
-        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(Some(gateway), THURSDAY_15_00);
+        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(gateway, THURSDAY_15_00);
         assert_eq!(rule, "rule1");
         assert!(read_limiter.is_some());
         assert!(write_limiter.is_some());
 
-        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(Some(gateway), THURSDAY_19_00);
+        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(gateway, THURSDAY_19_00);
         assert_eq!(rule, "somewhere");
         assert!(read_limiter.is_some());
         assert!(write_limiter.is_some());
 
-        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(Some(private), THURSDAY_19_00);
+        let (rule, read_limiter, write_limiter) = cache.lookup_rate_limiter(private, THURSDAY_19_00);
         assert_eq!(rule, "rule2");
         assert!(read_limiter.is_some());
         assert!(write_limiter.is_some());
-- 
2.30.2





             reply	other threads:[~2022-01-28 14:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-28 14:10 Dominik Csapak [this message]
2022-01-31  9:02 ` [pbs-devel] applied: " Dietmar Maurer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220128141030.1373160-1-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal