The control queue handler passed the total length of all
descriptors (header + data + status) as used_len to add_used.
Per virtio spec section 2.6.8, used_len must only count bytes
written to device-writable descriptors. The device only writes
the 1-byte status/ack field.
Windows NetKVM >= 0.1.285 strictly checks this value and calls
NdisMRemoveMiniport when len != sizeof(virtio_net_ctrl_ack),
removing the network adapter immediately after activation.
Signed-off-by: CMGS <ilskdw@gmail.com>
Windows NetKVM driver (>= 0.1.271) issues unsupported command classes
even when they are not even advertised.
According to the Virtio 1.2 specification:
RX, VLAN, and ANNOUNCE control paths are only meaningful when their
corresponding features are negotiated in sections 5.1.3.1, 5.1.6.5.1.2,
5.1.6.5.2.2, and 5.1.6.5.4.1.
RX and VLAN are explicitly described as best-effort in sections
5.1.6.5.1 and 5.1.6.5.3.
Instead of returning an error to the guest, return success to the guest.
Fixes: #7925
Signed-off-by: Wei Liu <liuwe@microsoft.com>
The libc crate provides all functionality provided by the net_gen crate.
Removing the net_gen crate reduces the maintenance burden.
The switch to libc required some fixes, most notably the switch from a
`Vec<u8>` to a `CString` for the `net_util::Tap.if_name` field.
On-behalf-of: SAP julian.schindel@sap.com
Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>
error: this argument is passed by value, but not consumed in the function body
--> net_util/src/tap.rs:685:17
|
685 | ifname: String,
| ^^^^^^ help: consider changing the type to: `&str`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
= note: requested on the command line with `-D clippy::needless-pass-by-value`
Signed-off-by: Rob Bradford <rbradford@meta.com>
A buggy or malicious guest may write an inappropriate value into
virtqueue's next_avail field. This will result in an error
when iterating over the queue:
863837ef86/virtio-queue/src/queue.rs (L708)
but this error is (logged and) ignored if pop_descriptor_chain()
is used:
863837ef86/virtio-queue/src/queue.rs (L583)
A reasonable approach, implemented here, is to mark the device as
NEEDS_RESET and ignore further queue events until the guest
reinitializes the device.
How this patch was tested:
Linux kernel was patched to trigger a bad next_avail when the
virtqueue queue counter reaches 5000:
--------------- START OF LINUX KERNEL PATCH ----------
$ git diff
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b784aab668670..989f2a0c64a77 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -15,6 +15,9 @@
#include <linux/spinlock.h>
#include <xen/xen.h>
+
+void virtqueue_kick_always(struct virtqueue *vq);
+
#ifdef DEBUG
/* For development, we want to crash whenever the ring is screwed. */
#define BAD_RING(_vq, fmt, args...) \
@@ -677,6 +680,12 @@ static inline int virtqueue_add_split(
struct virtqueue *_vq,
* new available array entries. */
virtio_wmb(vq->weak_barriers);
vq->split.avail_idx_shadow++;
+ {
+ if ((vq->split.avail_idx_shadow % 100) == 0)
+ printk(KERN_ERR "avail idx: %d",
+ (int)vq->split.avail_idx_shadow);
+ if (vq->split.avail_idx_shadow == 5000)
+ vq->split.avail_idx_shadow = 0;
+ }
vq->split.vring.avail->idx = cpu_to_virtio16(_vq->vdev,
vq->split.avail_idx_shadow);
vq->num_added++;
@@ -689,6 +698,11 @@ static inline int virtqueue_add_split(
struct virtqueue *_vq,
if (unlikely(vq->num_added == (1 << 16) - 1))
virtqueue_kick(_vq);
+ {
+ if (unlikely(vq->split.avail_idx_shadow == 0))
+ virtqueue_kick_always(_vq);
+ }
+
return 0;
unmap_release:
@@ -2515,6 +2529,11 @@ bool virtqueue_kick(struct virtqueue *vq)
}
EXPORT_SYMBOL_GPL(virtqueue_kick);
+void virtqueue_kick_always(struct virtqueue *vq)
+{
+ virtqueue_kick_prepare(vq);
+ virtqueue_notify(vq);
+}
/**
* virtqueue_get_buf_ctx - get the next used buffer
* @_vq: the struct virtqueue we're talking about.
--------------- END OF LINUX KERNEL PATCH ----------
Then the kernel was booted, and the host pinged until the
nic became unresponsive:
ping -i 0.002 192.168.4.1
Device status was confirmed using
cat /sys/class/net/eth0/device/status
(it was 0x4f).
Then the device was re-initialized:
DEV_NAME=$(basename $(readlink -f /sys/class/net/eth0/device))
echo $DEV_NAME | tee /sys/bus/virtio/drivers/virtio_net/unbind
echo $DEV_NAME | tee /sys/bus/virtio/drivers/virtio_net/bind
ip link set eth0 up
At this point networking became healthly again.
Signed-off-by: Peter Oskolkov <posk@google.com>
This should be guaranteed by GuestMemory and GuestMemoryRegion, but
those traits are currently safe, so add checks to guard against
incorrect implementations of them.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
This better aligns with the rest of the code and makes it clearer
that these tests can run "as is" in a normal hosted environments
without the special test environment.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
TL;DR: Massive quality of life improvement for devs
Cloud Hypervisor uses the Cargo test framework for multiple tests:
- normal unit tests
- unit tests requiring special environment (the Tap device tests)
- integration tests requiring a special environment
This prevented the execution of `cargo test --workspace`, which results
in a very poor developer experience. Although
`./scripts/run_unit_tests.sh` exists, there are valid reasons why devs
cannot or even don't want to use it.
By adding a new `chv_testenv` rustc config, we can conditionally only
activate tests when the `./scripts/` magic runs them. This improves
the general developer experience by a lot.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Consuming `&Arc<T>` as argument is almost always an antipattern as it
hides whether the callee is going to take over (shared) ownership
(by .clone()) or not. Instead, it is better to consume `&dyn T` or
`Arc<dyn T>` to be more explicit. This commit cleans up the code.
The change is very mechanic and was very easy to implement across the
code base.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Fix clippy warning `uninlined_format_args` reported by rustc rustc
1.89.0 (29483883e 2025-08-04).
```console
warning: variables can be used directly in the `format!` string
--> block/src/lib.rs:649:17
|
649 | info!("{} failed to create io_uring instance: {}", error_msg, e);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
= note: `#[warn(clippy::uninlined_format_args)]` on by default
help: change this to
|
649 - info!("{} failed to create io_uring instance: {}", error_msg, e);
649 + info!("{error_msg} failed to create io_uring instance: {e}");
|
```
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
This step was done manually by searching for "unsafe fn" in
the code base and adding corresponding Safety sections.
`clippy::missing_safety_doc` only works for public functions
but none of the corresponding functions is public.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
One can call `to_vec()` anyway if one needs an owned copy. This change
further helps to prevent needless copies in upcoming changes.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
The virtio_features_to_tap_offload() defined in ctrl_queue.rs
is duplicated. Remove it and use the one defined in lib.rs
instead.
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
This streamlines the Error implementation in the Cloud Hypervisor code
base to match the remaining parts so that everything follows the agreed
conventions. These are leftovers missed in the previous commits.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This streamlines the code base to follow best practices for
error handling in Rust: Each error struct implements
std::error::Error (most due via thiserror::Error derive macro)
and sets its source accordingly.
This allows future work that nicely prints the error chains,
for example.
So far, the convention is that each error prints its
sub error as part of its Display::fmt() impl.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Allow tap interfaces to be configured with an IPv6 address. The change
is fairly straightforward: we need to update the API types and CLI
parsing to accept either an IPv6 or IPv4 and then match on the IP
address type when the tap device is configured.
For IPv6 addresses, the netmask (prefix) must be provided at the same
time as the address itself (in the SIOCSIFADDR ioctl). They cannot be
configured separately. So we remove the separate "set_netmask" function
and convert "set_ip_addr" to also accept a netmask. For IPv4 addresses,
the IP address and netmask were already always set together, so this
should have no functional impact for users of IPv4 addresses.
Signed-off-by: Gregory Anders <ganders@cloudflare.com>
Rust has a new way of constructing other error and clippy complains if
we are still using the older way to construct error message. Thus,
migrate to the new approach suggested by the clippy.
Warning from beta compiler:
error: this can be `std::io::Error::other(_)`
--> block/src/vhdx/mod.rs:142:17
|
| / std::io::Error::new(
| | std::io::ErrorKind::Other,
| | format!("Failed to update VHDx header: {e}"),
| | )
| |_________________^
|
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#io_other_error
help: use `std::io::Error::other`
std::io::Error::other(
format!("Failed to update VHDx header: {e}"),
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
Replace `map_or()` on false condition with `is_some_and` to provide
better readability, as suggestted by v1.84.0-beta.1 `cargo clippy`.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
As clippy of rust-toolchain version 1.83.0-beta.1 suggests, use `c""` to
construct nul-terminated `CStr`.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Asserting on .is_ok()/.is_err() leads to hard to debug failures (as if
the test fails, it will only say "assertion failed: false". We replace
these with `.unwrap()`, which also prints the exact error variant that
was unexpectedly encountered (we can to this these days thanks to
efforts to implement Display and Debug for our error types). If the
assert!((...).is_ok()) was followed by an .unwrap() anyway, we just drop
the assert.
Inspired by and quoted from @roypat.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
By introducing `imports_granularity="Module"` format strategy,
effectively groups imports from the same module into one line or block,
improving maintainability and readability.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Historically the Cloud Hypervisor coding style has been to ensure that
all imports are ordered and placed in a single group. Unfortunately
cargo fmt has no support for ensuring that all imports are in a single
group so if whitespace lines were added as part of the import statements
then they would only be odered correctly in the group.
By adopting "group_imports="StdExternalCrate" we can enforce a style
where imports are placed in at most three groups for std, external
crates and the crate itself. Choosing a style enforceable by the tooling
reduces the reviewer burden.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Replace the specific Bitmap implementation from the type signature used
for functions that take memory. This allows more flexibility when using
these functions in particular when these functions are used by the
vhost-user-net backend. An updated vhost-user-backend crate requires
extra constraints on the Bitmap implementation used (it must support
BitmapReplace which AtomicBitmap does not.)
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
With the nightly toolchain (2024-02-18) cargo check will flag up
redundant imports either because they are pulled in by the prelude on
earlier match.
Remove those redundant imports.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
This commit changes existing behavior of named TAP interfaces.
When booting a VM with configuration for a named TAP interface,
cloud-hypervisor will create the interface and apply a given
IP configuration to that interface. If the named interface
already exists on the system, the configuration is NOT overwritten.
Setting the ip and netmask fields in a tap interface configuration
for a named tap interface now works by handing this configuration
to the virtio_devices::Net object when it is created with a name.
This commit also touches net_util to make sure that the ip configuration
of existing TAP interfaces is not modified with ip or netmask handed to
open_tap.
Signed-off-by: Markus Sütter <markus.suetter@secunet.com>
This fixes all typos found by the typos utility with respect to the config file.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
Update to the latest vm-memory and all the crates that also depend upon
it.
Fix some deprecation warnings.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
warning: this argument is a mutable reference, but not used mutably
--> net_util/src/queue_pair.rs:167:14
|
167 | tap: &mut Tap,
| ^^^^^^^^ help: consider changing to: `&Tap`
|
= warning: changing this function will impact semver compatibility
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
warning: casting raw pointers to the same type and constness is unnecessary (`*const protocol::MemoryRange` -> `*const protocol::MemoryRange`)
--> vm-migration/src/protocol.rs:280:17
|
280 | self.data.as_ptr() as *const MemoryRange as *const u8,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `self.data.as_ptr()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
= note: `#[warn(clippy::unnecessary_cast)]` on by default
Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
