From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.5 (2021-03-20) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-4.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=3.4.5 Received: by atuin.qyliss.net (Postfix, from userid 496) id 540371ED68; Mon, 17 May 2021 18:57:42 +0000 (UTC) Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 062871ED41; Mon, 17 May 2021 18:57:31 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 496) id B89241ED96; Mon, 17 May 2021 18:57:28 +0000 (UTC) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by atuin.qyliss.net (Postfix) with ESMTPS id D9BFB1ED35 for ; Mon, 17 May 2021 18:57:24 +0000 (UTC) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 3FD415C0181; Mon, 17 May 2021 14:57:23 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 17 May 2021 14:57:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h= from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; s=fm3; bh=j2co00QiUGYUxWYCQjKnPTvvSu OwxZmbMOgFQC1m2us=; b=y6kkslH3Y2fEuXFAJIJo/plfgj++bFqT3R663ovNuv 1I2ZXz+E2Y+RV9GCxiwUS2KkAVtbOXpxMaC3/FXoeQl0c699uj3ZCAMMrzfPwQwJ zYK4mHANEgcDEMjGb3ANW5JVzOlUHxxT3RjGfBv7I5BPKIHzqeOGq8fd1hW+IHnx rniLT3b5//PgV5pjrbDgJHBQTLXLxu44PBfC4bBDqURD8BgSCawEACRiY/vHmSzI MXR8UhtD9KqSkUhBGgNF81qQ1itZ66kVUz9nb+wrtIk9YVogdjP+xEuH67IGJYUM oxslHiaQET92momHZOwF985TnHd4fqTA52nolvcgE1uA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :message-id:mime-version:subject:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=j2co00QiUGYUxWYCQ jKnPTvvSuOwxZmbMOgFQC1m2us=; b=ozc37fYsByrF6yD6xNPDuon0fSbvD1Yck LSXAJWRgMBzdQGV89s9Oo84GVuJs0xkbNsJdlU9QDbOzzbl4Y2cR5l66n4Ym1gyp L4DIZSsOUU+Kxb83+VFVHeIz85yXoDT7Zs8yv7RS/LrRvcpVG0S6YjQFT3R0ni/g JLLXXx7buo9wQJBdQVpF62wG6i80JMfv4Jwaq17RUKmDvaTjLgT8Lx8Qx+YVfflp Bt1F94HQ88zuKf5HmXxIc3HuZy556zxuByvpv/KtG928wO0qhSXWdaf4cfh4c88v g7AsqwiZ4EETa4TGCJxkuF0zt6d005Zia2aXgPCEuMqKD/fDhPSpA== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvdeihedguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkofgggfestdekredtredttdenucfhrhhomheptehlhihsshgr ucftohhsshcuoehhihesrghlhihsshgrrdhisheqnecuggftrfgrthhtvghrnhepkeelte ekieduledugffhhfetfedvleeffeejudfhgeduhedtieefhfehjeduueejnecuffhomhgr ihhnpehorghsihhsqdhophgvnhdrohhrghdpshhpvggtthhruhhmqdhoshdrohhrghdpvg hnrggslhgvrdhmrghppdhmrggtrdhmrghppdhunhifrhgrphdrthhopdhprghrshgvrdhm rghpnecukfhppeegiedrkedtrddufeekrdejfeenucevlhhushhtvghrufhiiigvpedtne curfgrrhgrmhepmhgrihhlfhhrohhmpehqhihlihhsshesgidvvddtrdhqhihlihhsshdr nhgvth X-ME-Proxy: Received: from x220.qyliss.net (p2e508a49.dip0.t-ipconnect.de [46.80.138.73]) by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 17 May 2021 14:57:22 -0400 (EDT) Received: by x220.qyliss.net (Postfix, from userid 1000) id 5A76487C; Mon, 17 May 2021 18:57:21 +0000 (UTC) From: Alyssa Ross To: devel@spectrum-os.org Subject: [PATCH crosvm v2] crosvm: support setting guest MAC from tap-fd Date: Mon, 17 May 2021 18:57:00 +0000 Message-Id: <20210517185700.3591932-1-hi@alyssa.is> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: NLSYGRKLUQAFX32S2RXLGX5TILB2UMI7 X-Message-ID-Hash: NLSYGRKLUQAFX32S2RXLGX5TILB2UMI7 X-MailFrom: qyliss@x220.qyliss.net X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Cole Helbling X-Mailman-Version: 3.3.4 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: This adds a mac= option to crosvm's --tap-fd option. The virtio-net driver in the guest will read the desired MAC from virtio configuration space. See the documentation for VIRTIO_NET_F_MAC in the Virtio spec[1]. [1]: https://docs.oasis-open.org/virtio/virtio/v1.1/virtio-v1.1.html Thanks-to: Puck Meerburg --- v1: https://spectrum-os.org/lists/archives/spectrum-devel/20210409222026.13886-1-hi@alyssa.is/ This is a new version of the patch, updated for a much more recent crosvm. I'm posting it again because I had to change it enough that I feel it could use a re-review. The biggest change is that I gave up on a NetOptions struct, since upstream has added a new option, and it seems that their approach is going to be just passing ever more function arguments around. No use fighting that for my single addition. Also, change from RawFd to crosvm's RawDescriptor. It would be nice if we could use SafeDescriptor earlier here (which represents fd ownership), but I don't think that's really easily doable. devices/src/virtio/net.rs | 12 ++++++-- src/crosvm.rs | 8 +++-- src/linux.rs | 12 ++++---- src/main.rs | 63 ++++++++++++++++++++++++++++++--------- 4 files changed, 71 insertions(+), 24 deletions(-) diff --git a/devices/src/virtio/net.rs b/devices/src/virtio/net.rs index 92368c440..4d0ea1560 100644 --- a/devices/src/virtio/net.rs +++ b/devices/src/virtio/net.rs @@ -419,6 +419,7 @@ where } pub struct Net { + mac_address: Option, queue_sizes: Box<[u16]>, workers_kill_evt: Vec, kill_evts: Vec, @@ -439,6 +440,7 @@ where ip_addr: Ipv4Addr, netmask: Ipv4Addr, mac_addr: MacAddress, + guest_mac_addr: Option, vq_pairs: u16, ) -> Result, NetError> { let multi_queue = if vq_pairs > 1 { true } else { false }; @@ -450,12 +452,12 @@ where tap.enable().map_err(NetError::TapEnable)?; - Net::from(base_features, tap, vq_pairs) + Net::with_tap(base_features, tap, vq_pairs, guest_mac_addr) } /// Creates a new virtio network device from a tap device that has already been /// configured. - pub fn from(base_features: u64, tap: T, vq_pairs: u16) -> Result, NetError> { + pub fn with_tap(base_features: u64, tap: T, vq_pairs: u16, mac_address: Option) -> Result, NetError> { let taps = tap.into_mq_taps(vq_pairs).map_err(NetError::TapOpen)?; // This would also validate a tap created by Self::new(), but that's a good thing as it @@ -488,7 +490,12 @@ where workers_kill_evt.push(worker_kill_evt); } + if mac_address.is_some() { + avail_features |= 1 << virtio_net::VIRTIO_NET_F_MAC; + } + Ok(Net { + mac_address, queue_sizes: vec![QUEUE_SIZE; (vq_pairs * 2 + 1) as usize].into_boxed_slice(), workers_kill_evt, kill_evts, @@ -503,6 +510,7 @@ where let vq_pairs = self.queue_sizes.len() as u16 / 2; VirtioNetConfig { + mac: self.mac_address.map(|m| m.octets()).unwrap_or_else(Default::default), max_vq_pairs: Le16::from(vq_pairs), // Other field has meaningful value when the corresponding feature // is enabled, but all these features aren't supported now. diff --git a/src/crosvm.rs b/src/crosvm.rs index 04e267d6c..1dde7d769 100644 --- a/src/crosvm.rs +++ b/src/crosvm.rs @@ -171,6 +171,10 @@ impl Default for SharedDir { } } +pub struct TapFdOptions { + pub mac: Option, +} + /// Aggregate of all configurable options for a running VM. pub struct Config { pub vcpu_count: Option, @@ -194,7 +198,7 @@ pub struct Config { pub mac_address: Option, pub net_vq_pairs: Option, pub vhost_net: bool, - pub tap_fd: Vec, + pub tap_fd: BTreeMap, pub cid: Option, pub wayland_socket_paths: BTreeMap, pub wayland_dmabuf: bool, @@ -253,7 +257,7 @@ impl Default for Config { mac_address: None, net_vq_pairs: None, vhost_net: false, - tap_fd: Vec::new(), + tap_fd: BTreeMap::new(), cid: None, #[cfg(feature = "gpu")] gpu_parameters: None, diff --git a/src/linux.rs b/src/linux.rs index f452fef3f..42c07df4f 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -75,7 +75,7 @@ use vm_memory::{GuestAddress, GuestMemory}; #[cfg(all(target_arch = "x86_64", feature = "gdb"))] use crate::gdb::{gdb_thread, GdbStub}; -use crate::{Config, DiskOption, Executable, SharedDir, SharedDirKind, TouchDeviceOption}; +use crate::{Config, DiskOption, Executable, SharedDir, SharedDirKind, TapFdOptions, TouchDeviceOption}; use arch::{ self, LinuxArch, RunnableLinuxVm, SerialHardware, SerialParameters, VcpuAffinity, VirtioDeviceStub, VmComponents, VmImage, @@ -675,7 +675,7 @@ fn create_balloon_device(cfg: &Config, socket: BalloonControlResponseSocket) -> }) } -fn create_tap_net_device(cfg: &Config, tap_fd: RawDescriptor) -> DeviceResult { +fn create_tap_net_device(cfg: &Config, tap_fd: RawDescriptor, options: &TapFdOptions) -> DeviceResult { // Safe because we ensure that we get a unique handle to the fd. let tap = unsafe { Tap::from_raw_descriptor( @@ -691,7 +691,7 @@ fn create_tap_net_device(cfg: &Config, tap_fd: RawDescriptor) -> DeviceResult { vq_pairs = 1; } let features = virtio::base_features(cfg.protected_vm); - let dev = virtio::Net::from(features, tap, vq_pairs).map_err(Error::NetDeviceNew)?; + let dev = virtio::Net::with_tap(features, tap, vq_pairs, options.mac).map_err(Error::NetDeviceNew)?; Ok(VirtioDeviceStub { dev: Box::new(dev), @@ -725,7 +725,7 @@ fn create_net_device( .map_err(Error::VhostNetDeviceNew)?; Box::new(dev) as Box } else { - let dev = virtio::Net::::new(features, host_ip, netmask, mac_address, vq_pairs) + let dev = virtio::Net::::new(features, host_ip, netmask, mac_address, None, vq_pairs) .map_err(Error::NetDeviceNew)?; Box::new(dev) as Box }; @@ -1311,8 +1311,8 @@ fn create_virtio_devices( devs.push(create_balloon_device(cfg, balloon_device_socket)?); // We checked above that if the IP is defined, then the netmask is, too. - for tap_fd in &cfg.tap_fd { - devs.push(create_tap_net_device(cfg, *tap_fd)?); + for (tap_fd, options) in &cfg.tap_fd { + devs.push(create_tap_net_device(cfg, *tap_fd, options)?); } if let (Some(host_ip), Some(netmask), Some(mac_address)) = diff --git a/src/main.rs b/src/main.rs index 5d02af02f..f8bc0d14e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -28,7 +28,7 @@ use base::{ }; use crosvm::{ argument::{self, print_help, set_arguments, Argument}, - platform, BindMount, Config, DiskOption, Executable, GidMap, SharedDir, TouchDeviceOption, + platform, BindMount, Config, DiskOption, Executable, GidMap, SharedDir, TapFdOptions, TouchDeviceOption, DISK_ID_LEN, }; #[cfg(feature = "gpu")] @@ -1319,17 +1319,52 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument:: } "vhost-net" => cfg.vhost_net = true, "tap-fd" => { - cfg.tap_fd.push( - value - .unwrap() - .parse() - .map_err(|_| argument::Error::InvalidValue { - value: value.unwrap().to_owned(), - expected: String::from( - "this value for `tap-fd` must be an unsigned integer", - ), - })?, - ); + let mut components = value.unwrap().split(','); + + let fd: RawDescriptor = components + .next() + .and_then(|x| x.parse().ok()) + .ok_or_else(|| argument::Error::InvalidValue { + value: value.unwrap().to_owned(), + expected: String::from("this value for `tap-fd` must be an unsigned integer"), + })?; + + let mut mac = None; + for c in components { + let mut kv = c.splitn(2, '='); + let (kind, value) = match (kv.next(), kv.next()) { + (Some(kind), Some(value)) => (kind, value), + _ => { + return Err(argument::Error::InvalidValue { + value: c.to_owned(), + expected: String::from("option must be of the form `kind=value`"), + }) + } + }; + match kind { + "mac" => { + mac = Some(value.parse().map_err(|_| argument::Error::InvalidValue { + value: value.to_owned(), + expected: String::from( + "`mac` needs to be in the form \"XX:XX:XX:XX:XX:XX\"", + ), + })?) + } + _ => { + return Err(argument::Error::InvalidValue { + value: kind.to_owned(), + expected: String::from("unrecognized option"), + }) + } + } + } + if cfg.tap_fd.contains_key(&fd) { + return Err(argument::Error::TooManyArguments(format!( + "TAP FD already used: '{}'", + name + ))); + } + cfg.tap_fd.insert(fd, TapFdOptions { mac }); } #[cfg(feature = "gpu")] "gpu" => { @@ -1644,8 +1679,8 @@ writeback=BOOL - Indicates whether the VM can use writeback caching (default: fa Argument::value("plugin-gid-map-file", "PATH", "Path to the file listing supplemental GIDs that should be mapped in plugin jail. Can be given more than once."), Argument::flag("vhost-net", "Use vhost for networking."), Argument::value("tap-fd", - "fd", - "File descriptor for configured tap device. A different virtual network card will be added each time this argument is given."), + "FD[,mac=MAC]", + "File descriptor for configured tap device. A different virtual network card will be added each time this argument is given. MAC is the MAC address that will be set in the guest."), #[cfg(feature = "gpu")] Argument::flag_or_value("gpu", "[width=INT,height=INT]", base-commit: f35d2c43ff19520855cffee761dc8899c5a439a1 -- 2.31.1