From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 9BB056990B for ; Tue, 27 Jul 2021 17:03:33 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 886C91BA7D for ; Tue, 27 Jul 2021 17:03:03 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id A7CA01BA6C for ; Tue, 27 Jul 2021 17:03:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6A13D429C7 for ; Tue, 27 Jul 2021 17:03:01 +0200 (CEST) Message-ID: <679e56cc-9cd9-ae44-72ee-e01e3be053d4@proxmox.com> Date: Tue, 27 Jul 2021 17:02:59 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.0 Content-Language: en-US To: Oguz Bektas , Proxmox VE development discussion References: <20210727133709.1311314-1-o.bektas@proxmox.com> <166e8460-05ff-5791-6103-7e2a5fc796d4@proxmox.com> From: Thomas Lamprecht In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.616 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.438 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH container] vmstatus: include detected IP address of running containers X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Jul 2021 15:03:33 -0000 On 27.07.21 16:17, Oguz Bektas wrote: > On Tue, Jul 27, 2021 at 04:03:17PM +0200, Thomas Lamprecht wrote: >> On 27.07.21 15:37, Oguz Bektas wrote: >>> add a helper 'find_lxc_ip_address' to fetch IP address of container from >>> its network namespace using lxc-info. >>> >>> for the moment it can be queried with the pct tool: >>> $ pct status 1000 --verbose >>> cpu: 0 >>> cpus: 1 >>> disk: 6422528 >>> diskread: 368640 >>> diskwrite: 0 >>> ipaddress: 192.168.31.83 <---- >>> maxdisk: 4294967296 >>> maxmem: 536870912 >>> maxswap: 536870912 >>> mem: 864256 >>> name: CT1000 >>> netin: 3281265 >>> netout: 15794 >>> pid: 34897 >>> status: running >>> swap: 94208 >>> type: lxc >>> uptime: 11088 >>> vmid: 1000 >>> >> >> >> And why use `lxc-info`, this is called very often and we know about setups with >> 1500+ CTs on a single host, so it'd be good to check if adding 1000s forks every >> status call could be avoided. > > lxc-info already queries this information from netlink, otherwise switching > network namespace and communicating to netlink in perl would be hacky (at least > that's the impression i got from looking at the relevant lxc code [0][1]). Does not have to be hacky if done nicely and we can do that in rust and use only the bindings in perl.. Mira already did quite some work for the contrack stuff, that could possibly be re-used/shaped into something for that use case. But we cannot really do away the fork their either, as unshare into another NS is only valid for the current's process future children... > > [0]: https://github.com/lxc/lxc/blob/f1c64634c40a7218165538b89aca320fa258b3c1/src/lxc/lxccontainer.c#L2235 > [1]: https://github.com/lxc/lxc/blob/f1c64634c40a7218165538b89aca320fa258b3c1/src/lxc/lxccontainer.c#L2429 > > fabian also mentioned we can use 'iproute2': sure, would be useful as it actually gets you the full CIDR and possibly any other detail. > $ lxc-info --pid 1000 > PID: 34897 > $ ip netns attach 1000 34897 > $ ip -n 1000 a show three commands isn't exactly better than the "no command" I wished for ;-) > 1: lo: mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000 > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > inet 127.0.0.1/8 scope host lo > valid_lft forever preferred_lft forever > inet6 ::1/128 scope host > valid_lft forever preferred_lft forever > 2: eth0@if84: mtu 1500 qdisc noqueue state UP group default qlen 1000 > link/ether fa:2e:e1:b6:a4:b0 brd ff:ff:ff:ff:ff:ff link-netns 1000 > inet 192.168.31.83/20 scope global eth0 > valid_lft forever preferred_lft forever > inet6 fe80::f82e:e1ff:feb6:a4b0/64 scope link > valid_lft forever preferred_lft forever > > but that leaves the namespace file in /run/netns/ which requires extra cleanup > and handling when container is rebooting or shutting down, so this way seemed > like it would be cleaner. yeah no, that seems icky. > > for avoiding the forks in bigger setups it might make sense to call it once and > cache the result until there's a network hotplug or similar, or only poll every N minutes. the latter is adding a lot of complexity, the former can work out but would need to be in /run or the like, else every worker would just cache 1000s of entries eventually, and that's not really that what a simple and clean solution would look like either. We could replicate what we do for VMs, adding a `$full` boolean flag and only query that info if it's true, that'd be at least dead simple. > >> what about multiple, you only print the last match which is quite confusing... >> You forgot to add this to the API/CLI return schema in $PVE::LXC::vmstatus_return_properties, >> which may highly probably actually want to be an array... > > okay will do that in v2 with an array to account for multiple addresses > and also: On 27.07.21 16:03, Thomas Lamprecht wrote: > You forgot to add this to the API/CLI return schema in $PVE::LXC::vmstatus_return_properties, > which may highly probably actually want to be an array...