Opened 16 years ago

Closed 13 years ago

Last modified 13 years ago

#7 closed Enhancement (fixed)

DHT support

Reported by: ddv Owned by: charles
Priority: Normal Milestone: 1.70
Component: libtransmission Version: Other
Severity: Normal Keywords: dht
Cc: jch@…

Description

DHT is a very important feature for a torrent client since it makes possibel to download from a busy tracker or if the tracker is down.

Some private trackers have banned clients with DHT, but that has in a large scale been clients which has not accepted a flag, which disappel DHT for the tracker. So if Transmission follow the rules, DHT should not be a problem.

Attachments (17)

dht-0.1.tar.gz (19.3 KB) - added by charles 13 years ago.
transmission-dht.png (53.8 KB) - added by charles 13 years ago.
transmission-dht.README (1.5 KB) - added by charles 13 years ago.
transmission-dht-20090612.patch (14.6 KB) - added by charles 13 years ago.
transmission-dht-20090617.patch (27.8 KB) - added by charles 13 years ago.
dht-0.4.tar.gz (20.8 KB) - added by charles 13 years ago.
dht-0.4.tar.gz.asc (197 bytes) - added by charles 13 years ago.
transmission-dht-20090618.patch (29.7 KB) - added by charles 13 years ago.
Transmission-Mac-20090519.diff (13.8 KB) - added by tiennou 13 years ago.
DHT Mac OS X GUI modifications
prefer-dht.patch (1.4 KB) - added by jch 13 years ago.
provide-node-count.patch (3.4 KB) - added by jch 13 years ago.
transmission-dht-2009.06.19.01.diff (119.9 KB) - added by charles 13 years ago.
updated diff
add-node-from-peer.patch (609 bytes) - added by jch 13 years ago.
transmission-dht-20090619.patch (1.8 KB) - added by jch 13 years ago.
dht-memmem.patch (831 bytes) - added by jch 13 years ago.
dht-warnings.patch (612 bytes) - added by jch 13 years ago.
incorrect-port-test.patch (570 bytes) - added by jch 13 years ago.

Download all attachments as: .zip

Change History (118)

comment:1 Changed 16 years ago by jah

i seconds this ticket

comment:2 Changed 16 years ago by ddv

I think one question that needs to be answered, is which type of DHT to support. Azureus uses one protocol, the mainline client uses another. Azureus uses its DHT as a tracker backup or supplement, while mainline uses it as tracker replacement.

The official client is Mainline, but the Azureus DHT connect to many more clients. Doing both would probably be a waste of time, and I do not know which "protocol" would be most efficient and simple to implement

comment:3 Changed 15 years ago by anonymous

implementations of DHT in C: KadC - GPL GNUnet DHT - GPL

comment:5 Changed 15 years ago by sarge

libtorrent just added dht support; c++; bsd

comment:6 Changed 14 years ago by John Clay

  • Component changed from Transmission to libtransmission
  • Owner somebody deleted
  • Severity changed from Major to Normal
  • Version changed from 0.5 to Other

comment:7 Changed 14 years ago by tiennou

I've taken a quick look at the proposed stuff above :

  • KadC :

Right now it don't supports Bittorrent DHT (supports Overnet, and eMule (almost)), so I guess this would require hacking KadC (which looks like create a better separation between the 'real' DHT Kademlia stuff, and the stuff around it) to add BT's DHT, or recreate a C Kademlia implementation from scratch...

  • GNUnet :

I guess their DHT stuff is too much interwoven into it to be of use (tried building it, and got numerous 'missing header foo' errors, most notably about core GNUnet stuff). It seems to be cleaner than KadC though (but most of the stuff in KadC looks like helper stuff)...

  • For the others, I don't know what the stance about C++ stuff in our code, so I didn't care to take a look.

comment:8 Changed 14 years ago by tiennou

  • Keywords dht added

Right now, I'm in the process of cleaning KadC source code, so we can have a Kademlia-agnostic implementation, and plug it to a Bittorrent DHT protocol implementation ;-)

comment:9 Changed 14 years ago by dak180

This may be useful: DHT Protocol

comment:10 Changed 14 years ago by indolering

(spam removed)

Last edited 11 years ago by charles (previous) (diff)

comment:11 Changed 14 years ago by the-me

as a non-developing, transmission-loving user I strongly agree. DHT is a feature which is highly convenient and concerns the Transmissions fundamental purpose: file distribtion. And it is a must-have for modern bittorrent clients in my opinion.

that this is ticket no. 7 speaks for itself, I guess :-)

Please, guys, you have a terrific product. Make it even more terrific by keeping Transmission on the front line of usefulness. It may not be that other products support more ways of distribution. Much in contrast to more configuration options - these are not so important.

comment:12 Changed 13 years ago by sell

(spam removed)

Last edited 11 years ago by charles (previous) (diff)

comment:14 Changed 13 years ago by add

(spam removed)

Last edited 11 years ago by charles (previous) (diff)

comment:15 Changed 13 years ago by storney

As we know that the developers work on Transmission for free, in their spare time, but I would postulate that clean C implementation of the mainline DHT is the single most important thing for Bittorrent and filesharing in general, from a practical or political POV. As such it deserves higher priority and severity ratings.

How long will the main trackers remain? When OiNK died the hydra spouted a few more heads rather quickly but that doesn't mean that the full power has been restored. According to TorrentFreak? the Pirate Bay was supporting 50% of ALL torrents BEFORE Demonoid was shut down,shopWholesaleTrainers SneakerswholesalerSellSale and Demonoid tracked roughly 25% of the torrents out there. The DHT is a way to float until a new hydra sprouts, until new dissidents can find their voice.

The developers of Transmission, and all FOSS developers for that matter, have my endless thanks for their work. I don't think that this one piddily comment will change the mind of people who have a LOT more invested in this than I do. But I strongly believe in the importance of at least recognizing how important a clean C implementation of the mainline DHT is but for Bittorrent and Free Speech in general. One way to do that is by raising the priority to High and the severity to Major, as the DHT is highly important to keeping the hydra alive, given the major severity of the threats to it ; p

Version 0, edited 13 years ago by storney (next)

comment:16 Changed 13 years ago by molok

Could someone clean up the spam in this ticket? Also, I wonder if developers want to implement DHT or they think it is not an important feature.

comment:17 Changed 13 years ago by charles

Judging from the comments in this reddit thread, there are still users out there who would like to have DHT implemented.

molok: if someone contributes a good patch, it will get used. I think the developers' consensus is that the effort-to-benefit ratio is pretty high on this, which is why we haven't done it ourselves.

comment:18 Changed 13 years ago by jch

  • Cc jch@… added

Changed 13 years ago by charles

Changed 13 years ago by charles

Changed 13 years ago by charles

Changed 13 years ago by charles

comment:19 Changed 13 years ago by charles

jch: this is great news! Thanks for starting work on this.

Is there anything you need to move this patch along further?

comment:20 Changed 13 years ago by tiennou

Hi there ! Just to point out I'm seeing duplication of efforts there ;-).

I'm working on DHT support too (http://github.com/tiennou/libdht2/tree/master). I'm using code originally written by the libevent developer, and I'm currently trying to cleanup its ability to be decoupled from the actual protocol type (this means libdht could provide support for both mainline DHT and AZ, provided we have spec for it). When that's ready, I'll have to write a benc-parser/writer using libT bencode stuff, and fix bugs in algorithms ;-).

comment:21 Changed 13 years ago by jch

Is there anything you need to move this patch along further?

Yep. I need help from somebody familiar with the libtransmission internals.

The code I've written is cleanly separated into client-independent DHT implementation (dht.c and dht.h) and the interface with libtransmission, which is in trdht.c.

I intend to maintain the former, which I'm also using in a project of mine. On the other hand, I'm neither competent nor willing to maintain the interface with libtransmission, which needs some work from somebody who understands libtransmission.

In particular, somebody needs to work out whether they want to use a specific thread for the DHT, as I have done, or whether they prefer to hook into libtransmission's event loop. Furthermore, they'll need to work out when to send announces to the DHT (every 28 minutes or whenever the user presses the announce now button in some UI widget). Finally, they'll need to make sure they don't announce private torrents.

Once that's done, I'll be glad to take care of the few remaining protocol issues.

--Juliusz

comment:22 follow-up: Changed 13 years ago by jch

I'm working on DHT support too (http://github.com/tiennou/libdht2/tree/master).

Cool.

this means libdht could provide support for both mainline DHT and AZ

My personal opinion is that AZ should be deprecated, and Azureus/Vuze? should switch to the mainline protocol. The cost of Kademlia is logarithmic in the number of nodes, so participating in two DHTs is twice as expensive as participating in a single DHT that is twice as large.

comment:23 in reply to: ↑ 22 Changed 13 years ago by tiennou

Replying to jch:

I'm working on DHT support too (http://github.com/tiennou/libdht2/tree/master).

Cool.

this means libdht could provide support for both mainline DHT and AZ

My personal opinion is that AZ should be deprecated, and Azureus/Vuze? should switch to the mainline protocol. The cost of Kademlia is logarithmic in the number of nodes, so participating in two DHTs is twice as expensive as participating in a single DHT that is twice as large.

Agreed. I want to know which one of the two implementation we're more likely to use. I'll take a look at yours this week, I hope we can manage a merge or something like that (if you're inclined to do so, that is). I started learning about how DHT works because I found it an useful tool for decentralized storage of... stuff ;-).

I'm also willing to handle the libtransmission interface, I have some experience with it, and I already thought about it, since I'm writing a DHT too ;-).

comment:24 Changed 13 years ago by charles

  • Owner set to charles
  • Status changed from new to assigned

I haven't looked at the new implementation yet, but these are my general feelings on what I'd accept into svn:

  1. Working code is better than non-working code. jch's patch seems to be a lot further along, at least in terms of actually running...
  1. I'm only interested in mainline DHT.
  1. It would be very nice for the code to use libevent s.t. it can operate within the libtransmission/libevent.
  1. It would be great for the two of you to work this out so that all I have to do is commit the final patch. ;)

comment:25 follow-up: Changed 13 years ago by jch

I hope we can manage a merge or something like that

I hope so too.

Please take into account that while I have a good understanding of network protocols in general and a good understanding of the maths behind Kademlia, I know nothing whatsoever about transmission, and almost nothing about GUI design.

In other words: I am interested in writing the most efficient implementation of DHT on the market. I am not interested in working on libtransmission or transmission itself.

comment:26 in reply to: ↑ 25 Changed 13 years ago by tiennou

Replying to charles:

  1. libdht works, its only issue is that it doesn't use a bencoded protocol, so it's basically in it's own DHT. My current goal is trying to graft a filtering API on top of its IO, so I can convert from its protocol structs to bencoded dicts and vice-versa.
  1. The integration with libevent is likely to make a mess out of dht-0.1, but it has the advantage of not caring about IO filters (the things I'm banging my head against in libdht), so it's much easier to get right (I could even copy-paste the code in libdht to dht-0.1 to fix this. (Because libevent doesn't have a high-level UDP API, I can't use bufferevent in libdht because it uses send/recv() or readv/writev(), which don't like unconnected sockets).
  1. I guess we're already on it ;-).

Replying to jch:

I hope we can manage a merge or something like that

I hope so too.

Please take into account that while I have a good understanding of network protocols in general and a good understanding of the maths behind Kademlia, I know nothing whatsoever about transmission, and almost nothing about GUI design.

In other words: I am interested in writing the most efficient implementation of DHT on the market. I am not interested in working on libtransmission or transmission itself.

Understood. Rest assured, I'll handle the wrapper stuff, provided there's sufficient API to :

  • query a key in the DHT, and get back results (a callback is preferred).
  • announce a key in the DHT.
  • store and restore known nodes at bootstrap.

I have a few questions/comparison on your code (charles, first point is for you too) :

  • What about IPv6 ? libdht uses libdnet (libdumbnet on IIRC solaris), which provides an abstraction on top of struct sockaddr. I can foresee a "clash" between libT IP abstraction and libdht, so I want to know if you could take a look at switching to libdnet (it's a small library, and it could be made even smaller, because most of the stuff in it won't be used), or maybe ask jhujhiti ;-). Right now dht-0.1 is IPv4 only.
  • If I'm reading correctly, you're using globals. Don't know how well this will integrate in libT, though that seems easy to fix: make a struct containing the socket and the top-level bucket, along with most of the static variables from the top of the file, and make dht_init return a pointer to this struct.

On my side of things, the libdht code is more high-level (since it was designed to be protocol-agnostic, there's even an implementation of some kind of DHT-backed IRC server). I do agree that it's a whole more complicated (3 files instead of 1, has minimal documentation atm), but it already have some nice abstractions (for our purposes, mainly IPv6, and it uses libevent (though I need v2). Right now it just need to write/debug an API to convert protocol stuff into bencoded dicts. I'm having some problem with ironing the last few bugs in there, so if you could take a look at Valgrinding the backend-rework branch on Github (Valgrind still crashes on OS X).

comment:27 follow-up: Changed 13 years ago by jch

What about IPv6

The first version of my code did include full support for IPv6. Then I realised that it's not so simple -- the Kademlia algorithm supposes transitive connectivity, you cannot just dump a bunch of IPv4 and IPv6 addresses together and hope that it will still work.

After realising that I dumped the support for IPv6, but retained a dual-stack API (the public functions use sockaddr, not sockaddr_in). I'll think about how to generalise Kademlia to the dual-stack (non-transitive) situation when I'm done with the IPv4 version.

If your library is dual-stack, I'm very much interested in hearing how you solved this issue.

if you could take a look at switching to libdnet

I've had a look. I'm not switching.

If I'm reading correctly, you're using globals [...] that seems easy to fix

Yep. Using globals is reasonable, since there's just a single DHT; if you've got multiple DHT sessions, you're doing something wrong. (In principle, the DHT should be implemented as a single, client-wide daemon that is shared by all the Bittorrent clients.)

But as you rightly note, if Charles disagrees, we can always put the globals into a single struct later on. I'm sticking with globals right now, since it's easier to examine globals with a debugger.

--Juliusz

comment:28 in reply to: ↑ 27 ; follow-up: Changed 13 years ago by tiennou

Replying to jch:

What about IPv6

The first version of my code did include full support for IPv6. Then I realised that it's not so simple -- the Kademlia algorithm supposes transitive connectivity, you cannot just dump a bunch of IPv4 and IPv6 addresses together and hope that it will still work. After realising that I dumped the support for IPv6, but retained a dual-stack API (the public functions use sockaddr, not sockaddr_in). I'll think about how to generalise Kademlia to the dual-stack (non-transitive) situation when I'm done with the IPv4 version. If your library is dual-stack, I'm very much interested in hearing how you solved this issue.

I didn't know about the non-transitive issue (I found a paper describing some of the issues encountered). I'm pretty sure libdht doesn't care about this problem (though I was aware of the problem, there is a discussion about IPv4/6 DHT in the Bittorrent forum), so you make a point.

If I'm reading correctly, you're using globals [...] that seems easy to fix

Yep. Using globals is reasonable, since there's just a single DHT; if you've got multiple DHT sessions, you're doing something wrong. (In principle, the DHT should be implemented as a single, client-wide daemon that is shared by all the Bittorrent clients.) But as you rightly note, if Charles disagrees, we can always put the globals into a single struct later on. I'm sticking with globals right now, since it's easier to examine globals with a debugger.

Okay, no problem for now then.

Would it be possible for you to put dht-0.1 under GitHub? ? I already have a mirror of Transmission code there, and it will make concurrent development easier (like the libevent stuff I'm likely to add). If don't want to create an account there, I can make a repository under mine (if you allow me to, that is ;-)).

BTW, Charles, you're misinterpreting the BT_PORT message in current trunk ;-).

comment:29 in reply to: ↑ 28 Changed 13 years ago by charles

Replying to tiennou:

BTW, Charles, you're misinterpreting the BT_PORT message in current trunk ;-).

Fixed in r8415, thanks

comment:30 Changed 13 years ago by jch

Would it be possible for you to put dht-0.1 under GitHub??

I'm using darcs:

$ darcs get http://www.pps.jussieu.fr/~jch/software/repos/dht/

comment:31 Changed 13 years ago by jch

A new version of the patch is on

http://www.pps.jussieu.fr/~jch/software/files/transmission-dht-20090617.patch

You'll need version 0.2 of the dht code, which is on

http://www.pps.jussieu.fr/~jch/software/files/dht-0.2.tar.gz http://www.pps.jussieu.fr/~jch/software/files/dht-0.2.tar.gz.asc

The main news is that this version is almost entirely event-driven -- we fully integrate with libevent's loop, and only launch a thread for bootstrapping. (Charles, if you object to this thread, I can do it from the event loop too, but it would complicate the code for little gain -- the bootstrapping thread only runs for a couple of minutes after startup.)

Other than that, we now save/restore the list of good nodes at shutdown/startup. Additionally, we are able to bootstrap from any Bittorrent peers that we meet.

Missing:

  • bootstrapping from .torrent files;
  • reattempting to bootstrap if we failed at startup (for example, because the network was down);
  • user-interface (enabling/disabling the DHT, displaying the DHT status somewhere).

--Juliusz

comment:32 Changed 13 years ago by charles

Sounds great! It's nice to see this ticket getting so much action.

Don't worry about the UI at this point... that requires large changes because of how many clients we've got nowadays. As long as the API is in place for enabling/disabling and getting the status, livings and I can take care of the UI.

Changed 13 years ago by charles

comment:33 Changed 13 years ago by jch

http://www.pps.jussieu.fr/~jch/software/files/dht-0.3.tar.gz http://www.pps.jussieu.fr/~jch/software/files/dht-0.3.tar.gz.asc

Bug fixes only, no user-visible changes. The patch to transmission doesn't change since this morning.

--Juliusz

comment:34 Changed 13 years ago by jch

New version:

http://www.pps.jussieu.fr/~jch/software/files/dht-0.4.tar.gz http://www.pps.jussieu.fr/~jch/software/files/dht-0.4.tar.gz.asc http://www.pps.jussieu.fr/~jch/software/files/transmission-dht-20090618.patch

The DHT core fixes a stupid bug that caused tokens in announces to be checked incorrectly. It also implements backtracking during searches, so that a few dead nodes near the target will not reduce the number of announcements that we make. (AFAIK, mainline doesn't backtrack, while Azureus does. I'm quite positive that backtracking is the correct thing to do, and I'm also positive that we won't break anything for mainline.)

The transmission patch integrates announcing into the trackerPulse function, rather than hacking them into the DHT event loop, which means that adding a torrent immediately schedules a new announce. This also means that whoever adds this stuff into the GUI will be able to provide a announce now button.

Various minor GUI-minded tweaks to the transmission interface too, in particular the ability to retrieve the number of nodes in the DHT and the addition of a torrent->dhtAnnounceInProgress boolean field which is properly maintained. Not really used right now, just nice things to have in the GUI.

Charles, this version is ready to be committed.

--Juliusz

comment:35 Changed 13 years ago by livings124

jch: I'm having trouble compiling this on Mac. It looks like memmem() doesn't exist, unless I'm missing something.

Changed 13 years ago by charles

Changed 13 years ago by charles

Changed 13 years ago by charles

comment:36 Changed 13 years ago by charles

tr_memmem() is now available for public use in r8428

comment:37 Changed 13 years ago by charles

that should read r8424

comment:38 Changed 13 years ago by charles

jch: I'm reading over this patch right now and am pretty happy about this -- not only is DHT finally getting done, but even better, the patch looks pretty good and pretty clean. If I commit this, are you available for looking over future DHT bug reports?

comment:39 Changed 13 years ago by tiennou

Note: you're NULL-ing session in dht_uninit, this causes a crash.

comment:40 follow-up: Changed 13 years ago by tiennou

jch: And, could you look at adding the v key as specified in http://www.rasterbar.com/products/libtorrent/dht_extensions.html ? It could be helpful, in case someone notices a misbehavior.

Otherwise, I'm currently running it on OS X, and after a few tweaks to make DHT counts appear in the peers list, a small #define for the memmem issue, and fix for the bug above, it runs smoothly ;-). Thanks for your work !

comment:41 Changed 13 years ago by livings124

tiennou: Perhaps you can submit a patch for all of that?

comment:42 Changed 13 years ago by charles

tiennou: I can't try the code out ATM but I don't see the NULLing error. Can you provide a diff?

jch: a few thoughts on the code. I want to run these by you to see if you think they're a good idea before I patch.

  • As I said before, overall the code is very good :)
  • I see we can't use tr_memmem() because it's at the wrong point in the library stack. The libt/utils.c implementation could be copied into dht.c...
  • I'm pretty sure the functionality in the proposed tr_bencDict*StrN() functions already exists in tr_bencDict*Raw(). The latter doesn't handle replacing existing keys, but tr-dht.c doesn't need it to.
  • tr_dhtStatus() blocks for a full second even when invoked from the event thread. This seems like a needless pause... but if it's an intentional side-effect for jitter, it should be commented.
  • tr_dhtAddNode() doesn't seem to have any callers. Is this function to be called from the mac/gtk/qt/daemon when the end user has a specific address?

Minor portability / consistency items

  • dht.c and tr-dht.c use "unsigned short" for the ports. this should probably be uint16_t in dht.c, and tr_port in tr-dht.c.
  • tr-dht.c should use tr_cryptoWeakRandInt() instead of random().
  • tr-dht.c should use tr_new/tr_free instead of malloc/free
  • tr_dhtPort()'s tr_session argument should be const
  • tr_dhtEnabled()'s tr_session argument should be const, and return tr_bool
  • tr-dht.c should use tr_buildPath() instead of tr_strdup_printf()

comment:43 Changed 13 years ago by tiennou

@livings: Sure, but it's minimal. I just added some accessors in Torrent, and some stuff in InfoWindowController?. I'm also stuck with the lack of memmem, which I worked around by downloading the GPLed version from BSD. This means duplication, as libT also has tr_memmem. Also note the patch was hand-edited (I removed everything contained in jch last patch).

@charles: Sorry, the crash had escaped my testing, it's not related to the line I thought (last line of dht_uninit). I'm getting a crash down in :

#0  0x0006e7e3 in tr_globalLock (session=0xc01ea50b) at /Volumes/myHD/Projects/Transmission/Transmission-git/libtransmission/session.c:874
#1  0x00097472 in managerLock (manager=0x7a90f0) at /Volumes/myHD/Projects/Transmission/Transmission-git/libtransmission/peer-mgr.c:183
#2  0x0009cbe4 in reconnectPulse (vmgr=0x7a90f0) at /Volumes/myHD/Projects/Transmission/Transmission-git/libtransmission/peer-mgr.c:2402
#3  0x0008f039 in timerCallback (fd=-1, event=1, vtimer=0xf214970) at /Volumes/myHD/Projects/Transmission/Transmission-git/libtransmission/trevent.c:302
#4  0x000b2b19 in event_process_active (base=0x2033400) at /Volumes/myHD/Projects/Transmission/Transmission-git/third-party/libevent/event.c:392
#5  0x000b2f0a in event_base_loop (base=0x2033400, flags=0) at /Volumes/myHD/Projects/Transmission/Transmission-git/third-party/libevent/event.c:544
#6  0x000b2d33 in event_loop (flags=0) at /Volumes/myHD/Projects/Transmission/Transmission-git/third-party/libevent/event.c:468
#7  0x000b2b6d in event_dispatch () at /Volumes/myHD/Projects/Transmission/Transmission-git/third-party/libevent/event.c:406
#8  0x0008ed20 in libeventThreadFunc (veh=0x7a8770) at /Volumes/myHD/Projects/Transmission/Transmission-git/libtransmission/trevent.c:238
#9  0x0007d6bf in ThreadFunc (_t=0x7a8220) at /Volumes/myHD/Projects/Transmission/Transmission-git/libtransmission/platform.c:106
#10 0x970ce155 in _pthread_start ()
#11 0x970ce012 in thread_start ()

It seems to happen consistently at exit.

Changed 13 years ago by tiennou

DHT Mac OS X GUI modifications

comment:44 Changed 13 years ago by jch

jch: a few thoughts on the code. I want to run these by you to see if you think they're a good idea before I patch.

I think it'll be easier to debate once we have a common code base.

  • As I said before, overall the code is very good :)

Thanks.

  • I see we can't use tr_memmem() because it's at the wrong point in the

library stack. The libt/utils.c implementation could be copied into dht.c...

Aye, will do.

  • I'm pretty sure the functionality in the proposed tr_bencDict*StrN()

functions already exists in tr_bencDict*Raw(). The latter doesn't handle replacing existing keys, but tr-dht.c doesn't need it to.

I'm a little bit confused by your bencoding code, so I'd be grateful if you could do that.

  • tr_dhtStatus() blocks for a full second even when invoked from the

event thread. This seems like a needless pause... but if it's an intentional side-effect for jitter, it should be commented.

It actually blocks for 1ms only, but your point stands.

No need to apply jitter here, since tr_dhtStatus doesn't send any packets around. I fully agree that we should handle the case when we're called from the event thread specially.

(By the way, as a side comment, do not call this function more than once every few seconds -- it walks the full routing table to compute the number of nodes, so it's not as fast as you might expect.)

  • tr_dhtAddNode() doesn't seem to have any callers.

It's called in peer-msgs.c:1449 (when we receive a PORT message from a peer), and in tr-dht.c line 89 (when we bootstrap from the dht.dat file).

Still missing is the ability to bootstrap from a .torrent file, but I'm having some doubts whether it's a good idea; in any case, that should only be done as a last resort, and I'm finding it difficult to work out the right conditions.

At any rate, the current version usually manages to bootstrap even without help from the .torrent files.

  • dht.c and tr-dht.c use "unsigned short" for the ports. this should

probably be uint16_t in dht.c, and tr_port in tr-dht.c.

Fine, I'll do the dht.c code ASAP.

  • tr-dht.c should use tr_cryptoWeakRandInt() instead of random().
  • tr-dht.c should use tr_new/tr_free instead of malloc/free
  • tr-dht.c should use tr_buildPath() instead of tr_strdup_printf()

Agreed.

  • tr_dhtPort()'s tr_session argument should be const
  • tr_dhtEnabled()'s tr_session argument should be const, and return

tr_bool

I only ever use const for plain datatypes, never for structs. There are good reasons for that -- using const breaks when you later implement caching or memoisation.

Juliusz

comment:45 Changed 13 years ago by jch

If I commit this, are you available for looking over future DHT bug reports?

I'll be maintaining the dht.c code for the foreseeable future -- it's actually split from a projet I'm doing for Real Work (a massively concurrent seeder, planned to be able to handle 10000 simultaneous peers); the integration into libtransmission is just a side-effect (a convenient way to test my code).

I don't promise to do any maintenance work for any of the libtransmission code, but that's stuff that you're certainly more ocmpetent to handle than me.

--Juliusz

Changed 13 years ago by jch

comment:46 Changed 13 years ago by jch

PEX has a tendency to cause fragmented swarms -- swarms in which there are small, well-connected cliques that have little or no connections between themselves. I can point you to a number of papers on the subject if you want to learn more.

The patch prefer-dht.patch tries to avoid that by preferring DHT peers to PEX peers when possible. Unfortunately, since PEX tends to be faster than the DHT, it has little effect until you have run your client for a few hours.

Changed 13 years ago by jch

comment:47 Changed 13 years ago by jch

In provide-node-count.patch, the interface to dhtStatus is changed to provide a node count, which could be useful for the GUI.

--Juliusz

Changed 13 years ago by charles

updated diff

comment:48 Changed 13 years ago by charles

Attached is transmission-dht-2009.06.19.01.diff. Here's what it is:

baseline:

  • dht-0.4
  • transmission-dht-20090618.patch
  • prefer-dht.patch

changes to dht-0.4:

  • silence minor compiler warnings making some funcs static
  • silence minor compiler warnings by adding "void" to empty arg list
  • added a simple automake template, which might be overkill.

changes to tr-dht:

  • use tr_bencDict*Raw() instead of adding new tr_bencDict*StrN() funcs
  • use tr_port instead of unsigned short for port's types
  • use tr_buildPath() instead of tr_strdup_printf() for paths
  • use tr_wait() instead of usleep()
  • use tr_new(), tr_free() instead of malloc(), free()
  • use tr_bool instead of int for flag args
  • use tr_cryptoRandBuf() instead of reading from /dev/rand
  • use tr_cryptoWeakRandInt() instead of random() for jitter
  • fix a couple of small memory leaks

unresolved:

  • provide-node-count.patch not applied yet.
  • Transmission-Mac-20090519.diff not applied; needs approval from livings
  • the crash-on-shutdown bug tiennou is seeing

comment:49 follow-up: Changed 13 years ago by charles

actually, would it be easier for everyone if I just committed what we've got so far -- so that we can use that as a reference -- instead of piling diffs on top of each other?

comment:50 follow-ups: Changed 13 years ago by charles

Other items I've got for mission-dht-2009.06.19.01.diff:

  • jch in handshake.c, we check tor->session->isPexEnabled before calling tr_peerIoEnableDHT(). Should that be tor->session->isDHTEnabled?
  • dht /dev/urandom is going to break on mingw. It would be nice if we could #ifdef in the use of RAND_pseudo_bytes from openssl.
  • dht: memmem()
  • libT: add a separate preference to tr_sessionInit() for the DHT flag
  • libT: tr_dhtUinit() uses libevent, so it must be called from tr_closeAllConnections() rather than tr_sessionClose()
  • libT: tr_dhtInit() uses libevent, so it must be called in tr_sessionInitImpl() instead of tr_sessionInit()
  • libT: looks like I missed couple of calls to random() in checkAndStartImpl() and trackerPulse()
  • charles: next time, don't include Makefile.in in the diff ;)

comment:51 Changed 13 years ago by livings124

I have local changes comprable to tiennou's changes ready

comment:52 in reply to: ↑ 49 Changed 13 years ago by jch

Replying to charles:

actually, would it be easier for everyone if I just committed what we've got so far -- so that we can use that as a reference

Yes, please do. I'm working in a local Git repository, it would be much easier if I could just track your subversion.

comment:53 in reply to: ↑ 50 Changed 13 years ago by jch

  • jch in handshake.c, we check tor->session->isPexEnabled before calling tr_peerIoEnableDHT(). Should that be tor->session->isDHTEnabled?

Yes, my bug.

  • dht /dev/urandom is going to break on mingw. It would be nice if we could #ifdef in the use of RAND_pseudo_bytes from openssl.

Right. I'll provide an upcall, as I've done with dht_hash. My hope is that the dht.c included in transmission can remain pristine, this will make life easier for everyone.

  • dht: memmem()

Will do.

--Juliusz

comment:54 Changed 13 years ago by jch

Charles, I don't see tr-dht.* in your patch.

-- Juliusz

comment:55 Changed 13 years ago by charles

I've committed transmission-dht-2009.06.19.01.diff into svn, so trunk should be the baseline for subsequent patches.

comment:56 in reply to: ↑ 40 ; follow-up: Changed 13 years ago by jch

Replying to tiennou:

jch: And, could you look at adding the v key as specified in http://www.rasterbar.com/products/libtorrent/dht_extensions.html ? It could be helpful, in case someone notices a misbehavior.

Good idea. It's just 7 extra bytes per message.

Charles, how shall I generate the identifier (pointers to code more than welcome)? It has the format

XXnn

where XX is a two-letter code identifying the implementation, and nn is a 16-bit number identifying the version.

Looking at the nodes I see right now, most do not specify a version number. The ones that do specify

  UT 15358
  UT 13666
  UT 14578

and similar, indicating that these are various µTorrent versions. I also see one node that says

  Az 126

which is surprising, since I was under the impression that Azureus doesn't support mainline DHT.

Concerning the other extension on that page: it makes a lot of sense, and I already parse and handle it correctly. However, I'm not generating it, since I'm afraid that other clients might not be able to parse us correctly. If you have any contact with the rasterbar folks, ask them whether they know anything I don't.

comment:57 in reply to: ↑ 56 Changed 13 years ago by charles

Replying to jch:

Charles, how shall I generate the identifier (pointers to code more than welcome)? It has the format

XXnn

where XX is a two-letter code identifying the implementation, and nn is a 16-bit number identifying the version.

"TR" would be the two-letter code, of course.

For the 16-bit number we could either use a release number or svn build number. I'd lean toward the svn build but either is fine. I'd probably do something like

#include "version.h"
...
char buf[5];
tr_snprintf( buf, sizeof(buf), "TR%02x", SVN_REVISION_NUM );

If you have any contact with the rasterbar folks, ask them whether they know anything I don't.

You might want to drop into the #transmission channel on freenode. I think all the transmission devs at a minimum leave their connections idling there, and libtorrent (Rasterbar)'s author idles there under the irc nick of "hydri".

comment:58 Changed 13 years ago by charles

provide-node-count patch applied in r8435.

jch:

it doesn't look like dht_hash() is used anywhere:

[charles@localhost T]$ grep dht_hash */*[chmp]
libtransmission/tr-dht.c:dht_hash(void *hash_return, int hash_size,

also it looks like this code in tr_dhtAddNode() is unused, unless I'm missing something:

{
    char buf[50];
    inet_ntop(AF_INET, &address->addr.addr4, buf, 50);
}

Should these be removed?

Changed 13 years ago by jch

comment:59 follow-up: Changed 13 years ago by charles

same question for

struct timeval now;

gettimeofday(&now, NULL);

doesn't seem to be used in tr-dht's event_callback()

comment:60 Changed 13 years ago by charles

  • Milestone changed from Sometime to 1.70

comment:61 follow-up: Changed 13 years ago by charles

oh, nvm about dht_hash() being removed, I see it now.

That's a strange way to go about it though. Could we pass in a function pointer to dht_init() instead?

comment:62 Changed 13 years ago by jch

Replying to charles:

it doesn't look like dht_hash() is used anywhere:

It is -- it's an upcall from the DHT core. See the function make_token in dht.c.

It must be a strong cryptographic hash (or else significant security issue), but other than that, it can be anything you wish. It doesn't need to be especially fast.

also it looks like this code in tr_dhtAddNode() is unused, unless I'm missing something:

{
    char buf[50];
    inet_ntop(AF_INET, &address->addr.addr4, buf, 50);
}

Yep, it's a remainder from some old debugging code. Please remove it.

comment:63 in reply to: ↑ 59 Changed 13 years ago by jch

Replying to charles:

same question for

struct timeval now;

gettimeofday(&now, NULL);

doesn't seem to be used in tr-dht's event_callback()

Indeed -- libevent computes deadlines for us. Please remove it.

comment:64 in reply to: ↑ 61 ; follow-up: Changed 13 years ago by jch

Replying to charles:

oh, nvm about dht_hash() being removed, I see it now.

That's a strange way to go about it though. Could we pass in a function pointer to dht_init() instead?

I thought about it, but then, it's not like dht_hash is likely to change at runtime (actually it must not). I'm afraid that the number of parameters to dht_init will grow very fast, as we expand the number of upcalls.

If you really mind, I can make it into a callback, but I'd rather leave it the way it is.

comment:65 in reply to: ↑ 64 ; follow-up: Changed 13 years ago by charles

Replying to jch:

Yep, it's a remainder from some old debugging code. Please remove it.

removed in r8436

Replying to jch:

Indeed -- libevent computes deadlines for us. Please remove it.

removed in r8436

Replying to jch:

If you really mind, I can make it into a callback, but I'd rather leave it the way it is.

Let's leave it the way it is then.

Could you explain a little more of what's going on in

if(len % 6 == 2) {
    /* This hack allows reading of uTorrent files, which I find
      convenient. */
    len -= 2;
}

comment:66 in reply to: ↑ 65 Changed 13 years ago by jch

Replying to charles:

Could you explain a little more of what's going on in

if(len % 6 == 2) {
    /* This hack allows reading of uTorrent files, which I find
      convenient. */
    len -= 2;

Sure. The format of the dht.dat file is inspired by the format used by µTorrent. µTorrent's format is similar, except that it has two extra keys, and adds two bytes at the end of the nodes key. Removing those two extra bytes allows my code to read the dht.dat of µTorrent, which was very convenient when transmission wasn't yet able to bootstrap by itself.

Now that we don't need µTorrent's help any longer, I suggest you simply do

  if(len % 6 == 0) {
     nodes = tr_new( uint8_t, len );
     memcpy( nodes, raw, len );
  } else {
     /* whatever you do to print a warning */
  }
  tr_bencFree(&benc);

comment:67 follow-up: Changed 13 years ago by charles

Another question: I think this is a bug... dht_periodic() can only return -1 (via its recvfrom() call or when it gets an IPv6 socket), or a 0 or 1 (in its last line, "return find_bucket(myid)->count > 2;"). However, tr-dht.c's event_callback() checks rc, rather than errno, for EINVAL and EFAULT....

comment:68 in reply to: ↑ 67 ; follow-up: Changed 13 years ago by jch

Replying to charles:

However, tr-dht.c's event_callback() checks rc, rather than errno, for EINVAL and EFAULT....

Correct, as usual.

comment:69 in reply to: ↑ 50 Changed 13 years ago by jch

Replying to charles:

  • dht /dev/urandom is going to break on mingw. It would be nice if we could #ifdef in the use of RAND_pseudo_bytes from openssl.

Okay, in dht-0.5, you need to provide a function

int dht_random_bytes(void *buf, size_t size);

(0.5 is not released yet, as I'll add the requested « v4: » handling to that version.)

comment:70 in reply to: ↑ 68 Changed 13 years ago by charles

However, tr-dht.c's event_callback() checks rc, rather than errno, for EINVAL and EFAULT....

Correct, as usual.

fixed in r8437

Changed 13 years ago by jch

comment:71 Changed 13 years ago by jch

20090619 does the following:

  • includes the v key in messages;
  • moves /dev/urandom into tr-dht.c so that you can customise it.

You will need to import

http://www.pps.jussieu.fr/~jch/software/files/dht-0.5.tar.gz

before you can apply this patch.

comment:72 Changed 13 years ago by livings124

jch: This still isn't working on Mac because of memmem.

Changed 13 years ago by jch

comment:73 Changed 13 years ago by charles

updated to dht-0.5 in r8440.

jch: did I do the `v' argument right?

jch: dht_socket isn't being closed by anyone. Whose responsibility is it? We pass it as the first argument to dht_uninit(), but it's never used there... is this a bug in dht, or tr-dht?

comment:74 Changed 13 years ago by jch

livings124: sorry for that. Could you please test dht-memmem.patch and let me know if it works for you?

comment:75 Changed 13 years ago by jch

dht_socket isn't being closed by anyone. Whose responsibility is it? We pass it as the first argument to dht_uninit(), but it's never used there... is this a bug in dht, or tr-dht?

It's supposed to be closed by tr-dht, in tr_dhtUninit, for symmetry with tr_dhtUninit. Sorry for the omission.

comment:76 Changed 13 years ago by jch

did I do the `v' argument right?

Perhaps I'm confused by subversion (does it have sticky tags, like CVS used to?), but I don't see the changes I made in transmission-dht-20090619.patch. It won't compile without that.

comment:77 Changed 13 years ago by livings124

jch: Yup, that patch did the trick.

comment:78 follow-up: Changed 13 years ago by livings124

I'm getting this warning with your patch:

/Transmission/third-party/dht/dht.c:2097: warning: return discards qualifiers from pointer target type

(dht.c:2097 is return h + i;)

Changed 13 years ago by jch

comment:79 in reply to: ↑ 78 Changed 13 years ago by jch

/Transmission/third-party/dht/dht.c:2097: warning: return discards qualifiers from pointer target type

Thanks for the notice -- fixed upstream. See dht-warnings.patch.

comment:80 follow-up: Changed 13 years ago by charles

dht-warnings.patch applied in r8451.

jch: is there a public upstream repo that I can use to look at the latest version of dht.[ch]?

comment:81 in reply to: ↑ 80 Changed 13 years ago by jch

Replying to charles:

jch: is there a public upstream repo that I can use to look at the latest version of dht.[ch]?

Yes, but it's using Darcs, which you may not be familiar with. http://www.darcs.net

Crash course: you get a local copy by doing

darcs get http://www.pps.jussieu.fr/~jch/software/repos/dht/

You can then track my repository by doing

darcs pull

and send me your locally committed changes by doing

darcs send

comment:82 Changed 13 years ago by livings124

There is a crash report related to DHT at http://forum.transmissionbt.com/viewtopic.php?f=4&t=7657

comment:83 Changed 13 years ago by charles

Money quote:

Thread 1 Crashed:
0   org.m0k.transmission                0x000a655c dht_periodic + 23525
1   org.m0k.transmission                0x0009ae0f event_callback + 63
2   org.m0k.transmission                0x000aa136 event_base_loop + 1209
3   org.m0k.transmission                0x000aa552 event_dispatch + 35
4   org.m0k.transmission                0x0007cebf libeventThreadFunc + 191
5   libSystem.B.dylib                   0x92f33095 _pthread_start + 321
6   libSystem.B.dylib                   0x92f32f52 thread_start + 34

livings124: this isn't terribly helpful. I thought the nightly builds had line numbers & debug info built into them now...?

comment:84 Changed 13 years ago by livings124

They do have debug info, sometimes it doesn't show up...

comment:85 follow-up: Changed 13 years ago by jch

Note to everyone: I'm as puzzled as anyone about the reported crash. (Still, crashing in the wee hours is rather respectable performance for a first version.)

Note to somebody: the UPnP and NAT-PNP code should be updated to open the UDP port. (It's not strictly necessary, but it will help.)

Note to nobody in particular: I need to think some more about bootstrapping. It's somewhat tricky, since you don't want to be too aggressive, but still want to ensure that bootstrapping happens even if the DHT has suffered some form of catastrophic failure (think all Bittorrents in the world rebooting at the same time because of a global thermonuclear war).

Note to self: I need to contact the rtorrent developers to explain that their approach to rate-limiting the DHT is wrong. Time to dust my trusty old IRC client....

Note to Charles: I don't understand your piece selection algorithm, but I have no idea what is the right channel to contact you over. Is e-mail okay?

--Juliusz

comment:86 in reply to: ↑ 85 Changed 13 years ago by charles

Replying to jch:

(Still, crashing in the wee hours is rather respectable performance for a first version.)

:)

Note to Charles: I don't understand your piece selection algorithm, but I have no idea what is the right channel to contact you over. Is e-mail okay?

Sure.

comment:87 Changed 13 years ago by charles

jch:

Is it intentional that DHT uses the same port as the incoming BitTorrent? port? Will this interfere with incoming peers from connecting to us?

comment:88 Changed 13 years ago by jch

Is it intentional that DHT uses the same port as the incoming BitTorrent?? port?

Yes.

Will this interfere with incoming peers from connecting to us?

No, the spaces of TCP and UDP ports are disjoint -- an incoming packet is first identified as a TCP or UDP port, and only then is it dispatched depending on the port number.

While there's nothing in the protocol that requires that the port numbers be equal, there are a number of advantages to keeping them so:

  • it's simpler on the brain, both for us and for people who set up port forwarding;
  • there's fewer incomprehensible items in the preferences dialog;
  • older versions of uTorrent are rumoured to get confused if you don't do that.

I really don't see any advantages to allowing the user to set the UDP port independently from the TCP port.

--Juliusz

comment:89 Changed 13 years ago by jch

Okay, I found a buffer overflow in the current code which could in principle explain the crash if the user was being bombarded by truncated packets. Still, I find it rather unlikely.

It's a read-only buffer overflow in the packet parser, so no need to panic -- the only harm that happens is that we read beyond our data, we don't actually corrupt anything in memory.

--Juliusz

comment:90 Changed 13 years ago by charles

jch:

thanks for the information. It makes perfect sense for DHT to use UDP, but for some reason I'd forgotten that.

Also, new info on the crash. Another user has reported a similar crash. This is still off the dht-0.5 base but I thought I'd pass it along anyway:

Thread 1 Crashed:
0   libSystem.B.dylib                   0x91cc90e0 memcmp + 224
1   org.m0k.transmission                0x00096a6c dht_periodic + 112
2   org.m0k.transmission                0x00091514 event_callback + 52
3   org.m0k.transmission                0x0009d630 event_base_loop + 1344
4   org.m0k.transmission                0x00074aa8 libeventThreadFunc + 160
5   libSystem.B.dylib                   0x91d020c4 _pthread_start + 316

I'll sync our svn/trunk with your upstream asap.

comment:91 follow-up: Changed 13 years ago by charles

better crash report:

Please indicate why this post is abusive, and provide any other useful information.
Spam / advertising / junk
Personal details
Proprietary code
Other

comments (optional)


email (optional)


   1.
      Thread 1 Crashed:
   2.
      0   libSystem.B.dylib                   0x967f1440 bcmp + 32
   3.
      1   org.m0k.transmission                0x000b6263 parse_message + 384 (dht.c:2159)
   4.
      2   org.m0k.transmission                0x000b3074 dht_periodic + 595 (dht.c:1435)
   5.
      3   org.m0k.transmission                0x000afd21 event_callback + 79 (tr-dht.c:362)
   6.
      4   org.m0k.transmission                0x000b9549 event_process_active + 302
   7.
      5   org.m0k.transmission                0x000b993a event_base_loop + 465
   8.
      6   org.m0k.transmission                0x000b9763 event_loop + 44
   9.
      7   org.m0k.transmission                0x000b959d event_dispatch + 24
  10.
      8   org.m0k.transmission                0x0008eece libeventThreadFunc + 230 (trevent.c:238)
  11.
      9   org.m0k.transmission                0x0007d857 ThreadFunc + 34 (platform.c:112)
  12.
      10  libSystem.B.dylib                   0x96819155 _pthread_start + 321
  13.
      11  libSystem.B.dylib                   0x96819012 thread_start + 34

This is with an up-to-date revision from darcs

comment:92 follow-up: Changed 13 years ago by charles

Juliusz: this might be a dumb idea, but maybe parse_message should be an upcall s.t. a real benc parser could be used?

comment:93 in reply to: ↑ 91 ; follow-up: Changed 13 years ago by jch

Replying to charles:

better crash report:

Great, that's most likely the buffer overflow I've just found. I'm testing right now, dht-0.6 should be ready in a few hours.

--Juliusz

comment:94 in reply to: ↑ 93 ; follow-up: Changed 13 years ago by charles

Juliusz:

I think one possible problem is that size_t is an unsigned long on OS X. So if needlelen > haystacklen, I think this loop test could underflow, which would cause behavior consistent with the bcmp() crash we're seeing.

for(i = 0; i < haystacklen - needlelen; i++)

Also, that memmem implementation has another bug, probably not relevant to the task at hand: something like memmem( "a", 1, "a", 1 ) will return NULL because of using < rather than <= in the test.

Possible revision:

static void *
memmem(const void *haystack, size_t haystacklen,
       const void *needle, size_t needlelen)
{
    const char *h = haystack;
    const char *n = needle;
    size_t i;

    if(needlelen > haystacklen)
        return NULL;

    for(i = 0; i <= haystacklen - needlelen; i++) {
        if(memcmp(h + i, n, needlelen) == 0)
            return (void*)(h + i);
    }
    return NULL;
}

comment:95 Changed 13 years ago by jch

Nah, that's not the problem.

The problem is that parse_message doesn't check the string lengths it receives; so if it receives a corrupt packet with something like

2000:...

it will happily read 2000 bytes, which may overflow the buffer.

I've got a fixed version, which I'm testing right now. I'll get back to you as soon as I'ver released it.

comment:96 in reply to: ↑ 92 Changed 13 years ago by jch

Replying to charles:

Juliusz: this might be a dumb idea, but maybe parse_message should be an upcall s.t. a real benc parser could be used?

Might be a good idea. I really don't know yet.

I'm in the process of integrating dht.c into my own Bittorrent software (the one it was originally written for), so it's not a good idea to go changing the interfaces right now. Once that's done, we can think about the right interfaces.

In the meantime, feel free to suggest an interface.

comment:97 in reply to: ↑ 94 Changed 13 years ago by jch

Replying to charles:

I think one possible problem is that size_t is an unsigned long on OS X.

Also, that memmem implementation has another bug, probably not relevant to the task at hand:

Applied your patch, thanks.

--Juliusz

comment:98 Changed 13 years ago by jch

Okay, it looks like this version works.

Charles, please import dht-0.6 from

http://www.pps.jussieu.fr/~jch/software/files/dht-0.6.tar.gz http://www.pps.jussieu.fr/~jch/software/files/dht-0.6.tar.gz.asc

Fixes a buffer overflow (when reading) in parse_message. Fixes some bugs (not relevant to dht.c) in the homebrew implementation of memmem. Fixes slightly incorrect metric computation which could very slightly impact the performance of the DHT when a lot of nodes die at the same time.

--Juliusz

Changed 13 years ago by jch

comment:99 Changed 13 years ago by charles

updated to dht-0.6 in r8482.

comment:100 Changed 13 years ago by charles

  • Resolution set to fixed
  • Status changed from assigned to closed

No longer crashing for me. (Yes I'm on Linux, so I had to force the homerolled implementation of memmem() to build ;)

I think this finishes out the implementation phase of DHT in Transmission. This ticket has now reached 100 comments, so I'm going to close it as "fixed" and let any bugs that pop up go in a new ticket.

comment:101 Changed 13 years ago by jch

In tr-dht.c, around line 140:

    if(rc == 0) {
        if(( have_id = tr_bencDictFindRaw( &benc, "id", &raw, &len ) && len==20 ))
            memcpy( myid, raw, len );
        if( tr_bencDictFindRaw( &benc, "nodes", &raw, &len ) && !(len%6) )
            nodes = tr_memdup( raw, len );
        tr_bencFree( &benc );
    }

I may be mis-reading this (I avoid putting side effects into conditionals myself, since I'm not very good at reading this kind of code), but I think it does the wrong thing if len is not 20.

--Juliusz

Note: See TracTickets for help on using tickets.