* [PATCH ] net_sched: actions - Add default lookup
@ 2013-10-30 11:25 Jamal Hadi Salim
2013-10-30 14:00 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2013-10-30 11:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Eric W. Biederman, Alexander Duyck
[-- Attachment #1: Type: text/plain, Size: 53 bytes --]
Attached. Tested with simple action.
cheers,
jamal
[-- Attachment #2: p1 --]
[-- Type: text/plain, Size: 613 bytes --]
commit 80f80120ba4b85daf00c53eceebe7f0ad0b58a57
Author: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Wed Oct 30 07:03:55 2013 -0400
Provide default lookup function for actions that dont provide one
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index fd70728..3c974a0 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -279,6 +279,10 @@ int tcf_register_action(struct tc_action_ops *act)
}
act->next = NULL;
*ap = act;
+
+ if (!act->lookup)
+ act->lookup = tcf_hash_search;
+
write_unlock(&act_mod_lock);
return 0;
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH ] net_sched: actions - Add default lookup
2013-10-30 11:25 [PATCH ] net_sched: actions - Add default lookup Jamal Hadi Salim
@ 2013-10-30 14:00 ` Eric Dumazet
2013-11-04 4:12 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2013-10-30 14:00 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David Miller, netdev, Eric W. Biederman, Alexander Duyck
On Wed, 2013-10-30 at 07:25 -0400, Jamal Hadi Salim wrote:
> Attached. Tested with simple action.
>
> cheers,
> jamal
Why not setting .lookup to tcf_hash_search
in the few actions not already doing that ?
This would be more consistent.
# git grep -n tcf_hash_search
include/net/act_api.h:92:int tcf_hash_search(struct tc_action *a, u32 index);
net/sched/act_api.c:198:int tcf_hash_search(struct tc_action *a, u32 index)
net/sched/act_api.c:209:EXPORT_SYMBOL(tcf_hash_search);
net/sched/act_csum.c:588: .lookup = tcf_hash_search,
net/sched/act_gact.c:209: .lookup = tcf_hash_search,
net/sched/act_ipt.c:301: .lookup = tcf_hash_search,
net/sched/act_ipt.c:315: .lookup = tcf_hash_search,
net/sched/act_mirred.c:274: .lookup = tcf_hash_search,
net/sched/act_nat.c:311: .lookup = tcf_hash_search,
net/sched/act_pedit.c:246: .lookup = tcf_hash_search,
net/sched/act_police.c:410: .lookup = tcf_hash_search,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH ] net_sched: actions - Add default lookup
2013-10-30 14:00 ` Eric Dumazet
@ 2013-11-04 4:12 ` David Miller
2013-11-04 4:17 ` David Miller
2013-11-05 12:19 ` Jamal Hadi Salim
0 siblings, 2 replies; 7+ messages in thread
From: David Miller @ 2013-11-04 4:12 UTC (permalink / raw)
To: eric.dumazet; +Cc: jhs, netdev, ebiederm, alexander.h.duyck
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 30 Oct 2013 07:00:05 -0700
> On Wed, 2013-10-30 at 07:25 -0400, Jamal Hadi Salim wrote:
>> Attached. Tested with simple action.
>>
>> cheers,
>> jamal
>
> Why not setting .lookup to tcf_hash_search
> in the few actions not already doing that ?
>
> This would be more consistent.
> # git grep -n tcf_hash_search
> include/net/act_api.h:92:int tcf_hash_search(struct tc_action *a, u32 index);
> net/sched/act_api.c:198:int tcf_hash_search(struct tc_action *a, u32 index)
> net/sched/act_api.c:209:EXPORT_SYMBOL(tcf_hash_search);
> net/sched/act_csum.c:588: .lookup = tcf_hash_search,
> net/sched/act_gact.c:209: .lookup = tcf_hash_search,
> net/sched/act_ipt.c:301: .lookup = tcf_hash_search,
> net/sched/act_ipt.c:315: .lookup = tcf_hash_search,
> net/sched/act_mirred.c:274: .lookup = tcf_hash_search,
> net/sched/act_nat.c:311: .lookup = tcf_hash_search,
> net/sched/act_pedit.c:246: .lookup = tcf_hash_search,
> net/sched/act_police.c:410: .lookup = tcf_hash_search,
Right, and BUG() if we try to register and action with a NULL .lookup
member.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH ] net_sched: actions - Add default lookup
2013-11-04 4:12 ` David Miller
@ 2013-11-04 4:17 ` David Miller
2013-11-17 15:46 ` Jamal Hadi Salim
2013-11-05 12:19 ` Jamal Hadi Salim
1 sibling, 1 reply; 7+ messages in thread
From: David Miller @ 2013-11-04 4:17 UTC (permalink / raw)
To: eric.dumazet; +Cc: jhs, netdev, ebiederm, alexander.h.duyck
From: David Miller <davem@davemloft.net>
Date: Sun, 03 Nov 2013 23:12:32 -0500 (EST)
> Right, and BUG() if we try to register and action with a NULL .lookup
> member.
I return an error, BUG() is too harsh.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH ] net_sched: actions - Add default lookup
2013-11-04 4:12 ` David Miller
2013-11-04 4:17 ` David Miller
@ 2013-11-05 12:19 ` Jamal Hadi Salim
1 sibling, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2013-11-05 12:19 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: netdev, ebiederm, alexander.h.duyck
Apologies for the latency
My intention was to eventually remove it everywhere since this is needed
by all actions. An action could override it, otherwise they get the
default.
cheers,
jamal
On 11/03/13 23:12, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 30 Oct 2013 07:00:05 -0700
>
>> On Wed, 2013-10-30 at 07:25 -0400, Jamal Hadi Salim wrote:
>>> Attached. Tested with simple action.
>>>
>>> cheers,
>>> jamal
>>
>> Why not setting .lookup to tcf_hash_search
>> in the few actions not already doing that ?
>>
>> This would be more consistent.
>> # git grep -n tcf_hash_search
>> include/net/act_api.h:92:int tcf_hash_search(struct tc_action *a, u32 index);
>> net/sched/act_api.c:198:int tcf_hash_search(struct tc_action *a, u32 index)
>> net/sched/act_api.c:209:EXPORT_SYMBOL(tcf_hash_search);
>> net/sched/act_csum.c:588: .lookup = tcf_hash_search,
>> net/sched/act_gact.c:209: .lookup = tcf_hash_search,
>> net/sched/act_ipt.c:301: .lookup = tcf_hash_search,
>> net/sched/act_ipt.c:315: .lookup = tcf_hash_search,
>> net/sched/act_mirred.c:274: .lookup = tcf_hash_search,
>> net/sched/act_nat.c:311: .lookup = tcf_hash_search,
>> net/sched/act_pedit.c:246: .lookup = tcf_hash_search,
>> net/sched/act_police.c:410: .lookup = tcf_hash_search,
>
> Right, and BUG() if we try to register and action with a NULL .lookup
> member.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH ] net_sched: actions - Add default lookup
2013-11-04 4:17 ` David Miller
@ 2013-11-17 15:46 ` Jamal Hadi Salim
2013-11-17 16:23 ` Jamal Hadi Salim
0 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2013-11-17 15:46 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: netdev, ebiederm, alexander.h.duyck
On 11/03/13 23:17, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Sun, 03 Nov 2013 23:12:32 -0500 (EST)
>
>> Right, and BUG() if we try to register and action with a NULL .lookup
>> member.
>
> I return an error, BUG() is too harsh.
>
Sorry - I was distracted, but have time now.
I wanted to fix this then send the other patches - but confused.
We are setting the default if someone registers an action with
a NULL .lookup. Why do we want to return an error?
cheers,
jamal
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH ] net_sched: actions - Add default lookup
2013-11-17 15:46 ` Jamal Hadi Salim
@ 2013-11-17 16:23 ` Jamal Hadi Salim
0 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2013-11-17 16:23 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: netdev, ebiederm, alexander.h.duyck
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
Follow up to that patch would be this one.
I also want to do the same for the walker, but will wait
response.
cheers,
jamal
On 11/17/13 10:46, Jamal Hadi Salim wrote:
> On 11/03/13 23:17, David Miller wrote:
>> From: David Miller <davem@davemloft.net>
>> Date: Sun, 03 Nov 2013 23:12:32 -0500 (EST)
>>
>>> Right, and BUG() if we try to register and action with a NULL .lookup
>>> member.
>>
>> I return an error, BUG() is too harsh.
>>
>
> Sorry - I was distracted, but have time now.
> I wanted to fix this then send the other patches - but confused.
> We are setting the default if someone registers an action with
> a NULL .lookup. Why do we want to return an error?
>
> cheers,
> jamal
>
[-- Attachment #2: p-act2 --]
[-- Type: text/plain, Size: 3153 bytes --]
commit 926db1197d055ee933265ad4d5f5ae2392bcc654
Author: Jamal Hadi Salim <hadi@mojatatu.com>
Date: Sun Nov 17 11:20:16 2013 -0500
Use default lookup functions. Users are free to override when needed.
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 3a4c0ca..4225a93 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -585,7 +585,6 @@ static struct tc_action_ops act_csum_ops = {
.act = tcf_csum,
.dump = tcf_csum_dump,
.cleanup = tcf_csum_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_csum_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index fd2b3cf..15851da 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -206,7 +206,6 @@ static struct tc_action_ops act_gact_ops = {
.act = tcf_gact,
.dump = tcf_gact_dump,
.cleanup = tcf_gact_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_gact_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 60d88b6..1d3e191 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -298,7 +298,6 @@ static struct tc_action_ops act_ipt_ops = {
.act = tcf_ipt,
.dump = tcf_ipt_dump,
.cleanup = tcf_ipt_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_ipt_init,
.walk = tcf_generic_walker
};
@@ -312,7 +311,6 @@ static struct tc_action_ops act_xt_ops = {
.act = tcf_ipt,
.dump = tcf_ipt_dump,
.cleanup = tcf_ipt_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_ipt_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 977c10e..6cb16ec 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -271,7 +271,6 @@ static struct tc_action_ops act_mirred_ops = {
.act = tcf_mirred,
.dump = tcf_mirred_dump,
.cleanup = tcf_mirred_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_mirred_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 876f0ef..30c13de 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -308,7 +308,6 @@ static struct tc_action_ops act_nat_ops = {
.act = tcf_nat,
.dump = tcf_nat_dump,
.cleanup = tcf_nat_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_nat_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 7ed78c9..ab4fc56 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -243,7 +243,6 @@ static struct tc_action_ops act_pedit_ops = {
.act = tcf_pedit,
.dump = tcf_pedit_dump,
.cleanup = tcf_pedit_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_pedit_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 272d8e9..16a62c3 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -407,7 +407,6 @@ static struct tc_action_ops act_police_ops = {
.act = tcf_act_police,
.dump = tcf_act_police_dump,
.cleanup = tcf_act_police_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_act_police_locate,
.walk = tcf_act_police_walker
};
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-11-17 16:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-30 11:25 [PATCH ] net_sched: actions - Add default lookup Jamal Hadi Salim
2013-10-30 14:00 ` Eric Dumazet
2013-11-04 4:12 ` David Miller
2013-11-04 4:17 ` David Miller
2013-11-17 15:46 ` Jamal Hadi Salim
2013-11-17 16:23 ` Jamal Hadi Salim
2013-11-05 12:19 ` Jamal Hadi Salim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.