Denis Vlasenko wrote:
> While hunting down oversized inlines
> I stumbled on tcf_generic_walker().
>
> It is defined in two separate files:
> once as an inline in include/net/pkt_act.h
> (really big one, ~750 bytes of code)
> and once as a static function in net/sched/act_police.c
>
> These two instances are not identical.
> Second one has one extra parameter (int type)
> and it uses it like this:
>
> for (i = 0; i < MY_TAB_SIZE; i++) {
> p = tcf_police_ht[tcf_police_hash(i)];
>
> for (; p; p = p->next) {
> index++;
> if (index < s_i)
> continue;
> a->priv = p;
> a->order = index;
> r = (struct rtattr*) skb->tail;
> RTA_PUT(skb, a->order, 0, NULL);
> + if (type == RTM_DELACTION)
> + err = tcf_action_dump_1(skb, a, 0, 1);
> + else
> + err = tcf_action_dump_1(skb, a, 0, 0);
>
> Having two functions with same name is rather confusing.
> Worse, they are both are called via five different
> tc_action_ops structs:
>
> static struct tc_action_ops act_ipt_ops = {
> ...
> .walk = tcf_generic_walker
>
> and I fail to understand how it is supposed to work,
> considering the fact that these two tcf_generic_walker's
> have different prototypes.
This code is a lazy hack to achieve "genericness" by using
a "generic" inline functions and #defines to create
specialized instances.
> 1) What should I do with tcf_generic_walker?
> 2) Should I deinline huge inlines in include/net/pkt_act.h?
> If yes, to which .c file should I move them?
Unforunately you can't do this without further restructuring,
look at how the tc actions use that file:
/* use generic hash table */
#define MY_TAB_SIZE 8
#define MY_TAB_MASK (MY_TAB_SIZE - 1)
static u32 idx_gen;
static struct tcf_mirred *tcf_mirred_ht[MY_TAB_SIZE];
static DEFINE_RWLOCK(mirred_lock);
/* ovewrride the defaults */
#define tcf_st tcf_mirred
#define tc_st tc_mirred
#define tcf_t_lock mirred_lock
#define tcf_ht tcf_mirred_ht
#define CONFIG_NET_ACT_INIT 1
#include <net/pkt_act.h>
What needs to be done is to put a pointer to the hash, its size
and its lock in struct tc_action_ops and move the "generic"
functions to a seperate .c file and make them work on a struct
tcf_act_common.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
