netdev - Re: [PATCH net-next V2] Documentation/networking: Add net DIM documentation

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: <31507706-8a7a-5afd-9df3-90fc7ec53442@gmail.com>
Date:   Wed, 21 Mar 2018 12:33:51 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Tal Gilboa <talgi@...lanox.com>,
        "David S. Miller" <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: Re: [PATCH net-next V2] Documentation/networking: Add net DIM
 documentation

Hi Tal,

On 03/21/2018 11:33 AM, Tal Gilboa wrote:
> Net DIM is a generic algorithm, purposed for dynamically
> optimizing network devices interrupt moderation. This
> document describes how it works and how to use it.

Thanks a lot for providing this documentation, this is very helpful! A
few things that could be good to be expanded upon a little bit:

- HW must support configuring a timeout per channel
- HW must support configuring a number of packets before getting an
interrupt per channel

Does that sound about right?

[snip]

> +In order to use Net DIM from a networking driver, the driver needs to call the
> +main net_dim() function. The recommended method is to call net_dim() on each
> +interrupt.

I would make it a bit clearer that this is on each invocation of the
interrupt service routine function. With NAPI + net DIM working in
concert you would not actually get one packet per interrupt
consistently, it would largely depend on the rate, right?

> Since Net DIM has a built-in moderation and it might decide to skip
> +iterations under certain conditions, there is no need to moderate the net_dim()
> +calls as well. As mentioned above, the driver needs to provide an object of type
> +struct net_dim to the net_dim() function call. It is advised for each entity
> +using Net DIM to hold a struct net_dim as part of its data structure and use it
> +as the main Net DIM API object. The struct net_dim_sample should hold the latest
> +bytes, packets and interrupts count. No need to perform any calculations, just
> +include the raw data.
> +
> +The net_dim() call itself does not return anything. Instead Net DIM relies on
> +the driver to provide a callback function, which is called when the algorithm
> +decides to make a change in the interrupt moderation parameters. This callback
> +will be scheduled and run in a separate thread in order not to add overhead to
> +the data flow. After the work is done, Net DIM algorithm needs to be set to
> +the proper state in order to move to the next iteration.
> +
> +
> +Part IV: Example
> +=================
> +
> +The following code demonstrates how to register a driver to Net DIM. The actual
> +usage is not complete but it should make the outline of the usage clear.

It could be worth to touch a word or two about reflecting the use of Net
DIM within the driver into ethtool_coalesce::use_adaptive_rx_coalesce
and ethtool_coalesce::use_adaptive_tx_coalesce?

> +
> +my_driver.c:
> +
> +#include <linux/net_dim.h>
> +
> +/* Callback for net DIM to schedule on a decision to change moderation */
> +void my_driver_do_dim_work(struct work_struct *work)
> +{
> +	/* Get struct net_dim from struct work_struct */
> +	struct net_dim *dim = container_of(work, struct net_dim,
> +					   work);
> +	/* Do interrupt moderation related stuff */
> +	...
> +
> +	/* Signal net DIM work is done and it should move to next iteration */
> +	dim->state = NET_DIM_START_MEASURE;
> +}
> +
> +/* My driver's interrupt handler */
> +int my_driver_handle_interrupt(struct my_driver_entity *my_entity, ...)
> +{
> +	...
> +	/* A struct to hold current measured data */
> +	struct net_dim_sample dim_sample;
> +	...
> +	/* Initiate data sample struct with current data */
> +	net_dim_sample(my_entity->events,
> +		       my_entity->packets,
> +		       my_entity->bytes,
> +		       &dim_sample);
> +	/* Call net DIM */
> +	net_dim(&my_entity->dim, dim_sample);
> +	...
> +}
> +
> +/* My entity's initialization function (my_entity was already allocated) */
> +int my_driver_init_my_entity(struct my_driver_entity *my_entity, ...)
> +{
> +	...
> +	/* Initiate struct work_struct with my driver's callback function */
> +	INIT_WORK(&my_entity->dim.work, my_driver_do_dim_work);
> +	...
> +}
> 


-- 
Florian