hi Sorin, The change looks good overall. I have one comment in OvsInitConfiguredSwitchNics() which I had asked in the review for the v1 patch as well. Not sure if you got a chance to address that.
thanks, -- Nithin > On Sep 18, 2015, at 2:31 AM, Sorin Vinturis > <svintu...@cloudbasesolutions.com> wrote: > > The internal/external vports will have the actual OS-based names, which > represent the NIC interface alias that is displayed by running > 'Get-NetAdapter' Hyper-V PS command. > > Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> > --- > datapath-windows/ovsext/Vport.c | 85 +++++++++++++++++++---------------------- > datapath-windows/ovsext/Vport.h | 5 --- > 2 files changed, 40 insertions(+), 50 deletions(-) > > diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c > index ea10692..b1a1e01 100644 > --- a/datapath-windows/ovsext/Vport.c > +++ b/datapath-windows/ovsext/Vport.c > @@ -95,7 +95,7 @@ static VOID OvsTunnelVportPendingInit(PVOID context, > static VOID OvsTunnelVportPendingRemove(PVOID context, > NTSTATUS status, > UINT32 *replyLen); > - > +static VOID AssignNicNameSpecial(POVS_VPORT_ENTRY vport); > > /* > * Functions implemented in relaton to NDIS port manipulation. > @@ -191,10 +191,8 @@ HvUpdatePort(POVS_SWITCH_CONTEXT switchContext, > portParam->PortId, 0); > /* > * Update properties only for NETDEV ports for supprting PS script > - * We don't allow changing the names of the internal or external ports > */ > - if (vport == NULL || (( vport->portType != NdisSwitchPortTypeSynthetic) > && > - ( vport->portType != NdisSwitchPortTypeEmulated))) { > + if (vport == NULL) { > goto update_port_done; > } > > @@ -439,6 +437,7 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext, > case NdisSwitchNicTypeInternal: > RtlCopyMemory(&vport->netCfgInstanceId, &nicParam->NetCfgInstanceId, > sizeof (GUID)); > + AssignNicNameSpecial(vport); > break; > case NdisSwitchNicTypeSynthetic: > case NdisSwitchNicTypeEmulated: > @@ -968,36 +967,47 @@ OvsInitBridgeInternalVport(POVS_VPORT_ENTRY vport) > > /* > * -------------------------------------------------------------------------- > - * For external vports 'portFriendlyName' provided by Hyper-V is over-written > - * by synthetic names. > + * For external and internal vports 'portFriendlyName' parameter, provided by > + * Hyper-V, is overwritten with the interface alias name. > * -------------------------------------------------------------------------- > */ > static VOID > AssignNicNameSpecial(POVS_VPORT_ENTRY vport) > { > - size_t len; > + NTSTATUS status = STATUS_SUCCESS; > + WCHAR interfaceName[IF_MAX_STRING_SIZE] = { 0 }; > + NET_LUID interfaceLuid = { 0 }; > + size_t len = 0; > > - if (vport->portType == NdisSwitchPortTypeExternal) { > - if (vport->nicIndex == 0) { > - ASSERT(vport->nicIndex == 0); > - RtlStringCbPrintfW(vport->portFriendlyName.String, > - IF_MAX_STRING_SIZE, > - L"%s.virtualAdapter", > OVS_DPPORT_EXTERNAL_NAME_W); > + ASSERT(vport->portType == NdisSwitchPortTypeExternal || > + vport->portType == NdisSwitchPortTypeInternal); > + > + status = ConvertInterfaceGuidToLuid(&vport->netCfgInstanceId, > + &interfaceLuid); > + if (status == STATUS_SUCCESS) { > + 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, > + &len); > + vport->portFriendlyName.Length = (USHORT)len; > } else { > - RtlStringCbPrintfW(vport->portFriendlyName.String, > - IF_MAX_STRING_SIZE, > - L"%s.%lu", OVS_DPPORT_EXTERNAL_NAME_W, > - (UINT32)vport->nicIndex); > + OVS_LOG_INFO("Fail to convert interface LUID to alias, status: > %x", > + status); > } > } else { > - RtlStringCbPrintfW(vport->portFriendlyName.String, > - IF_MAX_STRING_SIZE, > - L"%s", OVS_DPPORT_INTERNAL_NAME_W); > + OVS_LOG_INFO("Fail to convert interface GUID to LUID, status: %x", > + status); > } > - > - RtlStringCbLengthW(vport->portFriendlyName.String, IF_MAX_STRING_SIZE, > - &len); > - vport->portFriendlyName.Length = (USHORT)len; > } > > > @@ -1382,6 +1392,7 @@ OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT > switchContext) > if (vport) { > OvsInitPhysNicVport(vport, virtExtVport, > nicParam->NicIndex); > + OvsInitVportWithNicParam(switchContext, vport, nicParam); > status = InitHvVportCommon(switchContext, vport, TRUE); > if (status != NDIS_STATUS_SUCCESS) { > OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG); > @@ -1392,13 +1403,15 @@ OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT > switchContext) > vport = OvsFindVportByPortIdAndNicIndex(switchContext, > nicParam->PortId, > nicParam->NicIndex); > + OvsInitVportWithNicParam(switchContext, vport, nicParam); > } > if (vport == NULL) { > OVS_LOG_ERROR("Fail to allocate vport"); > continue; > } > - OvsInitVportWithNicParam(switchContext, vport, nicParam); > if (nicParam->NicType == NdisSwitchNicTypeInternal) { > + /* Overwrite the 'portFriendlyName' of the internal vport. */ > + AssignNicNameSpecial(vport); Why do we need this call to AssignNicNameSpecial()? I asked this question in the previous review as well. Like I mentioned, we’d need to allocate a new vport, and assign name only for external NICs (ie. index > 0). For internal NICs, we’d have already assigned the special name in OvsAddConfiguredSwitchPorts() -> InitHvVportCommon(). > OvsInternalAdapterUp(vport->portNo, &nicParam->NetCfgInstanceId); > } > } > @@ -2093,9 +2106,7 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT > usrParamsCtx, > PCHAR portName; > ULONG portNameLen; > UINT32 portType; > - BOOLEAN isBridgeInternal = FALSE; > BOOLEAN vportAllocated = FALSE, vportInitialized = FALSE; > - BOOLEAN addInternalPortAsNetdev = FALSE; > > static const NL_POLICY ovsVportPolicy[] = { > [OVS_VPORT_ATTR_PORT_NO] = { .type = NL_A_U32, .optional = TRUE }, > @@ -2138,24 +2149,12 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT > usrParamsCtx, > goto Cleanup; > } > > - if (portName && portType == OVS_VPORT_TYPE_NETDEV && > - !strcmp(OVS_DPPORT_INTERNAL_NAME_A, portName)) { > - addInternalPortAsNetdev = TRUE; > - } > - > - if (portName && portType == OVS_VPORT_TYPE_INTERNAL && > - strcmp(OVS_DPPORT_INTERNAL_NAME_A, portName)) { > - isBridgeInternal = TRUE; > - } > - > - if (portType == OVS_VPORT_TYPE_INTERNAL && !isBridgeInternal) { > - vport = gOvsSwitchContext->internalVport; > - } else if (portType == OVS_VPORT_TYPE_NETDEV) { > + if (portType == OVS_VPORT_TYPE_NETDEV) { > /* External ports can also be looked up like VIF ports. */ > vport = OvsFindVportByHvNameA(gOvsSwitchContext, portName); > } else { > ASSERT(OvsIsTunnelVportType(portType) || > - (portType == OVS_VPORT_TYPE_INTERNAL && isBridgeInternal)); > + portType == OVS_VPORT_TYPE_INTERNAL); > > vport = (POVS_VPORT_ENTRY)OvsAllocateVport(); > if (vport == NULL) { > @@ -2221,10 +2220,6 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT > usrParamsCtx, > goto Cleanup; > } > > - /* Initialize the vport with OVS specific properties. */ > - if (addInternalPortAsNetdev != TRUE) { > - vport->ovsType = portType; > - } > if (vportAttrs[OVS_VPORT_ATTR_PORT_NO] != NULL) { > /* > * XXX: when we implement the limit for ovs port number to be > diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h > index ba21c62..ead55a9 100644 > --- a/datapath-windows/ovsext/Vport.h > +++ b/datapath-windows/ovsext/Vport.h > @@ -34,11 +34,6 @@ > */ > #define OVS_DPPORT_NUMBER_LOCAL 0 > > -#define OVS_DPPORT_INTERNAL_NAME_A "internal" > -#define OVS_DPPORT_INTERNAL_NAME_W L"internal" > -#define OVS_DPPORT_EXTERNAL_NAME_A "external" > -#define OVS_DPPORT_EXTERNAL_NAME_W L"external" > - > /* > * A Vport, or Virtual Port, is a port on the OVS. It can be one of the > * following types. Some of the Vports are "real" ports on the hyper-v switch, > -- > 1.9.0.msysgit.0 > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=PtJvYDEqho5jh2hAQ0i3eGD0u2IEwwDQ3votDGo-nx4&s=-u9SRUnalZagEUwblelvvVt2X7a-bmgd4XxbKplH_Ac&e= > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev