在 2024/4/25 上午12:18, Jakub Kicinski 写道:
On Wed, 24 Apr 2024 21:41:55 +0800 Heng Qi wrote:
+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,
The init callback mostly fills in static data, can we not
declare the driver information statically and move the init
code into the core?
Now the init callback is used as following
In dim.c:
+int net_dim_init_irq_moder(struct net_device *dev)
+{
+ if (dev->netdev_ops && dev->netdev_ops->ndo_init_irq_moder)
+ return dev->netdev_ops->ndo_init_irq_moder(dev);
+
+ return 0;
+}
+EXPORT_SYMBOL(net_dim_init_irq_moder);
In dev.c
@@ -10258,6 +10259,10 @@ int register_netdevice(struct net_device *dev)
if (ret)
return ret;
+ ret = net_dim_init_irq_moder(dev);
+ if (ret)
+ return ret;
+
spin_lock_init(&dev->addr_list_lock);
netdev_set_addr_lockdep_class(dev);
The collected flags, mode, and work must obtain driver-specific
values from the driver. If I'm not wrong, you don't want an interface
like .ndo_init_irq_moder, but instead provide a generic interface in
dim.c (e.g. net_dim_init_irq_moder() with parameters dev and struct
dim_irq_moder or separate flags,mode,work). Then this func is called
by the driver in the probe phase?
....
+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.*
I see.
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?
Maybe create the bits but automatically add them for the driver?
Ok. I think it works.
Thanks!
diff --git a/net/ethtool/coalesce.c b/net/ethtool/coalesce.c
index 83112c1a71ae..56777d36f7f1 100644
--- a/net/ethtool/coalesce.c
+++ b/net/ethtool/coalesce.c
@@ -243,6 +243,8 @@ ethnl_set_coalesce_validate(struct ethnl_req_info *req_info,
/* make sure that only supported parameters are present */
supported_params = ops->supported_coalesce_params;
+ if (dev->moder->coal_flags ...)
+ supported_params |= ETHTOOL_COALESCE_...;
for (a = ETHTOOL_A_COALESCE_RX_USECS; a < __ETHTOOL_A_COALESCE_CNT; a++)
if (tb[a] && !(supported_params & attr_to_mask(a))) {
NL_SET_ERR_MSG_ATTR(info->extack, tb[a],
