Tree-wide cleanup of new clippy warning in Rust 1.54 that warns about
needless borrows:
error: this expression borrows a reference (`&...`) that is
immediately dereferenced by the compiler
https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
BUG=b:197251702
TEST=bin/clippy # with rust-toolchain = 1.54.0
Change-Id: Ib702ec524d4623d264a00ec11dbc2150c411a67b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3108321
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Allen Webb <allenwebb@google.com>
Reviewed-by: Tomasz Jeznach <tjeznach@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
A given net device should expose the minimum MTU of all its
tap interfaces. This ensures that the guest won't inadvertently
produce frames that the host will have to drop, if a lower MTU
is in use on the host (common with cellular links).
BUG=b:171098377
TEST=create tap with MTU < 1500 and start crosvm
Change-Id: I6add14b532e4c2d070d2fe4410272421afc1303f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2917620
Commit-Queue: Stephen Barber <smbarber@chromium.org>
Tested-by: Stephen Barber <smbarber@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Add a vhost-user net device executable which communiates with a VMM having vhost-user net device.
Example command:
vhost-user-net-device \
--tap 10.0.2.2,255.255.255.0,12:34:56:78:9a:bc \
--socket /tmp/vhost-net.sock
BUG=b:179755448
TEST=run as a backend of crosvm with vhost-user net device
Cq-Depend: chromium:2861979
Change-Id: Ib723cdd00381b29464855378e568a9fa3de01536
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2807205
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
This CL transitions most structs to RawDescriptor and the
associated traits if possible.
BUG=b:162363783
TEST=./build_test
Change-Id: Iabae6ac212787836d77de2b9ffb5d451421ab0dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2530911
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Michael Hoyle <mikehoyle@google.com>
Commit-Queue: Michael Hoyle <mikehoyle@google.com>
This is a layer above PollContext with a more generic interface.
As PollContext is used so widely, this is quite a large change
in order to accomodate the interface update, especially with the
use of RawDescriptor. In some cases this has caused an echo
of updates to RawDescriptor, which is fine because of our eventual
goal to move the whole codebase to it regardless.
Note there are a few instances of forcing the RawDescriptor update
chain to stop, ex. ioctl. This is to keep the scope of this CL
concentrated and avoid changing entire other areas.
Note that this CL leaves out a few additional pieces of work:
- The sole usage of EpollContext over PollContext (event_loop),
which poses a bigger challenge for interface changes
- Full PollToken renaming, which is a tiny change turned difficult
due to the unavailability of type aliases for traits.
- Renaming certain methods which have been updated to use
RawDescriptor such as keep_fds. Some have enough dependencies that
they are worth avoiding to keep this CL pointed, but will be
addressed in future CLs to make sure the whole codebase is on the
fd->descriptor train
BUG=b:162363783
TEST=./build_test
Change-Id: Iff2cfe8f90dea55f1388f8e91bdc698e121a8e43
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2455726
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Michael Hoyle <mikehoyle@google.com>
For now, this crate simply re-exports all of sys_util, but it will
be updated to provide new interfaces when needed. This is the
first step to making crosvm not directly depend on sys_util, so
that we can make the interface changes we need without fear of
negatively affecting (i.e. completely breaking) other usages
within chromeos.
BUG=b:162363783
TEST=./build_test
Change-Id: I7d0aa3d8a1f66af1c7fee8fd649723ef17027150
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2325168
Tested-by: Michael Hoyle <mikehoyle@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Michael Hoyle <mikehoyle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
In particular, Android targets use c_int whereas normal Linux targets
use c_ulong. An ioctl number should always fit in 16 bits, so casting
from c_uint (as bindgen uses for the constants) to either other type
should be safe.
BUG=b:158290206
TEST=cargo test
Change-Id: I3d808ba9a5588a75c029a299e3609d97e328e3cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2288229
Tested-by: kokoro <noreply+kokoro@google.com>
Auto-Submit: Andrew Walbran <qwandor@google.com>
Commit-Queue: Andrew Walbran <qwandor@google.com>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
In order to support 2 vq pairs for virtio-net, this patch create 2
sockets in one tap interface, 2 vq pairs(rx/tx) and 2 threads in
device model, So one vq pair has one thread and one socket in tap
corresponding.
On my SkyLake desktop with crosvm and ubuntu 18.04 guest:
If run one iperf server on host and one iperf client in guest, 2 vq
pairs has the same netwrok bandwidth as 1 vq pair, the bandwidth is
5.99Gbits/sec
If run two iperf server on host and two iperf client in guest,
In 1 vq pair mode, two iperf cliens bandwidth are 3.19Gbits/sec and
3.18Gbits/sec.
In 2 vq pairs mode, two iperf clients bandwidth are 4.87Gbits/sec and
4.86Gbits/sec.
So 2 vq pairs improve net bandwidth 52.7% compared with 1 vq pair in
this case.
BUG=chromium:1064482
TEST=Run iperf test in guest
Change-Id: I1fa14d7e24085552dc828a89a883d4a2ada34789
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2099754
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Xiong Zhang <xiong.y.zhang@intel.corp-partner.google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
As described in:
https://doc.rust-lang.org/edition-guide/rust-2018/ownership-and-lifetimes/default-match-bindings.html
which also covers the new mental model that the Rust Book will use for
teaching binding modes and has been found to be more friendly for both
beginners and experienced users.
Before:
match *opt {
Some(ref v) => ...,
None => ...,
}
After:
match opt {
Some(v) => ...,
None => ...,
}
TEST=cargo check --all-features
TEST=local kokoro
Change-Id: I3c5800a9be36aaf5d3290ae3bd3116f699cb00b7
Reviewed-on: https://chromium-review.googlesource.com/1566669
Commit-Ready: David Tolnay <dtolnay@chromium.org>
Tested-by: David Tolnay <dtolnay@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
In Rust 2018 edition, `extern crate` is no longer required for importing
from other crates. Instead of writing:
extern crate dep;
use dep::Thing;
we write:
use dep::Thing;
In this approach, macros are imported individually from the declaring
crate rather than through #[macro_use]. Before:
#[macro_use]
extern crate sys_util;
After:
use sys_util::{debug, error};
The only place that `extern crate` continues to be required is in
importing the compiler's proc_macro API into a procedural macro crate.
This will hopefully be fixed in a future Rust release.
extern crate proc_macro;
TEST=cargo check
TEST=cargo check --all-features
TEST=cargo check --target aarch64-unknown-linux-gnu
TEST=local kokoro
Change-Id: I0b43768c0d81f2a250b1959fb97ba35cbac56293
Reviewed-on: https://chromium-review.googlesource.com/1565302
Commit-Ready: David Tolnay <dtolnay@chromium.org>
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: David Tolnay <dtolnay@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: David Tolnay <dtolnay@chromium.org>
Separated out of CL:1513058 to make it possible to land parts
individually while the affected crate has no other significant CLs
pending. This avoids repeatedly introducing non-textual conflicts with
new code that adds `use` statements.
TEST=cargo check
TEST=cargo check --all-features
TEST=cargo check --target aarch64-unknown-linux-gnu
Change-Id: Ibb78b8254693a68a55780ebea3e895c146a14119
Reviewed-on: https://chromium-review.googlesource.com/1519701
Commit-Ready: David Tolnay <dtolnay@chromium.org>
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: David Tolnay <dtolnay@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: David Tolnay <dtolnay@chromium.org>
Checks for the IFF_NO_PI and IFF_VNET_HDR flags, failing if those are
not set.
Sets the offload and vnet header sizes to the required values, instead
of trusting the values on the interface.
Bug=b/128686192
Change-Id: Ibbbfbf3cdedd6e64cdcfb446bcdfb26b4fd38395
Reviewed-on: https://chromium-review.googlesource.com/1526771
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
The description method is deprecated and its signature forces less
helpful error messages than what Display can provide.
BUG=none
TEST=cargo check --all-features
TEST=cargo check --target aarch64-unknown-linux-gnu
Change-Id: I27fc99d59d0ef457c5273dc53e4c563ef439c2c0
Reviewed-on: https://chromium-review.googlesource.com/1497735
Commit-Ready: David Tolnay <dtolnay@chromium.org>
Tested-by: David Tolnay <dtolnay@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
I have been running into Debug-printed error messages too often and
needing to look up in the source code each level of nested errors to
find out from the comment on the error variant what the short name of
the variant means in human terms. Worse, many errors (like the one shown
below) already had error strings written but were being printed from the
calling code in the less helpful Debug representation anyway.
Before:
[ERROR:src/main.rs:705] The architecture failed to build the vm: NoVarEmpty
After:
[ERROR:src/main.rs:705] The architecture failed to build the vm: /var/empty doesn't exist, can't jail devices.
TEST=cargo check --all-features
TEST=FEATURES=test emerge-amd64-generic crosvm
Change-Id: I77122c7d6861b2d610de2fff718896918ab21e10
Reviewed-on: https://chromium-review.googlesource.com/1469225
Commit-Ready: David Tolnay <dtolnay@chromium.org>
Tested-by: David Tolnay <dtolnay@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
We use the tap device interface name in some ioctls. When we are
creating a Tap struct from a raw fd make sure that we also grab the
interface name so that these ioctls don't fail later.
BUG=b:80150167
TEST=run the plugin_net_config test
Change-Id: Ic308ebd55d0545c1b445fc6abdf017fdc7ab594b
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1341104
Hopefully the changes are self-explanatory and uncontroversial. This
eliminates much of the noise from `cargo clippy` and, for my purposes,
gives me a reasonable way to use it as a tool when writing and reviewing
code.
Here is the Clippy invocation I was using:
cargo +nightly clippy -- -W clippy::correctness -A renamed_and_removed_lints -Aclippy::{blacklisted_name,borrowed_box,cast_lossless,cast_ptr_alignment,enum_variant_names,identity_op,if_same_then_else,mut_from_ref,needless_pass_by_value,new_without_default,new_without_default_derive,or_fun_call,ptr_arg,should_implement_trait,single_match,too_many_arguments,trivially_copy_pass_by_ref,unreadable_literal,unsafe_vector_initialization,useless_transmute}
TEST=cargo check --features wl-dmabuf,gpu,usb-emulation
TEST=boot linux
Change-Id: I55eb1b4a72beb2f762480e3333a921909314a0a2
Reviewed-on: https://chromium-review.googlesource.com/1356911
Commit-Ready: David Tolnay <dtolnay@chromium.org>
Tested-by: David Tolnay <dtolnay@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Now that cargo fmt has landed, run it over everything at once to bring
rust source to the standard formatting.
TEST=cargo test
BUG=None
Change-Id: Ic95a48725e5a40dcbd33ba6d5aef2bd01e91865b
Reviewed-on: https://chromium-review.googlesource.com/1259287
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
Allow the process that spawned crosvm to pass in a configured tap file
descriptor for networking. If this option is provided then crosvm will
ignore the other networking related command line flags (like mac
address, netmask, etc).
Passing in a configured tap device allows us to run crosvm without
having to give it CAP_NET_ADMIN.
BUG=none
TEST=Start a container and verify that networking still works
Change-Id: I70b9e6ae030d66c4882e4e48804dc2f29d9874ba
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1081394
Reviewed-by: Zach Reizner <zachr@chromium.org>
Now that there are no users of that interface, we should remove it.
TEST=./build_test
BUG=chromium:816692
Change-Id: Ifdbde22984f557b945e49559ba47076e99db923b
Reviewed-on: https://chromium-review.googlesource.com/1000103
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
sys_util errors always have a backing errno. Use these to allow
clients to always get access to the underlying errno when using
ioctl-based operations.
BUG=none
TEST=cargo test
Change-Id: I8853eeb2b9b38e5509c0ef01f507e461d5d36267
Reviewed-on: https://chromium-review.googlesource.com/927804
Commit-Ready: Stephen Barber <smbarber@chromium.org>
Tested-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Allow get/set for the host mac on the tap interface. Also add read accessors
for the host IP address and netmask, and make using IFF_VNET_HDR optional.
BUG=none
TEST=./build_test
Change-Id: I9999bf5aa8aa35b8cae702d9bc6f94602d6fe32e
Reviewed-on: https://chromium-review.googlesource.com/918406
Commit-Ready: Stephen Barber <smbarber@chromium.org>
Tested-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
This slightly advances the use of fakes to test higher level
application logic. The fakes are rudimentary at this point, but I
wanted to get feedback on the addition of generics in order to
facilitate swaping concrete implementations out with fakes in higher
level code.
BUG=none
TEST=./build_test and
cargo test -p crosvm -p data_model -p syscall_defines -p kernel_loader
-p net_util -p x86_64 -p virtio_sys -p kvm_sys -p vhost -p io_jail -p
net_sys -p sys_util -p kvm
Change-Id: Ib64581014391f49cff30ada10677bbbcd0088f20
Reviewed-on: https://chromium-review.googlesource.com/689740
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Mutable references being declared mutable themselves is unnecessary and
now generates a warning.
Change-Id: I29c7652fb86e17a8eda21efc728dd09b726c304f
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/717733
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
The compiler was complaining about std::fmt::Debug being an unused
import. Remove the import.
BUG=none
TEST=build_test and see no warnings about unused imports
Change-Id: I5e606982dffee575b3048b6c45cb5222e63ae3dc
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/669305
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
We can't really mock out the underlying TAP ioctls unless we introduce
another layer of abstraction. Instead, this CL allows a test to pass
if the reason that it failed was a permission denial as we would
expect running on Paladins as non-root.
Also adds net_util to the build_test test targets.
BUG=none
TEST=Run unit tests:
cargo test -p crosvm -p data_model -p syscall_defines -p kernel_loader -p net_util -p x86_64 -p virtio_sys -p kvm_sys -p vhost -p io_jail -p net_sys -p sys_util -p kvm
Also ran ./build_test
Change-Id: I5c761bd75d3a6d5829f4dd07fb8031612944e912
Reviewed-on: https://chromium-review.googlesource.com/649958
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>
