[ovs-dev] [PATCH 6/6 v3] datapath-windows: Fix HvUpdateNic() to handle name changes

Guru Shetty guru at ovn.org
Wed Nov 25 21:55:30 UTC 2015


On 25 November 2015 at 12:00, Nithin Raju <nithin at vmware.com> wrote:

> If the name of an internal or external NIC changes, we need to
> disconnect the NIC from OVS since the name is the key. In this
> change, we generate a link down event. It is as though we got a
> call to HvDisconnectNic() for the old name and got a HvConnectNic()
> for the new name.
>
> Also, HvCreateNic() has been cleaned up to remove the code to look
> for existing vport. We won't have a vport now since we'd have deleted
> the vport in HvDeleteNic().
>
> Signed-off-by: Nithin Raju <nithin at vmware.com>
> Acked-by: Sairam Venugopal <vsairam at vmware.com>
> Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
>

Series Applied, thanks!


>
> ---
>  datapath-windows/ovsext/Vport.c | 100
> ++++++++++++++++++++--------------------
>  datapath-windows/ovsext/Vport.h |  34 ++++++++++++--
>  2 files changed, 80 insertions(+), 54 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Vport.c
> b/datapath-windows/ovsext/Vport.c
> index ef21fca..a7576d3 100644
> --- a/datapath-windows/ovsext/Vport.c
> +++ b/datapath-windows/ovsext/Vport.c
> @@ -327,9 +327,8 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
>          goto done;
>      }
>
> -    if (nicParam->NicType == NdisSwitchNicTypeInternal ||
> -        (nicParam->NicType == NdisSwitchNicTypeExternal &&
> -         nicParam->NicIndex != 0)) {
> +    if (OvsIsInternalNIC(nicParam->NicType) ||
> +        OvsIsRealExternalNIC(nicParam->NicType, nicParam->NicIndex)) {
>          GetNICAlias(&nicParam->NetCfgInstanceId, &portFriendlyName);
>      }
>
> @@ -339,44 +338,22 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
>       * structure for each such NIC, and each NIC inherits a lot of
> properties
>       * from the parent external port.
>       */
> -    if (nicParam->NicType == NdisSwitchNicTypeExternal &&
> -        nicParam->NicIndex != 0) {
> +    if (OvsIsRealExternalNIC(nicParam->NicType, nicParam->NicIndex)) {
> +        NDIS_SWITCH_PORT_PARAMETERS portParam;
>          POVS_VPORT_ENTRY virtExtVport =
>              (POVS_VPORT_ENTRY)switchContext->virtualExternalVport;
>
> -        vport = OvsFindVportByPortIdAndNicIndex(switchContext,
> -                                                nicParam->PortId,
> -                                                nicParam->NicIndex);
> -        if (vport == NULL) {
> -            NDIS_SWITCH_PORT_PARAMETERS portParam;
> -            /* Find by interface name */
> -            WCHAR interfaceName[IF_MAX_STRING_SIZE] = { 0 };
> -            NET_LUID interfaceLuid = { 0 };
> -            size_t len = 0;
> -            status =
> ConvertInterfaceGuidToLuid(&nicParam->NetCfgInstanceId,
> -                                                &interfaceLuid);
> -            if (status == STATUS_SUCCESS) {
> -                status = ConvertInterfaceLuidToAlias(&interfaceLuid,
> -                                                     interfaceName,
> -                                                     IF_MAX_STRING_SIZE +
> 1);
> -                if (status == STATUS_SUCCESS) {
> -                    RtlStringCbLengthW(interfaceName,
> -                                       IF_MAX_STRING_SIZE,
> -                                       &len);
> -                    vport = OvsFindVportByHvNameW(switchContext,
> -                                                  interfaceName,
> -                                                  len);
> -                }
> -            }
> -
> -            OvsCopyPortParamsFromVport(virtExtVport, &portParam);
> -            NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
> -            status = HvCreatePort(switchContext, &portParam,
> -                                  nicParam->NicIndex);
> -            NdisAcquireRWLockWrite(switchContext->dispatchLock,
> &lockState, 0);
> -            if (status != NDIS_STATUS_SUCCESS) {
> -                goto add_nic_done;
> -            }
> +        ASSERT(virtExtVport);
> +        ASSERT(OvsFindVportByPortIdAndNicIndex(switchContext,
> +                                               nicParam->PortId,
> +                                               nicParam->NicIndex) ==
> NULL);
> +        OvsCopyPortParamsFromVport(virtExtVport, &portParam);
> +        NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
> +        status = HvCreatePort(switchContext, &portParam,
> +                              nicParam->NicIndex);
> +        NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState,
> 0);
> +        if (status != NDIS_STATUS_SUCCESS) {
> +            goto add_nic_done;
>          }
>      }
>
> @@ -390,9 +367,8 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
>          goto add_nic_done;
>      }
>      OvsInitVportWithNicParam(switchContext, vport, nicParam);
> -    if (nicParam->NicType == NdisSwitchNicTypeInternal ||
> -        (nicParam->NicType == NdisSwitchNicTypeExternal &&
> -         nicParam->NicIndex != 0)) {
> +    if (OvsIsInternalNIC(nicParam->NicType) ||
> +        OvsIsRealExternalNIC(nicParam->NicType, nicParam->NicIndex)) {
>          RtlCopyMemory(&vport->portFriendlyName, &portFriendlyName,
>                        sizeof portFriendlyName);
>      }
> @@ -470,6 +446,8 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext,
>      LOCK_STATE_EX lockState;
>      UINT32 event = 0;
>      IF_COUNTED_STRING portFriendlyName = {0};
> +    BOOLEAN nameChanged = FALSE;
> +    BOOLEAN aliasLookup = FALSE;
>
>      VPORT_NIC_ENTER(nicParam);
>
> @@ -483,9 +461,9 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext,
>
>      /* GetNICAlias() must be called outside of a lock. */
>      if (nicParam->NicType == NdisSwitchNicTypeInternal ||
> -        (nicParam->NicType == NdisSwitchNicTypeExternal &&
> -         nicParam->NicIndex != 0)) {
> +        OvsIsRealExternalNIC(nicParam->NicType, nicParam->NicIndex)) {
>          GetNICAlias(&nicParam->NetCfgInstanceId, &portFriendlyName);
> +        aliasLookup = TRUE;
>      }
>
>      NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0);
> @@ -502,8 +480,15 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext,
>      case NdisSwitchNicTypeInternal:
>          RtlCopyMemory(&vport->netCfgInstanceId,
> &nicParam->NetCfgInstanceId,
>                        sizeof (GUID));
> -        RtlCopyMemory(&vport->portFriendlyName, &portFriendlyName,
> -                      sizeof portFriendlyName);
> +        if (aliasLookup) {
> +            if (RtlCompareMemory(&vport->portFriendlyName,
> +                    &portFriendlyName, vport->portFriendlyName.Length) !=
> +                    vport->portFriendlyName.Length) {
> +                RtlCopyMemory(&vport->portFriendlyName, &portFriendlyName,
> +                    sizeof portFriendlyName);
> +                nameChanged = TRUE;
> +            }
> +        }
>          break;
>      case NdisSwitchNicTypeSynthetic:
>      case NdisSwitchNicTypeEmulated:
> @@ -536,6 +521,17 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext,
>      }
>      vport->numaNodeId = nicParam->NumaNodeId;
>
> +    if (nameChanged) {
> +        OVS_EVENT_ENTRY event;
> +        event.portNo = vport->portNo;
> +        event.ovsType = vport->ovsType;
> +        event.upcallPid = vport->upcallPid;
> +        RtlCopyMemory(&event.ovsName, &vport->ovsName, sizeof
> event.ovsName);
> +        event.type = OVS_EVENT_LINK_DOWN;
> +        OvsRemoveAndDeleteVport(NULL, switchContext, vport, FALSE, TRUE);
> +        OvsPostEvent(&event);
> +    }
> +
>      NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
>
>      /*
> @@ -598,14 +594,15 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
>      RtlCopyMemory(&event.ovsName, &vport->ovsName, sizeof event.ovsName);
>      event.type = OVS_EVENT_LINK_DOWN;
>
> -    NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
> -
>      /*
>       * Delete the port from the hash tables accessible to userspace.
> After this
>       * point, userspace should not be able to access this port.
>       */
> -    OvsRemoveAndDeleteVport(NULL, switchContext, vport, FALSE, TRUE);
> -    OvsPostEvent(&event);
> +    if (OvsIsRealExternalVport(vport)) {
> +        OvsRemoveAndDeleteVport(NULL, switchContext, vport, FALSE, TRUE);
> +        OvsPostEvent(&event);
> +    }
> +    NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
>
>      if (isInternalPort) {
>          OvsInternalAdapterDown();
> @@ -650,8 +647,8 @@ HvDeleteNic(POVS_SWITCH_CONTEXT switchContext,
>      vport->nicState = NdisSwitchNicStateUnknown;
>      vport->ovsState = OVS_STATE_PORT_CREATED;
>
> -    if (vport->portType == NdisSwitchPortTypeExternal &&
> -        vport->nicIndex != 0) {
> +    if (OvsIsRealExternalVport(vport)) {
> +        /* This vport was created in HvCreateNic(). */
>          OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, FALSE);
>      }
>
> @@ -926,6 +923,7 @@ OvsInitVportWithNicParam(POVS_SWITCH_CONTEXT
> switchContext,
>      vport->mtu = nicParam->MTU;
>      vport->nicState = nicParam->NicState;
>      vport->nicIndex = nicParam->NicIndex;
> +    vport->nicType = nicParam->NicType;
>      vport->numaNodeId = nicParam->NumaNodeId;
>
>      switch (vport->nicState) {
> diff --git a/datapath-windows/ovsext/Vport.h
> b/datapath-windows/ovsext/Vport.h
> index 0d56484..e9f3b03 100644
> --- a/datapath-windows/ovsext/Vport.h
> +++ b/datapath-windows/ovsext/Vport.h
> @@ -95,6 +95,7 @@ typedef struct _OVS_VPORT_ENTRY {
>      PVOID                  priv;
>      NDIS_SWITCH_PORT_ID    portId;
>      NDIS_SWITCH_NIC_INDEX  nicIndex;
> +    NDIS_SWITCH_NIC_TYPE   nicType;
>      UINT16                 numaNodeId;
>      NDIS_SWITCH_PORT_STATE portState;
>      NDIS_SWITCH_NIC_STATE  nicState;
> @@ -194,14 +195,41 @@ OvsIsInternalVportType(OVS_VPORT_TYPE ovsType)
>  }
>
>  static __inline BOOLEAN
> +OvsIsVirtualExternalVport(POVS_VPORT_ENTRY vport)
> +{
> +    return vport->nicType == NdisSwitchNicTypeExternal &&
> +           vport->nicIndex == 0;
> +}
> +
> +static __inline BOOLEAN
> +OvsIsRealExternalVport(POVS_VPORT_ENTRY vport)
> +{
> +    return vport->nicType == NdisSwitchNicTypeExternal &&
> +           vport->nicIndex != 0;
> +}
> +
> +static __inline BOOLEAN
>  OvsIsBridgeInternalVport(POVS_VPORT_ENTRY vport)
>  {
> -    if (vport->isBridgeInternal) {
> -       ASSERT(vport->ovsType == OVS_VPORT_TYPE_INTERNAL);
> -    }
> +    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;
> +}
> +
> +static __inline BOOLEAN
> +OvsIsRealExternalNIC(NDIS_SWITCH_NIC_TYPE   nicType,
> +                     NDIS_SWITCH_NIC_INDEX  nicIndex)
> +{
> +    return nicType == NdisSwitchNicTypeExternal &&
> +           nicIndex != 0;
> +}
> +
>  NTSTATUS OvsRemoveAndDeleteVport(PVOID usrParamsCtx,
>                                   POVS_SWITCH_CONTEXT switchContext,
>                                   POVS_VPORT_ENTRY vport,
> --
> 1.8.5.6
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list