patchmldonkey, a multi-networks file-sharing client - Patches: patch #4776, EDK: Avoid LowID if...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #4776: EDK: Avoid LowID if max_indirect_connections is reached

Submitter:  spiralvoice <spiralvoice>
Submitted:  Sun 08 Jan 2006 07:06:31 PM UTC
   
 
Category:  Bug fixes Priority:  7 - High
Status:  Done Assigned to:  None
Open/Closed:  Closed

Jump to the original submission

Thu 12 Jan 2006 09:03:33 AM UTC, comment #13: 

Applied 2006/01/12

spiralvoice <spiralvoice>
Group administrator
Wed 11 Jan 2006 08:42:17 PM UTC, comment #12: 

Updated patch, small logging changes
z: thanks for your help, patch looks good now IMHO

spiralvoice <spiralvoice>
Group administrator
Tue 10 Jan 2006 07:05:40 PM UTC, comment #11: 

If you say the connecting state doesn't matter, why does your patch still check this and slowly Hashtbl.find when you have already focused on the possible IPs by having a connecting_servers list?

Please check the attached patch to see if it does what you want; it is a modified version.

Also, I notice that your patches have tabs in them.  Most of the source doesn't use tabs, so converting tabs to spaces in your chosen editor or using diff -t can help to retain consistent formatting/indentation.

z <zet>
Group Member
Tue 10 Jan 2006 04:24:33 PM UTC, comment #10: 

Improved patch to get rid of the deletion of old connecting server list entries every minute

spiralvoice <spiralvoice>
Group administrator
Tue 10 Jan 2006 03:33:22 AM UTC, comment #9: 

updated patch

spiralvoice <spiralvoice>
Group administrator
Tue 10 Jan 2006 02:54:10 AM UTC, comment #8: 

"If servers are always in the "Connecting" state when performing
callbacks"

This is not the case, connecting an EDK server puts the state to
Connecting, the server immediatly closes the socket to perform the
callback. Because of the closed socket the server state is NotConnected when the callback arrives. Normally this happens so
quickly that no GUI can display it, logging revealed that behaviour.

add_connecting_server works like designed, I am adding a flag
for servers when last connect was performed. If this was more
than 1 minute ago that entry will be deleted from !connecting_server_list because remove_connecting_server can not
be called when the server socket is closed for whatever reason.

spiralvoice <spiralvoice>
Group administrator
Mon 09 Jan 2006 11:31:31 PM UTC, comment #7: 

commment #6 is fixed in patch #4781

spiralvoice <spiralvoice>
Group administrator
Mon 09 Jan 2006 09:21:51 PM UTC, comment #6: 

remove_server should also call disconnect_server, I had this
problem already with Sancho GUI, you have to call "remove" twice
for a connected server until its really removed.

spiralvoice <spiralvoice>
Group administrator
Mon 09 Jan 2006 04:12:42 AM UTC, comment #5: 

Also, the ip would need to be removed (if it were in the list) during remove_server since that doesn't seem to go through disconnect_server (set_server_state) AFAICT.

z <zet>
Group Member
Mon 09 Jan 2006 03:16:11 AM UTC, comment #4: 

s/f t (a,b,c,d)/matches_3 t (a,b,c,d)/


z <zet>
Group Member
Mon 09 Jan 2006 02:24:18 AM UTC, comment #3: 

If servers are always in the "Connecting" state when performing
callbacks you can add to/remove from a new
"connecting_server_ips" list in "set_server_state". 
I believe that checking this small list would be quick.

ie:

let connecting_servers = ref []

let set_server_state s state =
...
  if state = Connecting then (AddIpToList) else (removeIpFromList)
...

let rec matches_3 l (a,b,c,d) =
    match l with
      [] -> false
      | (x,y,z,_) :: _ when x=a && y=b && c=z -> true
      | _ :: t -> f t (a,b,c,d)

if matches_3 !connecting_server_ips ip ...

Or whatever you think is best.

z <zet>
Group Member
Sun 08 Jan 2006 11:17:23 PM UTC, comment #2: 

Implemented most of your suggestions, only the Razorback 2.0
problem with its hard-coded IP address remains.
I think Hashtbl.iter servers_by_key will be too expensive for
checking the first three octets.

spiralvoice <spiralvoice>
Group administrator
Sun 08 Jan 2006 08:38:49 PM UTC, comment #1: 

You are re-calculating what I would call
'real_max_indirect_connections' each and
every time you use it, based on a percent
of max_connections. 

(!!max_opened_connections * !!max_indirect_connections / 100);

Why not calculate it just once and use the
result directly?

This value can only change when max_connections
and max_indirect_connections change, so perhaps
adding a function "calc_real_max" on those two
options as hooks would be better than recalculating
the same thing all the time.

let calc_real_max () =
 !real_max := !!max_opened_connections * !!max_indirect_connections / 100;

You also re-check if an IP is blocked a bunch of
times.  An ip is blocked or it isn't and rechecking
against the blocked list is not cheap.
let is_ip_blocked = Ip_set.ip_blocked from_ip in
... and then reusing "is_ip_blocked" seems better.

Hardcoding IP addresses is always a bad idea,
especially when it is embedded like this...

Also, you are accepting anyone in the server address
list to gain an indirect connection at any time.
If I run Bob's TinyServer and get my server address
into all mldonkey server lists, I can get indirect
connections at all times avoiding the connection limit. 
Wouldn't it be better to accept a server address (and
perhaps matching only the first three octets of the
quad address like 195.245.244.* to get around your
razorback problem) only if the server is in the
'Connecting' state?  (I assume it is in this state
while trying to establish a connection)

z <zet>
Group Member
Sun 08 Jan 2006 07:06:31 PM UTC, original submission:  

If max_indirect_connections was reached connecting EDK servers
was not possible because the server callback was not accepted
as it is regarded as a indirect connections. This led to LowID warnings.

minimum max_indirect_connections is now 20
maximum max_indirect_connections is now 50

spiralvoice <spiralvoice>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #9440:  4776.patch added by spiralvoice (13KiB - application/octet-stream)
file #9433:  max_ind.diff added by zet (14KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

CC list is empty

 

There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

 

Follow 15 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2006-01-12 spiralvoice StatusReady For Test Done
    Open/ClosedOpen Closed
2006-01-11 spiralvoice Attached File#5742 Removed
2006-01-11 spiralvoice Attached File- Added 4776.patch, #5755
2006-01-10 zet Attached File- Added max_ind.diff, #5745
2006-01-10 zet Attached File#5744 Removed
2006-01-10 zet Attached File- Added max_ind.diff, #5744
2006-01-10 spiralvoice Attached File#5741 Removed
2006-01-10 spiralvoice Attached File- Added 4776.patch, #5742
2006-01-10 spiralvoice Attached File#5728 Removed
2006-01-10 spiralvoice Attached File#5731 Removed
2006-01-10 spiralvoice Attached File- Added 4776.patch, #5741
2006-01-08 spiralvoice Attached File- Added 4776.patch, #5731
2006-01-08 spiralvoice StatusNone Ready For Test
2006-01-08 spiralvoice Attached File- Added max_max_indir_conns.patch, #5728

Back to the top

Powered by Savane 3.13-4b48.
Corresponding source code