On Wed, May 24, 2023 at 02:57:10AM -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@xxxxxxxxxxxxxxxxxxx>
Hi Shradha,
thanks for your patch.
Some nits from my side.
And some friendly advice: please consider using checkpatch
...
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 0103ff914024..ab791e4ca63c 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1040,6 +1040,13 @@ static int netvsc_detach(struct net_device *ndev,
>
> rndis_filter_device_remove(hdev, nvdev);
>
> + /*
> + * Free the rx indirection table and reset the table size to 0.
> + * With the netvsc_attach call it will get allocated again.
> + */
nit: In Networking code multi-line comments look like this:
/* Free the rx indirection table and reset the table size to 0.
* With the netvsc_attach call it will get allocated again.
*/
> + ndev_ctx->rx_table_sz = 0;
> + kfree(ndev_ctx->rx_table);
> +
> return 0;
> }
>
...
> @@ -1548,6 +1548,20 @@ 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 = kzalloc(sizeof(u16) * ndc->rx_table_sz,
> + GFP_KERNEL);
nit: kcalloc seems appropriate here.
> + if (!ndc->rx_table) {
> + netdev_err(net, "Error in allocating rx indirection table of size %d\n",
> + ndc->rx_table_sz);
nit: No need to log memory allocation errors, the mm core does that.
Also, the alignment of the line above should match of the opening '('
of the preceding line.
f(a,
b);
> + goto out;
> + }
> +
> /* This guarantees that num_possible_rss_qs <= num_online_cpus */
> num_possible_rss_qs = min_t(u32, num_online_cpus(),
