[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
  • From: "Durrant, Paul" <xadimgnik@xxxxxxxxx>
  • Date: Thu, 19 May 2022 13:54:24 +0100
  • Delivery-date: Thu, 19 May 2022 12:54:30 +0000
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

On 13/05/2022 11:24, Martin Harvey wrote:
VifEnable allows the client to pass in an adapter callback,
which is called to push and return packets, and signal
adapter state changes. The parameters for this callback
were never fully specified (depending on varargs).
This change formally specifies those params in the Vif interface.

Signed-off-by: Martin Harvey <martin.harvey@xxxxxxxxxx>
---
  include/vif_interface.h | 104 +++++++++++++++++++++++++++---
  src/xenvif/vif.c        | 139 +++++++++++++++++++++++++++++-----------
  2 files changed, 195 insertions(+), 48 deletions(-)

diff --git a/include/vif_interface.h b/include/vif_interface.h
index c034657..c416f81 100644
--- a/include/vif_interface.h
+++ b/include/vif_interface.h
@@ -408,7 +408,7 @@ typedef VOID
      IN  PINTERFACE  Interface
      );
-/*! \typedef XENVIF_VIF_CALLBACK
+/*! \typedef XENVIF_VIF_CALLBACK_VARARGS
      \brief Provider to subscriber callback function
\param Argument An optional context argument passed to the callback
@@ -436,13 +436,72 @@ typedef VOID
      No additional arguments
  */
  typedef VOID
-(*XENVIF_VIF_CALLBACK)(
+(*XENVIF_VIF_CALLBACK_VARARGS)(
      IN  PVOID                       Argument OPTIONAL,
      IN  XENVIF_VIF_CALLBACK_TYPE    Type,
      ...
      );
-/*! \typedef XENVIF_VIF_ENABLE
+/*! \typedef XENVIF_VIF_ENABLE_VARARGS
+    \brief Enable the VIF interface
+
+    All packets queued for transmit will be rejected and no packets will
+    be queued for receive until this method completes.
+
+    \param Interface The interface header
+    \param Callback The subscriber's callback function
+    \param Argument An optional context argument passed to the callback
+*/
+typedef NTSTATUS
+(*XENVIF_VIF_ENABLE_VARARGS)(
+    IN  PINTERFACE          Interface,
+    IN  XENVIF_VIF_CALLBACK_VARARGS Callback,
+    IN  PVOID               Argument OPTIONAL
+    );
+
+
+typedef union _XENVIF_VIF_CALLBACK_PARAMS {
+    struct _XENVIF_VIF_CALLBACK_RETURN_PACKET_PARAMS {
+        PVOID Cookie;
+        PXENVIF_TRANSMITTER_PACKET_COMPLETION_INFO Completion;
+    } ReturnPacketParams;
+    struct _XENVIF_VIF_CALLBACK_QUEUE_PACKET_PARAMS {
+        ULONG Index;
+        PMDL Mdl;
+        ULONG Offset;
+        ULONG Length;
+        XENVIF_PACKET_CHECKSUM_FLAGS Flags;
+        USHORT MaximumSegmentSize;
+        USHORT TagControlInformation;
+        PXENVIF_PACKET_INFO Info;
+        PXENVIF_PACKET_HASH Hash;
+        BOOLEAN More;
+        PVOID Cookie;
+        PBOOLEAN NdisFinished;
+    } QueuePacketParams;
+    /*
+    struct _XENVIF_VIF_CALLBACK_STATE_CHANGE_PARAMS {
+        No parameters here
+    } StateChangeParams;
+    */
+} XENVIF_VIF_CALLBACK_PARAMS, *PXENVIF_VIF_CALLBACK_PARAMS;
+
+/*! \typedef XENVIF_VIF_CALLBACK
+    \brief Provider to subscriber callback function
+
+    \param Argument An optional context argument passed to the callback
+    \param Type The callback type
+    \param Params Additional paramaters required by \a Type
+*/
+
+typedef VOID
+(*XENVIF_VIF_CALLBACK)(
+    IN  PVOID                       Argument OPTIONAL,
+    IN  XENVIF_VIF_CALLBACK_TYPE    Type,
+    IN  PXENVIF_VIF_CALLBACK_PARAMS Params
+    );
+
+/*! \typedef XENVIF_VIF_ENABLE_VARARGS
      \brief Enable the VIF interface
All packets queued for transmit will be rejected and no packets will
@@ -834,7 +893,7 @@ struct _XENVIF_VIF_INTERFACE_V6 {
      INTERFACE                                       Interface;
      XENVIF_VIF_ACQUIRE                              Acquire;
      XENVIF_VIF_RELEASE                              Release;
-    XENVIF_VIF_ENABLE                               Enable;
+    XENVIF_VIF_ENABLE_VARARGS                       Enable;
      XENVIF_VIF_DISABLE                              Disable;
      XENVIF_VIF_QUERY_STATISTIC                      QueryStatistic;
      XENVIF_VIF_QUERY_RING_COUNT                     QueryRingCount;
@@ -868,7 +927,7 @@ struct _XENVIF_VIF_INTERFACE_V7 {
      INTERFACE                                       Interface;
      XENVIF_VIF_ACQUIRE                              Acquire;
      XENVIF_VIF_RELEASE                              Release;
-    XENVIF_VIF_ENABLE                               Enable;
+    XENVIF_VIF_ENABLE_VARARGS                       Enable;
      XENVIF_VIF_DISABLE                              Disable;
      XENVIF_VIF_QUERY_STATISTIC                      QueryStatistic;
      XENVIF_VIF_QUERY_RING_COUNT                     QueryRingCount;
@@ -902,7 +961,7 @@ struct _XENVIF_VIF_INTERFACE_V8 {
      INTERFACE                                       Interface;
      XENVIF_VIF_ACQUIRE                              Acquire;
      XENVIF_VIF_RELEASE                              Release;
-    XENVIF_VIF_ENABLE                               Enable;
+    XENVIF_VIF_ENABLE_VARARGS                       Enable;
      XENVIF_VIF_DISABLE                              Disable;
      XENVIF_VIF_QUERY_STATISTIC                      QueryStatistic;
      XENVIF_VIF_QUERY_RING_COUNT                     QueryRingCount;
@@ -928,10 +987,37 @@ struct _XENVIF_VIF_INTERFACE_V8 {
      XENVIF_VIF_MAC_QUERY_FILTER_LEVEL               MacQueryFilterLevel;
  };
-/* 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.

  Paul

+struct _XENVIF_VIF_INTERFACE_V9 {
+    INTERFACE                                       Interface;
+    XENVIF_VIF_ACQUIRE                              Acquire;
+    XENVIF_VIF_RELEASE                              Release;
+    XENVIF_VIF_ENABLE                               Enable;
+    XENVIF_VIF_DISABLE                              Disable;
+    XENVIF_VIF_QUERY_STATISTIC                      QueryStatistic;
+    XENVIF_VIF_QUERY_RING_COUNT                     QueryRingCount;
+    XENVIF_VIF_UPDATE_HASH_MAPPING                  UpdateHashMapping;
+    XENVIF_VIF_RECEIVER_RETURN_PACKET               ReceiverReturnPacket;
+    XENVIF_VIF_RECEIVER_SET_OFFLOAD_OPTIONS         ReceiverSetOffloadOptions;
+    XENVIF_VIF_RECEIVER_SET_BACKFILL_SIZE           ReceiverSetBackfillSize;
+    XENVIF_VIF_RECEIVER_QUERY_RING_SIZE             ReceiverQueryRingSize;
+    XENVIF_VIF_RECEIVER_SET_HASH_ALGORITHM          ReceiverSetHashAlgorithm;
+    XENVIF_VIF_RECEIVER_QUERY_HASH_CAPABILITIES     
ReceiverQueryHashCapabilities;
+    XENVIF_VIF_RECEIVER_UPDATE_HASH_PARAMETERS      
ReceiverUpdateHashParameters;
+    XENVIF_VIF_TRANSMITTER_QUEUE_PACKET             TransmitterQueuePacket;
+    XENVIF_VIF_TRANSMITTER_QUERY_OFFLOAD_OPTIONS    
TransmitterQueryOffloadOptions;
+    XENVIF_VIF_TRANSMITTER_QUERY_LARGE_PACKET_SIZE  
TransmitterQueryLargePacketSize;
+    XENVIF_VIF_TRANSMITTER_QUERY_RING_SIZE          TransmitterQueryRingSize;
+    XENVIF_VIF_MAC_QUERY_STATE                      MacQueryState;
+    XENVIF_VIF_MAC_QUERY_MAXIMUM_FRAME_SIZE         MacQueryMaximumFrameSize;
+    XENVIF_VIF_MAC_QUERY_PERMANENT_ADDRESS          MacQueryPermanentAddress;
+    XENVIF_VIF_MAC_QUERY_CURRENT_ADDRESS            MacQueryCurrentAddress;
+    XENVIF_VIF_MAC_QUERY_MULTICAST_ADDRESSES        MacQueryMulticastAddresses;
+    XENVIF_VIF_MAC_SET_MULTICAST_ADDRESSES          MacSetMulticastAddresses;
+    XENVIF_VIF_MAC_SET_FILTER_LEVEL                 MacSetFilterLevel;
+    XENVIF_VIF_MAC_QUERY_FILTER_LEVEL               MacQueryFilterLevel;
+};
-typedef XENVIF_VIF_INTERFACE_V9 XENVIF_VIF_INTERFACE, *PXENVIF_VIF_INTERFACE;
+typedef struct _XENVIF_VIF_INTERFACE_V9 XENVIF_VIF_INTERFACE, 
*PXENVIF_VIF_INTERFACE;
/*! \def XENVIF_VIF
      \brief Macro at assist in method invocation
diff --git a/src/xenvif/vif.c b/src/xenvif/vif.c
index b026e5b..0c43e0c 100644
--- a/src/xenvif/vif.c
+++ b/src/xenvif/vif.c
@@ -51,7 +51,8 @@ struct _XENVIF_VIF_CONTEXT {
      PXENVIF_FRONTEND            Frontend;
      BOOLEAN                     Enabled;
      ULONG                       Version;
-    XENVIF_VIF_CALLBACK         Callback;
+    XENVIF_VIF_CALLBACK_VARARGS CallbackVarargs;
+    XENVIF_VIF_CALLBACK         CallbackParams;
      PVOID                       Argument;
      PXENVIF_THREAD              MacThread;
      KEVENT                      MacEvent;
@@ -106,8 +107,19 @@ VifMac(
              break;
if (Context->Enabled)
-            Context->Callback(Context->Argument,
-                              XENVIF_MAC_STATE_CHANGE);
+        {
+            if (Context->Version < 9) {
+                Context->CallbackVarargs(Context->Argument,
+                                XENVIF_MAC_STATE_CHANGE);
+            } else {
+                XENVIF_VIF_CALLBACK_PARAMS Params;
+
+                RtlZeroMemory(&Params, sizeof(Params));
+                Context->CallbackParams(Context->Argument,
+                                XENVIF_MAC_STATE_CHANGE,
+                                &Params);
+            }
+        }
KeSetEvent(&Context->MacEvent, IO_NO_INCREMENT, FALSE);
      }
@@ -138,10 +150,11 @@ VifSuspendCallbackLate(
  }
static NTSTATUS
-VifEnable(
+VifEnableCommon(
      IN  PINTERFACE          Interface,
-    IN  XENVIF_VIF_CALLBACK Callback,
-    IN  PVOID               Argument
+    IN  PVOID               Callback,
+    IN  PVOID               Argument,
+    IN  BOOLEAN             Varargs
      )
  {
      PXENVIF_VIF_CONTEXT     Context = Interface->Context;
@@ -157,7 +170,13 @@ VifEnable(
      if (Context->Enabled)
          goto done;
- Context->Callback = Callback;
+    if (Varargs) {
+        Context->CallbackVarargs = (XENVIF_VIF_CALLBACK_VARARGS) Callback;
+        Context->CallbackParams = NULL;
+    } else {
+        Context->CallbackVarargs = NULL;
+        Context->CallbackParams = (XENVIF_VIF_CALLBACK)Callback;
+    }
      Context->Argument = Argument;
Context->Enabled = TRUE;
@@ -226,7 +245,8 @@ fail1:
      KeMemoryBarrier();
Context->Argument = NULL;
-    Context->Callback = NULL;
+    Context->CallbackVarargs = NULL;
+    Context->CallbackParams = NULL;
if (Exclusive)
          ReleaseMrswLockExclusive(&Context->Lock, Irql, FALSE);
@@ -236,6 +256,28 @@ fail1:
      return status;
  }
+static FORCEINLINE NTSTATUS
+VifEnableVarargs(
+    IN  PINTERFACE                      Interface,
+    IN  XENVIF_VIF_CALLBACK_VARARGS     Callback,
+    IN  PVOID                           Argument
+    ) {
+    BUG_ON(Interface->Version >= 9);
+
+    return VifEnableCommon(Interface, (PVOID)Callback, Argument, TRUE);
+}
+
+static FORCEINLINE NTSTATUS
+VifEnableParams(
+    IN  PINTERFACE                      Interface,
+    IN  XENVIF_VIF_CALLBACK             Callback,
+    IN  PVOID                           Argument
+    ) {
+    BUG_ON(Interface->Version < 9);
+
+    return VifEnableCommon(Interface, (PVOID)Callback, Argument, FALSE);
+}
+
  static VOID
  VifDisable(
      IN  PINTERFACE      Interface
@@ -285,7 +327,8 @@ VifDisable(
      XENBUS_SUSPEND(Release, &Context->SuspendInterface);
Context->Argument = NULL;
-    Context->Callback = NULL;
+    Context->CallbackVarargs = NULL;
+    Context->CallbackParams = NULL;
ReleaseMrswLockShared(&Context->Lock); @@ -832,7 +875,7 @@ static struct _XENVIF_VIF_INTERFACE_V6 VifInterfaceVersion6 = {
      { sizeof (struct _XENVIF_VIF_INTERFACE_V6), 6, NULL, NULL, NULL },
      VifAcquire,
      VifRelease,
-    VifEnable,
+    VifEnableVarargs,
      VifDisable,
      VifQueryStatistic,
      VifQueryRingCount,
@@ -862,7 +905,7 @@ static struct _XENVIF_VIF_INTERFACE_V7 VifInterfaceVersion7 
= {
      { sizeof (struct _XENVIF_VIF_INTERFACE_V7), 7, NULL, NULL, NULL },
      VifAcquire,
      VifRelease,
-    VifEnable,
+    VifEnableVarargs,
      VifDisable,
      VifQueryStatistic,
      VifQueryRingCount,
@@ -892,7 +935,7 @@ static struct _XENVIF_VIF_INTERFACE_V8 VifInterfaceVersion8 
= {
      { sizeof (struct _XENVIF_VIF_INTERFACE_V8), 8, NULL, NULL, NULL },
      VifAcquire,
      VifRelease,
-    VifEnable,
+    VifEnableVarargs,
      VifDisable,
      VifQueryStatistic,
      VifQueryRingCount,
@@ -918,11 +961,11 @@ static struct _XENVIF_VIF_INTERFACE_V8 
VifInterfaceVersion8 = {
      VifMacQueryFilterLevel
  };
-static XENVIF_VIF_INTERFACE_V9 VifInterfaceVersion9 = {
-    { sizeof (XENVIF_VIF_INTERFACE_V9), 9, NULL, NULL, NULL },
+static struct _XENVIF_VIF_INTERFACE_V9 VifInterfaceVersion9 = {
+    { sizeof (struct _XENVIF_VIF_INTERFACE_V9), 9, NULL, NULL, NULL },
      VifAcquire,
      VifRelease,
-    VifEnable,
+    VifEnableParams,
      VifDisable,
      VifQueryStatistic,
      VifQueryRingCount,
@@ -1064,12 +1107,12 @@ VifGetInterface(
          break;
      }
      case 9: {
-        XENVIF_VIF_INTERFACE_V9 *VifInterface;
+        struct _XENVIF_VIF_INTERFACE_V9 *VifInterface;
- VifInterface = (XENVIF_VIF_INTERFACE_V9 *)Interface;
+        VifInterface = (struct _XENVIF_VIF_INTERFACE_V9 *)Interface;
status = STATUS_BUFFER_OVERFLOW;
-        if (Size < sizeof (XENVIF_VIF_INTERFACE_V9))
+        if (Size < sizeof (struct _XENVIF_VIF_INTERFACE_V9))
              break;
*VifInterface = VifInterfaceVersion9;
@@ -1134,7 +1177,7 @@ __VifReceiverQueuePacketVersion6(
      UNREFERENCED_PARAMETER(Index);
      UNREFERENCED_PARAMETER(More);
- Context->Callback(Context->Argument,
+    Context->CallbackVarargs(Context->Argument,
                        XENVIF_RECEIVER_QUEUE_PACKET,
                        Mdl,
                        Offset,
@@ -1165,7 +1208,7 @@ __VifReceiverQueuePacketVersion7(
  {
      UNREFERENCED_PARAMETER(Index);
- Context->Callback(Context->Argument,
+    Context->CallbackVarargs(Context->Argument,
                        XENVIF_RECEIVER_QUEUE_PACKET,
                        Mdl,
                        Offset,
@@ -1195,7 +1238,7 @@ __VifReceiverQueuePacketVersion8(
      IN  PVOID                           Cookie
      )
  {
-    Context->Callback(Context->Argument,
+    Context->CallbackVarargs(Context->Argument,
                        XENVIF_RECEIVER_QUEUE_PACKET,
                        Index,
                        Mdl,
@@ -1227,21 +1270,27 @@ __VifReceiverQueuePacket(
      IN  PVOID                           Cookie
      )
  {
+    XENVIF_VIF_CALLBACK_PARAMS Params;
      BOOLEAN Finished = FALSE;
-    Context->Callback(Context->Argument,
-                      XENVIF_RECEIVER_QUEUE_PACKET,
-                      Index,
-                      Mdl,
-                      Offset,
-                      Length,
-                      Flags,
-                      MaximumSegmentSize,
-                      TagControlInformation,
-                      Info,
-                      Hash,
-                      More,
-                      Cookie,
-                      &Finished);
+
+    RtlZeroMemory(&Params, sizeof(Params));
+
+    Params.QueuePacketParams.Index = Index;
+    Params.QueuePacketParams.Mdl = Mdl;
+    Params.QueuePacketParams.Offset = Offset;
+    Params.QueuePacketParams.Length = Length;
+    Params.QueuePacketParams.Flags = Flags;
+    Params.QueuePacketParams.MaximumSegmentSize = MaximumSegmentSize;
+    Params.QueuePacketParams.TagControlInformation = TagControlInformation;
+    Params.QueuePacketParams.Info = Info;
+    Params.QueuePacketParams.Hash = Hash;
+    Params.QueuePacketParams.More = More;
+    Params.QueuePacketParams.Cookie = Cookie;
+    Params.QueuePacketParams.NdisFinished = &Finished;
+
+    Context->CallbackParams(Context->Argument,
+                            XENVIF_RECEIVER_QUEUE_PACKET,
+                            &Params);
      return Finished;
  }
@@ -1347,10 +1396,22 @@ VifTransmitterReturnPacket(
  {
      BUG_ON(Context->Version < 6);
- Context->Callback(Context->Argument,
-                        XENVIF_TRANSMITTER_RETURN_PACKET,
-                        Cookie,
-                        Completion);
+    if (Context->Version < 9) {
+        Context->CallbackVarargs(Context->Argument,
+                           XENVIF_TRANSMITTER_RETURN_PACKET,
+                           Cookie,
+                           Completion);
+    } else {
+        XENVIF_VIF_CALLBACK_PARAMS Params;
+
+        RtlZeroMemory(&Params, sizeof(Params));
+        Params.ReturnPacketParams.Cookie = Cookie;
+        Params.ReturnPacketParams.Completion = Completion;
+
+        Context->CallbackParams(Context->Argument,
+                                XENVIF_TRANSMITTER_RETURN_PACKET,
+                                &Params);
+     }
  }
PXENVIF_THREAD




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.