lkml.org 
[lkml]   [2006]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: tcf_generic_walker(): what's going on?
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-04-06 10:55    [W:0.030 / U:0.948 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site