On Sun, 2017-06-04 at 15:51 +0200, Jiri Pirko wrote: > Sun, Jun 04, 2017 at 03:43:14PM CEST, dledford@xxxxxxxxxx wrote: > > > > > This breaks uapi... > > > > We don't really care about this particular uapi breakage. > > > > First, this is the ib_uverbs_*ex*_create_qp struct, so this is > > already > > part of the "extensible uAPI" we created back when we included XRC > > support into the mainline kernel. A fundamental part of that uAPI > > was > > that we were allowed to extend structs (but not reorganize them) so > > that we could add new stuff to the end as we added features, but > > old > > apps would continue to work. As a result, it was understood that > > the > > uapi structs would continue to grow. > > > > Given that fact, if we put a reserved1 element into a struct to pad > > it > > out to a given size (which we did roughly 1 year ago with > > commit c70285f880e88 (IB/uverbs: Extend create QP to get RWQ > > indirection table) which made this change to the ex_create_qp > > struct: > > > > __u32 create_flags; > > + __u32 rwq_ind_tbl_handle; > > + __u32 reserved1; > > }; > > > > that doesn't mean a user of the uapi should directly reference this > > element, it means we added 32 bits out of a 64 bit line and we'll > > add > > the other 32 bits later, so don't reference that pad element by > > name, > > use the preferred means of clearing the struct prior to use: > > memset(sizeof(struct)) or calloc or equivalent. Given that only > > user > > space verbs providers that have been updated to support the RWQ > > indirection table would possibly ever even have seen this API > > element, > > and given that those should be fairly rare at this point (I doubt > > that > > any exist besides possibly libibverbs or rdma-core, I'm not sure if > > this support went into libibverbs before we merged it into rdma- > > core, > > but if it didn't, then only rdma-core would have been updated to > > know > > about these last two struct elements), I'm not going to have > > Mellanox > > put a bunch of ugly cruft in here for probably non-existent, but > > very > > recently updated if it even exists, consumers that don't follow > > best > > practice when accessing/clearing elements of an extensible struct. > > I understand all the reasons why this breakage should not cause any > harm, don't get me wrong Doug. It's just, it is a breakage, no matter > how pink you paint it :) Either the uapi is guaranteed to be backward > compatible, or not. Nothing in between. Just saying... We can make it explicit then that the uapi requires the use of memset/calloc on structs to zero them, and people are not allowed to access any reserved elements. Under that understanding, this is *not* a uapi break and it is an acceptable extension. This was more or less understood already, just adding a comment to the uapi header is all we need. -- Doug Ledford <dledford@xxxxxxxxxx> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
