在 2021/4/13 19:17, Salil Mehta 写道:
From: lipeng (Y) Sent: Tuesday, April 13, 2021 10:30 AM To: Salil Mehta salil.mehta@huawei.com; Linuxarm linuxarm@huawei.com; linuxarm@openeuler.org Cc: Zhuangyuzeng (Yisen) yisen.zhuang@huawei.com; huangdaode huangdaode@huawei.com; linyunsheng linyunsheng@huawei.com; shenjian (K) shenjian15@huawei.com; moyufeng moyufeng@huawei.com; zhangjiaran zhangjiaran@huawei.com; huangguangbin (A) huangguangbin2@huawei.com; Jonathan Cameron jonathan.cameron@huawei.com Subject: Re: [PATCH RFC net] net: hns3: fixes+refactors the broken set channel fallback logic
在 2021/4/13 17:12, Salil Mehta 写道:
The fallback logic of the set channels when set_channel() fails to configure TX Sched/RSS H/W configuration or for the code which brings down/restores client before/therafter is not handled properly.
This patch fixes and refactors that code to improve the readibility.
[INTERNAL ONLY] Fixes: <Tag TODO> ("Comment TODO") Signed-off-by: Salil Mehta salil.mehta@huawei.com
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 72 +++++++++++-------- 1 file changed, 43 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index bf4302a5cf95..41d8bdb34f6e 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c @@ -4692,23 +4692,56 @@ static int hns3_reset_notify(struct hnae3_handle
*handle,
static int hns3_change_channels(struct hnae3_handle *handle, u32
new_tqp_num,
bool rxfh_configured){
- int ret;
- const struct hnae3_ae_ops *ops = handle->ae_algo->ops;
- u32 org_tqp_num = handle->kinfo.num_tqps;
- struct device *dev = &handle->pdev->dev;
- u32 req_tqp_num = new_tqp_num;
- bool revert_old_config = false;
- int ret, retval = 0;
- /* bring down the client */
- ret = hns3_reset_notify(handle, HNAE3_DOWN_CLIENT);
- if (ret)
return ret;
- ret = handle->ae_algo->ops->set_channels(handle, new_tqp_num,
rxfh_configured);- if (ret) {
dev_err(&handle->pdev->dev,"Change tqp num(%u) fail.\n", new_tqp_num);
- ret = hns3_reset_notify(handle, HNAE3_UNINIT_CLIENT);
- if (ret) return ret;
+revert_old_tpqs_config:
- /* update the traffic sched and RSS config in the H/W as per new TPQ num
*/
- ret = ops->set_channels(handle, req_tqp_num, rxfh_configured);
- if (ret) {
dev_err(dev, "failed(=%d) to update TX sched/RSS h/w cfg for %s TPQs\n",ret, revert_old_config?"old":"new");/* try reverting back h/w config with old TPQ num */if (!revert_old_config) {dev_warn(dev,"will try to revert TX sched/RSS h/w cfg with old TPQs\n");req_tqp_num = org_tqp_num;revert_old_config = true;retval = ret;goto revert_old_tpqs_config;}/* bad, we could not revert to h/w config with old TQP number */goto err_set_chan_fail;why you changelike this ?
when setting fail, the port should work OK just like setting before.
The port can get into problematic state if the client remains in un-initialized state after errors are encountered during the process of setting the channels for new number of TPQs. Port cannot be said to be in sane state.
Conditions(after desperate attempt to revert to old TPQ config fails):
- half un-initialized state of the client or
- client un-initialized but not re-initialized or
- client re-initialization fails later
This condition can only be recovered when you reset or re-load the driver.
do you have any test case for it ?
when user change channel larger by ethtool command , and the memory not enough, it will fail.
we should recovered too.
- /* restore the client */ ret = hns3_reset_notify(handle, HNAE3_INIT_CLIENT); if (ret)
return ret;
goto err_set_chan_fail;ret = hns3_reset_notify(handle, HNAE3_UP_CLIENT); if (ret)
hns3_reset_notify(handle, HNAE3_UNINIT_CLIENT);
goto err_set_chan_fail;- return retval;
+err_set_chan_fail:
- dev_warn(dev, "device could be in insane state. Reset rqd. or reload the
driver!\n");
return ret;} @@ -4720,7 +4753,6 @@ int hns3_set_channels(struct net_device *netdev, struct hnae3_knic_private_info *kinfo = &h->kinfo; bool rxfh_configured = netif_is_rxfh_configured(netdev); u32 new_tqp_num = ch->combined_count;
u16 org_tqp_num; int ret;
if (hns3_nic_resetting(netdev))
@@ -4750,28 +4782,10 @@ int hns3_set_channels(struct net_device *netdev, "set channels: tqp_num=%u, rxfh=%d\n", new_tqp_num, rxfh_configured);
- ret = hns3_reset_notify(h, HNAE3_DOWN_CLIENT);
- if (ret)
return ret;- ret = hns3_reset_notify(h, HNAE3_UNINIT_CLIENT);
- if (ret)
return ret;- org_tqp_num = h->kinfo.num_tqps; ret = hns3_change_channels(h, new_tqp_num, rxfh_configured); if (ret) {
int ret1;netdev_warn(netdev,"Change channels fail, revert to old value\n");ret1 = hns3_change_channels(h, org_tqp_num, rxfh_configured);if (ret1) {netdev_err(netdev,"revert to old channel fail\n");return ret1;}
netdev_err(netdev, "fail(=%d) to set number of channels to %u\n", ret,new_tqp_num); return ret;