Fix the name of the registry parameter for MAC address

[selvanair]

• Replace registry entry MAC by NetworkAddress.
• Add a directive to remove any old entry named MAC
  to support upgrade.

No validation of input is done. Windows accepts MAC
as 12 hex characters with an optional hyphen between
bytes but not colons. Also the MAC should be a valid
"locally administered address".

See also issue #97

Signed-off-by: Selva Nair [email protected]

[cron2]

Oi. You see me trying to look at a C code patch and making sense out of it, but it's all windows magic.

Can you help me by explaining what was wrong before and what your change does? I can see that the code talks about "Read MAC parameter from registry (NetworkAddress keyword)", which hints at "the key was named wrongly before".

How does this get populated with default values? Defaulting to "PermanentAddress" (which is randomly created in tapReadConfiguration())

[selvanair]

Oi. You see me trying to look at a C code patch and making sense out of it, but it's all windows magic.

Can you help me by explaining what was wrong before and what your change does? I can see that the code talks about "Read MAC parameter from registry (NetworkAddress keyword)", which hints at "the key was named wrongly before".

How does this get populated with default values? Defaulting to "PermanentAddress" (which is randomly created in tapReadConfiguration())

The entries in the advanced property dialog is automatically linked to registry keys with the same name and they get updated when the user edits them. So the key will have either either an empty value (i.e., invalid) or what the user sets in the property dialog (a valid MAC or some junk as no checks are done). It will never get the permanent address.

Normally, MAC is internally set in the driver (or burned in hardware) and there is no requirement for it to be user-configurable. And, in our case, if this key exists in registry and has a valid value, we use it, else the permanent address is used.

Now about the change of the key name: we read all custom parameters except MAC by directly accessing the registry using NdisReadConfiguration where the key name is an input parameter like "MTU", "AllowNonadmin" etc.. We used to do that for "MAC" too and it used to work.

That was until commit f4f646 changed it to use NdisReadNetworkAddress() which is apparently the right way to do it. But that function behind the scenes reads a registry key named "NetworkAddress" [1]. We can't store it as "MAC" or something else and expect that function to find it. Hence the only change required is in the name of the registry key for that property and that's what the patch does. I suppose it was overlooked when that commit was made.

The patch also adds a directive to delete any existing parameter named MAC. Otherwise the property dialog for existing adapters after driver upgrade will get two entries both displayed as "Mac Address" but only one of those will actually work. Avoid that confusion by explicitly deleting that entry if found. Its not required for new adapters but can't hurt.

[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/ndis/nf-ndis-ndisreadnetworkaddress

[cron2]

Thank you. This all makes lots of sense. Still, windows magic :-)