* [PATCH] hv_netvsc: Allocate rx indirection table size dynamically
@ 2023-05-24 5:57 Shradha Gupta
2023-05-24 7:03 ` Simon Horman
2023-05-24 8:27 ` Steen Hegelund
0 siblings, 2 replies; 3+ messages in thread
From: Shradha Gupta @ 2023-05-24 5:57 UTC (permalink / raw)
To: linux-kernel, linux-hyperv, netdev
Cc: Shradha Gupta, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Michael Kelley, David S. Miller
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>
---
drivers/net/hyperv/hyperv_net.h | 5 ++++-
drivers/net/hyperv/netvsc_drv.c | 11 +++++++----
drivers/net/hyperv/rndis_filter.c | 23 +++++++++++++++++++----
3 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index dd5919ec408b..1dbdb65ca8f0 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -74,6 +74,7 @@ struct ndis_recv_scale_cap { /* NDIS_RECEIVE_SCALE_CAPABILITIES */
#define NDIS_RSS_HASH_SECRET_KEY_MAX_SIZE_REVISION_2 40
#define ITAB_NUM 128
+#define ITAB_NUM_MAX 256
struct ndis_recv_scale_param { /* NDIS_RECEIVE_SCALE_PARAMETERS */
struct ndis_obj_header hdr;
@@ -1034,7 +1035,9 @@ struct net_device_context {
u32 tx_table[VRSS_SEND_TAB_SIZE];
- u16 rx_table[ITAB_NUM];
+ u16 *rx_table;
+
+ int rx_table_sz;
/* Ethtool settings */
u8 duplex;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 0103ff914024..5b8a7d5f9a15 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1747,7 +1747,9 @@ static u32 netvsc_get_rxfh_key_size(struct net_device *dev)
static u32 netvsc_rss_indir_size(struct net_device *dev)
{
- return ITAB_NUM;
+ struct net_device_context *ndc = netdev_priv(dev);
+
+ return ndc->rx_table_sz;
}
static int netvsc_get_rxfh(struct net_device *dev, u32 *indir, u8 *key,
@@ -1766,7 +1768,7 @@ static int netvsc_get_rxfh(struct net_device *dev, u32 *indir, u8 *key,
rndis_dev = ndev->extension;
if (indir) {
- for (i = 0; i < ITAB_NUM; i++)
+ for (i = 0; i < ndc->rx_table_sz; i++)
indir[i] = ndc->rx_table[i];
}
@@ -1792,11 +1794,11 @@ static int netvsc_set_rxfh(struct net_device *dev, const u32 *indir,
rndis_dev = ndev->extension;
if (indir) {
- for (i = 0; i < ITAB_NUM; i++)
+ for (i = 0; i < ndc->rx_table_sz; i++)
if (indir[i] >= ndev->num_chn)
return -EINVAL;
- for (i = 0; i < ITAB_NUM; i++)
+ for (i = 0; i < ndc->rx_table_sz; i++)
ndc->rx_table[i] = indir[i];
}
@@ -2638,6 +2640,7 @@ static void netvsc_remove(struct hv_device *dev)
hv_set_drvdata(dev, NULL);
+ kfree(ndev_ctx->rx_table);
free_percpu(ndev_ctx->vf_stats);
free_netdev(net);
}
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index eea777ec2541..af031e711cb2 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -927,7 +927,7 @@ static int rndis_set_rss_param_msg(struct rndis_device *rdev,
struct rndis_set_request *set;
struct rndis_set_complete *set_complete;
u32 extlen = sizeof(struct ndis_recv_scale_param) +
- 4 * ITAB_NUM + NETVSC_HASH_KEYLEN;
+ 4 * ndc->rx_table_sz + NETVSC_HASH_KEYLEN;
struct ndis_recv_scale_param *rssp;
u32 *itab;
u8 *keyp;
@@ -953,7 +953,7 @@ static int rndis_set_rss_param_msg(struct rndis_device *rdev,
rssp->hashinfo = NDIS_HASH_FUNC_TOEPLITZ | NDIS_HASH_IPV4 |
NDIS_HASH_TCP_IPV4 | NDIS_HASH_IPV6 |
NDIS_HASH_TCP_IPV6;
- rssp->indirect_tabsize = 4*ITAB_NUM;
+ rssp->indirect_tabsize = 4 * ndc->rx_table_sz;
rssp->indirect_taboffset = sizeof(struct ndis_recv_scale_param);
rssp->hashkey_size = NETVSC_HASH_KEYLEN;
rssp->hashkey_offset = rssp->indirect_taboffset +
@@ -961,7 +961,7 @@ static int rndis_set_rss_param_msg(struct rndis_device *rdev,
/* Set indirection table entries */
itab = (u32 *)(rssp + 1);
- for (i = 0; i < ITAB_NUM; i++)
+ for (i = 0; i < ndc->rx_table_sz; i++)
itab[i] = ndc->rx_table[i];
/* Set hask key values */
@@ -1548,6 +1548,21 @@ 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);
+ if (ndc->rx_table) {
+ netdev_err(net, "Error in allocating rx indirection table of size %d\n",
+ ndc->rx_table_sz);
+ goto out;
+ }
+
/* 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 +1573,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);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] hv_netvsc: Allocate rx indirection table size dynamically
2023-05-24 5:57 [PATCH] hv_netvsc: Allocate rx indirection table size dynamically Shradha Gupta
@ 2023-05-24 7:03 ` Simon Horman
2023-05-24 8:27 ` Steen Hegelund
1 sibling, 0 replies; 3+ messages in thread
From: Simon Horman @ 2023-05-24 7:03 UTC (permalink / raw)
To: Shradha Gupta
Cc: linux-kernel, linux-hyperv, netdev, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Long Li, Michael Kelley, David S. Miller
On Tue, May 23, 2023 at 10:57:24PM -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>
Hi Shradha,
thanks for your patch.
> @@ -1548,6 +1548,21 @@ 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) {
nit: the line above is not indented correctly,
it should line up with the inside of the opening parentheses
on the preceding line.
Also, I don't think the curly-brackets are needed.
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);
> + if (ndc->rx_table) {
More importantly, it looks like the condition is inverted here.
Which seems unlikely to lead to anything good happening.
if (!ndc->rx_table) {
> + netdev_err(net, "Error in allocating rx indirection table of size %d\n",
> + ndc->rx_table_sz);
> + goto out;
> + }
> +
> /* 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 +1573,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);
> }
--
pw-bot: cr
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] hv_netvsc: Allocate rx indirection table size dynamically
2023-05-24 5:57 [PATCH] hv_netvsc: Allocate rx indirection table size dynamically Shradha Gupta
2023-05-24 7:03 ` Simon Horman
@ 2023-05-24 8:27 ` Steen Hegelund
1 sibling, 0 replies; 3+ messages in thread
From: Steen Hegelund @ 2023-05-24 8:27 UTC (permalink / raw)
To: Shradha Gupta, linux-kernel, linux-hyperv, netdev
Cc: Eric Dumazet, Jakub Kicinski, Paolo Abeni, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Michael Kelley,
David S. Miller
Hi Shradha,
On Tue, 2023-05-23 at 22:57 -0700, Shradha Gupta wrote:
> [You don't often get email from shradhagupta@linux.microsoft.com. Learn why
> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> 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>
> ---
> drivers/net/hyperv/hyperv_net.h | 5 ++++-
> drivers/net/hyperv/netvsc_drv.c | 11 +++++++----
> drivers/net/hyperv/rndis_filter.c | 23 +++++++++++++++++++----
> 3 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index dd5919ec408b..1dbdb65ca8f0 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -74,6 +74,7 @@ struct ndis_recv_scale_cap { /*
> NDIS_RECEIVE_SCALE_CAPABILITIES */
> #define NDIS_RSS_HASH_SECRET_KEY_MAX_SIZE_REVISION_2 40
>
> #define ITAB_NUM 128
> +#define ITAB_NUM_MAX 256
>
> struct ndis_recv_scale_param { /* NDIS_RECEIVE_SCALE_PARAMETERS */
> struct ndis_obj_header hdr;
> @@ -1034,7 +1035,9 @@ struct net_device_context {
>
> u32 tx_table[VRSS_SEND_TAB_SIZE];
>
> - u16 rx_table[ITAB_NUM];
> + u16 *rx_table;
> +
> + int rx_table_sz;
>
> /* Ethtool settings */
> u8 duplex;
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 0103ff914024..5b8a7d5f9a15 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1747,7 +1747,9 @@ static u32 netvsc_get_rxfh_key_size(struct net_device
> *dev)
>
> static u32 netvsc_rss_indir_size(struct net_device *dev)
> {
> - return ITAB_NUM;
> + struct net_device_context *ndc = netdev_priv(dev);
> +
> + return ndc->rx_table_sz;
> }
>
> static int netvsc_get_rxfh(struct net_device *dev, u32 *indir, u8 *key,
> @@ -1766,7 +1768,7 @@ static int netvsc_get_rxfh(struct net_device *dev, u32
> *indir, u8 *key,
>
> rndis_dev = ndev->extension;
> if (indir) {
> - for (i = 0; i < ITAB_NUM; i++)
> + for (i = 0; i < ndc->rx_table_sz; i++)
> indir[i] = ndc->rx_table[i];
> }
>
> @@ -1792,11 +1794,11 @@ static int netvsc_set_rxfh(struct net_device *dev,
> const u32 *indir,
>
> rndis_dev = ndev->extension;
> if (indir) {
> - for (i = 0; i < ITAB_NUM; i++)
> + for (i = 0; i < ndc->rx_table_sz; i++)
> if (indir[i] >= ndev->num_chn)
> return -EINVAL;
>
> - for (i = 0; i < ITAB_NUM; i++)
> + for (i = 0; i < ndc->rx_table_sz; i++)
> ndc->rx_table[i] = indir[i];
> }
>
> @@ -2638,6 +2640,7 @@ static void netvsc_remove(struct hv_device *dev)
>
> hv_set_drvdata(dev, NULL);
>
> + kfree(ndev_ctx->rx_table);
> free_percpu(ndev_ctx->vf_stats);
> free_netdev(net);
> }
> diff --git a/drivers/net/hyperv/rndis_filter.c
> b/drivers/net/hyperv/rndis_filter.c
> index eea777ec2541..af031e711cb2 100644
> --- a/drivers/net/hyperv/rndis_filter.c
> +++ b/drivers/net/hyperv/rndis_filter.c
> @@ -927,7 +927,7 @@ static int rndis_set_rss_param_msg(struct rndis_device
> *rdev,
> struct rndis_set_request *set;
> struct rndis_set_complete *set_complete;
> u32 extlen = sizeof(struct ndis_recv_scale_param) +
> - 4 * ITAB_NUM + NETVSC_HASH_KEYLEN;
> + 4 * ndc->rx_table_sz + NETVSC_HASH_KEYLEN;
> struct ndis_recv_scale_param *rssp;
> u32 *itab;
> u8 *keyp;
> @@ -953,7 +953,7 @@ static int rndis_set_rss_param_msg(struct rndis_device
> *rdev,
> rssp->hashinfo = NDIS_HASH_FUNC_TOEPLITZ | NDIS_HASH_IPV4 |
> NDIS_HASH_TCP_IPV4 | NDIS_HASH_IPV6 |
> NDIS_HASH_TCP_IPV6;
> - rssp->indirect_tabsize = 4*ITAB_NUM;
> + rssp->indirect_tabsize = 4 * ndc->rx_table_sz;
> rssp->indirect_taboffset = sizeof(struct ndis_recv_scale_param);
> rssp->hashkey_size = NETVSC_HASH_KEYLEN;
> rssp->hashkey_offset = rssp->indirect_taboffset +
> @@ -961,7 +961,7 @@ static int rndis_set_rss_param_msg(struct rndis_device
> *rdev,
>
> /* Set indirection table entries */
> itab = (u32 *)(rssp + 1);
> - for (i = 0; i < ITAB_NUM; i++)
> + for (i = 0; i < ndc->rx_table_sz; i++)
> itab[i] = ndc->rx_table[i];
>
> /* Set hask key values */
> @@ -1548,6 +1548,21 @@ 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);
> + if (ndc->rx_table) {
> + netdev_err(net, "Error in allocating rx indirection table of
> size %d\n",
> + ndc->rx_table_sz);
> + goto out;
> + }
> +
You are now always allocating the rx_table in the rndis_filter_device_add()
function, but this function is called both from the netvsc_probe function
and the netvsc_attach function, but as far as I can see the rx_table only
gets freed in the remove function not in the netvsc_detach.
So if there is a chance that the attach/detach will be called more times
on a existing device you will have a leak.
> /* 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 +1573,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);
> }
> --
> 2.34.1
>
>
Best Regards
Steen
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-24 8:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-24 5:57 [PATCH] hv_netvsc: Allocate rx indirection table size dynamically Shradha Gupta
2023-05-24 7:03 ` Simon Horman
2023-05-24 8:27 ` Steen Hegelund
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).