Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#5128 closed Bug (fixed)

Miss-alignement in libutp

Reported by: waby38 Owned by: livings124
Priority: Normal Milestone: 2.74
Component: libtransmission Version: 2.73
Severity: Normal Keywords: libutp alignment
Cc:

Description

Some arch like SH4 don't like / support unaligned access, others like x86/ARM support it but with poor performance.

In this extracted part of code, in get_familly() call, "&_in._in6addr" can be unaligned, and will be used by the IN6_IS_ADDR_V4MAPPED macro who make uint32_t access. This generate unaligned memory access which end up with exception on SH4, and poor performance on others arch.

#define IN6_IS_ADDR_V4MAPPED(a) \
        ((((__const uint32_t *) (a))[0] == 0)                                 \
         && (((__const uint32_t *) (a))[1] == 0)                              \
         && (((__const uint32_t *) (a))[2] == htonl (0xffff)))

struct PACKED_ATTRIBUTE PackedSockAddr {

        // The values are always stored here in network byte order
        union {
                byte _in6[16];          // IPv6
                uint16 _in6w[8];        // IPv6, word based (for convenience)
                uint32 _in6d[4];        // Dword access
                in6_addr _in6addr;      // For convenience
        } _in;

        (...)

        byte get_family() const
        {
                return (IN6_IS_ADDR_V4MAPPED(&_in._in6addr) != 0) ? AF_INET : AF_INET6;
        }
};

I 've make a small patch to fix it. (gcc only ?)

--- third-party/libutp/utp.cpp~	2012-10-24 15:01:08.000000000 +0200
+++ third-party/libutp/utp.cpp	2012-11-07 12:07:02.000000000 +0100
@@ -194,7 +194,7 @@ struct PACKED_ATTRIBUTE PackedSockAddr {
 		snprintf(i, len - (i-s), ":%u", _port);
 		return s;
 	}
-};
+} __attribute__ ((aligned (4)));
 
 struct PACKED_ATTRIBUTE RST_Info {
 	PackedSockAddr addr;

Change History (8)

comment:1 Changed 8 years ago by jordan

  • Resolution set to invalid
  • Status changed from new to closed

Hi waby38,

I've passed this upstream to the libutp people at https://github.com/bittorrent/libutp/issues/37. Thanks for the report!

comment:2 Changed 8 years ago by cfpp2p

  • Resolution invalid deleted
  • Status changed from closed to reopened

Thanks waby38 and jordan !!!

A variation the patch submitted by waby38 had been committed by the libutp people. https://github.com/bittorrent/libutp/commit/a9caf98596e9a8fc8b53b90375a6e6f3dabd2b44

It might be wise to update trunk /Transmission/trunk/third-party/libutp to that of https://github.com/bittorrent/libutp

There have been very few changes to libutp-master since 2010 but this is one of the few.

It may be that this will help with the many crash reports for uTP submitted by transmission users. #5044 #5002 #4960 #4935 #5062 Perhaps others I haven't found.

	Why unaligned access is bad
	===========================
	
	The effects of performing an unaligned memory access vary from architecture
	to architecture. It would be easy to write a whole document on the differences
	here; a summary of the common scenarios is presented below:

http://www.mjmwired.net/kernel/Documentation/unaligned-memory-access.txt#43

comment:3 Changed 8 years ago by livings124

  • Component changed from Transmission to libtransmission
  • Milestone changed from None Set to 2.80
  • Resolution set to fixed
  • Status changed from reopened to closed

Updated libutp in r13620.

comment:4 Changed 8 years ago by cfpp2p

this doesn't fix the crashes I listed in comment:2 as I had hoped.

see: https://trac.transmissionbt.com/ticket/5095#comment:20

comment:5 Changed 8 years ago by jordan

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:6 Changed 8 years ago by jordan

  • Keywords alignment added; alignement removed
  • Owner set to livings124
  • Status changed from reopened to new

comment:7 Changed 8 years ago by jordan

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

comment:8 Changed 8 years ago by jordan

  • Milestone changed from 2.80 to 2.74
Note: See TracTickets for help on using tickets.