From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-dl1-f50.google.com (mail-dl1-f50.google.com [74.125.82.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B754D36EA86 for ; Wed, 1 Apr 2026 07:52:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775029947; cv=none; b=X47rGwBL5pNWoqGJSrkOFqWyaEi8dNlobSkcVF2s+AeiwTHyo1cBsqlwmFBJUX/avnmpovZyLQ1jhbdCRlalD+cJ48DmoX3wzJpRl7aGGZVMzyaElJUidyKZaSY0nV4lHbtQOTQQ58Kr/dC1r/Orlu0AgWkQc9GNOrIptnbVF50= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775029947; c=relaxed/simple; bh=/xKPjENbw+y9tVTlpGDDqxkzbi3Dsy4WscFa4T8jtsw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=XmDPIlY7QQQ0dRJh+8SXbZf5j9oDmdutU88qXyYMDUER61Azd3WD14QxYnyy5Lvc3J8ItzHSQdNK2bdBqAlnw77ixi0Ss8k3TyijxlZI5TU1fwORNeGv/Fbpyx3snZYiurcD5mRJWfbjjiD2c+FDUwQ7HHPDdpad+9bT8UgJlVY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=TBin9MS9; arc=none smtp.client-ip=74.125.82.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="TBin9MS9" Received: by mail-dl1-f50.google.com with SMTP id a92af1059eb24-128b9b7e3edso4015551c88.0 for ; Wed, 01 Apr 2026 00:52:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1775029945; x=1775634745; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=SbYMgx5b2U+ll1ogdXc2obIoQ1UDlBi3tYpZOOvXiXo=; b=TBin9MS9HwvQcidj7DxebJH6m1DYhgQPhWB1PWVTrLolVthDAQRBC/LRjX0qt7T2Ux Mmqj/OH7IlbjPrOxlOQRgYACFzWS4LqB0E8akFnYG71KSLFReGEbceVYCVKc8UHSErhN 9MccYFvUHwkEd9ND+6YuT1oSimm4Wtfccw79Py/C8AiFjcziaaPT4SQYYSvy6os2mbOf jPlKIqo+iAJGKdMDEEszaz6HmoLQt9txhttDF/F0kk7+Y9Qi7RvvTh8wazCRKasVXcpE +31FqpXID4Js80W0gbyXEmc1HJ1F1ACjBw8KRszWQunnrbgT6Unou6LGlbhJaM4JbTmD VQIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775029945; x=1775634745; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=SbYMgx5b2U+ll1ogdXc2obIoQ1UDlBi3tYpZOOvXiXo=; b=NNA/xY3SGa59e0bkb5qS5SaDrd5SC/Qq+BLq9F2iCvsXmoV3rz/b3Z/D04jw+iw7eW Z4oWuezLnxy/PCyXsOs6d+HwV7WUgRsiILa8yFN304Mzdz96yMUY3tUnAE7BtVCvCpF6 ChJZD5FqaTdWTOVWwGH/BeIWz7tuGQjVImOXG+7waQxGs8v/0KeGwBB4nC0eTO0iXex6 FhcHCDi3kIwLFe1Uis6Qa/1L7O47GgzmdFGdw6s+BGlWqmxqVtDR5ryc2+WaB7QxmMW0 1UzlTu6c8XSOqaHJ9g7VzZjUVxb41OxO9H8UivswOaJ5OkOFQV+jWDQn2RLbF66MW4hL t7Ng== X-Gm-Message-State: AOJu0Yy/ABjs9IdCILOEW8lFH76llbjdeZp0OT4lhMYWwcubWOmsWxYG c4z1S96GMA715UHuL7JWLgFcc8udEU9StFIxVTkLxipuWVoxs51i9ujw X-Gm-Gg: ATEYQzx6fkX8J2DSWpkF1lYFah8WdjCeoNR2Q13XGGL5BdeeobpL18HOZiwhN2vq+UF WoJjAKsv9HyJDcdWJwqx+uOqPwWGjDwqZSgDu+xAdnTrtndPexRSucU1XE9w58GGLrczRvY+yaN eiFUSlbX3c7fNXcf6NANN0Yi//MNruCcNRMPf3wc45ZVVlphCl1q3HDu94bcoD1FLx6s7oZ6uPW LeNf8azquOgYYWIz0cS8r+/9LwRkvCLdIBKNJ6pUXh3IXwcLw8bG8ViuzU/pL1aAMZNPEbKr0MO 4Qf/k5P+jsdPDtOCDsW2TQF5RY+hIMeyuCL5Ds8/Qu9It8k9nPS79/Kj1tXwQhJFNYhH3xXDGuG USRZPc9zeXOs7wSRGlIyCcvgcQe37Kdil4IH4dOscn5Ybhd0SfBE/4bRiEyKPEHNHEfY8GziUVK PVAe+M9E5W1jgErSLpwStSVO0tc9kusfMnpusrnpSG+JPmXyI7glL4Fshz4VrJ X-Received: by 2002:a05:7022:f10f:b0:119:e569:fbb4 with SMTP id a92af1059eb24-12be65579eemr1471488c88.35.1775029944651; Wed, 01 Apr 2026 00:52:24 -0700 (PDT) Received: from ?IPV6:2620:10d:c085:21e1::139a? ([2620:10d:c090:400::5:51bb]) by smtp.gmail.com with ESMTPSA id a92af1059eb24-12ab9864810sm13149653c88.12.2026.04.01.00.52.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 Apr 2026 00:52:23 -0700 (PDT) Message-ID: Date: Wed, 1 Apr 2026 00:52:22 -0700 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next v03 1/6] hinic3: Add ethtool queue ops To: Fan Gong , Zhu Yikai , netdev@vger.kernel.org, "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Andrew Lunn , Ioana Ciornei Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, luosifu , Xin Guo , Zhou Shuai , Wu Like , Shi Jing , Zheng Jiezhen , Maxime Chevallier References: <17b3bbb0c917ee09ddd7a164cdefca6eb63793ec.1774940117.git.zhuyikai1@h-partners.com> Content-Language: en-US From: Mohsin Bashir In-Reply-To: <17b3bbb0c917ee09ddd7a164cdefca6eb63793ec.1774940117.git.zhuyikai1@h-partners.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit > +static int hinic3_set_ringparam(struct net_device *netdev, > + struct ethtool_ringparam *ring, > + struct kernel_ethtool_ringparam *kernel_ring, > + struct netlink_ext_ack *extack) > +{ > + struct hinic3_nic_dev *nic_dev = netdev_priv(netdev); > + struct hinic3_dyna_txrxq_params q_params = {}; > + u32 new_sq_depth, new_rq_depth; > + int err; > + > + err = hinic3_check_ringparam_valid(netdev, ring); > + if (err) > + return err; > + > + new_sq_depth = 1U << ilog2(ring->tx_pending); > + new_rq_depth = 1U << ilog2(ring->rx_pending); > + if (new_sq_depth == nic_dev->q_params.sq_depth && > + new_rq_depth == nic_dev->q_params.rq_depth) > + return 0; > + So non power of 2 values would trimmed to the nearest power of 2 even when the driver support these values. If such values are not supported, should these be rejected here? or if we want to force the nearest power of 2, should we use NL_SET_ERR_MSG to inform the user about rounding? > +int > +hinic3_change_channel_settings(struct net_device *netdev, > + struct hinic3_dyna_txrxq_params *trxq_params) > +{ > + struct hinic3_nic_dev *nic_dev = netdev_priv(netdev); > + struct hinic3_dyna_qp_params new_qp_params = {}; > + struct hinic3_dyna_qp_params cur_qp_params = {}; > + bool need_teardown = false; > + unsigned long flags; > + int err; > + > + mutex_lock(&nic_dev->channel_cfg_lock); > + > + hinic3_config_num_qps(netdev, trxq_params); > + > + err = hinic3_alloc_channel_resources(netdev, &new_qp_params, > + trxq_params); > + if (err) { > + netdev_err(netdev, "Failed to alloc channel resources\n"); > + mutex_unlock(&nic_dev->channel_cfg_lock); > + return err; > + } > + > + spin_lock_irqsave(&nic_dev->channel_res_lock, flags); > + if (!test_and_set_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags)) > + need_teardown = true; > + spin_unlock_irqrestore(&nic_dev->channel_res_lock, flags); > + > + if (need_teardown) { > + hinic3_vport_down(netdev); > + hinic3_close_channel(netdev); > + hinic3_uninit_qps(nic_dev, &cur_qp_params); > + hinic3_free_channel_resources(netdev, &cur_qp_params, > + &nic_dev->q_params); > + } > + > + if (nic_dev->num_qp_irq > trxq_params->num_qps) > + hinic3_qp_irq_change(netdev, trxq_params->num_qps); > + > + spin_lock_irqsave(&nic_dev->channel_res_lock, flags); > + nic_dev->q_params = *trxq_params; [1] Here we are overwriting the old parameters > + spin_unlock_irqrestore(&nic_dev->channel_res_lock, flags); > + > + hinic3_init_qps(nic_dev, &new_qp_params); > + > + err = hinic3_open_channel(netdev); > + if (err) > + goto err_uninit_qps; > + > + err = hinic3_vport_up(netdev); > + if (err) > + goto err_close_channel; > + > + spin_lock_irqsave(&nic_dev->channel_res_lock, flags); > + clear_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags); > + spin_unlock_irqrestore(&nic_dev->channel_res_lock, flags); > + > + mutex_unlock(&nic_dev->channel_cfg_lock); > + > + return 0; > + > +err_close_channel: > + hinic3_close_channel(netdev); > +err_uninit_qps: > + spin_lock_irqsave(&nic_dev->channel_res_lock, flags); > + memset(&nic_dev->q_params, 0, sizeof(nic_dev->q_params)); Looks like we are asking for trouble here. The old parameters were already overwritten at [1] and now we are destroying the new ones? > + spin_unlock_irqrestore(&nic_dev->channel_res_lock, flags); > + > + hinic3_uninit_qps(nic_dev, &new_qp_params); > + hinic3_free_channel_resources(netdev, &new_qp_params, trxq_params); > + > + mutex_unlock(&nic_dev->channel_cfg_lock); > + > + return err; > +} > +