Re: [PATCH v2 01/10] ethdev: introduce flow pre-configuration hints

[Thomas Monjalon <thomas@monjalon.net>]

26/01/2022 11:52, Bruce Richardson:
> The scenario is as follows. Suppose we have the initial state as below:
> 
> struct x_dev_cfg {
>    int x;
> };
> 
> int
> x_dev_cfg(int dev_id, struct x_dev_cfg *cfg)
> {
>    struct x_dev *dev = x_devs[id];
>    // some setup/config may go here
>    return dev->configure(cfg, sizeof(cfg)); // sizeof(cfg) == 4
> }
> 
> Now, supposing we need to add in a new field into the config structure, a
> very common occurance. This will indeed break the ABI, so we need to use
> ABI versioning, to ensure that apps passing in the old structure, only call
> a function which expects the old structure. Therefore, we need a copy of
> the old structure, and a function to work on it. This gives this result:
> 
> struct x_dev_cfg {
> 	int x;
> 	bool flag; // new field;
> };
> 
> struct x_dev_cfg_v22 { // needed for ABI-versioned function
> 	int x;
> };
> 
> /* this function will only be called by *newly-linked* code, which uses
>  * the new structure */
> int
> x_dev_cfg(int dev_id, struct x_dev_cfg *cfg)
> {
>    struct x_dev *dev = x_devs[id];
>    // some setup/config may go here
>    return dev->configure(cfg, sizeof(cfg)); // sizeof(cfg) is now 8
> }
> 
> /* this function is called by apps linked against old version */
> int
> x_dev_cfg_v22(int dev_id, struct x_dev_cfg_v22 *cfg)
> {
>    struct x_dev *dev = x_devs[id];
>    // some setup/config may go here
>    return dev->configure((void *)cfg, sizeof(cfg)); // sizeof(cfg) is still 4
> }
> 
> With the above library code, we have different functions using the
> different structures, so ABI compatibility is preserved - apps passing in a
> 4-byte struct call a function using the 4-byte struct, while newer apps can
> use the 8-byte version.
> 
> The final part of the puzzle is then how drivers react to this change.
> Originally, all drivers only use "x" in the config structure because that
> is all that there is. That will still continue to work fine in the above
> case, as both 4-byte and 8-byte structs have the same x value at the same
> offset. i.e. no driver updates for x_dev is needed.
> 
> On the other hand, if there are drivers that do want/need the new field,
> they can also get to use it, but they do need to check for its presence
> before they do so, i.e they would work as below:
> 
> 	if (size_param > struct(x_dev_cfg_v22)) { // or "== struct(x_dev_cfg)"
> 		// use flags field
> 	}
> 
> Hope this is clear now.

Yes, this is the kind of explanation we need in our guideline doc.
Alternatives can be documented as well.
If we can list pros/cons in the doc, it will be easier to choose
the best approach and to explain the choice during code review.