Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 26 May 2023 10:15:23 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Shradha Gupta <shradhagupta@linux.microsoft.com>
Cc:     linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
        netdev@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "K. Y. Srinivasan" <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
        Long Li <longli@microsoft.com>,
        Michael Kelley <mikelley@microsoft.com>,
        "David S. Miller" <davem@davemloft.net>,
        Steen Hegelund <steen.hegelund@microchip.com>
Subject: Re: [PATCH v3] hv_netvsc: Allocate rx indirection table size
 dynamically
Message-ID: <ZHBqm0fGHPMyN0vz@corigine.com>
References: <1685080949-18316-1-git-send-email-shradhagupta@linux.microsoft.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1685080949-18316-1-git-send-email-shradhagupta@linux.microsoft.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?mGe8ai8GQndydlCBmhZhBcEh+4JakDgkXJE38R6uETtGKDv9tFI2o1qv/gFr?=
 =?us-ascii?Q?r4ptjOIa6do3V8m5PuaXec0bxX/NTGUcgVpcwxXckI1+123IEQCp7Mu0CbwQ?=
 =?us-ascii?Q?w4fDv7s72VG98j3Ehh0eHd4GZhWDPuVpELoVihYHs0gz4TaHpjBXcdAguM98?=
 =?us-ascii?Q?YkEJ2VGVoqi7riv5wXG3MbYO0KMJSLPzfyZlRPcTaAncAueAF1i9Pnu7VYyO?=
 =?us-ascii?Q?siq4XpqXeTB4mKqzo9Fs77QjTMwpSl4s2xVKBk/jbqXB2kzWfQcgh60m9Qi+?=
 =?us-ascii?Q?SxmCVwFFbT6NZvxqGrLTzkD858ebZhrs2UxetiFiwWI1V1RXN0Fey5urnlms?=
 =?us-ascii?Q?UQfAYuHpqQZChDly8EnBLM9XQyRTijsDP8KWdRodhmh8kd8gw0hPJBeU5KEQ?=
 =?us-ascii?Q?yIdJEqXeGMe/Q1JlWNC7qMI50qQr7yithi43fqf47+xV12EN/aUUibfGbO/k?=
 =?us-ascii?Q?jUTL8OJcvp0A8wY9g3HCn9tsRLuFxKgjqn2PjD2t/ZxjZeqLiXnHrew9QF1s?=
 =?us-ascii?Q?Fp6u1GaZm/HJOYAkX1po6rWl/T2PJ0XXsOFI4XHa4VpfP0Nl04CuA/z5P+Vx?=
 =?us-ascii?Q?+pXyYSy0ys9u6PYen78KTgHS7fggrJ8hQ2jtSykOkSiDggMndazwwkr46N1p?=
 =?us-ascii?Q?SdfQblRrxYim5VyVnJzwUXW2KRGzjPqndX2GRJXYLNeSarKiKKWqOXr0CJLv?=
 =?us-ascii?Q?aC6w4jezoio7zlr7N30WTjYz9yDp8EC3RkhQXVg0TYwAPRZKCBOf32NPwCJx?=
 =?us-ascii?Q?WZ6XdniXc8I5a1HYqO8qPLtr+CDIyXNWSUjzcVgoC3G01yygtcvrtuNBFXdh?=
 =?us-ascii?Q?hPOAm3aVP8HpEaRnCyTxzrMLtIQ/xmXcf0Iyn9u4fxiCfCPbd04L/5n3+ya7?=
 =?us-ascii?Q?FSAYkGTHVeZkTE6Y6YMwBduXnufmJTAV6ozHZZhBkbQNyAHLFRs4i284nxmY?=
 =?us-ascii?Q?iaO/heYmqGZX++7qWPNKK7GKM/QWJSvR31LBNrhpjbROPIzq1EeGRAj8Db6F?=
 =?us-ascii?Q?jsnD29uLZOLPIqtBf+qAhgnHFX8mTRIMCrAANKL4fhgxaxBjPIIGLsRzkd8z?=
 =?us-ascii?Q?xNaFb+SkycIdGPdFacJTdXCOg9TuPk8BoFj2jQsk/Qh1iNIfYxgz/+qUenaX?=
 =?us-ascii?Q?/tToxw60MDptBxdJAuI5NqLgGkUOG7713L2ulVdLONEQ4fN/UWaetN2az8LP?=
 =?us-ascii?Q?TLvGs9FVXyQ2zFauVEyxNqI5u/pS2ffRV8a6lxQLi/7QhSBwF8yHlEXLp764?=
 =?us-ascii?Q?SGqel6VWeyNL/vCkHeywYKILnIYN0WkOoHsh+HI/nh8rhn2UssgWjMQz1Z0c?=
 =?us-ascii?Q?9wiXh6HQ2BiyaBMB9zWwlk4VFqLBHTOC0rbxgoETDjtE8ZJ/I3/RK89kngyK?=
 =?us-ascii?Q?lg7MtGNIRmxmtat4pn1p+uiBncykdy3vQWJwlhoNSMYwmELXFtceegbpJQRX?=
 =?us-ascii?Q?aimzy9mmFJHMIBWpRNQDHzECPWppWKZSuh1GW5o++nKiBVHvDBDnkJgdBJYT?=
 =?us-ascii?Q?/hvRVWVFclPeExiWBDvnmSMbzReX5pZMewamjFrT6xHPyIWb4/Mx93JLRvvM?=
 =?us-ascii?Q?0SAo1ftALs+gNo8DK1MM6y077jzZDPYqtqzb4O6CtTYx+JY/8CpPTSIHkiEh?=
 =?us-ascii?Q?PT5YSQesw120FywWtL985PO3yFdvuvs7bQ0aeErEykaWI2LNQXd2w2GaEZV+?=
 =?us-ascii?Q?PdfNXQ=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b213d2b5-c771-4bfe-47a1-08db5dc15f5d
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 May 2023 08:15:31.4453
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TCX6yzmwFwlyiwzRw3Ih3C/WLl7WfRxgw3oeS9BPhr7ZAXdr9ZovNYRaJ4iIY2muQzSTXAr3zmXWh/+P/VGAfb6/P4ZkXxKDIk3Vd2X0cms=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR13MB3970
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, May 25, 2023 at 11:02:29PM -0700, Shradha Gupta wrote:
> Allocate the size of rx indirection table dynamically in netvsc
> from the value of size provided by OID_GEN_RECEIVE_SCALE_CAPABILITIES
> query instead of using a constant value of ITAB_NUM.
> 
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>

> @@ -1548,6 +1549,17 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
>  	if (ret || rsscap.num_recv_que < 2)
>  		goto out;
>  
> +	if (rsscap.num_indirect_tabent &&
> +	    rsscap.num_indirect_tabent <= ITAB_NUM_MAX)
> +		ndc->rx_table_sz = rsscap.num_indirect_tabent;
> +	else
> +		ndc->rx_table_sz = ITAB_NUM;
> +
> +	ndc->rx_table = kcalloc(ndc->rx_table_sz, sizeof(u16),
> +				GFP_KERNEL);

nit: the above could fit on a single line.

> +	if (!ndc->rx_table)

I think you need to set ret to an error value here,
as err_dev_remv will call return ERR_PTR(ret).

> +		goto err_dev_remv;
> +
>  	/* This guarantees that num_possible_rss_qs <= num_online_cpus */
>  	num_possible_rss_qs = min_t(u32, num_online_cpus(),
>  				    rsscap.num_recv_que);
> @@ -1558,7 +1570,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
>  	net_device->num_chn = min(net_device->max_chn, device_info->num_chn);
>  
>  	if (!netif_is_rxfh_configured(net)) {
> -		for (i = 0; i < ITAB_NUM; i++)
> +		for (i = 0; i < ndc->rx_table_sz; i++)
>  			ndc->rx_table[i] = ethtool_rxfh_indir_default(
>  						i, net_device->num_chn);
>  	}
> @@ -1596,11 +1608,18 @@ void rndis_filter_device_remove(struct hv_device *dev,
>  				struct netvsc_device *net_dev)
>  {
>  	struct rndis_device *rndis_dev = net_dev->extension;
> +	struct net_device *net = hv_get_drvdata(dev);
> +	struct net_device_context *ndc = netdev_priv(net);

nit: Please use reverse xmas tree order - longest line to shortest -
     for local variable declaration sin networking code.

	struct rndis_device *rndis_dev = net_dev->extension;
	struct net_device *net = hv_get_drvdata(dev);
	struct net_device_context *ndc;

	ndc = netdev_priv(net);

>  
>  	/* Halt and release the rndis device */
>  	rndis_filter_halt_device(net_dev, rndis_dev);
>  
>  	netvsc_device_remove(dev);
> +
> +	ndc->rx_table_sz = 0;
> +	kfree(ndc->rx_table);
> +	ndc->rx_table = NULL;
> +
>  }
>  
>  int rndis_filter_open(struct netvsc_device *nvdev)

-- 
pw-bot: cr