* [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
@ 2012-07-25 14:36 Or Gerlitz
2012-07-25 14:57 ` Ben Hutchings
2012-07-25 14:57 ` Ben Hutchings
0 siblings, 2 replies; 6+ messages in thread
From: Or Gerlitz @ 2012-07-25 14:36 UTC (permalink / raw)
To: davem; +Cc: netdev, oren, Amir Vadai, Ben Hutchings, Or Gerlitz
From: Amir Vadai <amirv@mellanox.com>
RFS filter id can't have the special value RPS_NO_FILTER,
need to skip it when allocating id's.
Also, changed an ifdef into a more elegant IS_DEFINED.
CC: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
Addressing feedback from Ben Hutchings
resending as of typo in my signature... sorry
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index aa9c2f6..866829b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
struct mlx4_en_dev *mdev = priv->mdev;
int err = 0;
char name[25];
- struct cpu_rmap *rmap =
-#ifdef CONFIG_RFS_ACCEL
- priv->dev->rx_cpu_rmap;
-#else
- NULL;
-#endif
+ struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
+ priv->dev->rx_cpu_rmap : NULL;
cq->dev = mdev->pndev[priv->port];
cq->mcq.set_ci_db = cq->wqres.db.db;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8864d8b..edd9cb8 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
filter->flow_id = flow_id;
- filter->id = priv->last_filter_id++;
+ filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
list_add_tail(&filter->next, &priv->filters);
hlist_add_head(&filter->filter_chain,
--
1.7.8.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
2012-07-25 14:36 [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER Or Gerlitz
@ 2012-07-25 14:57 ` Ben Hutchings
2012-07-25 15:04 ` Or Gerlitz
2012-07-25 14:57 ` Ben Hutchings
1 sibling, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2012-07-25 14:57 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, netdev, oren, Amir Vadai
On Wed, 2012-07-25 at 17:36 +0300, Or Gerlitz wrote:
> From: Amir Vadai <amirv@mellanox.com>
>
> RFS filter id can't have the special value RPS_NO_FILTER,
> need to skip it when allocating id's.
>
> Also, changed an ifdef into a more elegant IS_DEFINED.
>
> CC: Ben Hutchings <bhutchings@solarflare.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
>
> Addressing feedback from Ben Hutchings
>
> resending as of typo in my signature... sorry
--signoff is so much easier than typing it every time :-)
> drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
> 2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> index aa9c2f6..866829b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
> struct mlx4_en_dev *mdev = priv->mdev;
> int err = 0;
> char name[25];
> - struct cpu_rmap *rmap =
> -#ifdef CONFIG_RFS_ACCEL
> - priv->dev->rx_cpu_rmap;
> -#else
> - NULL;
> -#endif
> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
> + priv->dev->rx_cpu_rmap : NULL;
This is a separate change.
> cq->dev = mdev->pndev[priv->port];
> cq->mcq.set_ci_db = cq->wqres.db.db;
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 8864d8b..edd9cb8 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
>
> filter->flow_id = flow_id;
>
> - filter->id = priv->last_filter_id++;
> + filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
This should do for now.
Ideally the filter ID would be based on the index used in hardware. We
would need to change the API to allow for devices where this cannot be
determined synchronously, as in this driver. (And the filter ID might
also need to be wider.)
Ben.
>
> list_add_tail(&filter->next, &priv->filters);
> hlist_add_head(&filter->filter_chain,
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
2012-07-25 14:36 [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER Or Gerlitz
2012-07-25 14:57 ` Ben Hutchings
@ 2012-07-25 14:57 ` Ben Hutchings
1 sibling, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2012-07-25 14:57 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, netdev, oren, Amir Vadai
On Wed, 2012-07-25 at 17:36 +0300, Or Gerlitz wrote:
> From: Amir Vadai <amirv@mellanox.com>
>
> RFS filter id can't have the special value RPS_NO_FILTER,
> need to skip it when allocating id's.
>
> Also, changed an ifdef into a more elegant IS_DEFINED.
>
> CC: Ben Hutchings <bhutchings@solarflare.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
>
> Addressing feedback from Ben Hutchings
>
> resending as of typo in my signature... sorry
--signoff is so much easier than typing it every time :-)
> drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
> 2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> index aa9c2f6..866829b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
> struct mlx4_en_dev *mdev = priv->mdev;
> int err = 0;
> char name[25];
> - struct cpu_rmap *rmap =
> -#ifdef CONFIG_RFS_ACCEL
> - priv->dev->rx_cpu_rmap;
> -#else
> - NULL;
> -#endif
> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
> + priv->dev->rx_cpu_rmap : NULL;
This ought to be a separate change really.
> cq->dev = mdev->pndev[priv->port];
> cq->mcq.set_ci_db = cq->wqres.db.db;
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 8864d8b..edd9cb8 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
>
> filter->flow_id = flow_id;
>
> - filter->id = priv->last_filter_id++;
> + filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
This should do for now.
Ideally the filter ID would be based on the index used in hardware. We
would need to change the API to allow for devices where this cannot be
determined synchronously, as in this driver. (And the filter ID might
also need to be wider.)
Ben.
>
> list_add_tail(&filter->next, &priv->filters);
> hlist_add_head(&filter->filter_chain,
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
2012-07-25 14:57 ` Ben Hutchings
@ 2012-07-25 15:04 ` Or Gerlitz
2012-07-25 22:23 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Or Gerlitz @ 2012-07-25 15:04 UTC (permalink / raw)
To: Ben Hutchings; +Cc: davem, netdev, oren, Amir Vadai
On 25/07/2012 17:57, Ben Hutchings wrote:
>
> resending as of typo in my signature... sorry
>
> --signoff is so much easier than typing it every time :-)
I know and I do that on the regular basis, today did that differently
and here's the result...
Or.
>
>
>> drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
>> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
>> 2 files changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> index aa9c2f6..866829b 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
>> struct mlx4_en_dev *mdev = priv->mdev;
>> int err = 0;
>> char name[25];
>> - struct cpu_rmap *rmap =
>> -#ifdef CONFIG_RFS_ACCEL
>> - priv->dev->rx_cpu_rmap;
>> -#else
>> - NULL;
>> -#endif
>> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
>> + priv->dev->rx_cpu_rmap : NULL;
>
> This is a separate change.
OK, will send two patches
>
>
>> cq->dev = mdev->pndev[priv->port];
>> cq->mcq.set_ci_db = cq->wqres.db.db;
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> index 8864d8b..edd9cb8 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> @@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
>>
>> filter->flow_id = flow_id;
>>
>> - filter->id = priv->last_filter_id++;
>> + filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
>
> This should do for now.
thanks
Or.
>
>
> Ideally the filter ID would be based on the index used in hardware. We
> would need to change the API to allow for devices where this cannot be
> determined synchronously, as in this driver. (And the filter ID might
> also need to be wider.)
>
> Ben.
>
>>
>> list_add_tail(&filter->next,&priv->filters);
>> hlist_add_head(&filter->filter_chain,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
2012-07-25 15:04 ` Or Gerlitz
@ 2012-07-25 22:23 ` David Miller
2012-07-25 22:29 ` Ben Hutchings
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2012-07-25 22:23 UTC (permalink / raw)
To: ogerlitz; +Cc: bhutchings, netdev, oren, amirv
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Wed, 25 Jul 2012 18:04:35 +0300
> On 25/07/2012 17:57, Ben Hutchings wrote:
>>> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv,
>>> struct mlx4_en_cq *cq,
>>> struct mlx4_en_dev *mdev = priv->mdev;
>>> int err = 0;
>>> char name[25];
>>> - struct cpu_rmap *rmap =
>>> -#ifdef CONFIG_RFS_ACCEL
>>> - priv->dev->rx_cpu_rmap;
>>> -#else
>>> - NULL;
>>> -#endif
>>> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
>>> + priv->dev->rx_cpu_rmap : NULL;
>>
>> This is a separate change.
>
> OK, will send two patches
This change breaks the build.
You can't do this check at run-time, because the reason you need to
check CONFIG_RFS_ACCEL is because if that's disabled then the netdev
structure doesn't even have the ->rx_cpu_rmap member.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
2012-07-25 22:23 ` David Miller
@ 2012-07-25 22:29 ` Ben Hutchings
0 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2012-07-25 22:29 UTC (permalink / raw)
To: David Miller; +Cc: ogerlitz, netdev, oren, amirv
On Wed, 2012-07-25 at 15:23 -0700, David Miller wrote:
> From: Or Gerlitz <ogerlitz@mellanox.com>
> Date: Wed, 25 Jul 2012 18:04:35 +0300
>
> > On 25/07/2012 17:57, Ben Hutchings wrote:
> >>> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv,
> >>> struct mlx4_en_cq *cq,
> >>> struct mlx4_en_dev *mdev = priv->mdev;
> >>> int err = 0;
> >>> char name[25];
> >>> - struct cpu_rmap *rmap =
> >>> -#ifdef CONFIG_RFS_ACCEL
> >>> - priv->dev->rx_cpu_rmap;
> >>> -#else
> >>> - NULL;
> >>> -#endif
> >>> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
> >>> + priv->dev->rx_cpu_rmap : NULL;
> >>
> >> This is a separate change.
> >
> > OK, will send two patches
>
> This change breaks the build.
>
> You can't do this check at run-time, because the reason you need to
> check CONFIG_RFS_ACCEL is because if that's disabled then the netdev
> structure doesn't even have the ->rx_cpu_rmap member.
Yes, sorry for suggesting that, Or.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-25 22:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-25 14:36 [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER Or Gerlitz
2012-07-25 14:57 ` Ben Hutchings
2012-07-25 15:04 ` Or Gerlitz
2012-07-25 22:23 ` David Miller
2012-07-25 22:29 ` Ben Hutchings
2012-07-25 14:57 ` Ben Hutchings
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox