[<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