On Thu, Aug 03, 2023 at 02:29:28PM +1000, David Gibson wrote:On Thu, Aug 03, 2023 at 12:09:16PM +1000, David Gibson wrote:Uhhh... then I realized I don't really see what that entails either. The basic problem is this, given a structure: struct req { struct nlmsghdr nlh; a_t a; b_t b; c_t c; } req; then, what's the correct req.nlh.nlmsg_length? sizeof(req) will probably work in practice, but as you say could be an overestimate if struct req has end padding. So try: payload_len = sizeof(req) - offsetof(struct req, a); NLMSG_LENGTH(payload_len) But.. that still relies on sizeof(req) so will overestimate in exactly the same circumstances. So try: NLMSG_LENGTH(sizeof(a_t) + sizeof(b_t) + sizeof(c_t)) For one thing that's bulky and annoying... but also it will *under*estimate the length if struct req has any mid-structure padding. Ok, so try: struct req { struct nlmsghdr nlg; struct payload { a_t a; b_t b; c_t c; } } req; NLMSG_LENGTH(sizeof(struct payload)) Well, if struct req has end padding, but struct payload doesn't, then this will be correct, but struct payload isn't guaranteed to lack end padding any more than struct req. At that point I'm out of ideas. So, I'm inclined to stick with sizeof(req) for its simplicity. This has the implicit requirement that we're always careful and ABI aware about the construction of our structures so that they don't have unexpected end padding. But.. then we're also relying on the structures having mid-padding only at the places that netlink expects it, which already requires some awareness of the ABI, so I'm not sure that avoiding the NLMSG_LENGTH() really loses us anything. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibsonOn Thu, Aug 03, 2023 at 12:47:29AM +0200, Stefano Brivio wrote:[snip]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.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.-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