[PATCH v2 01/10] ethdev: introduce flow pre-configuration hints
Thomas Monjalon
thomas at monjalon.net
Wed Jan 26 12:21:31 CET 2022
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.
More information about the dev
mailing list