virtio-nic: setting features should be idempotent?

[iximeow]

I think this might be the actual cause of #1048, but it might not be. either way, I'm pretty confident this is actually a bug in propolis:

propolis/lib/propolis/src/hw/virtio/viona.rs

Lines 716 to 723 in a28bee1

	fn set_features(&self, feat: u64) -> Result<(), ()> {
	self.hdl.set_features(feat).map_err(|_| ())?;
	if (feat & VIRTIO_NET_F_MQ) != 0 {
	self.hdl.set_pairs(PROPOLIS_MAX_MQ_PAIRS).map_err(|_| ())?;
	self.set_use_pairs(PROPOLIS_MAX_MQ_PAIRS)?;
	}
	Ok(())
	}

if a guest had already SET_FEATURES, and been using those queues, and then SET_FEATURES (maybe even the exact same bits) again, the second SET_FEATURES attempts to set_pairs(PROPOLIS_MAX_MQ_PAIRS) even though those queues had been enabled before and maybe even are in use.

in 1048 that is exactly what happens:

SET_FEATURES 0x1104388ab    # <--- somewhere early in boot
SET_PAIRS 0xb
SET_USEPAIRS 0xb
RING_INIT_MODERN 0x1 233496000/23349e000/23349f040 800
RING_SET_MSI 0x1 addr=0 msg=0
RING_INIT_MODERN 0x0 237ff1000/237ff2000/237ff2240 100
RING_SET_MSI 0x0 addr=0 msg=0
RING_INIT_MODERN 0x3 235002000/23500a000/23500b040 800
RING_SET_MSI 0x3 addr=0 msg=0
RING_INIT_MODERN 0x2 238158000/238159000/238159240 100
RING_SET_MSI 0x2 addr=0 msg=0
RING_INIT_MODERN 0x5 230505000/23050d000/23050e040 800
RING_SET_MSI 0x5 addr=0 msg=0
RING_INIT_MODERN 0x4 231468000/231469000/231469240 100
RING_SET_MSI 0x4 addr=0 msg=0
RING_INIT_MODERN 0x7 232057000/23205f000/232060040 800
RING_SET_MSI 0x7 addr=0 msg=0
RING_INIT_MODERN 0x6 232f99000/232f9a000/232f9a240 100
RING_SET_MSI 0x6 addr=0 msg=0
RING_SET_MSI 0x1 addr=fee0100c msg=49a1
RING_SET_MSI 0x0 addr=fee0100c msg=4991
RING_SET_MSI 0x3 addr=fee0200c msg=49b1
RING_SET_MSI 0x2 addr=fee0200c msg=49a1
RING_SET_MSI 0x5 addr=fee0400c msg=49b1
RING_SET_MSI 0x4 addr=fee0400c msg=49a1
RING_SET_MSI 0x7 addr=fee0800c msg=49b1
RING_SET_MSI 0x6 addr=fee0800c msg=49a1
SET_FEATURES 0x1104388ab    # <--- slightly later
SET_PAIRS 0xb
ioctl(SET_PAIRS) returning 0x10 # <--- device is now NEEDS_RESET

I'd thought this might be harmless-but-wonky (if we can't set_pairs() then we fail out before set_use_pairs() where we might fight with a guest's configuration), but since realizing this sets a bit that has the device asking very politely to be percussively maintenanced I'm pretty sure that setting device features to the same bit pattern over and over probably shouldn't... do that...

I'd want to double-check against the virtio spec just to be sure.