Looks good to me. Acked-by: Paul-Daniel Boca <pb...@cloudbasesolutions.com>
> -----Original Message----- > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean > Sent: Tuesday, May 10, 2016 2:58 AM > To: dev@openvswitch.org > Subject: [ovs-dev] [PATCH 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 <aserd...@cloudbasesolutions.com> > --- > datapath-windows/ovsext/Actions.c | 6 +-- > datapath-windows/ovsext/IpHelper.c | 6 +++ > datapath-windows/ovsext/Vport.c | 92 +++++++++++++++++------------------- > -- > datapath-windows/ovsext/Vport.h | 24 +--------- > 4 files changed, 50 insertions(+), 78 deletions(-) > > diff --git a/datapath-windows/ovsext/Actions.c b/datapath- > windows/ovsext/Actions.c > index 2c86469..0d887ff 100644 > --- a/datapath-windows/ovsext/Actions.c > +++ b/datapath-windows/ovsext/Actions.c > @@ -314,7 +314,7 @@ OvsDetectTunnelPkt(OvsForwardingContext > *ovsFwdCtx, > > if (!vport || > (vport->ovsType != OVS_VPORT_TYPE_NETDEV && > - !OvsIsBridgeInternalVport(vport))) { > + vport->ovsType != OVS_VPORT_TYPE_INTERNAL)) { > ovsFwdCtx->tunKey.dst = 0; > } > } > @@ -391,10 +391,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/IpHelper.c b/datapath- > windows/ovsext/IpHelper.c > index 75ae4de..4043e5c 100644 > --- a/datapath-windows/ovsext/IpHelper.c > +++ b/datapath-windows/ovsext/IpHelper.c > @@ -398,6 +398,9 @@ OvsGetRoute(SOCKADDR_INET *destinationAddress, > *vport = OvsFindVportByHvNameW(gOvsSwitchContext, > interfaceName, > len); > + if (*vport == NULL) { > + status = STATUS_INVALID_PARAMETER; > + } > } else { > status = STATUS_INVALID_PARAMETER; > } > @@ -609,6 +612,9 @@ > OvsAddIpInterfaceNotification(PMIB_IPINTERFACE_ROW ipRow) > status = ConvertInterfaceLuidToAlias(&ipRow->InterfaceLuid, > interfaceName, > IF_MAX_STRING_SIZE + 1); > + if (gOvsSwitchContext == NULL) { > + goto error; > + } > POVS_VPORT_ENTRY vport = > OvsFindVportByHvNameW(gOvsSwitchContext, > interfaceName, > sizeof(WCHAR) * > diff --git a/datapath-windows/ovsext/Vport.c b/datapath- > windows/ovsext/Vport.c > index ba0b5b2..eea27ed 100644 > --- a/datapath-windows/ovsext/Vport.c > +++ b/datapath-windows/ovsext/Vport.c > @@ -96,6 +96,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); > > /* > * -------------------------------------------------------------------------- > @@ -339,32 +342,49 @@ HvCreateNic(POVS_SWITCH_CONTEXT > switchContext, > OvsIsRealExternalNIC(nicParam->NicType, nicParam->NicIndex)) { > 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); > - NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0); > + char convertString[256]; > + RtlZeroMemory(convertString, 256); > + status = OvsConvertIfCountedStrToAnsiStr(&portFriendlyName, > + convertString, > + OVS_MAX_PORT_NAME_LENGTH); > 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; > + } > } > } > > + NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0); > vport = OvsFindVportByPortIdAndNicIndex(switchContext, nicParam- > >PortId, > nicParam->NicIndex); > if (vport == NULL) { > @@ -493,7 +513,7 @@ HvUpdateNic(POVS_SWITCH_CONTEXT > switchContext, > &portFriendlyName, vport->portFriendlyName.Length) != > vport->portFriendlyName.Length) { > RtlCopyMemory(&vport->portFriendlyName, &portFriendlyName, > - sizeof portFriendlyName); > + sizeof portFriendlyName); > nameChanged = TRUE; > } > } > @@ -607,7 +627,7 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT > switchContext, > * point, userspace should not be able to access this port. > */ > if (OvsIsRealExternalVport(vport)) { > - OvsRemoveAndDeleteVport(NULL, switchContext, vport, FALSE, TRUE); > + OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, FALSE); > OvsPostEvent(&event); > } > NdisReleaseRWLock(switchContext->dispatchLock, &lockState); > @@ -879,7 +899,6 @@ OvsInitVportWithPortParam(POVS_VPORT_ENTRY > vport, > vport->portId = portParam->PortId; > vport->nicState = NdisSwitchNicStateUnknown; > vport->isExternal = FALSE; > - vport->isBridgeInternal = FALSE; > > switch (vport->portType) { > case NdisSwitchPortTypeExternal: > @@ -921,7 +940,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); > > @@ -1002,7 +1020,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) { > @@ -1046,23 +1063,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. > @@ -1132,12 +1132,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->countInternalVports++; > break; > case NdisSwitchPortTypeSynthetic: > @@ -1198,10 +1197,6 @@ InitOvsVportCommon(POVS_SWITCH_CONTEXT > switchContext, > switchContext->numNonHvVports++; > break; > } > - case OVS_VPORT_TYPE_INTERNAL: > - if (vport->isBridgeInternal) { > - switchContext->numNonHvVports++; > - } > default: > break; > } > @@ -1252,14 +1247,12 @@ OvsRemoveAndDeleteVport(PVOID > usrParamsContext, > > switch (vport->ovsType) { > case OVS_VPORT_TYPE_INTERNAL: > - if (!vport->isBridgeInternal) { > - if (hvDelete && vport->isAbsentOnHv == FALSE) { > - switchContext->countInternalVports--; > - ASSERT(switchContext->countInternalVports >= 0); > - OvsInternalAdapterDown(vport->portNo, > vport->netCfgInstanceId); > - } > - hvSwitchPort = TRUE; > + if (hvDelete && vport->isAbsentOnHv == FALSE) { > + switchContext->countInternalVports--; > + ASSERT(switchContext->countInternalVports >= 0); > + OvsInternalAdapterDown(vport->portNo, vport->netCfgInstanceId); > } > + hvSwitchPort = TRUE; > break; > case OVS_VPORT_TYPE_VXLAN: > { > @@ -1519,8 +1512,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 00bd931..89a772a 100644 > --- a/datapath-windows/ovsext/Vport.h > +++ b/datapath-windows/ovsext/Vport.h > @@ -112,21 +112,7 @@ 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; > @@ -216,14 +202,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@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
