| 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
| ||
|
Date: Fri, 20 Aug 2021 21:27:13 +0300
From: Grygorii Strashko <grygorii.strashko@...com>
To: Yufeng Mo <moyufeng@...wei.com>, <davem@...emloft.net>,
<kuba@...nel.org>
CC: <netdev@...r.kernel.org>, <shenjian15@...wei.com>,
<lipeng321@...wei.com>, <yisen.zhuang@...wei.com>,
<linyunsheng@...wei.com>, <huangguangbin2@...wei.com>,
<chenhao288@...ilicon.com>, <salil.mehta@...wei.com>,
<linuxarm@...wei.com>, <linuxarm@...neuler.org>,
<dledford@...hat.com>, <jgg@...pe.ca>, <netanel@...zon.com>,
<akiyano@...zon.com>, <thomas.lendacky@....com>,
<irusskikh@...vell.com>, <michael.chan@...adcom.com>,
<edwin.peer@...adcom.com>, <rohitm@...lsio.com>,
<jacob.e.keller@...el.com>, <ioana.ciornei@....com>,
<vladimir.oltean@....com>, <sgoutham@...vell.com>,
<sbhatta@...vell.com>, <saeedm@...dia.com>,
<ecree.xilinx@...il.com>, <merez@...eaurora.org>,
<kvalo@...eaurora.org>, <linux-wireless@...r.kernel.org>
Subject: Re: [PATCH V3 net-next 2/4] ethtool: extend coalesce setting uAPI
with CQE mode
On 20/08/2021 10:35, Yufeng Mo wrote:
> In order to support more coalesce parameters through netlink,
> add two new parameter kernel_coal and extack for .set_coalesce
> and .get_coalesce, then some extra info can return to user with
> the netlink API.
>
> Signed-off-by: Yufeng Mo <moyufeng@...wei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@...wei.com>
> ---
> drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 8 ++++++--
> drivers/net/ethernet/amazon/ena/ena_ethtool.c | 8 ++++++--
> drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 8 ++++++--
> drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c | 8 ++++++--
> drivers/net/ethernet/broadcom/bcmsysport.c | 8 ++++++--
> drivers/net/ethernet/broadcom/bnx2.c | 12 ++++++++----
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 8 ++++++--
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 8 ++++++--
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 8 ++++++--
> drivers/net/ethernet/broadcom/tg3.c | 10 ++++++++--
> drivers/net/ethernet/brocade/bna/bnad_ethtool.c | 12 ++++++++----
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c | 8 ++++++--
> drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c | 4 +++-
> drivers/net/ethernet/chelsio/cxgb/cxgb2.c | 8 ++++++--
> drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++++++--
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c | 8 ++++++--
> drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 8 ++++++--
> drivers/net/ethernet/cisco/enic/enic_ethtool.c | 8 ++++++--
> drivers/net/ethernet/cortina/gemini.c | 8 ++++++--
> drivers/net/ethernet/emulex/benet/be_ethtool.c | 8 ++++++--
> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c | 8 ++++++--
> drivers/net/ethernet/freescale/enetc/enetc_ethtool.c | 8 ++++++--
> drivers/net/ethernet/freescale/fec_main.c | 14 +++++++++-----
> drivers/net/ethernet/freescale/gianfar_ethtool.c | 8 ++++++--
> drivers/net/ethernet/hisilicon/hip04_eth.c | 8 ++++++--
> drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 12 ++++++++++--
> drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 8 ++++++--
> drivers/net/ethernet/huawei/hinic/hinic_ethtool.c | 8 ++++++--
> drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 8 ++++++--
> drivers/net/ethernet/intel/e1000e/ethtool.c | 8 ++++++--
> drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 8 ++++++--
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 ++++++++++--
> drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 12 ++++++++++--
> drivers/net/ethernet/intel/ice/ice_ethtool.c | 12 ++++++++----
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 8 ++++++--
> drivers/net/ethernet/intel/igbvf/ethtool.c | 8 ++++++--
> drivers/net/ethernet/intel/igc/igc_ethtool.c | 8 ++++++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 8 ++++++--
> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 8 ++++++--
> drivers/net/ethernet/jme.c | 12 ++++++++----
> drivers/net/ethernet/marvell/mv643xx_eth.c | 12 ++++++++----
> drivers/net/ethernet/marvell/mvneta.c | 14 ++++++++++----
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 14 ++++++++++----
> .../net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 8 ++++++--
> drivers/net/ethernet/marvell/skge.c | 8 ++++++--
> drivers/net/ethernet/marvell/sky2.c | 8 ++++++--
> drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 8 ++++++--
> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 8 ++++++--
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 8 ++++++--
> .../net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c | 8 ++++++--
> drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 12 ++++++++----
> drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c | 8 ++++++--
> drivers/net/ethernet/ni/nixge.c | 14 ++++++++++----
> drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 8 ++++++--
> .../net/ethernet/qlogic/netxen/netxen_nic_ethtool.c | 8 ++++++--
> drivers/net/ethernet/qlogic/qede/qede.h | 4 +++-
> drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 8 ++++++--
> drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c | 8 ++++++--
> drivers/net/ethernet/realtek/r8169_main.c | 10 ++++++++--
> drivers/net/ethernet/samsung/sxgbe/sxgbe_ethtool.c | 8 ++++++--
> drivers/net/ethernet/sfc/ethtool.c | 8 ++++++--
> drivers/net/ethernet/sfc/falcon/ethtool.c | 8 ++++++--
> drivers/net/ethernet/socionext/netsec.c | 10 +++++++---
> drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 8 ++++++--
> drivers/net/ethernet/synopsys/dwc-xlgmac-ethtool.c | 14 ++++++++++----
> drivers/net/ethernet/tehuti/tehuti.c | 12 ++++++++----
> drivers/net/ethernet/ti/cpsw.c | 2 +-
> drivers/net/ethernet/ti/cpsw_ethtool.c | 8 ++++++--
> drivers/net/ethernet/ti/cpsw_new.c | 2 +-
> drivers/net/ethernet/ti/cpsw_priv.h | 8 ++++++--
> drivers/net/ethernet/ti/davinci_emac.c | 14 +++++++++++---
> drivers/net/ethernet/via/via-velocity.c | 8 ++++++--
> drivers/net/ethernet/xilinx/ll_temac_main.c | 14 ++++++++++----
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 18 ++++++++++++++----
> drivers/net/netdevsim/ethtool.c | 8 ++++++--
> drivers/net/tun.c | 8 ++++++--
> drivers/net/usb/r8152.c | 8 ++++++--
> drivers/net/virtio_net.c | 8 ++++++--
> drivers/net/vmxnet3/vmxnet3_ethtool.c | 12 ++++++++----
> drivers/net/wireless/ath/wil6210/ethtool.c | 14 ++++++++++----
> drivers/s390/net/qeth_ethtool.c | 4 +++-
> drivers/staging/qlge/qlge_ethtool.c | 10 ++++++++--
> include/linux/ethtool.h | 11 +++++++++--
> net/ethtool/coalesce.c | 10 +++++++---
> net/ethtool/ioctl.c | 15 ++++++++++++---
> 85 files changed, 576 insertions(+), 202 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
> index 823f683..a09ca21 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
> @@ -72,7 +72,9 @@ static void ipoib_get_drvinfo(struct net_device *netdev,
> }
>
> static int ipoib_get_coalesce(struct net_device *dev,
> - struct ethtool_coalesce *coal)
> + struct ethtool_coalesce *coal,
> + struct kernel_ethtool_coalesce *kernel_coal,
> + struct netlink_ext_ack *extack)
> {
> struct ipoib_dev_priv *priv = ipoib_priv(dev);
>
> @@ -83,7 +85,9 @@ static int ipoib_get_coalesce(struct net_device *dev,
> }
>
> static int ipoib_set_coalesce(struct net_device *dev,
> - struct ethtool_coalesce *coal)
> + struct ethtool_coalesce *coal,
> + struct kernel_ethtool_coalesce *kernel_coal,
> + struct netlink_ext_ack *extack)
> {
> struct ipoib_dev_priv *priv = ipoib_priv(dev);
> int ret;
[...]
This is very big change which is not only mix two separate changes, but also looks
half-done. From one side you're adding HW feature supported by limited number of HW,
from another - changing most of net drivers to support it by generating mix of legacy
and new kernel_ethtool_coalesce parameters.
There is also an issue - you do not account get/set_per_queue_coalesce() in any way.
Would it be possible to consider following, please?
- move extack change out of this series
- option (a)
add new callbacks in ethtool_ops as set_coalesce_cqe/get_coalesce_cqe for CQE support.
Only required drivers will need to be changed.
- option (b)
add struct ethtool_coalesce as first field of kernel_ethtool_coalesce
struct kernel_ethtool_coalesce {
/* legacy */
struct ethtool_coalesce coal;
/* new */
u8 use_cqe_mode_tx;
u8 use_cqe_mode_rx;
};
-- then b.1
drivers can be updated as
static int set_coalesce(struct net_device *dev,
struct kernel_ethtool_coalesce *kernel_coal)
{
struct ethtool_coalesce *coal = &kernel_coal->coal;
(which will clearly indicate migration to the new interface )
-- then b.2
add new callbacks in ethtool_ops as set_coalesce_ext/get_coalesce_ext (extended)
which will accept struct kernel_ethtool_coalesce as parameter an allow drivers to migrate when needed
(or as separate patch which will do only migration).
Personally, I like "b.2".
--
Best regards,
grygorii
Powered by blists - more mailing lists