netdev - Re: [RFCv3 PATCH net-next 2/2] net-next: hn3: add tx push support in hns3 ring param process

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] [day] [month] [year] [list]

Message-ID: <c26fc6ad-3517-48aa-1e30-5a1659e49c20@huawei.com>
Date:   Wed, 30 Mar 2022 16:56:31 +0800
From:   "wangjie (L)" <wangjie125@...wei.com>
To:     Michal Kubecek <mkubecek@...e.cz>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <netdev@...r.kernel.org>,
        <huangguangbin2@...wei.com>, <lipeng321@...wei.com>,
        <shenjian15@...wei.com>, <moyufeng@...wei.com>,
        <linyunsheng@...wei.com>, <salil.mehta@...wei.com>,
        <chenhao288@...ilicon.com>
Subject: Re: [RFCv3 PATCH net-next 2/2] net-next: hn3: add tx push support in
 hns3 ring param process



On 2022/3/30 3:44, Michal Kubecek wrote:
> On Tue, Mar 29, 2022 at 05:19:13PM +0800, Jie Wang wrote:
>> This patch adds tx push param to hns3 ring param and adapts the set and get
>> API of ring params. So users can set it by cmd ethtool -G and get it by cmd
>> ethtool -g.
>>
>> Signed-off-by: Jie Wang <wangjie125@...wei.com>
>> ---
>>  .../ethernet/hisilicon/hns3/hns3_ethtool.c    | 33 ++++++++++++++++++-
>>  1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> index 6469238ae090..5bc509f90d2a 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> @@ -664,6 +664,8 @@ static void hns3_get_ringparam(struct net_device *netdev,
>>  	param->tx_pending = priv->ring[0].desc_num;
>>  	param->rx_pending = priv->ring[rx_queue_index].desc_num;
>>  	kernel_param->rx_buf_len = priv->ring[rx_queue_index].buf_size;
>> +	kernel_param->tx_push = test_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE,
>> +					 &priv->state);
>>  }
>>
>>  static void hns3_get_pauseparam(struct net_device *netdev,
>> @@ -1114,6 +1116,30 @@ static int hns3_change_rx_buf_len(struct net_device *ndev, u32 rx_buf_len)
>>  	return 0;
>>  }
>>
>> +static int hns3_set_tx_push(struct net_device *netdev, u32 tx_push)
>> +{
>> +	struct hns3_nic_priv *priv = netdev_priv(netdev);
>> +	struct hnae3_handle *h = hns3_get_handle(netdev);
>> +	struct hnae3_ae_dev *ae_dev = pci_get_drvdata(h->pdev);
>> +	u32 old_state = test_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
>> +
>> +	if (!test_bit(HNAE3_DEV_SUPPORT_TX_PUSH_B, ae_dev->caps) && tx_push)
>> +		return -EOPNOTSUPP;
>> +
>> +	if (tx_push == old_state)
>> +		return 0;
>> +
>> +	netdev_info(netdev, "Changing tx push from %s to %s\n",
>> +		    old_state ? "on" : "off", tx_push ? "on" : "off");
>
> A nitpick: do we really want an unconditional log message for each
> change? If someone wants to monitor them, that's what the netlink
> notifications were created for.
>
Actually this log is no need to display for each change, I will use 
netdev_dbg instead.
> Michal
>
>> +
>> +	if (tx_push)
>> +		set_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
>> +	else
>> +		clear_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
>> +
>> +	return 0;
>> +}
>> +
>>  static int hns3_set_ringparam(struct net_device *ndev,
>>  			      struct ethtool_ringparam *param,
>>  			      struct kernel_ethtool_ringparam *kernel_param,