Skip to content

Commit

Permalink
datapath-windows: cleanup AssignNicNameSpecial()
Browse files Browse the repository at this point in the history 
AssignNicNameSpecial() needed to be called outside of a lock and was
moved out in a previous change. But, it was accessing vport structure
outside of the lock which isn't safe. In this change, we take care of
that.

I tried to trigger a call to HvUpdateNic() by renaming the interface
from the GUI and didn't see any callback. Other changes are tested.

Signed-off-by: Nithin Raju <[email protected]>
Acked-by: Sairam Venugopal <[email protected]>
Acked-by: Alin Gabriel Serdean <[email protected]>
Signed-off-by: Gurucharan Shetty <[email protected]>
  • Loading branch information
@nithinrajub @shettyg
nithinrajub authored and shettyg committed Nov 25, 2015
1 parent 12e888b commit 34be96c
Showing 1 changed file with 33 additions and 29 deletions.
62 changes: 33 additions & 29 deletions datapath-windows/ovsext/Vport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ static VOID OvsTunnelVportPendingInit(PVOID context,
static VOID OvsTunnelVportPendingRemove(PVOID context,
NTSTATUS status,
UINT32 *replyLen);
static VOID AssignNicNameSpecial(POVS_VPORT_ENTRY vport);
static NTSTATUS GetNICAlias(GUID *netCfgInstanceId,
IF_COUNTED_STRING *portFriendlyName);

/*
* --------------------------------------------------------------------------
Expand Down Expand Up @@ -311,7 +312,7 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
{
POVS_VPORT_ENTRY vport;
NDIS_STATUS status = NDIS_STATUS_SUCCESS;

IF_COUNTED_STRING portFriendlyName = {0};
LOCK_STATE_EX lockState;

VPORT_NIC_ENTER(nicParam);
Expand All @@ -326,6 +327,12 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
goto done;
}

if (nicParam->NicType == NdisSwitchNicTypeInternal ||
(nicParam->NicType == NdisSwitchNicTypeExternal &&
nicParam->NicIndex != 0)) {
GetNICAlias(&nicParam->NetCfgInstanceId, &portFriendlyName);
}

NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0);
/*
* There can be one or more NICs for the external port. We create a vport
Expand Down Expand Up @@ -386,17 +393,12 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
if (nicParam->NicType == NdisSwitchNicTypeInternal ||
(nicParam->NicType == NdisSwitchNicTypeExternal &&
nicParam->NicIndex != 0)) {
AssignNicNameSpecial(vport);
RtlCopyMemory(&vport->portFriendlyName, &portFriendlyName,
sizeof portFriendlyName);
}

add_nic_done:
NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
if (status == STATUS_SUCCESS &&
(vport->portType == NdisSwitchPortTypeInternal ||
(vport->portType == NdisSwitchPortTypeExternal &&
nicParam->NicIndex != 0))) {
AssignNicNameSpecial(vport);
}

done:
VPORT_NIC_EXIT(nicParam);
Expand Down Expand Up @@ -467,6 +469,7 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext,
POVS_VPORT_ENTRY vport;
LOCK_STATE_EX lockState;
UINT32 event = 0;
IF_COUNTED_STRING portFriendlyName = {0};

VPORT_NIC_ENTER(nicParam);

Expand All @@ -478,6 +481,13 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext,
goto update_nic_done;
}

/* GetNICAlias() must be called outside of a lock. */
if (nicParam->NicType == NdisSwitchNicTypeInternal ||
(nicParam->NicType == NdisSwitchNicTypeExternal &&
nicParam->NicIndex != 0)) {
GetNICAlias(&nicParam->NetCfgInstanceId, &portFriendlyName);
}

NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0);
vport = OvsFindVportByPortIdAndNicIndex(switchContext,
nicParam->PortId,
Expand All @@ -492,7 +502,8 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext,
case NdisSwitchNicTypeInternal:
RtlCopyMemory(&vport->netCfgInstanceId, &nicParam->NetCfgInstanceId,
sizeof (GUID));
AssignNicNameSpecial(vport);
RtlCopyMemory(&vport->portFriendlyName, &portFriendlyName,
sizeof portFriendlyName);
break;
case NdisSwitchNicTypeSynthetic:
case NdisSwitchNicTypeEmulated:
Expand Down Expand Up @@ -1033,18 +1044,16 @@ OvsInitBridgeInternalVport(POVS_VPORT_ENTRY vport)
* Hyper-V, is overwritten with the interface alias name.
* --------------------------------------------------------------------------
*/
static VOID
AssignNicNameSpecial(POVS_VPORT_ENTRY vport)
static NTSTATUS
GetNICAlias(GUID *netCfgInstanceId,
IF_COUNTED_STRING *portFriendlyName)
{
NTSTATUS status = STATUS_SUCCESS;
WCHAR interfaceName[IF_MAX_STRING_SIZE] = { 0 };
NET_LUID interfaceLuid = { 0 };
size_t len = 0;

ASSERT(vport->portType == NdisSwitchPortTypeExternal ||
vport->portType == NdisSwitchPortTypeInternal);

status = ConvertInterfaceGuidToLuid(&vport->netCfgInstanceId,
status = ConvertInterfaceGuidToLuid(netCfgInstanceId,
&interfaceLuid);
if (status == STATUS_SUCCESS) {
/*
Expand All @@ -1054,26 +1063,21 @@ AssignNicNameSpecial(POVS_VPORT_ENTRY vport)
status = ConvertInterfaceLuidToAlias(&interfaceLuid, interfaceName,
IF_MAX_STRING_SIZE + 1);
if (status == STATUS_SUCCESS) {
if (vport->portType == NdisSwitchPortTypeExternal &&
vport->nicIndex == 0) {
RtlStringCbPrintfW(vport->portFriendlyName.String, IF_MAX_STRING_SIZE,
L"%s.virtualAdapter", interfaceName);
} else {
RtlStringCbPrintfW(vport->portFriendlyName.String,
IF_MAX_STRING_SIZE, L"%s", interfaceName);
}

RtlStringCbLengthW(vport->portFriendlyName.String, IF_MAX_STRING_SIZE,
RtlStringCbPrintfW(portFriendlyName->String,
IF_MAX_STRING_SIZE, L"%s", interfaceName);
RtlStringCbLengthW(portFriendlyName->String, IF_MAX_STRING_SIZE,
&len);
vport->portFriendlyName.Length = (USHORT)len;
portFriendlyName->Length = (USHORT)len;
} else {
OVS_LOG_INFO("Fail to convert interface LUID to alias, status: %x",
OVS_LOG_ERROR("Fail to convert interface LUID to alias, status: %x",
status);
}
} else {
OVS_LOG_INFO("Fail to convert interface GUID to LUID, status: %x",
OVS_LOG_ERROR("Fail to convert interface GUID to LUID, status: %x",
status);
}

return status;
}


Expand Down

0 comments on commit 34be96c

Please sign in to comment.