From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk0-f172.google.com ([209.85.220.172]:44116 "EHLO mail-qk0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753121AbeCUTeG (ORCPT ); Wed, 21 Mar 2018 15:34:06 -0400 Received: by mail-qk0-f172.google.com with SMTP id h14so6708917qkj.11 for ; Wed, 21 Mar 2018 12:34:06 -0700 (PDT) Subject: Re: [PATCH net-next V2] Documentation/networking: Add net DIM documentation To: Tal Gilboa , "David S. Miller" Cc: "netdev@vger.kernel.org" , Tariq Toukan References: <1521657225-65392-1-git-send-email-talgi@mellanox.com> From: Florian Fainelli Message-ID: <31507706-8a7a-5afd-9df3-90fc7ec53442@gmail.com> Date: Wed, 21 Mar 2018 12:33:51 -0700 MIME-Version: 1.0 In-Reply-To: <1521657225-65392-1-git-send-email-talgi@mellanox.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: netdev-owner@vger.kernel.org List-ID: 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 > + > +/* 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