netdev - Re: [PATCH net-next 2/4] vmxnet3: add latency measurement support in vmxnet3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240515104626.GE154012@kernel.org>
Date: Wed, 15 May 2024 11:46:26 +0100
From: Simon Horman <horms@...nel.org>
To: Ronak Doshi <ronak.doshi@...adcom.com>
Cc: netdev@...r.kernel.org,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 2/4] vmxnet3: add latency measurement support in
 vmxnet3

On Tue, May 14, 2024 at 11:20:47AM -0700, Ronak Doshi wrote:
> This patch enhances vmxnet3 to support latency measurement.
> This support will help to track the latency in packet processing
> between guest virtual nic driver and host. For this purpose, we
> introduce a new timestamp ring in vmxnet3 which will be per Tx/Rx
> queue. This ring will be used to carry timestamp of the packets
> which will be used to calculate the latency.
> 
> Signed-off-by: Ronak Doshi <ronak.doshi@...adcom.com>
> Acked-by: Guolin Yang <guolin.yang@...adcom.com>

...

> index b3f3136cc8be..74cb63e3d311 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -143,6 +143,29 @@ vmxnet3_tq_stop(struct vmxnet3_tx_queue *tq, struct vmxnet3_adapter *adapter)
>  	netif_stop_subqueue(adapter->netdev, (tq - adapter->tx_queue));
>  }
>  
> +static u64
> +vmxnet3_get_cycles(int pmc)
> +{
> +	u32 low, high;
> +
> +	asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (pmc));
> +	return (low | ((u_int64_t)high << 32));
> +}

Hi Ronak,

This seems to open-code the rdpmc macro.

And it also seems to exclude compilation of this driver other than for x86.
This seems undesirable as, in general, networking drivers are supposed to
be architecture independent. I'd say, doubly so, for software devices.

Moreover, rdpmc outside of x86 architecture-specific code seems highly
unusual to me. So I wonder if there is a better approach to the problem at
hand.

If not, I would suggest making this feature optional and only compiled
for x86. That might mean factoring it out into a different file. I'm
unsure.

If not, I think the driver's Kconfig needs to be updated to reflect
that it can only be compiled for x86.

...