On Thu, 3 Aug 2023 14:29:28 +1000
David Gibson
On Thu, Aug 03, 2023 at 12:09:16PM +1000, David Gibson wrote:
On Thu, Aug 03, 2023 at 12:47:29AM +0200, Stefano Brivio wrote: [snip]
-void nl_link(int ns, unsigned int ifi, void *mac, int up, int mtu) +void nl_link_get_mac(int ns, unsigned int ifi, void *mac) { - int change = !MAC_IS_ZERO(mac) || up || mtu; struct req_t { struct nlmsghdr nlh; struct ifinfomsg ifm; - struct rtattr rta; - union { - unsigned char mac[ETH_ALEN]; - struct { - unsigned int mtu; - } mtu; - } set; } req = { - .nlh.nlmsg_type = change ? RTM_NEWLINK : RTM_GETLINK, - .nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)), - .nlh.nlmsg_flags = NLM_F_REQUEST | (change ? NLM_F_ACK : 0), + .nlh.nlmsg_type = RTM_GETLINK, + .nlh.nlmsg_len = sizeof(req),
I don't think there's a practical issue with this, but there were two reasons why I used NLMSG_LENGTH(sizeof(struct ifinfomsg)) instead:
- NLMSG_LENGTH() aligns to 4 bytes, not to whatever architecture-dependent alignment we might have: the message might actually be smaller
Oof... so. On the one hand, I see the issue; if these are different, I'm not sure what the effect will be. On the other hand, if we use NLMSG_LENGTH and it *is* longer than the structure size, we'll be saying that this message is longer than the datagram containing it. I'm not sure what the effect of that will be either.
Duh, sorry, I realized I had this backwards. NLSMSG_LENGTH() is the non-aligned length, sizeof() may include alignment. I'll rework based on that understanding.
Right, I was about to write you that... or rather, NLMSG_LENGTH() is the (presumably) lesser-aligned length. Also, if you check pretty much any example in iproute2, nlmsg_len is always set like that, using NLMSG_LENGTH() on the payload.
Not really sure what to do about this.
- I see that this works with gcc and clang, but, strictly speaking, is the size of the struct known "before" (sequence-point-wise) we're done initialising it? I have a very vague memory of this not working with gcc 2.9 or suchlike -- which is not a problem, as long as our new friend C11 actually supports this (but I'm not entirely sure).
I'm pretty sure it's ok, regardless of C11 state. It's not really a question of sequence points: those are about the ordering of run time operations. Even though the structure is being defined inline, determining it's size and layout will still happen at compile time, whereas the initialization is obviously a runtime event.
Ah, sorry, yes, of course. Still I remember that failing spectacularly in a distant past. But you just checked with gcc 4-ish I guess, so I guess it would have been fine anyway. -- Stefano