Re: [PATCH 2/2] Remove varargs from AdapterVifCallback.

["Durrant, Paul" <xadimgnik@xxxxxxxxx>]

On 19/05/2022 15:37, Martin Harvey wrote:
> > -----Original Message-----
> > From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
> > Durrant, Paul
> > Sent: 19 May 2022 13:54
> > To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> > Subject: Re: [PATCH 2/2] Remove varargs from AdapterVifCallback.
> >
> > > -/* V9 is exactly the same as V8 */
> > > -typedef struct _XENVIF_VIF_INTERFACE_V8 XENVIF_VIF_INTERFACE_V9;
> > No, you can't do that. The point of the versioning is that we make no 
> > incompatible change. You need to introduce v9 in its final form.
> OK. I have submitted two patches in the series in order to give you a choice - 
> as I will explain:
>
> 1) I assume you have noted the "-", to indicate that that these lines are 
> present after applying one of the two patches, but not both.
>
> 2) Would you like V9 to be just the interface as it is after [1/2] has been 
> applied to both xenvif and xennet (keep varargs), or as it is after both 
> patches have been applied to both xenvif and xennet.
>
> 3) I talked this thru with owen and we agree that the interface changes and 
> version bump for the interface and associated ID's should happen in one atomic 
> patch / change to the codebase. In which case, I should probably just be 
> submitting one patch. However, I have submitted it in two halves because I 
> don't know whether you want the varargs changes or not.
>
> 4) If we don't remove varargs (1 of 2 patches on both), I *have* to update the 
> version number, because the interfaces are not binary compatible. However, the 
> function prototypes for all the calls are, because the difference is hidden 
> away in varargs, hence the definition.
>
> So, going back to previous points:
>
> 1) Would you like one or both?
I think we can have both.

Let's have the infrastructure for the back-pressure in XENVIF first, but 
keep the same VIF interface for now (hence meaning no functional chang 
for the moment), noting in the commit comment that the new functionality 
will be used in a subsequent patch.
Then do the interface change. The general convention is to add a 
'_V<number>' to the end of anything you are superseding where the number 
is the version number which first saw that variant. As we have retired 
vif interface versions older than 6, I think you should rename:
XENVIF_VIF_CALLBACK -> XENVIF_VIF_CALLBACK_V6
XENVIF_VIF_ENABLE -> XENVIF_VIF_ENABLE_V6 (and have it reference 
XENVIF_VIF_CALLBACK_V6 rather than XENVIF_VIF_CALLBACK)
and then adjust the v6, v7 and v8 interface structs accordingly.

In the same patch define new versions of XENVIF_VIF_CALLBACK and 
XENVIF_VIF_ENABLE for your v9 interface, and also your 
XENVIF_VIF_CALLBACK_PARAMS union which the new XENVIF_VIF_CALLBACK will 
reference.
Hence the option of backpressure is now available.

These patches then need to be verified with an unmodified xennet still 
acquiring v8 of the interface.
Then update XENNET by copying the new header across into the repo and 
make use of the back-pressure... that can be done in one commit I think.
  Paul

> 2) If only one, do you want me to declare a v9 interface which has an identical 
> set of function calls (because the difference is hidden in varargs 
> implementation)?


> MH.