On Wed, 2021-04-07 at 16:00 -0700, Daniel Lenski wrote:
> Following David's initial implementation, I wrote something for
> OpenConnect that's pretty much the same as yours:
> https://gitlab.com/openconnect/openconnect/-/compare/5e6e9b850756157164f83cd4fedafb747fbbd50f...0bca5b32ac478b5d03b6e88f96bf29c6556610a5
> 
> 1. Uses GetAdaptersAddresses to list all the addresses
> 2. If/when it finds a clashing address, it uses
> GetUnicastIpAddressTable to determine the up/down state of the other
> interface
> 3. Only delete the address from the other interface if it's non-UP.
> 
> I was also annoyed that the GetAdaptersAddresses return structure
> doesn't provide the adapter state, and that I had to go for this
> convoluted O(n^2) design.
> 
> I guess this reassures me that there isn't an obviously-better way to
> do it.

Hm, your description doesn't match the code I see at that link.

You're using GetAdaptersAddresses() which gives you the UP/DOWN status
as well as the addresses, and you iterate over those. The loop is

 ∀ adapter, ∀ Unicast address on that adapter:
     Check if it's our Legacy IP or IPv6 address.

That isn't O(n²), is it? It's still O(n) of the total number of unicast
addresses in the system?

Once you've found an address which needs to be removed, you're *then*
using GetUnicastIpAddressTable() and searching through the results to
find the appropriate MIB_UNICASTIPADDRESS_ROW that you need to pass to
DeleteUnicastIpAddressEntry().

Does *every* field in the MIB_UNICASTIPADDRESS_ROW have to be filled
in, or is it just the Address, InterfaceLuid and InterfaceIndex? Can't
we get those from the table we get back from GetAdaptersAddresses()?


Alternatively, can't we start with GetUnicastIpAddressTable() as my
original code did, and if we want to check whether an interface is down
before we steal the address from it, use GetIfEntry2() to find out?

Using GetIfEntry2() is probably a saner way to find the InterfaceIndex
for the Wintun itself, which I was dredging the registry for manually.


I'd like to be consistent about clearing the 'conflicting' addresses
and setting the address on the Wintun interface. Whatever we do in
OpenConnect for Legacy IP we should also do for IPv6. It looks like
you're clearing the conflicting addresses for both families, but we
still aren't *setting* the IPv6 address from the C code?