| Message ID | 20210315064448.16391-1-boon.leong.ong@intel.com |
|---|---|
| Headers | show |
| Series | net: stmmac: add per-q coalesce support | expand |
On Mon, 15 Mar 2021 14:44:48 +0800 Ong Boon Leong wrote: > Extending the driver to support per-queue RX and TX coalesce settings in > order to support below commands: > > To show per-queue coalesce setting:- > $ ethtool --per-queue <DEVNAME> queue_mask <MASK> --show-coalesce > > To set per-queue coalesce setting:- > $ ethtool --per-queue <DEVNAME> queue_mask <MASK> --coalesce \ > [rx-usecs N] [rx-frames M] [tx-usecs P] [tx-frames Q] > > Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com> > -static int stmmac_get_coalesce(struct net_device *dev, > - struct ethtool_coalesce *ec) > +static int __stmmac_get_coalesce(struct net_device *dev, > + struct ethtool_coalesce *ec, > + int queue) > { > struct stmmac_priv *priv = netdev_priv(dev); > + u32 max_cnt; > + u32 rx_cnt; > + u32 tx_cnt; > > - ec->tx_coalesce_usecs = priv->tx_coal_timer; > - ec->tx_max_coalesced_frames = priv->tx_coal_frames; > + rx_cnt = priv->plat->rx_queues_to_use; > + tx_cnt = priv->plat->tx_queues_to_use; > + max_cnt = max(rx_cnt, tx_cnt); > > - if (priv->use_riwt) { > - ec->rx_max_coalesced_frames = priv->rx_coal_frames; > - ec->rx_coalesce_usecs = stmmac_riwt2usec(priv->rx_riwt, priv); > + if (queue < 0) > + queue = 0; > + else if (queue >= max_cnt) > + return -EINVAL; > + > + if (queue < tx_cnt) { > + ec->tx_coalesce_usecs = priv->tx_coal_timer[queue]; > + ec->tx_max_coalesced_frames = priv->tx_coal_frames[queue]; > + } else { > + ec->tx_coalesce_usecs = -1; > + ec->tx_max_coalesced_frames = -1; > + } > + > + if (priv->use_riwt && queue < rx_cnt) { > + ec->rx_max_coalesced_frames = priv->rx_coal_frames[queue]; > + ec->rx_coalesce_usecs = stmmac_riwt2usec(priv->rx_riwt[queue], > + priv); > + } else { > + ec->rx_max_coalesced_frames = -1; > + ec->rx_coalesce_usecs = -1; Why the use of negative values? why not leave them as 0? > } > > return 0; > } > > -static int stmmac_set_coalesce(struct net_device *dev, > +static int stmmac_get_coalesce(struct net_device *dev, > struct ethtool_coalesce *ec) > +{ > + return __stmmac_get_coalesce(dev, ec, -1); > +} > + > +static int stmmac_get_per_queue_coalesce(struct net_device *dev, u32 queue, > + struct ethtool_coalesce *ec) > +{ > + return __stmmac_get_coalesce(dev, ec, queue); > +} > + > +static int __stmmac_set_coalesce(struct net_device *dev, > + struct ethtool_coalesce *ec, > + int queue) > { > struct stmmac_priv *priv = netdev_priv(dev); > - u32 rx_cnt = priv->plat->rx_queues_to_use; > + bool all_queues = false; > unsigned int rx_riwt; > + u32 max_cnt; > + u32 rx_cnt; > + u32 tx_cnt; > + > + rx_cnt = priv->plat->rx_queues_to_use; > + tx_cnt = priv->plat->tx_queues_to_use; > + max_cnt = max(rx_cnt, tx_cnt); > + > + if (queue < 0) > + all_queues = true; > + else if (queue >= max_cnt) > + return -EINVAL; > + > + /* Check not supported parameters */ > + if (ec->rx_coalesce_usecs_irq || > + ec->rx_max_coalesced_frames_irq || ec->tx_coalesce_usecs_irq || > + ec->use_adaptive_rx_coalesce || ec->use_adaptive_tx_coalesce || > + ec->pkt_rate_low || ec->rx_coalesce_usecs_low || > + ec->rx_max_coalesced_frames_low || ec->tx_coalesce_usecs_high || > + ec->tx_max_coalesced_frames_low || ec->pkt_rate_high || > + ec->tx_coalesce_usecs_low || ec->rx_coalesce_usecs_high || > + ec->rx_max_coalesced_frames_high || > + ec->tx_max_coalesced_frames_irq || > + ec->stats_block_coalesce_usecs || > + ec->tx_max_coalesced_frames_high || ec->rate_sample_interval) > + return -EOPNOTSUPP; This shouldn't be needed now that supporter types are expressed in dev->ethtool_ops->supported_coalesce_params, no? > if (priv->use_riwt && (ec->rx_coalesce_usecs > 0)) { > rx_riwt = stmmac_usec2riwt(ec->rx_coalesce_usecs, priv); > @@ -785,8 +846,23 @@ static int stmmac_set_coalesce(struct net_device *dev, > if ((rx_riwt > MAX_DMA_RIWT) || (rx_riwt < MIN_DMA_RIWT)) > return -EINVAL; > > - priv->rx_riwt = rx_riwt; > - stmmac_rx_watchdog(priv, priv->ioaddr, priv->rx_riwt, rx_cnt); > + if (all_queues) { > + int i; > + > + for (i = 0; i < rx_cnt; i++) { > + priv->rx_riwt[i] = rx_riwt; > + stmmac_rx_watchdog(priv, priv->ioaddr, > + rx_riwt, i); > + priv->rx_coal_frames[i] = > + ec->rx_max_coalesced_frames; > + } > + } else if (queue < rx_cnt) { > + priv->rx_riwt[queue] = rx_riwt; > + stmmac_rx_watchdog(priv, priv->ioaddr, > + rx_riwt, queue); > + priv->rx_coal_frames[queue] = > + ec->rx_max_coalesced_frames; > + } > } > > if ((ec->tx_coalesce_usecs == 0) && > @@ -797,13 +873,37 @@ static int stmmac_set_coalesce(struct net_device *dev, > (ec->tx_max_coalesced_frames > STMMAC_TX_MAX_FRAMES)) > return -EINVAL; > > - /* Only copy relevant parameters, ignore all others. */ > - priv->tx_coal_frames = ec->tx_max_coalesced_frames; > - priv->tx_coal_timer = ec->tx_coalesce_usecs; > - priv->rx_coal_frames = ec->rx_max_coalesced_frames; > + if (all_queues) { > + int i; > + > + for (i = 0; i < tx_cnt; i++) { > + priv->tx_coal_frames[i] = > + ec->tx_max_coalesced_frames; > + priv->tx_coal_timer[i] = > + ec->tx_coalesce_usecs; > + } > + } else if (queue < tx_cnt) { > + priv->tx_coal_frames[queue] = > + ec->tx_max_coalesced_frames; > + priv->tx_coal_timer[queue] = > + ec->tx_coalesce_usecs; > + } > + > return 0; > }
>> + if (queue < tx_cnt) { >> + ec->tx_coalesce_usecs = priv->tx_coal_timer[queue]; >> + ec->tx_max_coalesced_frames = priv->tx_coal_frames[queue]; >> + } else { >> + ec->tx_coalesce_usecs = -1; >> + ec->tx_max_coalesced_frames = -1; >> + } >> + >> + if (priv->use_riwt && queue < rx_cnt) { >> + ec->rx_max_coalesced_frames = priv->rx_coal_frames[queue]; >> + ec->rx_coalesce_usecs = stmmac_riwt2usec(priv- >>rx_riwt[queue], >> + priv); >> + } else { >> + ec->rx_max_coalesced_frames = -1; >> + ec->rx_coalesce_usecs = -1; > >Why the use of negative values? why not leave them as 0? The initial logic was to return negative value to unsupported TXQ & RXQ since they are invalid. No preference here. So, we can leave it as all zeros. > >> } >> >> return 0; >> } >> >> -static int stmmac_set_coalesce(struct net_device *dev, >> +static int stmmac_get_coalesce(struct net_device *dev, >> struct ethtool_coalesce *ec) >> +{ >> + return __stmmac_get_coalesce(dev, ec, -1); >> +} >> + >> +static int stmmac_get_per_queue_coalesce(struct net_device *dev, u32 >queue, >> + struct ethtool_coalesce *ec) >> +{ >> + return __stmmac_get_coalesce(dev, ec, queue); >> +} >> + >> +static int __stmmac_set_coalesce(struct net_device *dev, >> + struct ethtool_coalesce *ec, >> + int queue) >> { >> struct stmmac_priv *priv = netdev_priv(dev); >> - u32 rx_cnt = priv->plat->rx_queues_to_use; >> + bool all_queues = false; >> unsigned int rx_riwt; >> + u32 max_cnt; >> + u32 rx_cnt; >> + u32 tx_cnt; >> + >> + rx_cnt = priv->plat->rx_queues_to_use; >> + tx_cnt = priv->plat->tx_queues_to_use; >> + max_cnt = max(rx_cnt, tx_cnt); >> + >> + if (queue < 0) >> + all_queues = true; >> + else if (queue >= max_cnt) >> + return -EINVAL; >> + >> + /* Check not supported parameters */ >> + if (ec->rx_coalesce_usecs_irq || >> + ec->rx_max_coalesced_frames_irq || ec->tx_coalesce_usecs_irq || >> + ec->use_adaptive_rx_coalesce || ec->use_adaptive_tx_coalesce || >> + ec->pkt_rate_low || ec->rx_coalesce_usecs_low || >> + ec->rx_max_coalesced_frames_low || ec->tx_coalesce_usecs_high || >> + ec->tx_max_coalesced_frames_low || ec->pkt_rate_high || >> + ec->tx_coalesce_usecs_low || ec->rx_coalesce_usecs_high || >> + ec->rx_max_coalesced_frames_high || >> + ec->tx_max_coalesced_frames_irq || >> + ec->stats_block_coalesce_usecs || >> + ec->tx_max_coalesced_frames_high || ec->rate_sample_interval) >> + return -EOPNOTSUPP; > >This shouldn't be needed now that supporter types are expressed in >dev->ethtool_ops->supported_coalesce_params, no? Wil fix this. Thanks for pointing out.