* [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@vger.kernel.org, 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@vger.kernel.org, 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: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
* 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
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 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).