: act_api: mark some functions static/remove unused function
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 1 line · Patrick McHardy
Mark some functions static in
act_api.
All the rest of the static conversions are good except for this.
Quoted 2 lines
tc_lookup_action_idwas unused
so I removed it.
Please dont. I am incrementally submitting patches.
cheers,
jamal
Quoted 10 lines · jamal
Quoted 4 lines · Patrick McHardy
tc_lookup_action_idwas unused
so I removed it.Please dont.
Ok, here is the same patch without removing tc_lookup_action_id. It is instead surrounded by #if 0.
Quoted 2 lines
I am incrementally submitting patches.
I was about to do more cleanup, please stop me if you are already doing any of this:
- move the functions from include/net/pkt_act.h to act_generic.c
- convert lastuse/install/expires to USER_HZ
- fix ipt: leaks memory at destruction, ovr doesn't work, doesn't refcount iptables module references
Is there already userspace-code for anything besides gact ? I would like to test my changes.
Regards
Patrick
# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2004/11/04 16:11:08+01:00 [email protected]
# [PKT_SCHED]: act_api: mark some functions static
#
# Signed-off-by: Patrick McHardy <[email protected]>
#
# net/sched/act_api.c # 2004/11/04 16:11:02+01:00 [email protected] +13 -11 # [PKT_SCHED]: act_api: mark some functions static # # Signed-off-by: Patrick McHardy <[email protected]> # diff -Nru a/net/sched/act_api.c b/net/sched/act_api.c --- a/net/sched/act_api.c 2004-11-04 16:11:32 +01:00 +++ b/net/sched/act_api.c 2004-11-04 16:11:32 +01:00 @@ -88,7 +88,7 @@ }
/* lookup by name */
-struct tc_action_ops *tc_lookup_action_n(char *kind) +static struct tc_action_ops *tc_lookup_action_n(char *kind)
{
struct tc_action_ops *a = NULL;
@@ -111,7 +111,7 @@
}
/* lookup by rtattr */
-struct tc_action_ops *tc_lookup_action(struct rtattr *kind) +static struct tc_action_ops *tc_lookup_action(struct rtattr *kind)
{
struct tc_action_ops *a = NULL;
@@ -134,6 +134,7 @@
return a;
}
+#if 0
/* lookup by id */
struct tc_action_ops *tc_lookup_action_id(u32 type) {
@@ -155,6 +156,7 @@
return a;
}
+#endif
int tcf_action_exec(struct sk_buff *skb,struct tc_action *act, struct tcf_result *res) {
@@ -504,7 +506,7 @@
return err;
}
-int tcf_action_get_1(struct rtattr *rta, struct tc_action *a, struct nlmsghdr *n, u32 pid) +static int tcf_action_get_1(struct rtattr *rta, struct tc_action *a, struct nlmsghdr *n, u32 pid)
{
struct tc_action_ops *a_o;
char act_name[4 + IFNAMSIZ + 1];
@@ -571,7 +573,7 @@
return err;
}
-void cleanup_a (struct tc_action *act)
+static void cleanup_a (struct tc_action *act)
{
struct tc_action *a;
@@ -587,7 +589,7 @@
}
}
-struct tc_action_ops *get_ao(struct rtattr *kind, struct tc_action *a) +static struct tc_action_ops *get_ao(struct rtattr *kind, struct tc_action *a)
{
char act_name[4 + IFNAMSIZ + 1];
struct tc_action_ops *a_o = NULL;
@@ -622,7 +624,7 @@
return a_o;
}
-struct tc_action *create_a(int i)
+static struct tc_action *create_a(int i)
{
struct tc_action *act = NULL;
@@ -639,7 +641,7 @@
return act;
}
-int tca_action_flush(struct rtattr *rta, struct nlmsghdr *n, u32 pid) +static int tca_action_flush(struct rtattr *rta, struct nlmsghdr *n, u32 pid)
{
struct sk_buff *skb;
unsigned char *b;
@@ -709,7 +711,7 @@
return err;
}
-int tca_action_gd(struct rtattr *rta, struct nlmsghdr *n, u32 pid, int event ) +static int tca_action_gd(struct rtattr *rta, struct nlmsghdr *n, u32 pid, int event )
{
int s = 0;
@@ -793,7 +795,7 @@
}
-int tcf_add_notify(struct tc_action *a, u32 pid, u32 seq, int event, unsigned flags) +static int tcf_add_notify(struct tc_action *a, u32 pid, u32 seq, int event, unsigned flags)
{
struct tcamsg *t;
struct nlmsghdr *nlh;
@@ -840,7 +842,7 @@
}
-int tcf_action_add(struct rtattr *rta, struct nlmsghdr *n, u32 pid, int ovr ) +static int tcf_action_add(struct rtattr *rta, struct nlmsghdr *n, u32 pid, int ovr )
{
int ret = 0;
struct tc_action *act = NULL;
@@ -923,7 +925,7 @@
return ret;
}
-char *
+static char *
find_dump_kind(struct nlmsghdr *n)
{
struct rtattr *tb1, *tb2[TCA_ACT_MAX+1];
Quoted 6 lines · Patrick McHardy
Quoted 2 lines · jamal
Please dont.
Ok, here is the same patch without removing
tc_lookup_action_id. It is instead surrounded by #if 0.
Why is this function bothering you? ;->
Why do you need to put the #if 0?
Quoted 5 lines
Quoted 2 lines
I am incrementally submitting patches.
I was about to do more cleanup, please stop me if you are already doing any of this:
My changes will be new stuff going in. So any cleanup is welcome.
Quoted 1 line
- move the functions from
include/net/pkt_act.htoact_generic.c
Some of the larger ones make sense to move. Whats the main reason for moving them?
Quoted 4 lines
- convert lastuse/install/expires to
USER_HZ
- fix ipt: leaks memory at destruction, ovr doesn't work, doesn't refcount iptables module references
These two make sense. I need help eyeballing the iptables stuff. I commented out some of the refcount attempts; also i think quiet a few targets are not friendly in getting accessed by anything other than iptables; I was going at some point fix them and send you guys patches. Maybe you can help me in this area as well.
Quoted 3 lines
Is there already userspace-code for anything besides gact ? I would like to test my changes.
Stephen is probably stalled somewhere because he hasnt been sucking in my updates to iproute2.
I have the latest and greatest on my test machine at home. But here are tc updates for ipt and mirred that i could find on my laptop. The ipt patch needs iptables libraries and may not work with latest iptables (although the one at home will surely work). The problem is some of the iptables functions were updated. It would be nice if i didnt have to maintain my own variants in user space.
cheers,
jamal
home/hadi/iproute2-2.6.9/tc/m_mirred.c new file +310 -0
@@ -0,0 +1,310 @@ +/* +* m_egress.c ingress/egress packet mirror/redir actions module +* +* This program is free software; you can distribute it and/or +* modify it under the terms of the GNU General Public License +* as published by the Free Software Foundation; either version +* 2 of the License, or (at your option) any later version. +* +* Authors: J Hadi Salim ([email protected]) +* +* TODO: Add Ingress support +* +*/ + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <syslog.h> +#include <fcntl.h> +#include <sys/socket.h> +#include <netinet/in.h> +#include <arpa/inet.h> +#include <string.h> +#include "utils.h" +#include "tc_util.h" +#include <linux/tc_act/tc_mirred.h> + +int mirred_d =1 ; + +static void +explain (void ) +{ +fprintf (stderr,"Usage: mirred <DIRECTION> <ACTION> [index INDEX] <dev DEVICENAME> \n" ); +fprintf (stderr,"where: \n" ); +fprintf (stderr,"DIRECTION := <ingress | egress>\n" ); +fprintf (stderr,"aCTION := <mirror | redirect>\n" ); +fprintf (stderr," : INDEX is the specific policy instance id\n" ); +fprintf (stderr," : DEVICENAME is the devicename \n" ); +} + +#define usage () return(-1) + +char *mirred_n2a (int action) +{ +switch (action) { +case TCA_EGRESS_REDIR : +return "Egress Redirect" ; +case TCA_INGRESS_REDIR : +return "Ingress Redirect" ; +case TCA_EGRESS_MIRROR : +return "Egress Mirror" ; +case TCA_INGRESS_MIRROR : +return "Ingress Mirror" ; +default : +return "unknown" ; + } +} + +int +parse_egress (struct action_util *a,int *argc_p,char ***argv_p,int tca_id,struct nlmsghdr *n) +{ + +int argc = *argc_p; +char **argv = *argv_p; +int ok =0 , iok =0 , mirror=0 ,redir=0 ; +struct tc_mirred p; +struct rtattr *tail; +char d[16 ]; +struct rtnl_handle rth; + +memset (d,0 ,sizeof (d)-1 ); +memset (&p,0 ,sizeof (struct tc_mirred )); + +while (argc >0 ) { + +if (matches (*argv,"action" ) ==0 ) { +break ; + }else if (matches (*argv,"egress" ) ==0 ) { +NEXT_ARG (); + ok++; +continue ; + }else { + +if (matches (*argv,"index" ) ==0 ) { +NEXT_ARG (); +if (get_u32 (&p.index, *argv,10 )) { +fprintf (stderr,"Illegal \"index\"\n" ); +return -1 ; + } + iok++; +if (!ok) { + argc--; + argv++; +break ; + } + }else if (!ok) { +fprintf (stderr,"was expecting egress (%s)\n" , *argv); +break ; + + }else if (!mirror &&matches (*argv,"mirror" ) ==0 ) { + mirror=1 ; +if (redir) { +fprintf (stderr,"Cant have both mirror and redir\n" ); +return -1 ; + } + p.eaction =TCA_EGRESS_MIRROR ; + p.action =TC_ACT_PIPE ; + ok++; + }else if (!redir &&matches (*argv,"redirect" ) ==0 ) { + redir=1 ; +if (mirror) { +fprintf (stderr,"Cant have both mirror and redir\n" ); +return -1 ; + } + p.eaction =TCA_EGRESS_REDIR ; + p.action =TC_ACT_STOLEN ; + ok++; + }else if ((redir || mirror) &&matches (*argv,"dev" ) ==0 ) { +NEXT_ARG (); +if (strlen (d)) +duparg ("dev" , *argv); + +strncpy (d, *argv,sizeof (d)-1 ); + argc--; + argv++; + +break ; + + } + } + +NEXT_ARG (); + } + +if (!ok && !iok) { +explain (); +return -1 ; + } + + + +if (d[0 ]) { +int idx; +if (rtnl_open (&rth,0 ) <0 ) { +fprintf (stderr,"Cannot open rtnetlink\n" ); +exit (1 ); + } +ll_init_map (&rth); + + +if ((idx =ll_name_to_index (d)) ==0 ) { +fprintf (stderr,"Cannot find device \"%s\"\n" , d); +rtnl_close (&rth); +return -1 ; + } + + p.ifindex = idx; +rtnl_close (&rth); + } + + +if (argc && p.eaction ==TCA_EGRESS_MIRROR ) { + +if (matches (*argv,"reclassify" ) ==0 ) { + p.action =TC_POLICE_RECLASSIFY ; +NEXT_ARG (); + }else if (matches (*argv,"pipe" ) ==0 ) { + p.action =TC_POLICE_PIPE ; +NEXT_ARG (); + }else if (matches (*argv,"drop" ) ==0 || +matches (*argv,"shot" ) ==0 ) { + p.action =TC_POLICE_SHOT ; +NEXT_ARG (); + }else if (matches (*argv,"continue" ) ==0 ) { + p.action =TC_POLICE_UNSPEC ; +NEXT_ARG (); + }else if (matches (*argv,"pass" ) ==0 ) { + p.action =TC_POLICE_OK ; +NEXT_ARG (); + } + + } + +if (argc) { +if (iok &&matches (*argv,"index" ) ==0 ) { +fprintf (stderr,"mirred: Illegal double index\n" ); +return -1 ; + }else { +if (matches (*argv,"index" ) ==0 ) { +NEXT_ARG (); +if (get_u32 (&p.index, *argv,10 )) { +fprintf (stderr,"mirred: Illegal \"index\"\n" ); +return -1 ; + } + argc--; + argv++; + } + } + } + +if (mirred_d) +fprintf (stdout,"Action %d device %s ifindex %d\n" ,p.action, d,p.ifindex); + + tail = (struct rtattr *) (((void *) n) +NLMSG_ALIGN (n->nlmsg_len)); +addattr_l (n,MAX_MSG , tca_id,NULL ,0 ); +addattr_l (n,MAX_MSG ,TCA_MIRRED_PARMS , &p,sizeof (p)); + tail->rta_len = + (((void *) n) +NLMSG_ALIGN (n->nlmsg_len)) - (void *) tail; + + *argc_p = argc; + *argv_p = argv; +return 0 ; +} + + +int +parse_mirred (struct action_util *a,int *argc_p,char ***argv_p,int tca_id,struct nlmsghdr *n) +{ + +int argc = *argc_p; +char **argv = *argv_p; + +if (argc <0 ) { +fprintf (stderr,"mirred bad arguement count %d\n" , argc); +return -1 ; + } + +if (matches (*argv,"mirred" ) ==0 ) { +NEXT_ARG (); + }else { +fprintf (stderr,"mirred bad arguement %s\n" , *argv); +return -1 ; + } + + +if (matches (*argv,"egress" ) ==0 ||matches (*argv,"index" ) ==0 ) { +int ret =parse_egress (a, &argc, &argv, tca_id, n); +if (ret ==0 ) { + *argc_p = argc; + *argv_p = argv; +return 0 ; + } + + }else if (matches (*argv,"ingress" ) ==0 ) { +fprintf (stderr,"mirred ingress not supported at the moment\n" ); + + }else { +fprintf (stderr,"mirred not supported %s\n" , *argv); + } + +return -1 ; + +} + +int +print_mirred (struct action_util *au,FILE * f,struct rtattr *arg) +{ +struct tc_mirred *p; +struct rtattr *tb[TCA_MIRRED_MAX +1 ]; +struct rtnl_handle rth; +const char *dev; +SPRINT_BUF (b1); + +if (arg ==NULL ) +return -1 ; + +memset (tb,0 ,sizeof (tb)); +parse_rtattr (tb,TCA_MIRRED_MAX ,RTA_DATA (arg),RTA_PAYLOAD (arg)); + +if (tb[TCA_MIRRED_PARMS ] ==NULL ) { +fprintf (f,"[NULL mirred parameters]" ); +return -1 ; + } + p =RTA_DATA (tb[TCA_MIRRED_PARMS ]); + +if (rtnl_open (&rth,0 ) <0 ) { +fprintf (stderr,"Cannot open rtnetlink\n" ); +return -1 ; + } + +ll_init_map (&rth); + + +if ((dev =ll_index_to_name (p->ifindex)) ==0 ) { +fprintf (stderr,"Cannot find device %d\n" , p->ifindex); +rtnl_close (&rth); +return -1 ; + } + +fprintf (f,"mirred (%s to device %s) %s" ,mirred_n2a (p->eaction), dev,action_n2a (p->action, b1,sizeof (b1))); + +fprintf (f,"\n " ); +fprintf (f,"\tindex %d ref %d bind %d" ,p->index,p->refcnt,p->bindcnt); + +if (show_stats) { +if (tb[TCA_MIRRED_TM ]) { +struct tcf_t *tm =RTA_DATA (tb[TCA_MIRRED_TM ]); +print_tm (f,tm); + } + } +fprintf (f,"\n " ); +rtnl_close (&rth); +return 0 ; +} + +struct action_util mirred_util = { + .id ="mirred" , + .parse_aopt = parse_mirred, + .print_aopt = print_mirred, +};
home/hadi/iproute2-2.6.9/include/linux/tc_act/tc_mirred.h new file +28 -0
@@ -0,0 +1,28 @@ +#ifndef __LINUX_TC_MIR_H +#define __LINUX_TC_MIR_H + +#include <linux/pkt_cls.h> + +#define TCA_ACT_MIRRED 8 +#define TCA_EGRESS_REDIR 1/* packet redirect to EGRESS*/ +#define TCA_EGRESS_MIRROR 2/* mirror packet to EGRESS */ +#define TCA_INGRESS_REDIR 3/* packet redirect to INGRESS*/ +#define TCA_INGRESS_MIRROR 4/* mirror packet to INGRESS */ + +struct tc_mirred +{ +tc_gen ; +int eaction;/* one of IN/EGRESS_MIRROR/REDIR */ +__u32 ifindex;/* ifindex of egress port */ +}; + +enum +{ +TCA_MIRRED_UNSPEC , +TCA_MIRRED_TM , +TCA_MIRRED_PARMS , + __TCA_MIRRED_MAX +}; +#define TCA_MIRRED_MAX (__TCA_MIRRED_MAX - 1) + +#endif
home/hadi/iproute2-2.6.9/configure +8 -1
@@ -26,12 +26,19 @@ fi rm -f /tmp/atmtest.c /tmp/atmtest -# hack for now +# hacks for now echo "TC actions" if [ -e "tc/m_gact.c" ] then + echo " GACT found" echo "TC_CONFIG_ACTION_GACT=y" >>Config echo "TC_CONFIG_ACTION_PROB=y" >>Config fi +if [ -e "tc/m_mirred.c" ] +then + echo " MIRRED found" + echo "TC_CONFIG_ACTION_MIRRED=y" >>Config +fi +
home/hadi/iproute2-2.6.9/doc/actions/mirred-usage new file +71 -0
71 lines not on this page · raw message
home/hadi/iproute2-2.6.9/tc/Makefile +1 -0
@@ -21,6 +21,7 @@ TCMODULES += q_hfsc.o TCMODULES += q_htb.o TCMODULES += m_gact.o +TCMODULES += m_mirred.o TCOBJ += $(TCMODULES)
root/iproute2-2.6.9-041019-mod/tc/m_ipt.c new file +683 -0
683 lines not on this page · raw message
root/iproute2-2.6.9-041019-mod/include/linux/tc_act/tc_ipt.h new file +21 -0
@@ -0,0 +1,21 @@ +#ifndef __LINUX_TC_IPT_H +#define __LINUX_TC_IPT_H + +#include <linux/pkt_cls.h> + +#define TCA_ACT_IPT 6 + +enum +{ +TCA_IPT_UNSPEC , +TCA_IPT_TABLE , +TCA_IPT_HOOK , +TCA_IPT_INDEX , +TCA_IPT_CNT , +TCA_IPT_TM , +TCA_IPT_TARG , + __TCA_IPT_MAX +}; +#define TCA_IPT_MAX (__TCA_IPT_MAX - 1) + +#endif
root/iproute2-2.6.9-041019-mod/tc/Makefile renamed from root/iproute2-2.6.9-041019/tc/Makefile +14 -1
@@ -3,6 +3,8 @@ include ../Config +IPT_INCL = /root/iptables-1.2.11/include + TCMODULES := TCMODULES += q_fifo.o TCMODULES += q_sfq.o
@@ -29,6 +31,12 @@ TCLIB += tc_cbq.o TCLIB += tc_estimator.o +ifeq ($(TC_CONFIG_ACTION_IPT),y) +LDLIBS += -L. m_ipt.o -ltc -lm -ldl +else +LDLIBS += -L. -ltc -lm -ldl +endif + CFLAGS += -DCONFIG_GACT -DCONFIG_GACT_PROB
@@ -52,10 +60,15 @@ ./$* >$@ rm $* -all: libtc.a tc $(TCSO) $(NETEM_DIST) +all: ipt libtc.a tc $(TCSO) $(NETEM_DIST) tc: $(TCOBJ) $(LIBNETLINK) $(LIBUTIL) $(TCLIB) +ipt: +ifeq ($(TC_CONFIG_ACTION_IPT),y) + $(CC) $(CCOPTS) -I$(KERNEL_INCLUDE) $(DEFINES) -I$(IPT_INCL) -I../include -c -o m_ipt.o m_ipt.c +#$(CC) $(CCOPTS) -I$(KERNEL_INCLUDE) -I../include $(DEFINES) -DCONFIG_NET_CLS_ACT -I$(IPT_INCL) -c -o m_ipt.o m_ipt.c +endif libtc.a: $(TCLIB) $(AR) rcs $@ $(TCLIB)
root/iproute2-2.6.9-041019-mod/configure renamed from root/iproute2-2.6.9-041019/configure +13 -1
@@ -26,12 +26,24 @@ fi rm -f /tmp/atmtest.c /tmp/atmtest -# hack for now +# hacks for now echo "TC actions" if [ -e "tc/m_gact.c" ] then + echo " GACT found" echo "TC_CONFIG_ACTION_GACT=y" >>Config echo "TC_CONFIG_ACTION_PROB=y" >>Config fi +if [ -e "tc/m_mirred.c" ] +then + echo " MIRRED found" + echo "TC_CONFIG_ACTION_MIRRED=y" >>Config +fi + +if [ -e "tc/m_ipt.c" ] +then + echo " IPT found" + echo "TC_CONFIG_ACTION_IPT=y" >>Config +fi
Quoted 10 lines · jamal
Quoted 3 lines · Patrick McHardy
Ok, here is the same patch without removing
tc_lookup_action_id. It is instead surrounded by #if 0.Why is this function bothering you? ;->
Why do you need to put the #if 0?
Because the compiler will warn about an unused static function.
Quoted 6 lines
Quoted 2 lines
- move the functions from
include/net/pkt_act.htoact_generic.cSome of the larger ones make sense to move. Whats the main reason for moving them?
I was talking about the larger ones. There are multiple large inline functions that are used by all(?) actions.
Quoted 7 lines
These two make sense. I need help eyeballing the iptables stuff. I commented out some of the refcount attempts; also i think quiet a few targets are not friendly in getting accessed by anything other than iptables; I was going at some point fix them and send you guys patches. Maybe you can help me in this area as well.
I can't think of a reason why targets would mind beeing used by ipt, but I'll look out for problems.
Quoted 6 lines
Quoted 5 lines
Is there already userspace-code for anything besides gact ? I would like to test my changes.
Quoted 6 lines
Stephen is probably stalled somewhere because he hasnt been sucking in my updates to iproute2.
I have the latest and greatest on my test machine at home. But here are tc updates for ipt and mirred that i could find on my laptop.
Thanks.
Quoted 5 lines
The ipt patch needs iptables libraries and may not work with latest
iptables (although the one at home will surely work). The problem is some of the iptables functions were updated. It would be nice if i didnt have to maintain my own variants in user space.
I can't see a way to avoid this currently.
Regards
Patrick
Quoted 5 lines · Patrick McHardy
Quoted 3 lines
Why is this function bothering you? ;->
Why do you need to put the #if 0?Because the compiler will warn about an unused static function.
fsck the compiler. Ok, fine if-0 it. I will unif-0 it when i submit code later on.
Quoted 7 lines
Quoted 4 lines
Some of the larger ones make sense to move. Whats the main reason for moving them?
I was talking about the larger ones. There are multiple large inline functions that are used by all(?) actions.
Sure go ahead.
Quoted 4 lines
I can't think of a reason why targets would mind beeing used by ipt, but I'll look out for problems.
Try a few targets; what i found is they were bugs in the targets code. Typically they wont check for something or other that i pass NULL to and just directly reference it. This could be both in kernel and user space.
Quoted 8 lines
Quoted 5 lines
The ipt patch needs iptables libraries and may not work with latest iptables (although the one at home will surely work). The problem is some of the iptables functions were updated. It would be nice if i didnt have to maintain my own variants in user space.
I can't see a way to avoid this currently.
I guess nobody has needed it before; take a look at the code i cutnpasted. See if some of that code could be made common and just referenced by me (since you guys maintain it).
Thanks a lot for your help Patrick.
cheers,
jamal
PS:- I am gonna disappear for a short while - so responses will have high latency.
BTW, at some point i would like to discuss an IMQ replacement which uses mirred and dummy device extension.
I want these patches to settle down for a while first and getting tested.
cheers,
jamal
Quoted 6 lines · Jamal Hadi Salim
BTW, at some point i would like to discuss an IMQ replacement which uses mirred and dummy device extension.
I want these patches to settle down for a while first and getting tested.
I'm not very familiar with the needs of the IMQ users these days, but I'm sure there will be lots of people with wishes :)
Regards
Patrick