Skip to content

Fix NIC settings fallback to WMI#725

Merged
lusassl-msft merged 5 commits into
mainfrom
lusassl-nicwmifallback
Sep 7, 2021
Merged

Fix NIC settings fallback to WMI#725
lusassl-msft merged 5 commits into
mainfrom
lusassl-nicwmifallback

Conversation

@lusassl-msft
Copy link
Copy Markdown
Contributor

Issue:
Fallback to WMI to query NIC settings was not working properly in some scenarios. If WMI was used to query the NIC settings, certain information was missing (IP addresses ...).

Reason:
There was no proper implementation to return the IP addresses in WMI scenario. We don't have all the information available which we get when using CIM with the latest cmdlets so this should be considered as a fallback just in case.

Fix:
Throw when we don't get any result via Get-NetworkConfiguration. Added some logic to get the IP addresses, subnet mask and gateway.

Validation:
Validated in lab and with affected customer. Please do not merge yet. Might require some more adjustments.

@dpaulson45
Copy link
Copy Markdown
Member

While reviewing this code, looks like i found a major issue with the original code.

$adapterConfiguration is actually only defined in a foreach loop and we are call that same variable a few other times outside of that loop. Need to look at this more.

@lusassl-msft
Copy link
Copy Markdown
Contributor Author

You mean this one?

#determine if IPv6 is enabled
$stopProcess = $false
foreach ($adapterConfiguration in $networkAdapterConfigurations) {
    $settingID = $adapterConfiguration.SettingID
    Write-Verbose "Working on '$($adapterConfiguration.Description)' | SettingID: $settingID"

    if ($settingID -eq $networkConfig.GUID -or
        $settingID -eq $networkConfig.InterfaceGuid) {
        foreach ($ipAddress in $adapterConfiguration.IPAddress) {
            if ($ipAddress.Contains(":")) {
                $ipv6Enabled = $true
                $stopProcess = $true
                break
            }
        }
    }

    if ($stopProcess) {
        break
    }
}

@dpaulson45
Copy link
Copy Markdown
Member

Yep, that is the one.

@lusassl-msft
Copy link
Copy Markdown
Contributor Author

I was wondering what this block was for. Think the $ipv6Enabled check could be done in the newly introduced logic below. Checked a few older versions of the script and it looks like the foreach was copied over from there.

If the registry goes likes this:
```
0011
0013
0016
0017
```
We will fail to go past 0015 as we don't reset the retry counter on the next successful attempt. This fixes that broken logic. Hopefully, there just isn't a break in the numbers greater than 3.
@lusassl-msft
Copy link
Copy Markdown
Contributor Author

Received feedback from customer that the WMI fallback works with the fix proposed.

@dpaulson45
Copy link
Copy Markdown
Member

Except the code is written correctly, took a closer look at it last night and understood what was going on a little bit more. Will push out additional changes here today.

@dpaulson45 dpaulson45 self-requested a review September 7, 2021 15:06
@lusassl-msft lusassl-msft merged commit f429491 into main Sep 7, 2021
@lusassl-msft lusassl-msft deleted the lusassl-nicwmifallback branch September 7, 2021 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants