-
Notifications
You must be signed in to change notification settings - Fork 337
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
[#3415][#3417]multi-homing: manage coma separated list of incoming interfaces #300
base: develop
Are you sure you want to change the base?
Conversation
59d7811
to
e821612
Compare
The following test was already broken prior to this PR: When I run locally |
allow to configure up to 100 characters in gtk outgoing interfaces control instead of 15, which was too small for IP v6 addresses
e821612
to
48040b1
Compare
It not very clean but since its at least related id like to mention doadin@633e312 It would be nice(since libtorrent supports it) to be able to use interface names instead of just ip's. Would also be nice to populate a dropdown menu in the gtkui(and webui?) of interfaces available. I think this can be done with netifaces.interfaces() then converting that list object to a menu. is ip should probabbly just be replaced with an all in one is interface? |
With this PR you can configure ipv4, ipv6, linux interface names, and probably Windows GUID interface identifiers. |
sorry late night replies but a few things, yes i agree menu could definitely be a different PR. Now could be just my 2 cents but I also just noticed you removed the is_ip, I think this is a step backwards instead of forward. While I think something like netifaces would provide better validation(and therefore a direction we should move toward) as it checks for real interfaces and not just if it looks good, I think having validation in general is good. I think maybe something like this could be helpful don't know if its even proper python just throwing out an idea with something like my updated is_ip, then:
I hope this comment/thought is clear.
|
I agree, the code can be rewritten to encapsulate the exception handling in is_ip, it would be cleaner. Regarding validating the ip addresses and interface names, it would be better done via libtorrent feedback, which is not done at the moment. Currently, one has to use command line netstat to make sure libtorrent listens to the desired addresses. So ideally for a clean code we would need:
|
I agree there is only so much that can be done on our side validation wise. I think currently our position is just check if a ip provided is a valid format which doesn't include names/guid . Which this PR it takes it a step back and just relies on the user more. Which imo is not a great plan. but i think with a move to netifaces we still don't get feedback from libtorrent to know if the interface given is usable but we also eliminate, user error, eg. where a user enters 2607::01::1 where they meant 2607::01:1 and then spends a while trying to find the issue (more validation is helpful, only human, mistakes can be made). This is just one example but in any case again just my 2 cents but more validation vs less is the way forward. for reference https://stackoverflow.com/questions/29913516/how-to-get-meaningful-network-interface-names-instead-of-guids-with-netifaces-un has examples to help with UI portion discussed here. IMO Obviously first we would need to decide how to proceed with the main topic of this PR. but since the main point deals with validation instead of re-writing this section of code to add/remove validation 10 more times in potential subsequent PRs/commits. I think it would be helpful to decide on a single path forward and proceed that way. I think end goal would be to have what qbittorrent has which is a list of interface names and then a sub list of ips associated with that interface. I think though one step at a time is fine and an interface list would do. I think particularly with the ui user needing to know the ip then manually enter it is not very user friendly. Its also a situation that all this in mind doesn't include ui support for multi-homing which will be a whole other situation. I think
|
Let's keep things simple. We won't get anywhere if we don't do incremental steps. I'll make PR1 compatible with your PR doadin@633e312, i.e. I'll restore the is_ip function. |
While I think we are on the same page it would nice to get some feedback from @cas-- as hes the one with power around here :) . Id hate for all your work to be unaccepted based on suggestions I made. |
@Strangelovian @doadin any progress on this? |
Sorry, no news from maintainers, this project is kind of abandonned |
I included these patches in my NixOS configuration and I can report dual-stack works again when the correct incoming addresses are specified. However GeoIP for IPv6 does not work: Every time I try to set the path to the GeoIP IPv6 database (in my thin client) and click 'Apply', it resets back to the default path. |
Sorry, "impossible" to test for me with thin client mode. I tested two modes: standalone gtk+ deluge, and daemon deluge + web UI. Both were Okay. I guess it never works without testing first all possible configurations... |
I tested in standlone gtk+ mode now as well, the same thing happens there: recording.mp4 |
Just to comment on that: While work has been very slow, I had Cas troubleshoot an issue with me just three months ago. I assume the "illness" they talked about on their Patreon is still a thing. |
Any update on this? I think a working IPv6 setup is important and urgent. |
In lieu of waiting for the merge of this awesome work, it is currently possible to configure Deluge to listen entirely on IPv6. While the normal UI won't allow {
// ...
"listen_interface": "[::]",
// ...
} Unfortunately this does not apply to the UI server, it still listens on IPv4 only. Looking into the sources, the
Unfortunately, there doesn't seem to be a way to set So, setting I'm having some subsequent issues getting |
Would love a resurrection of this, can I just change the files for myself and get dualstack working? |
It did and it works beautifully! |
…ace names as listen_interface Cf deluge-torrent@540d557 it patched ui/ console & gkt3, but not web. Inspired by deluge-torrent#300 (Translation should be ok: this string already exists) Also, resync listen & outgoing widths: they hold the same data types.
…Address" in web UI Deluge & libtorrent actually accept both IP address and device/interface names as listen_interface: deluge-torrent@540d557 it patched ui/ console & gkt3, but not web, this fixes it. (Inspired by deluge-torrent#300) (Translation should be ok: this string already exists) Also, resync listen & outgoing widths: they hold the same data types.
…Address" in web UI Deluge & libtorrent actually accept both IP address and device/interface names as listen_interface: deluge-torrent@540d557 it patched ui/ console & gkt3, but not web, this fixes it. (Inspired by deluge-torrent#300) (Translation should be ok: this string already exists) Also, resync listen & outgoing widths: they hold the same data types.
…Address" in web UI Deluge & libtorrent actually accept both IP address and device/interface names as listen_interface: deluge-torrent@540d557 it patched ui/ console & gkt3, but not web, this fixes it. (Inspired by deluge-torrent#300) (Translation should be ok: this string already exists) Also, resync listen & outgoing widths: they hold the same data types.
Deluge & libtorrent actually accept both IP address and device/interface namesgq as listen_interface: 540d557 which patched ui/ console & gkt3, but not web, this fixes it. (Inspired by #300) (Translation should be ok: this string already exists) Also, resync listen & outgoing widths: they hold the same data types. Closes: #458
Does 8df36c4 being merged close this issue? |
No, because it only removed the check for IPv4 address on |
Deluge & libtorrent actually accept both IP address and device/interface namesgq as listen_interface: 540d557 which patched ui/ console & gkt3, but not web, this fixes it. (Inspired by deluge-torrent#300) (Translation should be ok: this string already exists) Also, resync listen & outgoing widths: they hold the same data types. Closes: deluge-torrent#458
I developed this change to address some multi-homing needs I have with deluge.
The GTK and Web UI will now allow a list of IP addresses or network interface names, instead of a single IP address.
This list, appended with ports, will be passed to libtorrent.
I tested on debian sid. With port 23456, The following input works as expected:
lo,10.3.14.15, 2000:abcd:1234:5678::15
Then deluge(d) binds only:
One should note that lib torrent expects ipv6 addresses to be passed between square brackets.
But on the python side, brackets are not valid according to
ipaddress:IPV6Address
constructor.Therefore I added some extra IPV6 logic to add the surrounding brackets, prior to passing the listen_interfaces to lib torrent.
From the Deluge UI, the end user should provide IPV6 addresses without square brackets.
1st commit: fix binding for multiple incoming/outgoing interfaces
2nd commit: fix geoip for ipv6 (don't make sense without the first commit since ipv6 won't work)