this is line to debug, however I didn't find a way to trigger this check.

This is defined but not used?

I would suggest we change it to
typedef struct uuid hv_guid;
so we can avoid many lines of change below.

And IMHO we'd better not use the name "uuid" everywhere -- uuid is different from guid after all. :-)
And it seems a little confusing to have
" struct uuid instance_guid" in vmbus_channel_process_offer().
(hv_guid instance_guid seems much better).

It looks here we make non-trivial changes just to make use of the built-in API snprintf_uuid().
I suggest we define a new function snprintf_guid(it only needs a few lines of code, I think) so we can avoid the changes?

We won't need this change if we have the above typedef and snprintf_guid().

Can you please add a comment string of the GUID here so it can be easily grep-ed? :-)

Add a comment of the GUID string?

change this to snprintf_guid(), which can be added by us?

It's in the HV_KVP_OP_SET_IP_INFO case, which is used for guest failover (when a guest fails on host1 or host1 fails, the guest will be started on host2 with a static IP injected into the guest. To enable this, the replica feature needs to be enabled on both hosts.

I can help to test this.

Add a comment of the string GUID too?

Ditto

IMO it should be renamed to "next_cpu".

We'd better use this

channel->target_vcpu = hv_vmbus_g_context.hv_vcpu_index[0];

Here you convert the GUID to UUID and store the UUID into a variable named type_GUID... :-)

I don't think we need this change. Yes, we want to remove the ugly 'is perf' testing when the channel is created. But we will move the cpu selection to the drivers (nail channel to cpu0 when the channel is created). I think the only driver needs conversion is stor.

Both storVSC and netVSC needs "is_perf".

In the future, we'll add Hyper-V sockets driver, RDMA driver and PCIe driver (for device pass-through), which need that too.

So I suggest we keep the logic in the vmbus driver, rather than the individual drivers.

Nope, netvsc already uses its own CPU mapping logic, which nullifies the requirement of the is_perf parameter; well the code in this patch actually interferes netvsc's mapping. To be frank, cpu mapping logic is driver specific for drivers really need performance. VMBUS should or even can not aware of those.

Hmm, I wasn't reading the latest code.

I have a concern about the latest logic in netvsc code: assuming hn_chan_cnt==8 and the VM has 32 vCPUs and there are 2 NICs:

the first NIC's sc->hn_cpu == 0 and the the 8 channels are bound to CPU 0~7;
the second NIC's sc->hn_cpu == 1 and the the 8 channels are bound to CPU 1~8;
We can see cpu 1~7 are overloaded and cpu 9~31 are idle...

If we do the logic in VMBus driver, we can have a central place where the global channel-to-cpu binding info is available, e.g., NIC1 can use cpu 0~7 and NIC2 can use 8~16, and storVSC can use 17~21, etc.

Nope, you read the code wrong. For netvsc:

static uint32_t netvsc_gidx = 0

for hn0
(ring_cnt == 8 in your case)
lead_cpu = atomic_fetchadd_int(&netvsc_gidx, ring_cnt);
/* lead_cpu is 0 here */
chan_sub0 -> cpu0
chan_sub1 -> cpu1
...
chan_sub7 -> cpu7

for hn1
(ring_cnt == 8 in your case)
lead_cpu = atomic_fetchadd_int(&netvsc_gidx, ring_cnt);
/* lead_cpu is 8 here */
chan_sub0 -> cpu8
chan_sub1 -> cpu9
...
chan_sub7 -> cpu15

And so on, and so forth.

The old way actually is wrong: the channel/multi-channel offers are mixed together and messed up netvsc ring/cpu mapping badly, causes uneven load distribution.

Oh, sorry for the case of netvsc. I missed the ring_cnt... :-)

Yes, the old logic in VMBus driver is bad, but I think we should fix that in VMBus side, otherwise, we'll have duplicate the same logic to storvsc and the future hyper-v sockets, RDMA & PCIe drivers, and even if we duplicate the logic, the drivers always tend to use CPUs with smaller numbers, because now the individual drivers don't share the binding info.

Sharing the global index is easy part. But we do need to have APIs for drivers to configure their cpu mapping based on their own needs, e.g. options RSS will need special network ring binding. Well, and we do need to have tunables to let user do their own mapping, instead of making the decisions for them, e.g. network gate/switch application will never care about the cpu mapping of stors.

I think the main point of this patch is to get rid of the hv_guid. Let's just do it; but please leave the cpu mapping logic out. We could fix/rework it in another patch.

What I want to do is making the code more clean via decouple guid in the protocol v.s. uuid in the memory representation. Introduce a guid and a set of operations are overkill from my POV.

IMO we only need to add a snprintf_guid(), which is basically a wrapper of snprintf_uuid() + le_uuid_dec().

We don't need to introduce hv_guid -- it's already there and we can just update its definition to "typedef struct uuid hv_guid".

This way, the patch can be much smaller, I think.