[PATCH] add traceroute6 applet
Denys Vlasenko
vda.linux at googlemail.com
Mon Nov 23 05:24:18 UTC 2009
On Sunday 22 November 2009 13:03, Leonid Lisovskiy wrote:
> On Sat, Nov 21, 2009 at 6:28 PM, Denys Vlasenko
> <vda.linux at googlemail.com> wrote:
>
> > Reviewed.
>
> Patch reworked
Leonid. sockaddr abstraction in libc is actually not that bad,
it's appalling how often coders get fixated into "there are
only IPv4 and IPv6 worlds, I can just open-code these two cases".
Yes, you cant open code just them. But you should not.
Old, IPv4 only code:
static void
print_inetname(const struct sockaddr_in *from)
{
const char *ina;
ina = inet_ntoa(from->sin_addr);
if (option_mask32 & OPT_ADDR_NUM)
printf(" %s", ina);
else {
char *n = NULL;
if (from->sin_addr.s_addr != INADDR_ANY)
n = xmalloc_sockaddr2host_noport((struct sockaddr*)from);
printf(" %s (%s)", (n ? n : ina), ina);
free(n);
}
}
Your code blindly added #if IPv6 code:
static void
print_inetname(const struct sockaddr_in *from)
{
const char *ina;
#if ENABLE_TRACEROUTE6
char pa[MAXHOSTNAMELEN];
if (from->sin_family == AF_INET6)
ina = inet_ntop(AF_INET6,
&((const struct sockaddr_in6 *)from)->sin6_addr,
pa, sizeof(pa));
else
ina = inet_ntoa(from->sin_addr);
#else
ina = inet_ntoa(from->sin_addr);
#endif
if (option_mask32 & OPT_ADDR_NUM)
printf(" %s", ina);
else {
char *n = NULL;
if (from->sin_family != AF_INET
|| from->sin_addr.s_addr != INADDR_ANY
) {
n = xmalloc_sockaddr2host_noport((struct sockaddr*)from);
}
printf(" %s (%s)", (n ? n : ina), ina);
free(n);
}
}
What you can do instead is to realize that this code just converts
const struct sockaddr* to a string representing address in "dotted"
format (i.e., not back-resolved into hostname), *regardless* of
underlying address family.
bbox has a function for that: xmalloc_sockaddr2dotted[_noport](const struct sockaddr *)
Thus:
static void
print_inetname(const struct sockaddr *from)
{
char *ina = xmalloc_sockaddr2dotted_noport(from);
if (option_mask32 & OPT_ADDR_NUM)
printf(" %s", ina);
else {
char *n = NULL;
if (from->sa_family != AF_INET
|| ((struct sockaddr_in*)from)->sin_addr.s_addr != INADDR_ANY
) {
n = xmalloc_sockaddr2host_noport((struct sockaddr*)from);
}
printf(" %s (%s)", (n ? n : ina), ina);
free(n);
}
free(ina);
}
+#if ENABLE_TRACEROUTE6
+ /* We use recv_from_to() for IPv6 enabled traceroute only
+ * due to applet size reason - most applets don't use
+ * this library function at all */
+ cc = recv_from_to(rcvsock, recv_pkt, sizeof(recv_pkt), 0,
+ &fromp->u.sa, to, fromp->len);
+#else
+ socklen_t fromlen = fromp->len;
+
cc = recvfrom(rcvsock, recv_pkt, sizeof(recv_pkt), 0,
- (struct sockaddr *)fromp, &fromlen);
+ &fromp->u.sa, &fromlen);
+#endif
I bet what you won here you lose in code size (and simplicity)
everywhere else - now you propagate "to" only if ENABLE_TRACEROUTE6,
and code turns into an #ifdef forest:
- i = packet_ok(cc, &from, seq);
+#if ENABLE_TRACEROUTE6
+ i = packet6_ok(cc, &from_lsa, &to.sin6_addr, seq);
+#else
+ i = packet_ok(cc, &from_lsa.u.sin, seq);
+#endif
Since ENABLE_TRACEROUTE6 code does not reuse common code,
there is this growth:
function old new delta
common_traceroute_main - 4400 +4400
traceroute_main 2927 11 -2916
+#if ENABLE_TRACEROUTE6
+ else if (af == AF_INET6) {
+ struct sockaddr_in6 firsthop = dest_lsa->u.sin6;
+ len_and_sockaddr *source_lsa;
+ int probe_fd = xsocket(AF_INET6, SOCK_DGRAM, 0);
+
+ if (op & OPT_DEVICE) {
+ setsockopt_bindtodevice(probe_fd, device);
+ }
+ firsthop.sin6_port = htons(1025);
+ xconnect(probe_fd, (struct sockaddr*)&firsthop, sizeof(firsthop));
+ source_lsa = get_sock_lsa(probe_fd);
+ if (source_lsa == NULL)
+ bb_error_msg_and_die("can't get probe addr");
+ close(probe_fd);
+ source_lsa->u.sin6.sin6_port = 0;
+
+ xbind(sndsock, &source_lsa->u.sa, source_lsa->len);
+ xbind(rcvsock, &source_lsa->u.sa, source_lsa->len);
+ free(source_lsa);
+ }
+#endif
What this code does? Why we don't do it for IPv4?
It also can be optimized to use dest_lsa instead of
creating a copy, and written in af-agnostic manner:
else if (af == AF_INET6) {
len_and_sockaddr *source_lsa;
int probe_fd = xsocket(af, SOCK_DGRAM, 0);
if (op & OPT_DEVICE)
setsockopt_bindtodevice(probe_fd, device);
set_nport(dest_lsa, htons(1025));
/* dummy connect. makes kernel pick source IP (and port) */
xconnect(probe_fd, &dest_lsa->u.sa, dest_lsa->len);
/* read IP and port */
source_lsa = get_sock_lsa(probe_fd);
if (source_lsa == NULL)
bb_error_msg_and_die("can't get probe addr");
close(probe_fd);
/* bind our sockets to this IP (but not port) */
set_nport(source_lsa, 0);
xbind(sndsock, &source_lsa->u.sa, source_lsa->len);
xbind(rcvsock, &source_lsa->u.sa, source_lsa->len);
free(source_lsa);
}
+#if !ENABLE_TRACEROUTE6
+# define send_probe(seq, ttl, af) send_probe(seq, ttl)
+#endif
static void
-send_probe(int seq, int ttl)
+send_probe(int seq, int ttl, sa_family_t af)
af is always available as global dest_lsa->u.sa.sa_family,
no need to have af parameter.
len_and_sockaddr from_lsa, lastaddr;
int probe;
int unreachable = 0; /* counter */
int gotlastaddr = 0; /* flags */
int got_there = 0;
int first = 1;
memset(&lastaddr, 0, sizeof(lastaddr));
#if ENABLE_TRACEROUTE6
from_lsa.len = (af == AF_INET6) ? sizeof(struct sockaddr_in6)
: sizeof(struct sockaddr_in);
from_lsa.u.sa.sa_family = af;
lastaddr.len = (af == AF_INET6) ? sizeof(struct in6_addr)
: sizeof(struct in_addr);
#else
from_lsa.len = sizeof(struct sockaddr_in);
from_lsa.u.sa.sa_family = AF_INET;
lastaddr.len = sizeof(struct in_addr);
#endif
printf("%2d", ttl);
for (probe = 0; probe < nprobes; ++probe) {
int cc;
unsigned t1;
unsigned t2;
struct ip *ip;
Even though this will work, this is logically wrong.
You should not use the knowledge that len_and_sockaddr
is big enough for any address. Instead, xzalloc new
len_and_sockaddr exactly the same size as dest_lsa.
This is logically correct - it's address family agnostic.
#if ENABLE_TRACEROUTE6
struct sockaddr_in6 to;
#endif
Ugly again. Look how it can be done:
from_lsa = dup_sockaddr(dest_lsa);
lastaddr = xzalloc(dest_lsa->len);
to = xzalloc(dest_lsa->len);
for (ttl = first_ttl; ttl <= max_ttl; ++ttl) {
int probe;
int unreachable = 0; /* counter */
int gotlastaddr = 0; /* flags */
int got_there = 0;
int first = 1;
printf("%2d", ttl);
for (probe = 0; probe < nprobes; ++probe) {
int read_len;
unsigned t1;
unsigned t2;
struct ip *ip;
if (!first && pausemsecs > 0)
usleep(pausemsecs * 1000);
fflush_all();
t1 = monotonic_us();
send_probe(++seq, ttl);
first = 0;
while ((read_len = wait_for_reply(from_lsa, to)) != 0) {
t2 = monotonic_us();
i = packet_ok(read_len, from_lsa, to, seq);
/* Skip short packet */
if (i == 0)
continue;
if (!gotlastaddr
|| (memcmp(lastaddr, &from_lsa->u.sa, from_lsa->len) != 0)
) {
print(read_len, &from_lsa->u.sa, to);
memcpy(lastaddr, &from_lsa->u.sa, from_lsa->len);
gotlastaddr = 1;
}
Not a single line with sockaddr_in[6] casts and such!
Please review attached patch.
Please try current git, I tested only "traceroute -6 ::1" ...
--
vda
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 4.patch
Type: text/x-diff
Size: 25068 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20091123/c7e49ed3/attachment-0001.bin>
More information about the busybox
mailing list