[ovs-dev] [PATCH v2 4/4] datapath-windows: Fix VPORT when it is allocated by OVS

Paul Boca pboca at cloudbasesolutions.com
Thu Jul 28 11:14:11 UTC 2016


Acked-by: Paul Boca <pboca at cloudbasesolutions.com>

> -----Original Message-----
> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Alin Serdean
> Sent: Tuesday, July 26, 2016 6:45 PM
> To: dev at openvswitch.org
> Subject: [ovs-dev] [PATCH v2 4/4] datapath-windows: Fix VPORT when it is
> allocated by OVS
> 
> When an OID for an exeternal NIC create is done first try to find it
> by its friendly name.
> Also when an OID NIC delete is requested only remove it from the hyper-v
> hash array not from both.
> 
> Some parts of the code have been cleaned since they are not used anymore.
> 
> Signed-off-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
> ---
> v2: Rebase
> ---
>  datapath-windows/ovsext/Actions.c |  6 +--
>  datapath-windows/ovsext/Vport.c   | 89 ++++++++++++++++++------------------
> ---
>  datapath-windows/ovsext/Vport.h   | 23 ----------
>  3 files changed, 43 insertions(+), 75 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/Actions.c b/datapath-
> windows/ovsext/Actions.c
> index 9905a68..803b2b7 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -321,7 +321,7 @@ OvsDetectTunnelPkt(OvsForwardingContext
> *ovsFwdCtx,
> 
>              if (!vport ||
>                  (vport->ovsType != OVS_VPORT_TYPE_NETDEV &&
> -                 !OvsIsBridgeInternalVport(vport))) {
> +                 vport->ovsType != OVS_VPORT_TYPE_INTERNAL)) {
>                  ovsFwdCtx->tunKey.dst = 0;
>              }
>          }
> @@ -401,10 +401,6 @@ OvsAddPorts(OvsForwardingContext *ovsFwdCtx,
>      vport->stats.txBytes +=
>          NET_BUFFER_DATA_LENGTH(NET_BUFFER_LIST_FIRST_NB(ovsFwdCtx-
> >curNbl));
> 
> -    if (OvsIsBridgeInternalVport(vport)) {
> -        return NDIS_STATUS_SUCCESS;
> -    }
> -
>      if (OvsDetectTunnelPkt(ovsFwdCtx, vport, flowKey)) {
>          return NDIS_STATUS_SUCCESS;
>      }
> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-
> windows/ovsext/Vport.c
> index efb4b08..a2f7071 100644
> --- a/datapath-windows/ovsext/Vport.c
> +++ b/datapath-windows/ovsext/Vport.c
> @@ -97,6 +97,9 @@ static VOID OvsTunnelVportPendingRemove(PVOID
> context,
>                                          UINT32 *replyLen);
>  static NTSTATUS GetNICAlias(PNDIS_SWITCH_NIC_PARAMETERS nicParam,
>                              IF_COUNTED_STRING *portFriendlyName);
> +static NTSTATUS OvsConvertIfCountedStrToAnsiStr(PIF_COUNTED_STRING
> wStr,
> +                                                CHAR *str,
> +                                                UINT16 maxStrLen);
> 
>  /*
>   * --------------------------------------------------------------------------
> @@ -341,33 +344,53 @@ HvCreateNic(POVS_SWITCH_CONTEXT
> switchContext,
>          GetNICAlias(nicParam, &portFriendlyName);
>      }
> 
> -    NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0);
>      /*
>       * There can be one or more NICs for the external port. We create a vport
>       * structure for each such NIC, and each NIC inherits a lot of properties
>       * from the parent external port.
>       */
>      if (OvsIsRealExternalNIC(nicParam->NicType, nicParam->NicIndex)) {
> -        NDIS_SWITCH_PORT_PARAMETERS portParam;
> -        POVS_VPORT_ENTRY virtExtVport =
> -            (POVS_VPORT_ENTRY)switchContext->virtualExternalVport;
> -
> -        ASSERT(virtExtVport);
> +        /* The VPORT can be bound to OVS datapath already. Search for it
> +         * using its friendly name and if not found allocate a new port
> +         */
>          ASSERT(OvsFindVportByPortIdAndNicIndex(switchContext,
>                                                 nicParam->PortId,
>                                                 nicParam->NicIndex) == NULL);
> -        OvsCopyPortParamsFromVport(virtExtVport, &portParam);
> -        NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
> -        status = HvCreatePort(switchContext, &portParam,
> -                              nicParam->NicIndex);
> +        char convertString[256];
> +        RtlZeroMemory(convertString, 256);
> +        status = OvsConvertIfCountedStrToAnsiStr(&portFriendlyName,
> +                                                 convertString,
> +                                                 OVS_MAX_PORT_NAME_LENGTH);
>          NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0);
>          if (status != NDIS_STATUS_SUCCESS) {
> -            goto add_nic_done;
> +            goto done;
> +        }
> +        NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0);
> +        POVS_VPORT_ENTRY ovsVport =
> OvsFindVportByOvsName(switchContext,
> +                                                          convertString);
> +        if (ovsVport != NULL) {
> +            UpdateSwitchCtxWithVport(switchContext, ovsVport, FALSE);
> +            NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
> +        } else {
> +            NDIS_SWITCH_PORT_PARAMETERS portParam;
> +            POVS_VPORT_ENTRY virtExtVport =
> +                (POVS_VPORT_ENTRY)switchContext->virtualExternalVport;
> +
> +            ASSERT(virtExtVport);
> +            OvsCopyPortParamsFromVport(virtExtVport, &portParam);
> +            NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
> +            status = HvCreatePort(switchContext, &portParam,
> +                                  nicParam->NicIndex);
> +            if (status != NDIS_STATUS_SUCCESS) {
> +                goto done;
> +            }
>          }
>      }
> 
>      vport = OvsFindVportByPortIdAndNicIndex(switchContext, nicParam-
> >PortId,
>                                              nicParam->NicIndex);
> +
> +    NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0);
>      if (vport == NULL) {
>          OVS_LOG_ERROR("Create NIC without Switch Port,"
>                        " PortId: %x, NicIndex: %d",
> @@ -916,7 +939,6 @@ OvsInitVportWithPortParam(POVS_VPORT_ENTRY
> vport,
>      vport->portId = portParam->PortId;
>      vport->nicState = NdisSwitchNicStateUnknown;
>      vport->isExternal = FALSE;
> -    vport->isBridgeInternal = FALSE;
> 
>      switch (vport->portType) {
>      case NdisSwitchPortTypeExternal:
> @@ -958,7 +980,6 @@ OvsInitVportWithNicParam(POVS_SWITCH_CONTEXT
> switchContext,
>                           PNDIS_SWITCH_NIC_PARAMETERS nicParam)
>  {
>      ASSERT(vport->portId == nicParam->PortId);
> -    ASSERT(vport->ovsState == OVS_STATE_PORT_CREATED);
> 
>      UNREFERENCED_PARAMETER(switchContext);
> 
> @@ -1039,7 +1060,6 @@ OvsInitTunnelVport(PVOID userContext,
>      POVS_USER_PARAMS_CONTEXT usrParamsCtx =
>          (POVS_USER_PARAMS_CONTEXT)userContext;
> 
> -    vport->isBridgeInternal = FALSE;
>      vport->ovsType = ovsType;
>      vport->ovsState = OVS_STATE_PORT_CREATED;
>      switch (ovsType) {
> @@ -1086,23 +1106,6 @@ OvsInitTunnelVport(PVOID userContext,
> 
>  /*
>   * --------------------------------------------------------------------------
> - * Initializes a bridge internal vport ie. a port of type
> - * OVS_VPORT_TYPE_INTERNAL but not present on the Hyper-V switch.
> - * --------------------------------------------------------------------------
> - */
> -NTSTATUS
> -OvsInitBridgeInternalVport(POVS_VPORT_ENTRY vport)
> -{
> -    vport->isBridgeInternal = TRUE;
> -    vport->ovsType = OVS_VPORT_TYPE_INTERNAL;
> -    /* Mark the status to be connected, since there is no other initialization
> -     * for this port. */
> -    vport->ovsState = OVS_STATE_CONNECTED;
> -    return STATUS_SUCCESS;
> -}
> -
> -/*
> - * --------------------------------------------------------------------------
>   * For external and internal vports 'portFriendlyName' parameter, provided
> by
>    * Hyper-V, is overwritten with the interface alias name and NIC friendly
> name
>    * equivalent.
> @@ -1172,12 +1175,11 @@
> UpdateSwitchCtxWithVport(POVS_SWITCH_CONTEXT switchContext,
>          if (vport->nicIndex == 0) {
>              switchContext->virtualExternalPortId = vport->portId;
>              switchContext->virtualExternalVport = vport;
> -        } else {
> +        } else if (newPort == TRUE) {
>              switchContext->numPhysicalNics++;
>          }
>          break;
>      case NdisSwitchPortTypeInternal:
> -        ASSERT(vport->isBridgeInternal == FALSE);
>          switchContext->internalPortId = vport->portId;
>          switchContext->countInternalVports++;
>          break;
> @@ -1240,10 +1242,6 @@ InitOvsVportCommon(POVS_SWITCH_CONTEXT
> switchContext,
>          switchContext->numNonHvVports++;
>          break;
>      }
> -    case OVS_VPORT_TYPE_INTERNAL:
> -        if (vport->isBridgeInternal) {
> -            switchContext->numNonHvVports++;
> -        }
>      default:
>          break;
>      }
> @@ -1294,15 +1292,13 @@ OvsRemoveAndDeleteVport(PVOID
> usrParamsContext,
> 
>      switch (vport->ovsType) {
>      case OVS_VPORT_TYPE_INTERNAL:
> -        if (!vport->isBridgeInternal) {
> -            if (hvDelete && vport->isAbsentOnHv == FALSE) {
> -                switchContext->internalPortId = 0;
> -                switchContext->countInternalVports--;
> -                ASSERT(switchContext->countInternalVports >= 0);
> -                OvsInternalAdapterDown(vport->portNo, vport->netCfgInstanceId);
> -            }
> -            hvSwitchPort = TRUE;
> +        if (hvDelete && vport->isAbsentOnHv == FALSE) {
> +            switchContext->internalPortId = 0;
> +            switchContext->countInternalVports--;
> +            ASSERT(switchContext->countInternalVports >= 0);
> +            OvsInternalAdapterDown(vport->portNo, vport->netCfgInstanceId);
>          }
> +        hvSwitchPort = TRUE;
>          break;
>      case OVS_VPORT_TYPE_VXLAN:
>      {
> @@ -1563,8 +1559,7 @@ OvsClearAllSwitchVports(POVS_SWITCH_CONTEXT
> switchContext)
>              POVS_VPORT_ENTRY vport;
>              vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, portNoLink);
>              ASSERT(OvsIsTunnelVportType(vport->ovsType) ||
> -                   (vport->ovsType == OVS_VPORT_TYPE_INTERNAL &&
> -                    vport->isBridgeInternal) || vport->isAbsentOnHv == TRUE);
> +                   vport->isAbsentOnHv == TRUE);
>              OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE);
>          }
>      }
> diff --git a/datapath-windows/ovsext/Vport.h b/datapath-
> windows/ovsext/Vport.h
> index a7141d7..4dc4e00 100644
> --- a/datapath-windows/ovsext/Vport.h
> +++ b/datapath-windows/ovsext/Vport.h
> @@ -113,21 +113,6 @@ typedef struct _OVS_VPORT_ENTRY {
>      NDIS_SWITCH_NIC_FRIENDLYNAME nicFriendlyName;
>      NDIS_VM_NAME                 vmName;
>      GUID                         netCfgInstanceId;
> -    /*
> -     * OVS userpace has a notion of bridges which basically defines an
> -     * L2-domain. Each "bridge" has an "internal" port of type
> -     * OVS_VPORT_TYPE_INTERNAL. Such a port is connected to the OVS
> datapath in
> -     * one end, and the other end is a virtual adapter on the hypervisor host.
> -     * This is akin to the Hyper-V "internal" NIC. It is intuitive to map the
> -     * Hyper-V "internal" NIC to the OVS bridge's "internal" port, but there's
> -     * only one Hyper-V NIC but multiple bridges. To support multiple OVS
> bridge
> -     * "internal" ports, we use the flag 'isBridgeInternal' in each vport. We
> -     * support addition of multiple bridge-internal ports. A vport with
> -     * 'isBridgeInternal' == TRUE is a dummy port and has no backing
> currently.
> -     * If a flow actions specifies the output port to be a bridge-internal port,
> -     * the port is silently ignored.
> -     */
> -    BOOLEAN                      isBridgeInternal;
>      BOOLEAN                      isExternal;
>      UINT32                       upcallPid; /* netlink upcall port id */
>      PNL_ATTR                     portOptions;
> @@ -220,14 +205,6 @@ OvsIsRealExternalVport(POVS_VPORT_ENTRY vport)
>  }
> 
>  static __inline BOOLEAN
> -OvsIsBridgeInternalVport(POVS_VPORT_ENTRY vport)
> -{
> -    ASSERT(vport->isBridgeInternal != TRUE ||
> -           vport->ovsType == OVS_VPORT_TYPE_INTERNAL);
> -    return vport->isBridgeInternal == TRUE;
> -}
> -
> -static __inline BOOLEAN
>  OvsIsInternalNIC(NDIS_SWITCH_NIC_TYPE   nicType)
>  {
>      return nicType == NdisSwitchNicTypeInternal;
> --
> 1.9.5.msysgit.0
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev


More information about the dev mailing list