[00:28:53] --- abo has left [00:49:53] --- Simon Wilkinson has left [00:54:27] --- haba has become available [00:56:31] As soon as I want to link in ubik_VL_RegisterAddrs() into vos, it wants to link in stuff from strange places. /usr/src/openafs-1.5.71/src/vol/volume.c:3558: undefined reference to `DFlushVolume' /usr/src/openafs-1.5.71/lib/vlib.a(volume.o): In function `VPutVolume_r': /usr/src/openafs-1.5.71/src/vol/volume.c:3514: undefined reference to `DFlushVolume' [00:58:53] --- dev-zero@jabber.org has become available [01:06:09] Or to ask it the other way around, why would ubik_VL_RegisterAddrs need any more libraries that ubik_VL_GetAddrsU ? [01:17:19] OH, no, now I think I see it *bangs head* [01:17:30] (cut-paste-error methinks) [01:58:12] --- pod has become available [02:15:58] addrs.bulkaddrs_val = (afs_uint32 *) FS_HostAddrs_HBO; code = ubik_VL_RegisterAddrs(cstruct, 0, &FS_HostUUID, 0, &addrs); From viced.c line 1835. IN HOST BYTE ORDER???? [02:26:16] --- haba has left [02:26:24] --- Simon Wilkinson has become available [02:26:51] --- Simon Wilkinson has left [02:44:12] --- Jeffrey Altman has become available [02:44:39] --- jaltman has left: Replaced by new connection [02:44:40] --- jaltman has become available [02:49:54] --- manfred has become available [02:53:04] --- haba has become available [03:33:29] What do you think about the comments in this function (found at line ~200 in vos.c): /* * Parse a server name/address and return the address in HOST BYTE order */ afs_int32 GetServer(char *aname) { register struct hostent *th; afs_int32 addr; int b1, b2, b3, b4; register afs_int32 code; char hostname[MAXHOSTCHARS]; code = sscanf(aname, "%d.%d.%d.%d", &b1, &b2, &b3, &b4); if (code == 4) { addr = (b1 << 24) | (b2 << 16) | (b3 << 8) | b4; addr = ntohl(addr); /* convert to host order */ } else { th = gethostbyname(aname); if (!th) return 0; memcpy(&addr, th->h_addr, sizeof(addr)); } if (addr == htonl(0x7f000001)) { /* local host */ code = gethostname(hostname, MAXHOSTCHARS); if (code) return 0; th = gethostbyname(hostname); /* returns host byte order */ if (!th) return 0; memcpy(&addr, th->h_addr, sizeof(addr)); } return (addr); } [03:35:21] To me it looks like there like addr is in NET byte order and the ntohl() should be a htonl(). [04:02:17] --- manfred has left [04:24:32] --- manfred has become available [04:40:34] --- manfred has left [04:58:01] --- meffie has become available [05:18:11] --- dev-zero@jabber.org has left [05:18:28] --- dev-zero@jabber.org has become available [05:24:44] whichever it should be, the ordering appears to be inconsistent. [05:33:46] --- Jeffrey Altman has left [05:42:59] --- dev-zero@jabber.org has left [05:43:02] --- dev-zero@jabber.org has become available [05:47:12] jaltman (and others): The memcpy(&addr, th->h_addr, sizeof(addr)) of the th from gethostbyname() is the most prominent giveaway that the whole thing is meant to be in host byte order. When I assume that the function actually returns addr in net byte order, everything else fits and falls in place. [05:48:57] > if (addr == htonl(0x7f000001)) [05:49:56] ntohl is right: we're converting a number to NET so we can compre to something in NET [05:51:24] The addr = ntohl(addr); should be a addr = htonl(addr); [05:51:44] but for the result it does not make any difference. [05:52:22] the addr is in net, in contrast to all the comments sprinkled in this function. [05:53:34] Patch will be coming [06:03:01] --- dev-zero@jabber.org has left [06:03:55] --- dev-zero@jabber.org has become available [06:04:26] Please have a look at /afs/stacken.kth.se/home/haba/Public/vos-setaddrs.patch (against 1.5.71) [06:14:00] --- dev-zero@jabber.org has left [06:14:16] --- dev-zero@jabber.org has become available [06:16:28] --- dev-zero@jabber.org has left [06:16:43] --- dev-zero@jabber.org has become available [06:22:36] --- dev-zero@jabber.org has left [06:23:38] --- dev-zero@jabber.org has become available [06:36:46] --- dev-zero@jabber.org has left [06:37:01] --- dev-zero@jabber.org has become available [07:04:19] when some of yinz are bored, read the afs_osi_TimedSleep patch. note that it is not "in theory" different from what afs_osi_Wait is supposed to do, but see my first "comment". also, the interface is symmetric with all our other sleep stuff (uses an event instead of a wait handle) [07:22:29] --- deason has become available [07:49:43] --- dev-zero@jabber.org has left [07:50:17] --- dev-zero@jabber.org has become available [08:01:04] --- dev-zero@jabber.org has left [08:01:08] --- dev-zero@jabber.org has become available [08:05:47] --- reuteras has left [08:21:28] --- dev-zero@jabber.org has left [08:43:54] haba: push your patch to gerrit and we can comment on it there. [08:51:13] * haba feeling too green on this git stuff [08:52:08] > don't use ext3 Well, these are Solaris 9 servers using inode on UFS. [08:52:47] there's a wiki page that gives you recipes [08:52:55] http://www.dementia.org/twiki/bin/view/AFSLore/GitDevelopers [08:53:20] yes, that one [08:55:20] haba: are you sure that's the right byte order for the addresses? it looks like you're sending hbo... which is what the comments in viced and vlserver suggest, but I can't see how that's right [08:55:50] I can't shake the feeling that someone mixed up host- and net- order when writing a lot of this, which is making it rather confusing to read [08:55:57] I have until 18:00. That procedure won't get it done until then (including registering yet another ID). [08:56:00] ("this" as in, RegisterAddrs, not haba's patch) [08:58:28] deason: While debugging, I figured out: yes, it is mixed up but happens to work currently and will continue to work after my patch which in that sense mostly corrects the comments and one htonl() -> ntohl() which is not functional change. [08:59:22] maybe there first should be a patch that fixes the confusion and then a second patch for the new functionality. [08:59:44] well, I see you fixed the GetServer comments... so, GetServer actually returns an addr in NBO, right? [09:00:15] keep fixes separate from new functionality, yes [09:00:26] I would've thought you would just take the results from getserver then, to send on the wire.... you ntohl them again before sending, which is confusing me [09:00:41] deason: Yes. If you give GetServer a name, it uses gethostbyname which definitely returns NBO. [09:00:48] wire addresses must be NBO [09:02:03] jaltman: I don't know how it works, but look at the call for VL_RegisterAddrs in Do_VLRegisterRPC in viced.c. [09:02:19] I think the comments on that are just completely backwards [09:02:24] but consistently so, so it works [09:02:45] like, I mean, is it just me or is this comment just completely backwards? * This routine sets up the buffers for the VL_RegisterAddrs RPC. All addresses * in FS_HostAddrs[] are in NBO, while the RPC treats them as a "blob" of data * and so we need to convert each of them into HBO which is what the extra * array called FS_HostAddrs_HBO is used here. [09:04:05] deason: Precisely. And if I feed it into the RPC in HBO, it WORKS. If I feed it into the RPC in NBO, the next vos listaddr shows it backwards. [09:04:10] > wire addresses must be NBO Yes, _on the wire_. RPC arguments must be _host_ order, because XDR is going to take care of the conversion for you. [09:04:31] oh, that's where I got messed up, right right right [09:04:36] jhutz: Ok, that explains that. [09:04:41] If your client and server have the same host byte order, you won't notice, but if they are different, XDR will swap on one end but not the other. [09:05:53] you can do a vos listaddr -cell stacken.kth.se and find the uuid 008f157e-4d1f-157c-a3-e8-357c4d1faa77 with IP 1.2.3.5 and 10.10.10.10. [09:06:01] that comment above still seems wrong, but less so (the addresses are _not_ a 'blob'; they are converted by xdr) [09:06:48] the reason FS_HostAddrs[] is in NBO is because we are going to use those with Rx directly which requires NBO. The FS_HostAddrs_HBO[] is host byte order because that is as jhutz says. [09:07:10] Yes, the comment in viced.c says it's in HBO but for the wrong reason. [09:07:29] Yes, the comments surrounding this stuff are terribly broken, probably because they were at least partially written by someone who didn't actually understand what was going on. But the code is mostly right, aside from the "ntohl" that haba mentioned that should be htonl. [09:07:58] well well, on monday I will test what will blow up if I actually change addrs of life servers. [09:09:36] Until then - weekend! Have a nice weekend you too! (My weekend list contains "car repair", "model railroad" and "parent visit") [09:09:52] *poff* [09:10:30] --- haba has left [09:42:09] Guess what this always prints? (void)afs_snprintf(tmpStr, sizeof tmpStr, " CPS-%d is [", client->CPS.prlist_len); (void)STREAM_WRITE(tmpStr, strlen(tmpStr), 1, file); if (client->CPS.prlist_val) { for (i = 0; i > client->CPS.prlist_len; i++) { (void)afs_snprintf(tmpStr, sizeof tmpStr, " %d", client->CPS.prlist_val[i]); (void)STREAM_WRITE(tmpStr, strlen(tmpStr), 1, file); } } sprintf(tmpStr, "]\n"); [09:42:54] well, given the use of prlist_len, probably "nothing useful" [09:45:00] --- Russ has become available [09:49:14] --- kaj has left [10:01:37] --- meffie has left [10:56:59] --- deason has left [10:57:34] --- deason has become available [11:00:56] --- kaj has become available [11:09:16] --- kaj has left [11:09:34] --- jaltman has left: Disconnected [11:09:43] --- jaltman has become available [11:16:46] --- kaj has become available [13:28:50] --- Simon Wilkinson has become available [13:32:00] I wonder if we should have an XDR int type that essential means "I know what I'm doing, don't mess with this", and always takes data in network byte order. [13:37:32] for a single int, the current overhead seems really small; if you have a big array of them... there's always opaque [13:38:29] Yeh. It's more from a clarity perspective than anything else. [13:38:50] I guess, fundmentally, the problem is that a network address isn't an int - it's a specifically ordered set of octets. [13:59:05] Which can be quite a bit longer than an int once we do IPv6, too. [13:59:17] Oh yes. [13:59:48] Actually, in a number of places we already take an (int, uuid) union as a means of identifying a server. I suspect we'll end up seeing a lot more of them! [14:22:28] --- Simon Wilkinson has left [14:48:22] --- mdionne has become available [16:05:11] --- deason has left [16:19:18] --- dev-zero@jabber.org has become available [16:30:17] --- dev-zero@jabber.org has left: offline [17:58:00] --- Russ has left: Disconnected [18:23:38] --- Russ has become available [18:37:31] --- Russ has left: Disconnected [18:48:47] --- Russ has become available [19:17:33] --- Rrrrrred has become available [19:24:27] --- mdionne has left [19:57:13] --- deason has become available [20:15:23] --- manfred has become available [20:32:10] --- manfred has left [20:50:13] --- Rrrrrred has left [21:11:07] --- deason has left [23:30:09] --- dev-zero@jabber.org has become available [23:50:53] --- Simon Wilkinson has become available