Hi, Jess.
Thanks for your kind advices. It really helps.
I would take the advice #1 and remove the added OS checking script
since its totally useless and bug prone.
I decide to check for the definition of `IPV6_TCLASS` solely in the
`socket_fd.cc`. But I have three different
resolutions to fix these issues and do not know which one to take since
I am a newbie to write patches. Can you
give me some advices from your experiences? Thank you in advance!
#1
===
In the previous patch, I decide to define the `IPV6_CLASS` equal
to what `GNU/Linux` does by following a
similar fix found at the `gpsd/netlib.c` in github (link:
https://github.com/ukyg9e5r6k7gubiekd6/gpsd/blob/master/netlib.c).
It says,
```
/* work around the unfinished ipv6 implementation on hurd and
OSX <10.6 */
#ifndef IPV6_TCLASS
# if defined(__GNU__)
# define IPV6_TCLASS 61
# elif defined(__APPLE__)
# define IPV6_TCLASS 36
# endif
#endif
```
So I decide to define the `IPV6_TCLASS` equal to `67` by following
the `GNU/Linux` instead of the BSD for my own preference.
This may caused some undefined behaviours since the GNU Hurd haven't
support it yet. But if the GNU Hurd would just ignore the
undefined ToS setting, this will not be an issue.
It seems that hurd just adapt the linux ipv6 in `pfinet`.
In `./pfinet/linux-src/net/ipv6/ipv6_sockglue.c`
(https://git.savannah.gnu.org/cgit/hurd/hurd.git/tree/pfinet/linux-src/net/ipv6/ipv6_sockglue.c)
```
int ipv6_setsockopt(struct sock *sk, int level, int optname, char
*optval,
int optlen)
{
struct ipv6_pinfo *np = &sk->net_pinfo.af_inet6;
int val, valbool;
int retv = -ENOPROTOOPT;
// ... other unrelated checks, omit them for clearity
switch (optname) {
// ... other unrelated checks, omit them for clearity
case IPV6_V6ONLY:
if (sk->num) {
retv = -EINVAL;
goto out;
}
np->ipv6only = valbool;
retv = 0;
break;
// ... other unrelated checks, omit them for clearity
}
return retv;
}
```
The series of macros is defined at
`./pfinet/linux-src/include/linux/in6.h`
(https://git.savannah.gnu.org/cgit/hurd/hurd.git/tree/pfinet/linux-src/include/linux/in6.h).
But no `IPV6_TCLASS` is defined that contrast with the linux version.
(https://github.com/torvalds/linux/blob/master/include/uapi/linux/in6.h)
It might be reasonable to define the `IPV6_TCLASS` with what the
linux does,
since it would just return a -ENOPROTOOPT and that error would caught by
the `set_priority` function.
Therefore it result in the same effect like the #2 solution detailed in
the next section.
It might be reasonable to detect the exsits of GNU Hurd since I
think patches should do as minimal effect
as it could. I do not want to affect other OSes.
#2
===
Disabling the usage of `IPV6_TCLASS` when it is not defined. Now
I would have this quick fix, by changing
```
bool
SocketFd::set_priority(priority_type p) {
check_valid();
int opt = p;
if (m_ipv6_socket)
return setsockopt(m_fd, IPPROTO_IPV6, IPV6_TCLASS, &opt,
sizeof(opt)) == 0;
else
return setsockopt(m_fd, IPPROTO_IP, IP_TOS, &opt,
sizeof(opt)) == 0;
}
```
in `socket_fd.cc` to
```
bool
SocketFd::set_priority(priority_type p) {
check_valid();
int opt = p;
if (m_ipv6_socket)
#ifdef IPV6_TCLASS
return setsockopt(m_fd, IPPROTO_IPV6, IPV6_TCLASS, &opt,
sizeof(opt)) == 0;
#else
return false; // or should I always return a true?
#endif
else
return setsockopt(m_fd, IPPROTO_IP, IP_TOS, &opt,
sizeof(opt)) == 0;
}
```
I try to search in the libtorrent code base, I found this function
```
bool
HandshakeManager::setup_socket(SocketFd fd) {
if (!fd.set_nonblock())
return false;
ConnectionManager* m = manager->connection_manager();
if (m->priority() != ConnectionManager::iptos_default &&
!fd.set_priority(m->priority()))
return false;
// ... other unrelated checks, omit them for clearity
return true;
}
```
It seems that if I return a false, although it's the strictest
way, it will prevent the usage of `libtorrent` from the hurd under IPv6!
#3
===
After I examined the source code by greping the `set_priority`. It
shows
```
src/torrent/torrent.cc:download_set_priority(Download d,
uint32_t pri) {
src/torrent/torrent.cc: throw
internal_error("torrent::download_set_priority(...) could not find the
download in the resource manager.");
src/torrent/torrent.cc: throw
internal_error("torrent::download_set_priority(...) received an invalid
priority.");
src/torrent/torrent.cc:
manager->resource_manager()->set_priority(itr, pri);
src/torrent/torrent.h:void download_set_priority(Download d,
uint32_t pri) LIBTORRENT_EXPORT;
src/torrent/data/file.h: void set_priority(priority_t
t) { m_priority = t; }
src/torrent/data/block_list.h: void set_priority(priority_t
p) { m_priority = p; }
src/torrent/download/resource_manager.h: void
set_priority(uint16_t pri) { m_priority = pri; }
src/torrent/download/resource_manager.h: void
set_priority(iterator itr, uint16_t pri);
src/torrent/download/resource_manager.cc:ResourceManager::set_priority(iterator
itr, uint16_t pri) {
src/torrent/download/resource_manager.cc: itr->set_priority(pri);
src/torrent/utils/resume.cc:
(*listItr)->set_priority((priority_t)filesItr->get_key_value("priority"));
src/torrent/connection_manager.h: void
set_priority(priority_type p) { m_priority = p; }
src/net/socket_fd.h: bool set_priority(priority_type p);
src/net/socket_fd.cc:SocketFd::set_priority(priority_type p) {
src/download/delegator.cc: (*itr)->set_priority(PRIORITY_HIGH);
src/download/delegator.cc: (*itr)->set_priority(PRIORITY_NORMAL);
src/protocol/handshake_manager.cc: if (m->priority() !=
ConnectionManager::iptos_default && !fd.set_priority(m->priority()))
```
Follow what the #2 does, but always return a true, it seems cause
no demage to the `libtorrent` but it's not rigorous since we do not
actually set the priority.
Summary and Question
===
What solution should I pick, they all have pros and cons. I am a
newbie to patches and lack experiences, any suggestions or any
other better solutions?
#1. This is what I originally did. It breaks no other OSes and only
fix hurd itself, the error would be caught by the `libtorrent`,
but it relays on the implementation of GNU Hurd.
#2. This is the most safe and the most conservative way. It may
prevent softwares that link the `libtorrent` running under the IPv6
network when it just lacks the support of IPv6 traffic control. In
addition to that, it may affect other OSes that do not define
`IPV6_TCLASS`, not only hurd.
#3. This is the most radical way, it affects other OSes and may
cause more bugs. But it enable the potential usage of `libtorrent`
since ToS is oftenly not the end of the world.
hahawang
---
Miscellany
===
1. I am sorry for the HTML email and decide to use plain text. I am
using the yandex web mail client to send this mail, then choose the
`Without format`
option and hope it will send a plain-text email. If you are
using a text based mail client, can you tell me this mail is plain-text
or not?
2. I am not a native English speacker and even worse that I am not
familiar with email communication. For any etiquettes I am not follow,
any bad expressions,
bad formats, or any other things that may offend you, I am very sorry
for that. If you can point out these mistakes when you reply this email,
I am very happy because I could do better at the next time!