[PATCH ] net_sched: actions - Add default lookup

Attached. Tested with simple action.

cheers,
jamal

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,

From: Eric Dumazet <redacted>
Date: Wed, 30 Oct 2013 07:00:05 -0700

On Wed, 2013-10-30 at 07:25 -0400, Jamal Hadi Salim wrote:

quoted

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.

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.

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 <redacted>
Date: Wed, 30 Oct 2013 07:00:05 -0700

quoted

On Wed, 2013-10-30 at 07:25 -0400, Jamal Hadi Salim wrote:

quoted

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.

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)

quoted

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

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:

quoted

From: David Miller <davem@davemloft.net>
Date: Sun, 03 Nov 2013 23:12:32 -0500 (EST)

quoted

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