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: <640292b7-5fcd-44c2-bdd1-03702b7e60a1@linux.alibaba.com>
Date: Wed, 24 Apr 2024 21:41:55 +0800
From: Heng Qi <hengqi@...ux.alibaba.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, virtualization@...ts.linux.dev,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 Jason Wang <jasowang@...hat.com>, "Michael S . Tsirkin" <mst@...hat.com>,
 Brett Creeley <bcreeley@....com>, Ratheesh Kannoth <rkannoth@...vell.com>,
 Alexander Lobakin <aleksander.lobakin@...el.com>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Tal Gilboa <talgi@...dia.com>,
 Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
 Maxime Chevallier <maxime.chevallier@...tlin.com>,
 Jiri Pirko <jiri@...nulli.us>, Paul Greenwalt <paul.greenwalt@...el.com>,
 Ahmed Zaki <ahmed.zaki@...el.com>, Vladimir Oltean
 <vladimir.oltean@....com>, Kory Maincent <kory.maincent@...tlin.com>,
 Andrew Lunn <andrew@...n.ch>, "justinstitt@...gle.com"
 <justinstitt@...gle.com>
Subject: Re: [PATCH net-next v9 2/4] ethtool: provide customized dim profile
 management


>>> +++ b/include/linux/ethtool.h
>>> @@ -284,7 +284,11 @@ bool 
>>> ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
>>>   #define ETHTOOL_COALESCE_TX_AGGR_MAX_BYTES    BIT(24)
>>>   #define ETHTOOL_COALESCE_TX_AGGR_MAX_FRAMES    BIT(25)
>>>   #define ETHTOOL_COALESCE_TX_AGGR_TIME_USECS    BIT(26)
>>> -#define ETHTOOL_COALESCE_ALL_PARAMS        GENMASK(26, 0)
>>> +#define ETHTOOL_COALESCE_RX_EQE_PROFILE         BIT(27)
>>> +#define ETHTOOL_COALESCE_RX_CQE_PROFILE         BIT(28)
>>> +#define ETHTOOL_COALESCE_TX_EQE_PROFILE         BIT(29)
>>> +#define ETHTOOL_COALESCE_TX_CQE_PROFILE         BIT(30)
>>> +#define ETHTOOL_COALESCE_ALL_PARAMS        GENMASK(30, 0)
>> I think it's better to add these to netdev_ops, see below.


When modified according to v9's advice, I have the following structure, 
functions and ops:

+#define DIM_PROFILE_RX         BIT(0)  /* support rx dim profile 
modification */
+#define DIM_PROFILE_TX         BIT(1)  /* support tx dim profile 
modification */
+
+#define DIM_COALESCE_USEC      BIT(0)  /* support usec field 
modification */
+#define DIM_COALESCE_PKTS      BIT(1)  /* support pkts field 
modification */
+#define DIM_COALESCE_COMPS     BIT(2)  /* support comps field 
modification */
+
+struct dim_irq_moder {
+       /* See DIM_PROFILE_* */
+       u8 profile_flags;
+
+       /* See DIM_COALESCE_* for Rx and Tx */
+       u8 coal_flags;
+
+       /* Rx DIM period count mode: CQE or EQE */
+       u8 dim_rx_mode;
+
+       /* Tx DIM period count mode: CQE or EQE */
+       u8 dim_tx_mode;
+
+       /* DIM profile list for Rx */
+       struct dim_cq_moder *rx_profile;
+
+       /* DIM profile list for Tx */
+       struct dim_cq_moder *tx_profile;
+
+       /* Rx DIM worker function scheduled by net_dim() */
+       void (*rx_dim_work)(struct work_struct *work);
+
+       /* Tx DIM worker function scheduled by net_dim() */
+       void (*tx_dim_work)(struct work_struct *work);
+};
+

.....


+       .ndo_init_irq_moder     = virtnet_init_irq_moder,

....


+static int virtnet_init_irq_moder(struct net_device *dev)
+{
+        struct virtnet_info *vi = netdev_priv(dev);
+        struct dim_irq_moder *moder;
+        int len;
+
+        if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
+                return 0;
+
+        dev->irq_moder = kzalloc(sizeof(*dev->irq_moder), GFP_KERNEL);
+        if (!dev->irq_moder)
+                goto err_moder;
+
+        moder = dev->irq_moder;
+        len = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*moder->rx_profile);
+
+        moder->profile_flags |= DIM_PROFILE_RX;
+        moder->coal_flags |= DIM_COALESCE_USEC | DIM_COALESCE_PKTS;
+        moder->dim_rx_mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
+
+        moder->rx_dim_work = virtnet_rx_dim_work;
+
+        moder->rx_profile = kmemdup(dim_rx_profile[moder->dim_rx_mode],
+                                   len, GFP_KERNEL);
+        if (!moder->rx_profile)
+                goto err_profile;
+
+        return 0;
+
+err_profile:
+        kfree(moder);
+err_moder:
+        return -ENOMEM;
+}
+

......

+void net_dim_setting(struct net_device *dev, struct dim *dim, bool is_tx)
+{
+       struct dim_irq_moder *irq_moder = dev->irq_moder;
+
+       if (!irq_moder)
+               return;
+
+       if (is_tx) {
+               INIT_WORK(&dim->work, irq_moder->tx_dim_work);
+               dim->mode = irq_moder->dim_tx_mode;
+               return;
+       }
+
+       INIT_WORK(&dim->work, irq_moder->rx_dim_work);
+       dim->mode = irq_moder->dim_rx_mode;
+}

.....

+       for (i = 0; i < vi->max_queue_pairs; i++)
+               net_dim_setting(vi->dev, &vi->rq[i].dim, false);

.....

     ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES,          /* u32 */

     ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS,           /* u32 */

+ /* nest - _A_PROFILE_IRQ_MODERATION */

+  ETHTOOL_A_COALESCE_RX_PROFILE,

+   /* nest - _A_PROFILE_IRQ_MODERATION */
+  ETHTOOL_A_COALESCE_TX_PROFILE,

......


Almost clear implementation, but the only problem is when I want to

reuse "ethtool -C" and append ETHTOOL_A_COALESCE_RX_PROFILE

and ETHTOOL_A_COALESCE_TX_PROFILE, *ethnl_set_coalesce_validate()

will report an error because there are no ETHTOOL_COALESCE_RX_PROFILE

and ETHTOOL_COALESCE_TX_PROFILE, because they are replaced by

DIM_PROFILE_RX and DIM_PROFILE_TX in the field profile_flags.*


Should I reuse ETHTOOL_COALESCE_RX_PROFILE and

ETHTOOL_A_COALESCE_TX_PROFILE in ethtool_ops->.supported_coalesce_params

and remove the field profile_flags from struct dim_irq_moder?

Or let ethnl_set_coalesce_validate not verify these two flags?

Is there a better solution?


Thanks!



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ