* [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup
@ 2004-10-29 0:21 Thomas Graf
2004-10-29 0:22 ` [PATCH 1/6] PKT_SCHED: Add generic classifier routines Thomas Graf
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 0:21 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
Dave,
The following patchset is basically what we discussed before
but without the tcf_filter bits because it's not really worth
it. It will save us ~100 lines of code per classifier.
I tested the already splitted patchset this time :->
Jamal, I tried to test the action stuff as good as possible, but
can you give it a run with your test scripts?
I will submit a 2.4 version for patch 6 in a few minutes.
Cheers
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] PKT_SCHED: Add generic classifier routines
2004-10-29 0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
@ 2004-10-29 0:22 ` Thomas Graf
2004-10-29 10:23 ` [RESEND " Thomas Graf
2004-10-29 0:23 ` [PATCH 2/6] cls_fw: Cleanup fw_classify Thomas Graf
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 0:22 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
Adds generic routines used by classifier to:
- bind/unbind to classes
- configure action/police/indev
- dump action/police
Signed-off-by: Thomas Graf <tgraf@suug.ch>
--- linux-2.6.10-rc1-bk7.orig/include/net/pkt_cls.h 2004-10-28 22:03:20.000000000 +0200
+++ linux-2.6.10-rc1-bk7/include/net/pkt_cls.h 2004-10-28 23:53:05.000000000 +0200
@@ -3,6 +3,7 @@
#include <linux/pkt_cls.h>
#include <net/sch_generic.h>
+#include <net/act_api.h>
/* Basic packet classifier frontend definitions. */
@@ -41,4 +42,190 @@
return old_cl;
}
+static inline void
+tcf_bind_filter(struct tcf_proto *tp, struct tcf_result *r, unsigned long base)
+{
+ unsigned long cl;
+
+ cl = tp->q->ops->cl_ops->bind_tcf(tp->q, base, r->classid);
+ cl = cls_set_class(tp, &r->class, cl);
+ if (cl)
+ tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
+}
+
+static inline void
+tcf_unbind_filter(struct tcf_proto *tp, struct tcf_result *r)
+{
+ unsigned long cl;
+
+ if ((cl = __cls_set_class(&r->class, 0)) != 0)
+ tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
+}
+
+#ifdef CONFIG_NET_CLS_ACT
+static inline int
+tcf_change_act_police(struct tcf_proto *tp, struct tc_action **action,
+ struct rtattr *act_police_tlv, struct rtattr *rate_tlv)
+{
+ int ret;
+ struct tc_action *act;
+
+ act = kmalloc(sizeof(*act), GFP_KERNEL);
+ if (NULL == act)
+ return -ENOMEM;
+ memset(act, 0, sizeof(*act));
+
+ ret = tcf_action_init_1(act_police_tlv, rate_tlv, act, "police",
+ TCA_ACT_NOREPLACE, TCA_ACT_BIND);
+ if (ret < 0) {
+ tcf_action_destroy(act, TCA_ACT_UNBIND);
+ return ret;
+ }
+
+ act->type = TCA_OLD_COMPAT;
+
+ if (*action) {
+ tcf_tree_lock(tp);
+ act = xchg(action, act);
+ tcf_tree_unlock(tp);
+
+ tcf_action_destroy(act, TCA_ACT_UNBIND);
+ } else
+ *action = act;
+
+ return 0;
+}
+
+static inline int
+tcf_change_act(struct tcf_proto *tp, struct tc_action **action,
+ struct rtattr *act_tlv, struct rtattr *rate_tlv)
+{
+ int ret;
+ struct tc_action *act;
+
+ act = kmalloc(sizeof(*act), GFP_KERNEL);
+ if (NULL == act)
+ return -ENOMEM;
+ memset(act, 0, sizeof(*act));
+
+ ret = tcf_action_init(act_tlv, rate_tlv, act, NULL,
+ TCA_ACT_NOREPLACE, TCA_ACT_BIND);
+ if (ret < 0) {
+ tcf_action_destroy(act, TCA_ACT_UNBIND);
+ return ret;
+ }
+
+ if (*action) {
+ tcf_tree_lock(tp);
+ act = xchg(action, act);
+ tcf_tree_unlock(tp);
+
+ tcf_action_destroy(act, TCA_ACT_UNBIND);
+ } else
+ *action = act;
+
+ return 0;
+}
+
+static inline int
+tcf_dump_act(struct sk_buff *skb, struct tc_action *action,
+ int act_type, int compat_type)
+{
+ /*
+ * again for backward compatible mode - we want
+ * to work with both old and new modes of entering
+ * tc data even if iproute2 was newer - jhs
+ */
+ if (action) {
+ struct rtattr * p_rta = (struct rtattr*) skb->tail;
+
+ if (action->type != TCA_OLD_COMPAT) {
+ RTA_PUT(skb, act_type, 0, NULL);
+ if (tcf_action_dump(skb, action, 0, 0) < 0)
+ goto rtattr_failure;
+ } else {
+ RTA_PUT(skb, compat_type, 0, NULL);
+ if (tcf_action_dump_old(skb, action, 0, 0) < 0)
+ goto rtattr_failure;
+ }
+
+ p_rta->rta_len = skb->tail - (u8*)p_rta;
+ }
+ return 0;
+
+rtattr_failure:
+ return -1;
+}
+
+#ifdef CONFIG_NET_CLS_IND
+static inline int
+tcf_change_indev(struct tcf_proto *tp, char *indev, struct rtattr *indev_tlv)
+{
+ if (RTA_PAYLOAD(indev_tlv) >= IFNAMSIZ) {
+ printk("cls: bad indev name %s\n", (char *) RTA_DATA(indev_tlv));
+ return -EINVAL;
+ }
+
+ memset(indev, 0, IFNAMSIZ);
+ sprintf(indev, "%s", (char *) RTA_DATA(indev_tlv));
+
+ return 0;
+}
+
+static inline int
+tcf_match_indev(struct sk_buff *skb, char *indev)
+{
+ if (0 != indev[0]) {
+ if (NULL == skb->input_dev)
+ return 0;
+ else if (0 != strcmp(indev, skb->input_dev->name))
+ return 0;
+ }
+
+ return 1;
+}
+#endif /* CONFIG_NET_CLS_IND */
+#endif /* CONFIG_NET_CLS_ACT */
+
+
+#ifdef CONFIG_NET_CLS_POLICE
+static inline int
+tcf_change_police(struct tcf_proto *tp, struct tcf_police **police,
+ struct rtattr *police_tlv, struct rtattr *rate_tlv)
+{
+ struct tcf_police *p = tcf_police_locate(police_tlv, rate_tlv);
+
+ if (*police) {
+ tcf_tree_lock(tp);
+ p = xchg(police, p);
+ tcf_tree_unlock(tp);
+
+ tcf_police_release(p, TCA_ACT_UNBIND);
+ } else
+ *police = p;
+
+ return 0;
+}
+
+static inline int
+tcf_dump_police(struct sk_buff *skb, struct tcf_police *police,
+ int police_type)
+{
+ if (police) {
+ struct rtattr * p_rta = (struct rtattr*) skb->tail;
+
+ RTA_PUT(skb, police_type, 0, NULL);
+
+ if (tcf_police_dump(skb, police) < 0)
+ goto rtattr_failure;
+
+ p_rta->rta_len = skb->tail - (u8*)p_rta;
+ }
+ return 0;
+
+rtattr_failure:
+ return -1;
+}
+#endif /* CONFIG_NET_CLS_POLICE */
+
#endif
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/6] cls_fw: Cleanup fw_classify
2004-10-29 0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
2004-10-29 0:22 ` [PATCH 1/6] PKT_SCHED: Add generic classifier routines Thomas Graf
@ 2004-10-29 0:23 ` Thomas Graf
2004-10-29 0:24 ` [PATCH 3/6] cls_fw: Use generic routines to configure action/policer Thomas Graf
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 0:23 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
Cleans up fw_classify by using the generic routines and
adds a additional but unneeded "continue" to document that
an action may overrule the filter's match result.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
--- linux-2.6.10-rc1-bk7.orig/net/sched/cls_fw.c 2004-10-28 22:03:29.000000000 +0200
+++ linux-2.6.10-rc1-bk7/net/sched/cls_fw.c 2004-10-29 00:00:50.000000000 +0200
@@ -84,47 +84,39 @@
u32 id = 0;
#endif
- if (head == NULL)
- goto old_method;
-
- for (f=head->ht[fw_hash(id)]; f; f=f->next) {
- if (f->id == id) {
- *res = f->res;
+ if (head != NULL) {
+ for (f=head->ht[fw_hash(id)]; f; f=f->next) {
+ if (f->id == id) {
+ *res = f->res;
#ifdef CONFIG_NET_CLS_ACT
#ifdef CONFIG_NET_CLS_IND
- if (0 != f->indev[0]) {
- if (NULL == skb->input_dev) {
+ if (!tcf_match_indev(skb, f->indev))
+ continue;
+#endif /* CONFIG_NET_CLS_IND */
+ if (f->action) {
+ int act_res = tcf_action_exec(skb, f->action, res);
+ if (act_res >= 0)
+ return act_res;
continue;
- } else {
- if (0 != strcmp(f->indev, skb->input_dev->name)) {
- continue;
- }
}
- }
-#endif
- if (f->action) {
- int pol_res = tcf_action_exec(skb, f->action, res);
- if (pol_res >= 0)
- return pol_res;
- } else
-#else
+#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
- if (f->police)
- return tcf_police(skb, f->police);
-#endif
-#endif
+ if (f->police)
+ return tcf_police(skb, f->police);
+#endif /* CONFIG_NET_CLS_POLICE */
+#endif /* CONFIG_NET_CLS_ACT */
+ return 0;
+ }
+ }
+ } else {
+ /* old method */
+ if (id && (TC_H_MAJ(id) == 0 || !(TC_H_MAJ(id^tp->q->handle)))) {
+ res->classid = id;
+ res->class = 0;
return 0;
}
}
- return -1;
-old_method:
- if (id && (TC_H_MAJ(id) == 0 ||
- !(TC_H_MAJ(id^tp->q->handle)))) {
- res->classid = id;
- res->class = 0;
- return 0;
- }
return -1;
}
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/6] cls_fw: Use generic routines to configure action/policer
2004-10-29 0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
2004-10-29 0:22 ` [PATCH 1/6] PKT_SCHED: Add generic classifier routines Thomas Graf
2004-10-29 0:23 ` [PATCH 2/6] cls_fw: Cleanup fw_classify Thomas Graf
@ 2004-10-29 0:24 ` Thomas Graf
2004-10-29 0:24 ` [PATCH 4/6] cls_fw: Use generic routines to dump action/policer Thomas Graf
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 0:24 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
Adds a new function fw_change_attr using the new generic routines which
can be used to change attribute but also to initially set them to avoid
duplicated code.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
--- linux-2.6.10-rc1-bk7.orig/net/sched/cls_fw.c 2004-10-29 00:27:10.000000000 +0200
+++ linux-2.6.10-rc1-bk7/net/sched/cls_fw.c 2004-10-29 00:27:19.000000000 +0200
@@ -155,11 +155,8 @@
for (h=0; h<256; h++) {
while ((f=head->ht[h]) != NULL) {
- unsigned long cl;
head->ht[h] = f->next;
-
- if ((cl = __cls_set_class(&f->res.class, 0)) != 0)
- tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
+ tcf_unbind_filter(tp, &f->res);
#ifdef CONFIG_NET_CLS_ACT
if (f->action) {
tcf_action_destroy(f->action,TCA_ACT_UNBIND);
@@ -187,14 +184,10 @@
for (fp=&head->ht[fw_hash(f->id)]; *fp; fp = &(*fp)->next) {
if (*fp == f) {
- unsigned long cl;
-
tcf_tree_lock(tp);
*fp = f->next;
tcf_tree_unlock(tp);
-
- if ((cl = cls_set_class(tp, &f->res.class, 0)) != 0)
- tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
+ tcf_unbind_filter(tp, &f->res);
#ifdef CONFIG_NET_CLS_ACT
if (f->action) {
tcf_action_destroy(f->action,TCA_ACT_UNBIND);
@@ -212,21 +205,67 @@
return -EINVAL;
}
+static int
+fw_change_attrs(struct tcf_proto *tp, struct fw_filter *f,
+ struct rtattr **tb, struct rtattr **tca, unsigned long base)
+{
+ int err = -EINVAL;
+
+ if (tb[TCA_FW_CLASSID-1]) {
+ if (RTA_PAYLOAD(tb[TCA_FW_CLASSID-1]) != sizeof(u32))
+ goto errout;
+ f->res.classid = *(u32*)RTA_DATA(tb[TCA_FW_CLASSID-1]);
+ tcf_bind_filter(tp, &f->res, base);
+ }
+
+#ifdef CONFIG_NET_CLS_ACT
+ if (tb[TCA_FW_POLICE-1]) {
+ err = tcf_change_act_police(tp, &f->action, tb[TCA_FW_POLICE-1],
+ tca[TCA_RATE-1]);
+ if (err < 0)
+ goto errout;
+ }
+
+ if (tb[TCA_FW_ACT-1]) {
+ err = tcf_change_act(tp, &f->action, tb[TCA_FW_ACT-1],
+ tca[TCA_RATE-1]);
+ if (err < 0)
+ goto errout;
+ }
+
+#ifdef CONFIG_NET_CLS_IND
+ if (tb[TCA_FW_INDEV-1]) {
+ err = tcf_change_indev(tp, f->indev, tb[TCA_FW_INDEV-1]);
+ if (err < 0)
+ goto errout;
+ }
+#endif /* CONFIG_NET_CLS_IND */
+#else /* CONFIG_NET_CLS_ACT */
+#ifdef CONFIG_NET_CLS_POLICE
+ if (tb[TCA_FW_POLICE-1]) {
+ err = tcf_change_police(tp, &f->police, tb[TCA_FW_POLICE-1],
+ tca[TCA_RATE-1]);
+ if (err < 0)
+ goto errout;
+ }
+#endif /* CONFIG_NET_CLS_POLICE */
+#endif /* CONFIG_NET_CLS_ACT */
+
+ err = 0;
+errout:
+ return err;
+}
+
static int fw_change(struct tcf_proto *tp, unsigned long base,
u32 handle,
struct rtattr **tca,
unsigned long *arg)
{
struct fw_head *head = (struct fw_head*)tp->root;
- struct fw_filter *f;
+ struct fw_filter *f = (struct fw_filter *) *arg;
struct rtattr *opt = tca[TCA_OPTIONS-1];
struct rtattr *tb[TCA_FW_MAX];
int err;
-#ifdef CONFIG_NET_CLS_ACT
- struct tc_action *act = NULL;
- int ret;
-#endif
-
if (!opt)
return handle ? -EINVAL : 0;
@@ -234,85 +273,10 @@
if (rtattr_parse(tb, TCA_FW_MAX, RTA_DATA(opt), RTA_PAYLOAD(opt)) < 0)
return -EINVAL;
- if ((f = (struct fw_filter*)*arg) != NULL) {
- /* Node exists: adjust only classid */
-
+ if (f != NULL) {
if (f->id != handle && handle)
return -EINVAL;
- if (tb[TCA_FW_CLASSID-1]) {
- unsigned long cl;
-
- f->res.classid = *(u32*)RTA_DATA(tb[TCA_FW_CLASSID-1]);
- cl = tp->q->ops->cl_ops->bind_tcf(tp->q, base, f->res.classid);
- cl = cls_set_class(tp, &f->res.class, cl);
- if (cl)
- tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
- }
-#ifdef CONFIG_NET_CLS_ACT
- if (tb[TCA_FW_POLICE-1]) {
- act = kmalloc(sizeof(*act),GFP_KERNEL);
- if (NULL == act)
- return -ENOMEM;
-
- memset(act,0,sizeof(*act));
- ret = tcf_action_init_1(tb[TCA_FW_POLICE-1], tca[TCA_RATE-1] ,act,"police",TCA_ACT_NOREPLACE,TCA_ACT_BIND);
- if (0 > ret){
- tcf_action_destroy(act,TCA_ACT_UNBIND);
- return ret;
- }
- act->type = TCA_OLD_COMPAT;
-
- sch_tree_lock(tp->q);
- act = xchg(&f->action, act);
- sch_tree_unlock(tp->q);
-
- tcf_action_destroy(act,TCA_ACT_UNBIND);
-
- }
-
- if(tb[TCA_FW_ACT-1]) {
- act = kmalloc(sizeof(*act),GFP_KERNEL);
- if (NULL == act)
- return -ENOMEM;
- memset(act,0,sizeof(*act));
- ret = tcf_action_init(tb[TCA_FW_ACT-1], tca[TCA_RATE-1],act,NULL, TCA_ACT_NOREPLACE,TCA_ACT_BIND);
- if (0 > ret) {
- tcf_action_destroy(act,TCA_ACT_UNBIND);
- return ret;
- }
-
- sch_tree_lock(tp->q);
- act = xchg(&f->action, act);
- sch_tree_unlock(tp->q);
-
- tcf_action_destroy(act,TCA_ACT_UNBIND);
- }
-#ifdef CONFIG_NET_CLS_IND
- if(tb[TCA_FW_INDEV-1]) {
- struct rtattr *idev = tb[TCA_FW_INDEV-1];
- if (RTA_PAYLOAD(idev) >= IFNAMSIZ) {
- printk("cls_fw: bad indev name %s\n",(char*)RTA_DATA(idev));
- err = -EINVAL;
- goto errout;
- }
- memset(f->indev,0,IFNAMSIZ);
- sprintf(f->indev, "%s", (char*)RTA_DATA(idev));
- }
-#endif
-#else /* only POLICE defined */
-#ifdef CONFIG_NET_CLS_POLICE
- if (tb[TCA_FW_POLICE-1]) {
- struct tcf_police *police = tcf_police_locate(tb[TCA_FW_POLICE-1], tca[TCA_RATE-1]);
-
- tcf_tree_lock(tp);
- police = xchg(&f->police, police);
- tcf_tree_unlock(tp);
-
- tcf_police_release(police,TCA_ACT_UNBIND);
- }
-#endif
-#endif
- return 0;
+ return fw_change_attrs(tp, f, tb, tca, base);
}
if (!handle)
@@ -336,45 +300,9 @@
f->id = handle;
- if (tb[TCA_FW_CLASSID-1]) {
- err = -EINVAL;
- if (RTA_PAYLOAD(tb[TCA_FW_CLASSID-1]) != 4)
- goto errout;
- f->res.classid = *(u32*)RTA_DATA(tb[TCA_FW_CLASSID-1]);
- cls_set_class(tp, &f->res.class, tp->q->ops->cl_ops->bind_tcf(tp->q, base, f->res.classid));
- }
-
-#ifdef CONFIG_NET_CLS_ACT
- if(tb[TCA_FW_ACT-1]) {
- act = kmalloc(sizeof(*act),GFP_KERNEL);
- if (NULL == act)
- return -ENOMEM;
- memset(act,0,sizeof(*act));
- ret = tcf_action_init(tb[TCA_FW_ACT-1], tca[TCA_RATE-1],act,NULL,TCA_ACT_NOREPLACE,TCA_ACT_BIND);
- if (0 > ret) {
- tcf_action_destroy(act,TCA_ACT_UNBIND);
- return ret;
- }
- f->action= act;
- }
-#ifdef CONFIG_NET_CLS_IND
- if(tb[TCA_FW_INDEV-1]) {
- struct rtattr *idev = tb[TCA_FW_INDEV-1];
- if (RTA_PAYLOAD(idev) >= IFNAMSIZ) {
- printk("cls_fw: bad indev name %s\n",(char*)RTA_DATA(idev));
- err = -EINVAL;
- goto errout;
- }
- memset(f->indev,0,IFNAMSIZ);
- sprintf(f->indev, "%s", (char*)RTA_DATA(idev));
- }
-#endif
-#else
-#ifdef CONFIG_NET_CLS_POLICE
- if (tb[TCA_FW_POLICE-1])
- f->police = tcf_police_locate(tb[TCA_FW_POLICE-1], tca[TCA_RATE-1]);
-#endif
-#endif
+ err = fw_change_attrs(tp, f, tb, tca, base);
+ if (err < 0)
+ goto errout;
f->next = head->ht[fw_hash(handle)];
tcf_tree_lock(tp);
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/6] cls_fw: Use generic routines to dump action/policer
2004-10-29 0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
` (2 preceding siblings ...)
2004-10-29 0:24 ` [PATCH 3/6] cls_fw: Use generic routines to configure action/policer Thomas Graf
@ 2004-10-29 0:24 ` Thomas Graf
2004-10-29 0:25 ` [PATCH 5/6] cls_fw: Whitespace/ifdef fixes Thomas Graf
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 0:24 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
Signed-off-by: Thomas Graf <tgraf@suug.ch>
--- linux-2.6.10-rc1-bk7.orig/net/sched/cls_fw.c 2004-10-29 00:28:59.000000000 +0200
+++ linux-2.6.10-rc1-bk7/net/sched/cls_fw.c 2004-10-29 00:35:27.000000000 +0200
@@ -375,48 +375,18 @@
if (f->res.classid)
RTA_PUT(skb, TCA_FW_CLASSID, 4, &f->res.classid);
#ifdef CONFIG_NET_CLS_ACT
- /* again for backward compatible mode - we want
- * to work with both old and new modes of entering
- * tc data even if iproute2 was newer - jhs
- */
- if (f->action) {
- struct rtattr * p_rta = (struct rtattr*)skb->tail;
-
- if (f->action->type != TCA_OLD_COMPAT) {
- RTA_PUT(skb, TCA_FW_ACT, 0, NULL);
- if (tcf_action_dump(skb,f->action,0,0) < 0) {
- goto rtattr_failure;
- }
- } else {
- RTA_PUT(skb, TCA_FW_POLICE, 0, NULL);
- if (tcf_action_dump_old(skb,f->action,0,0) < 0) {
- goto rtattr_failure;
- }
- }
-
- p_rta->rta_len = skb->tail - (u8*)p_rta;
- }
+ if (tcf_dump_act(skb, f->action, TCA_FW_ACT, TCA_FW_POLICE) < 0)
+ goto rtattr_failure;
#ifdef CONFIG_NET_CLS_IND
- if(strlen(f->indev)) {
- struct rtattr * p_rta = (struct rtattr*)skb->tail;
+ if (strlen(f->indev))
RTA_PUT(skb, TCA_FW_INDEV, IFNAMSIZ, f->indev);
- p_rta->rta_len = skb->tail - (u8*)p_rta;
- }
-#endif
-#else
+#endif /* CONFIG_NET_CLS_IND */
+#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
- if (f->police) {
- struct rtattr * p_rta = (struct rtattr*)skb->tail;
-
- RTA_PUT(skb, TCA_FW_POLICE, 0, NULL);
-
- if (tcf_police_dump(skb, f->police) < 0)
- goto rtattr_failure;
-
- p_rta->rta_len = skb->tail - (u8*)p_rta;
- }
-#endif
-#endif
+ if (tcf_dump_police(skb, f->police, TCA_FW_POLICE) < 0)
+ goto rtattr_failure;
+#endif /* CONFIG_NET_CLS_POLICE */
+#endif /* CONFIG_NET_CLS_ACT */
rta->rta_len = skb->tail - b;
#ifdef CONFIG_NET_CLS_ACT
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/6] cls_fw: Whitespace/ifdef fixes
2004-10-29 0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
` (3 preceding siblings ...)
2004-10-29 0:24 ` [PATCH 4/6] cls_fw: Use generic routines to dump action/policer Thomas Graf
@ 2004-10-29 0:25 ` Thomas Graf
2004-10-29 0:26 ` [PATCH 6/6] PKT_SCHED: break is not enough to stop walking Thomas Graf
2004-10-29 10:26 ` [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT Thomas Graf
6 siblings, 0 replies; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 0:25 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
Signed-off-by: Thomas Graf <tgraf@suug.ch>
--- linux-2.6.10-rc1-bk7.orig/net/sched/cls_fw.c 2004-10-29 00:37:34.000000000 +0200
+++ linux-2.6.10-rc1-bk7/net/sched/cls_fw.c 2004-10-29 00:38:46.000000000 +0200
@@ -57,15 +57,15 @@
u32 id;
struct tcf_result res;
#ifdef CONFIG_NET_CLS_ACT
- struct tc_action *action;
+ struct tc_action *action;
#ifdef CONFIG_NET_CLS_IND
- char indev[IFNAMSIZ];
-#endif
-#else
+ char indev[IFNAMSIZ];
+#endif /* CONFIG_NET_CLS_IND */
+#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
struct tcf_police *police;
-#endif
-#endif
+#endif /* CONFIG_NET_CLS_POLICE */
+#endif /* CONFIG_NET_CLS_ACT */
};
static __inline__ int fw_hash(u32 handle)
@@ -158,14 +158,14 @@
head->ht[h] = f->next;
tcf_unbind_filter(tp, &f->res);
#ifdef CONFIG_NET_CLS_ACT
- if (f->action) {
- tcf_action_destroy(f->action,TCA_ACT_UNBIND);
- }
-#else
+ if (f->action)
+ tcf_action_destroy(f->action, TCA_ACT_UNBIND);
+#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
- tcf_police_release(f->police,TCA_ACT_UNBIND);
-#endif
-#endif
+ if (f->police)
+ tcf_police_release(f->police, TCA_ACT_UNBIND);
+#endif /* CONFIG_NET_CLS_POLICE */
+#endif /* CONFIG_NET_CLS_ACT */
kfree(f);
}
@@ -189,14 +189,13 @@
tcf_tree_unlock(tp);
tcf_unbind_filter(tp, &f->res);
#ifdef CONFIG_NET_CLS_ACT
- if (f->action) {
- tcf_action_destroy(f->action,TCA_ACT_UNBIND);
- }
-#else
+ if (f->action)
+ tcf_action_destroy(f->action,TCA_ACT_UNBIND);
+#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
tcf_police_release(f->police,TCA_ACT_UNBIND);
-#endif
-#endif
+#endif /* CONFIG_NET_CLS_POLICE */
+#endif /* CONFIG_NET_CLS_ACT */
kfree(f);
return 0;
}
@@ -358,15 +357,15 @@
t->tcm_handle = f->id;
- if (!f->res.classid
+ if (!f->res.classid
#ifdef CONFIG_NET_CLS_ACT
- && !f->action
+ && !f->action
#else
#ifdef CONFIG_NET_CLS_POLICE
- && !f->police
+ && !f->police
#endif
#endif
- )
+ )
return skb->len;
rta = (struct rtattr*)b;
@@ -390,19 +389,19 @@
rta->rta_len = skb->tail - b;
#ifdef CONFIG_NET_CLS_ACT
- if (f->action && f->action->type == TCA_OLD_COMPAT) {
- if (tcf_action_copy_stats(skb,f->action))
- goto rtattr_failure;
- }
-#else
+ if (f->action && f->action->type == TCA_OLD_COMPAT) {
+ if (tcf_action_copy_stats(skb,f->action))
+ goto rtattr_failure;
+ }
+#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
if (f->police) {
if (qdisc_copy_stats(skb, &f->police->stats,
- f->police->stats_lock))
+ f->police->stats_lock))
goto rtattr_failure;
}
-#endif
-#endif
+#endif /* CONFIG_NET_CLS_POLICE */
+#endif /* CONFIG_NET_CLS_ACT */
return skb->len;
rtattr_failure:
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/6] PKT_SCHED: break is not enough to stop walking
2004-10-29 0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
` (4 preceding siblings ...)
2004-10-29 0:25 ` [PATCH 5/6] cls_fw: Whitespace/ifdef fixes Thomas Graf
@ 2004-10-29 0:26 ` Thomas Graf
2004-10-29 10:26 ` [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT Thomas Graf
6 siblings, 0 replies; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 0:26 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
break is not enough to escape from the walking loops, since
multiple encapsulated loops are used to traverse the hash tables.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
diff -Nru linux-2.6.10-rc1-bk7.orig/net/sched/cls_fw.c linux-2.6.10-rc1-bk7/net/sched/cls_fw.c
--- linux-2.6.10-rc1-bk7.orig/net/sched/cls_fw.c 2004-10-29 00:43:38.000000000 +0200
+++ linux-2.6.10-rc1-bk7/net/sched/cls_fw.c 2004-10-29 00:50:29.000000000 +0200
@@ -338,7 +338,7 @@
}
if (arg->fn(tp, (unsigned long)f, arg) < 0) {
arg->stop = 1;
- break;
+ return;
}
arg->count++;
}
diff -Nru linux-2.6.10-rc1-bk7.orig/net/sched/cls_route.c linux-2.6.10-rc1-bk7/net/sched/cls_route.c
--- linux-2.6.10-rc1-bk7.orig/net/sched/cls_route.c 2004-10-28 22:03:29.000000000 +0200
+++ linux-2.6.10-rc1-bk7/net/sched/cls_route.c 2004-10-29 01:31:12.000000000 +0200
@@ -538,7 +538,7 @@
}
if (arg->fn(tp, (unsigned long)f, arg) < 0) {
arg->stop = 1;
- break;
+ return;
}
arg->count++;
}
diff -Nru linux-2.6.10-rc1-bk7.orig/net/sched/cls_rsvp.h linux-2.6.10-rc1-bk7/net/sched/cls_rsvp.h
--- linux-2.6.10-rc1-bk7.orig/net/sched/cls_rsvp.h 2004-10-28 22:03:29.000000000 +0200
+++ linux-2.6.10-rc1-bk7/net/sched/cls_rsvp.h 2004-10-29 01:31:49.000000000 +0200
@@ -601,7 +601,7 @@
}
if (arg->fn(tp, (unsigned long)f, arg) < 0) {
arg->stop = 1;
- break;
+ return;
}
arg->count++;
}
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RESEND 1/6] PKT_SCHED: Add generic classifier routines
2004-10-29 0:22 ` [PATCH 1/6] PKT_SCHED: Add generic classifier routines Thomas Graf
@ 2004-10-29 10:23 ` Thomas Graf
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 10:23 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
CONFIG_NET_CLS_IND was dependent on CONFIG_NET_CLS_ACT in cls_fw.c which
is not correct according to Kconfig and wouldn't make sense. This is a
revised patch which has this dependency removed.
Adds generic routines used by classifier to:
- bind/unbind to classes
- configure action/police/indev
- dump action/police
Signed-off-by: Thomas Graf <tgraf@suug.ch>
--- linux-2.6.10-rc1-bk7.orig/include/net/pkt_cls.h 2004-10-28 22:03:20.000000000 +0200
+++ linux-2.6.10-rc1-bk7/include/net/pkt_cls.h 2004-10-29 12:11:54.000000000 +0200
@@ -3,6 +3,7 @@
#include <linux/pkt_cls.h>
#include <net/sch_generic.h>
+#include <net/act_api.h>
/* Basic packet classifier frontend definitions. */
@@ -41,4 +42,189 @@
return old_cl;
}
+static inline void
+tcf_bind_filter(struct tcf_proto *tp, struct tcf_result *r, unsigned long base)
+{
+ unsigned long cl;
+
+ cl = tp->q->ops->cl_ops->bind_tcf(tp->q, base, r->classid);
+ cl = cls_set_class(tp, &r->class, cl);
+ if (cl)
+ tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
+}
+
+static inline void
+tcf_unbind_filter(struct tcf_proto *tp, struct tcf_result *r)
+{
+ unsigned long cl;
+
+ if ((cl = __cls_set_class(&r->class, 0)) != 0)
+ tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
+}
+
+#ifdef CONFIG_NET_CLS_ACT
+static inline int
+tcf_change_act_police(struct tcf_proto *tp, struct tc_action **action,
+ struct rtattr *act_police_tlv, struct rtattr *rate_tlv)
+{
+ int ret;
+ struct tc_action *act;
+
+ act = kmalloc(sizeof(*act), GFP_KERNEL);
+ if (NULL == act)
+ return -ENOMEM;
+ memset(act, 0, sizeof(*act));
+
+ ret = tcf_action_init_1(act_police_tlv, rate_tlv, act, "police",
+ TCA_ACT_NOREPLACE, TCA_ACT_BIND);
+ if (ret < 0) {
+ tcf_action_destroy(act, TCA_ACT_UNBIND);
+ return ret;
+ }
+
+ act->type = TCA_OLD_COMPAT;
+
+ if (*action) {
+ tcf_tree_lock(tp);
+ act = xchg(action, act);
+ tcf_tree_unlock(tp);
+
+ tcf_action_destroy(act, TCA_ACT_UNBIND);
+ } else
+ *action = act;
+
+ return 0;
+}
+
+static inline int
+tcf_change_act(struct tcf_proto *tp, struct tc_action **action,
+ struct rtattr *act_tlv, struct rtattr *rate_tlv)
+{
+ int ret;
+ struct tc_action *act;
+
+ act = kmalloc(sizeof(*act), GFP_KERNEL);
+ if (NULL == act)
+ return -ENOMEM;
+ memset(act, 0, sizeof(*act));
+
+ ret = tcf_action_init(act_tlv, rate_tlv, act, NULL,
+ TCA_ACT_NOREPLACE, TCA_ACT_BIND);
+ if (ret < 0) {
+ tcf_action_destroy(act, TCA_ACT_UNBIND);
+ return ret;
+ }
+
+ if (*action) {
+ tcf_tree_lock(tp);
+ act = xchg(action, act);
+ tcf_tree_unlock(tp);
+
+ tcf_action_destroy(act, TCA_ACT_UNBIND);
+ } else
+ *action = act;
+
+ return 0;
+}
+
+static inline int
+tcf_dump_act(struct sk_buff *skb, struct tc_action *action,
+ int act_type, int compat_type)
+{
+ /*
+ * again for backward compatible mode - we want
+ * to work with both old and new modes of entering
+ * tc data even if iproute2 was newer - jhs
+ */
+ if (action) {
+ struct rtattr * p_rta = (struct rtattr*) skb->tail;
+
+ if (action->type != TCA_OLD_COMPAT) {
+ RTA_PUT(skb, act_type, 0, NULL);
+ if (tcf_action_dump(skb, action, 0, 0) < 0)
+ goto rtattr_failure;
+ } else {
+ RTA_PUT(skb, compat_type, 0, NULL);
+ if (tcf_action_dump_old(skb, action, 0, 0) < 0)
+ goto rtattr_failure;
+ }
+
+ p_rta->rta_len = skb->tail - (u8*)p_rta;
+ }
+ return 0;
+
+rtattr_failure:
+ return -1;
+}
+#endif /* CONFIG_NET_CLS_ACT */
+
+#ifdef CONFIG_NET_CLS_IND
+static inline int
+tcf_change_indev(struct tcf_proto *tp, char *indev, struct rtattr *indev_tlv)
+{
+ if (RTA_PAYLOAD(indev_tlv) >= IFNAMSIZ) {
+ printk("cls: bad indev name %s\n", (char *) RTA_DATA(indev_tlv));
+ return -EINVAL;
+ }
+
+ memset(indev, 0, IFNAMSIZ);
+ sprintf(indev, "%s", (char *) RTA_DATA(indev_tlv));
+
+ return 0;
+}
+
+static inline int
+tcf_match_indev(struct sk_buff *skb, char *indev)
+{
+ if (0 != indev[0]) {
+ if (NULL == skb->input_dev)
+ return 0;
+ else if (0 != strcmp(indev, skb->input_dev->name))
+ return 0;
+ }
+
+ return 1;
+}
+#endif /* CONFIG_NET_CLS_IND */
+
+#ifdef CONFIG_NET_CLS_POLICE
+static inline int
+tcf_change_police(struct tcf_proto *tp, struct tcf_police **police,
+ struct rtattr *police_tlv, struct rtattr *rate_tlv)
+{
+ struct tcf_police *p = tcf_police_locate(police_tlv, rate_tlv);
+
+ if (*police) {
+ tcf_tree_lock(tp);
+ p = xchg(police, p);
+ tcf_tree_unlock(tp);
+
+ tcf_police_release(p, TCA_ACT_UNBIND);
+ } else
+ *police = p;
+
+ return 0;
+}
+
+static inline int
+tcf_dump_police(struct sk_buff *skb, struct tcf_police *police,
+ int police_type)
+{
+ if (police) {
+ struct rtattr * p_rta = (struct rtattr*) skb->tail;
+
+ RTA_PUT(skb, police_type, 0, NULL);
+
+ if (tcf_police_dump(skb, police) < 0)
+ goto rtattr_failure;
+
+ p_rta->rta_len = skb->tail - (u8*)p_rta;
+ }
+ return 0;
+
+rtattr_failure:
+ return -1;
+}
+#endif /* CONFIG_NET_CLS_POLICE */
+
#endif
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
` (5 preceding siblings ...)
2004-10-29 0:26 ` [PATCH 6/6] PKT_SCHED: break is not enough to stop walking Thomas Graf
@ 2004-10-29 10:26 ` Thomas Graf
2004-10-29 11:39 ` jamal
2004-11-02 0:39 ` David S. Miller
6 siblings, 2 replies; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 10:26 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, hadi
Signed-off-by: Thomas Graf <tgraf@suug.ch>
--- linux-2.6.10-rc1-bk7.orig/net/sched/cls_fw.c 2004-10-29 12:14:07.000000000 +0200
+++ linux-2.6.10-rc1-bk7/net/sched/cls_fw.c 2004-10-29 12:15:27.000000000 +0200
@@ -56,11 +56,11 @@
struct fw_filter *next;
u32 id;
struct tcf_result res;
-#ifdef CONFIG_NET_CLS_ACT
- struct tc_action *action;
#ifdef CONFIG_NET_CLS_IND
char indev[IFNAMSIZ];
#endif /* CONFIG_NET_CLS_IND */
+#ifdef CONFIG_NET_CLS_ACT
+ struct tc_action *action;
#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
struct tcf_police *police;
@@ -88,11 +88,11 @@
for (f=head->ht[fw_hash(id)]; f; f=f->next) {
if (f->id == id) {
*res = f->res;
-#ifdef CONFIG_NET_CLS_ACT
#ifdef CONFIG_NET_CLS_IND
if (!tcf_match_indev(skb, f->indev))
continue;
#endif /* CONFIG_NET_CLS_IND */
+#ifdef CONFIG_NET_CLS_ACT
if (f->action) {
int act_res = tcf_action_exec(skb, f->action, res);
if (act_res >= 0)
@@ -217,6 +217,14 @@
tcf_bind_filter(tp, &f->res, base);
}
+#ifdef CONFIG_NET_CLS_IND
+ if (tb[TCA_FW_INDEV-1]) {
+ err = tcf_change_indev(tp, f->indev, tb[TCA_FW_INDEV-1]);
+ if (err < 0)
+ goto errout;
+ }
+#endif /* CONFIG_NET_CLS_IND */
+
#ifdef CONFIG_NET_CLS_ACT
if (tb[TCA_FW_POLICE-1]) {
err = tcf_change_act_police(tp, &f->action, tb[TCA_FW_POLICE-1],
@@ -231,14 +239,6 @@
if (err < 0)
goto errout;
}
-
-#ifdef CONFIG_NET_CLS_IND
- if (tb[TCA_FW_INDEV-1]) {
- err = tcf_change_indev(tp, f->indev, tb[TCA_FW_INDEV-1]);
- if (err < 0)
- goto errout;
- }
-#endif /* CONFIG_NET_CLS_IND */
#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
if (tb[TCA_FW_POLICE-1]) {
@@ -373,13 +373,13 @@
if (f->res.classid)
RTA_PUT(skb, TCA_FW_CLASSID, 4, &f->res.classid);
-#ifdef CONFIG_NET_CLS_ACT
- if (tcf_dump_act(skb, f->action, TCA_FW_ACT, TCA_FW_POLICE) < 0)
- goto rtattr_failure;
#ifdef CONFIG_NET_CLS_IND
if (strlen(f->indev))
RTA_PUT(skb, TCA_FW_INDEV, IFNAMSIZ, f->indev);
#endif /* CONFIG_NET_CLS_IND */
+#ifdef CONFIG_NET_CLS_ACT
+ if (tcf_dump_act(skb, f->action, TCA_FW_ACT, TCA_FW_POLICE) < 0)
+ goto rtattr_failure;
#else /* CONFIG_NET_CLS_ACT */
#ifdef CONFIG_NET_CLS_POLICE
if (tcf_dump_police(skb, f->police, TCA_FW_POLICE) < 0)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 10:26 ` [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT Thomas Graf
@ 2004-10-29 11:39 ` jamal
2004-10-29 11:53 ` Thomas Graf
2004-11-02 0:39 ` David S. Miller
1 sibling, 1 reply; 17+ messages in thread
From: jamal @ 2004-10-29 11:39 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
Thomas, Can you submit equivalent patches for cls_u32.c?
Dave, these all look fine.
Thomas, I will let Dave suck these in first and test against resulting
bk.
cheers,
jamal
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 11:39 ` jamal
@ 2004-10-29 11:53 ` Thomas Graf
2004-10-29 12:35 ` jamal
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 11:53 UTC (permalink / raw)
To: jamal; +Cc: David S. Miller, netdev
* jamal <1099049980.1024.21.camel@jzny.localdomain> 2004-10-29 07:39
> Thomas, Can you submit equivalent patches for cls_u32.c?
Sure, already done and queued for submission. I will submit them today.
> Thomas, I will let Dave suck these in first and test against resulting
> bk.
OK.
Regarding the generic statistics for actions:
I thought about dumping the most common stats basic,queue,rate_est
in tcf_action_copy_stats and let the action module dump additional
stats in its get_stats implementation. Any objections in creating
struct tca_act_gen with the content of tca_gen and use it to
access a->priv from act_api.c to get my hands on the statistics
and stats_lock?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 11:53 ` Thomas Graf
@ 2004-10-29 12:35 ` jamal
2004-10-29 12:53 ` Thomas Graf
0 siblings, 1 reply; 17+ messages in thread
From: jamal @ 2004-10-29 12:35 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
On Fri, 2004-10-29 at 07:53, Thomas Graf wrote:
> * jamal <1099049980.1024.21.camel@jzny.localdomain> 2004-10-29 07:39
> > Thomas, Can you submit equivalent patches for cls_u32.c?
>
> Sure, already done and queued for submission. I will submit them today.
>
Ok, that will be good because it will help do a one shot test on bk.
> Regarding the generic statistics for actions:
>
> I thought about dumping the most common stats basic,queue,rate_est
> in tcf_action_copy_stats and let the action module dump additional
> stats in its get_stats implementation.
I think the action code seems clean to me as is.
Here are my thoughts:
Most of them just need queue stats (actually at the moment all of them)
so in tca_gen, struct tc_stats stats needs replacement to make sure they
only have a queue stat because it is generic. All actions call
qdisc_copy_stats calls in their stats dumpers. Replace that call with
the magic you have in the new stats dumping scheme. I think the same
lock can be used for all stats in an action (if new ones are added by an
action).
> Any objections in creating
> struct tca_act_gen with the content of tca_gen and use it to
> access a->priv from act_api.c to get my hands on the statistics
> and stats_lock?
Refer to above.
cheers,
jamal
PS:- I hope this is after the bk snapshot with current cleanups you have
is tested?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 12:35 ` jamal
@ 2004-10-29 12:53 ` Thomas Graf
2004-10-29 13:12 ` jamal
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 12:53 UTC (permalink / raw)
To: jamal; +Cc: David S. Miller, netdev
> > Regarding the generic statistics for actions:
> >
> > I thought about dumping the most common stats basic,queue,rate_est
> > in tcf_action_copy_stats and let the action module dump additional
> > stats in its get_stats implementation.
>
> I think the action code seems clean to me as is.
> Here are my thoughts:
> Most of them just need queue stats (actually at the moment all of them)
> so in tca_gen, struct tc_stats stats needs replacement to make sure they
> only have a queue stat because it is generic.
What about the rate estimator and basic stats in pkt_act.h? tca_gen
requires at least basic,queue and rate_est for me to compile.
> All actions call
> qdisc_copy_stats calls in their stats dumpers. Replace that call with
> the magic you have in the new stats dumping scheme.
That's a lot of duplicated code which could be avoided easly. It's
only a cosmetic thing but I don't see any reason for duplicating code :->
So this would be my way of doing it:
tcf_action_copy_stats {
start_copy()
copy_basic_stats()
copy_queue_stats()
copy_rate_est()
call get_stats() and let the action module dump additional stats
finish_copy
}
I'd also suggest to make a new TLV type TCA_ACT_STATS and not reuse
TCA_STATS.
Another issue... do we want the compatibility stuff and provide the
old tc_stats? I'd say no but it's your call.
> PS:- I hope this is after the bk snapshot with current cleanups you have
> is tested?
Sure. Always planning ahead.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 12:53 ` Thomas Graf
@ 2004-10-29 13:12 ` jamal
2004-10-29 14:24 ` Thomas Graf
0 siblings, 1 reply; 17+ messages in thread
From: jamal @ 2004-10-29 13:12 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
On Fri, 2004-10-29 at 08:53, Thomas Graf wrote:
> What about the rate estimator and basic stats in pkt_act.h? tca_gen
> requires at least basic,queue and rate_est for me to compile.
>
Ok. tca_gen sub for current stats.
Also most of them have no use for the estimator. I suppose if you ifdef
in tca_gen, then thats fine.
> > All actions call
> > qdisc_copy_stats calls in their stats dumpers. Replace that call with
> > the magic you have in the new stats dumping scheme.
>
> That's a lot of duplicated code which could be avoided easly. It's
> only a cosmetic thing but I don't see any reason for duplicating code :->
>
> So this would be my way of doing it:
>
> tcf_action_copy_stats {
> start_copy()
> copy_basic_stats()
> copy_queue_stats()
> copy_rate_est()
> call get_stats() and let the action module dump additional stats
> finish_copy
> }
>
Good idea.
> I'd also suggest to make a new TLV type TCA_ACT_STATS and not reuse
> TCA_STATS.
>
Fine by me.
> Another issue... do we want the compatibility stuff and provide the
> old tc_stats? I'd say no but it's your call.
>
no need for backward compat here. We have an opportunity since its not
widely deployed. I wont feel sorry for any apps that somehow depend on
old stats.
cheers,
jamal
PS:- iproute2 patch needed.
PPS:- I hope all the actions i queued to Dave are in ;->
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 13:12 ` jamal
@ 2004-10-29 14:24 ` Thomas Graf
2004-10-29 14:53 ` jamal
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Graf @ 2004-10-29 14:24 UTC (permalink / raw)
To: jamal; +Cc: David S. Miller, netdev
> no need for backward compat here. We have an opportunity since its not
> widely deployed. I wont feel sorry for any apps that somehow depend on
> old stats.
My thoughts as well.
> PS:- iproute2 patch needed.
Should be straight forward if my last patch made it in. Stephen?
> PPS:- I hope all the actions i queued to Dave are in ;->
So far gact, mirred and pedit, any more I need to take care of?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 14:24 ` Thomas Graf
@ 2004-10-29 14:53 ` jamal
0 siblings, 0 replies; 17+ messages in thread
From: jamal @ 2004-10-29 14:53 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
[-- Attachment #1: Type: text/plain, Size: 657 bytes --]
On Fri, 2004-10-29 at 10:24, Thomas Graf wrote:
> > no need for backward compat here. We have an opportunity since its not
> > widely deployed. I wont feel sorry for any apps that somehow depend on
> > old stats.
>
> My thoughts as well.
>
> > PS:- iproute2 patch needed.
>
> Should be straight forward if my last patch made it in. Stephen?
>
> > PPS:- I hope all the actions i queued to Dave are in ;->
>
> So far gact, mirred and pedit, any more I need to take care of?
ipt - You could wait for Dave to push the one he has in
or use the attached one - Depends on Dave but should be fair to update
attached one before he wakes up ;->
cheers,
jamal
[-- Attachment #2: ipt-2610-patch-0 --]
[-- Type: text/plain, Size: 11101 bytes --]
--- /dev/null 2003-09-15 09:40:47.000000000 -0400
+++ b/include/linux/tc_act/tc_ipt.h 2004-10-24 12:31:13.000000000 -0400
@@ -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
--- /dev/null 2003-09-15 09:40:47.000000000 -0400
+++ b/include/net/tc_act/tc_ipt.h 2004-10-24 12:31:13.000000000 -0400
@@ -0,0 +1,16 @@
+#ifndef __NET_TC_IPT_H
+#define __NET_TC_IPT_H
+
+#include <net/pkt_sched.h>
+
+struct ipt_entry_target;
+
+struct tcf_ipt
+{
+ tca_gen(ipt);
+ u32 hook;
+ char *tname;
+ struct ipt_entry_target *t;
+};
+
+#endif
--- /dev/null 2003-09-15 09:40:47.000000000 -0400
+++ b/net/sched/ipt.c 2004-10-24 12:53:41.000000000 -0400
@@ -0,0 +1,392 @@
+/*
+ * net/sched/ipt.c iptables target interface
+ *
+ *TODO: Add other tables. For now we only support the ipv4 table targets
+ *
+ * This program is free software; you can redistribute 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.
+ *
+ * Copyright: Jamal Hadi Salim (2002-4)
+ */
+
+#include <asm/uaccess.h>
+#include <asm/system.h>
+#include <asm/bitops.h>
+#include <linux/config.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/string.h>
+#include <linux/mm.h>
+#include <linux/socket.h>
+#include <linux/sockios.h>
+#include <linux/in.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <linux/rtnetlink.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/proc_fs.h>
+#include <net/sock.h>
+#include <net/pkt_sched.h>
+#include <linux/tc_act/tc_ipt.h>
+#include <net/tc_act/tc_ipt.h>
+
+#include <linux/netfilter_ipv4/ip_tables.h>
+
+/* use generic hash table */
+#define MY_TAB_SIZE 16
+#define MY_TAB_MASK 15
+
+u32 idx_gen;
+static struct tcf_ipt *tcf_ipt_ht[MY_TAB_SIZE];
+/* ipt hash table lock */
+static rwlock_t ipt_lock = RW_LOCK_UNLOCKED;
+
+/* ovewrride the defaults */
+#define tcf_st tcf_ipt
+#define tcf_t_lock ipt_lock
+#define tcf_ht tcf_ipt_ht
+
+#include <net/pkt_act.h>
+
+static inline int
+init_targ(struct tcf_ipt *p)
+{
+ struct ipt_target *target;
+ int ret = 0;
+ struct ipt_entry_target *t = p->t;
+ target = __ipt_find_target_lock(t->u.user.name, &ret);
+
+ if (!target) {
+ printk("init_targ: Failed to find %s\n",
+ t->u.kernel.target->name);
+ return -1;
+ }
+
+ DPRINTK("init_targ: found %s\n", target->name);
+ /* we really need proper ref counting
+ seems to be only needed for modules?? Talk to laforge */
+/* if (target->me)
+ __MOD_INC_USE_COUNT(target->me);
+*/
+ t->u.kernel.target = target;
+
+ __ipt_mutex_up();
+
+ if (t->u.kernel.target->checkentry
+ && !t->u.kernel.target->checkentry(p->tname, NULL, t->data,
+ t->u.target_size
+ - sizeof (*t), p->hook)) {
+/* if (t->u.kernel.target->me)
+ __MOD_DEC_USE_COUNT(t->u.kernel.target->me);
+*/
+ DPRINTK("ip_tables: check failed for `%s'.\n",
+ t->u.kernel.target->name);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+int
+tcf_ipt_init(struct rtattr *rta, struct rtattr *est, struct tc_action *a, int ovr, int bind)
+{
+ struct ipt_entry_target *t;
+ unsigned h;
+ struct rtattr *tb[TCA_IPT_MAX];
+ struct tcf_ipt *p;
+ int ret = 0;
+ u32 index = 0;
+ u32 hook = 0;
+
+ if (NULL == a || NULL == rta ||
+ (rtattr_parse(tb, TCA_IPT_MAX, RTA_DATA(rta), RTA_PAYLOAD(rta)) <
+ 0)) {
+ return -1;
+ }
+
+
+ if (tb[TCA_IPT_INDEX - 1]) {
+ index = *(u32 *) RTA_DATA(tb[TCA_IPT_INDEX - 1]);
+ DPRINTK("ipt index %d\n", index);
+ }
+
+ if (index && (p = tcf_hash_lookup(index)) != NULL) {
+ a->priv = (void *) p;
+ spin_lock(&p->lock);
+ if (bind) {
+ p->bindcnt += 1;
+ p->refcnt += 1;
+ }
+ if (ovr) {
+ goto override;
+ }
+ spin_unlock(&p->lock);
+ return ret;
+ }
+
+ if (NULL == tb[TCA_IPT_TARG - 1] || NULL == tb[TCA_IPT_HOOK - 1]) {
+ return -1;
+ }
+
+ p = kmalloc(sizeof (*p), GFP_KERNEL);
+ if (p == NULL)
+ return -1;
+
+ memset(p, 0, sizeof (*p));
+ p->refcnt = 1;
+ ret = 1;
+ spin_lock_init(&p->lock);
+ p->stats_lock = &p->lock;
+ if (bind)
+ p->bindcnt = 1;
+
+override:
+ hook = *(u32 *) RTA_DATA(tb[TCA_IPT_HOOK - 1]);
+
+ t = (struct ipt_entry_target *) RTA_DATA(tb[TCA_IPT_TARG - 1]);
+
+ p->t = kmalloc(t->u.target_size, GFP_KERNEL);
+ if (p->t == NULL) {
+ if (ovr) {
+ printk("ipt policy messed up \n");
+ spin_unlock(&p->lock);
+ return -1;
+ }
+ kfree(p);
+ return -1;
+ }
+
+ memcpy(p->t, RTA_DATA(tb[TCA_IPT_TARG - 1]), t->u.target_size);
+ DPRINTK(" target NAME %s size %d data[0] %x data[1] %x\n",
+ t->u.user.name, t->u.target_size, t->data[0], t->data[1]);
+
+ p->tname = kmalloc(IFNAMSIZ, GFP_KERNEL);
+
+ if (p->tname == NULL) {
+ if (ovr) {
+ printk("ipt policy messed up 2 \n");
+ spin_unlock(&p->lock);
+ return -1;
+ }
+ kfree(p->t);
+ kfree(p);
+ return -1;
+ } else {
+ int csize = IFNAMSIZ - 1;
+
+ memset(p->tname, 0, IFNAMSIZ);
+ if (tb[TCA_IPT_TABLE - 1]) {
+ if (strlen((char *) RTA_DATA(tb[TCA_IPT_TABLE - 1])) <
+ csize)
+ csize = strlen(RTA_DATA(tb[TCA_IPT_TABLE - 1]));
+ strncpy(p->tname, RTA_DATA(tb[TCA_IPT_TABLE - 1]),
+ csize);
+ DPRINTK("table name %s\n", p->tname);
+ } else {
+ strncpy(p->tname, "mangle", 1 + strlen("mangle"));
+ }
+ }
+
+ if (0 > init_targ(p)) {
+ if (ovr) {
+ printk("ipt policy messed up 2 \n");
+ spin_unlock(&p->lock);
+ return -1;
+ }
+ kfree(p->tname);
+ kfree(p->t);
+ kfree(p);
+ return -1;
+ }
+
+ if (ovr) {
+ spin_unlock(&p->lock);
+ return -1;
+ }
+
+ p->index = index ? : tcf_hash_new_index();
+
+ p->tm.lastuse = jiffies;
+ /*
+ p->tm.expires = jiffies;
+ */
+ p->tm.install = jiffies;
+#ifdef CONFIG_NET_ESTIMATOR
+ if (est) {
+ qdisc_new_estimator(&p->stats, p->stats_lock, est);
+ }
+#endif
+ h = tcf_hash(p->index);
+ write_lock_bh(&ipt_lock);
+ p->next = tcf_ipt_ht[h];
+ tcf_ipt_ht[h] = p;
+ write_unlock_bh(&ipt_lock);
+ a->priv = (void *) p;
+ return ret;
+
+}
+
+int
+tcf_ipt_cleanup(struct tc_action *a, int bind)
+{
+ struct tcf_ipt *p;
+ p = PRIV(a,ipt);
+ if (NULL != p)
+ return tcf_hash_release(p, bind);
+ return 0;
+}
+
+int
+tcf_ipt(struct sk_buff **pskb, struct tc_action *a)
+{
+ int ret = 0, result = 0;
+ struct tcf_ipt *p;
+ struct sk_buff *skb = *pskb;
+
+ p = PRIV(a,ipt);
+
+ if (NULL == p || NULL == skb) {
+ return -1;
+ }
+
+ spin_lock(&p->lock);
+
+ p->tm.lastuse = jiffies;
+ p->stats.bytes += skb->len;
+ p->stats.packets++;
+
+ if (skb_cloned(skb) ) {
+ if (pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
+ return -1;
+ }
+ }
+ /* yes, we have to worry about both in and out dev
+ worry later - danger - this API seems to have changed
+ from earlier kernels */
+
+ ret = p->t->u.kernel.target->target(&skb, skb->dev, NULL,
+ p->hook, p->t->data, (void *)NULL);
+ switch (ret) {
+ case NF_ACCEPT:
+ result = TC_ACT_OK;
+ break;
+ case NF_DROP:
+ result = TC_ACT_SHOT;
+ p->stats.drops++;
+ break;
+ case IPT_CONTINUE:
+ result = TC_ACT_PIPE;
+ break;
+ default:
+ if (net_ratelimit())
+ printk("Bogus netfilter code %d assume ACCEPT\n", ret);
+ result = TC_POLICE_OK;
+ break;
+ }
+ spin_unlock(&p->lock);
+ return result;
+
+}
+
+int
+tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
+{
+ struct ipt_entry_target *t;
+ struct tcf_t tm;
+ struct tc_cnt c;
+ unsigned char *b = skb->tail;
+
+ struct tcf_ipt *p;
+
+ p = PRIV(a,ipt);
+ if (NULL == p) {
+ printk("BUG: tcf_ipt_dump called with NULL params\n");
+ goto rtattr_failure;
+ }
+ /* for simple targets kernel size == user size
+ ** user name = target name
+ ** for foolproof you need to not assume this
+ */
+
+ t = kmalloc(p->t->u.user.target_size, GFP_ATOMIC);
+
+ if (NULL == t)
+ goto rtattr_failure;
+
+ c.bindcnt = p->bindcnt - bind;
+ c.refcnt = p->refcnt - ref;
+ memcpy(t, p->t, p->t->u.user.target_size);
+ strcpy(t->u.user.name, p->t->u.kernel.target->name);
+
+ DPRINTK("\ttcf_ipt_dump tablename %s length %d\n", p->tname,
+ strlen(p->tname));
+ DPRINTK
+ ("\tdump target name %s size %d size user %d data[0] %x data[1] %x\n",
+ p->t->u.kernel.target->name, p->t->u.target_size, p->t->u.user.target_size,
+ p->t->data[0], p->t->data[1]);
+ RTA_PUT(skb, TCA_IPT_TARG, p->t->u.user.target_size, t);
+ RTA_PUT(skb, TCA_IPT_INDEX, 4, &p->index);
+ RTA_PUT(skb, TCA_IPT_HOOK, 4, &p->hook);
+ RTA_PUT(skb, TCA_IPT_CNT, sizeof(struct tc_cnt), &c);
+ RTA_PUT(skb, TCA_IPT_TABLE, IFNAMSIZ, p->tname);
+ tm.install = jiffies - p->tm.install;
+ tm.lastuse = jiffies - p->tm.lastuse;
+ tm.expires = p->tm.expires;
+ RTA_PUT(skb, TCA_IPT_TM, sizeof (tm), &tm);
+ return skb->len;
+
+ rtattr_failure:
+ skb_trim(skb, b - skb->data);
+ return -1;
+}
+
+int
+tcf_ipt_stats(struct sk_buff *skb, struct tc_action *a)
+{
+ struct tcf_ipt *p;
+ p = PRIV(a,ipt);
+ if (NULL != p)
+ return qdisc_copy_stats(skb, &p->stats, p->stats_lock);
+
+ return 1;
+}
+
+struct tc_action_ops act_ipt_ops = {
+ .next = NULL,
+ .kind = "ipt",
+ .type = TCA_ACT_IPT,
+ .capab = TCA_CAP_NONE,
+ .owner = THIS_MODULE,
+ .act = tcf_ipt,
+ .get_stats = tcf_ipt_stats,
+ .dump = tcf_ipt_dump,
+ .cleanup = tcf_ipt_cleanup,
+ .lookup = tcf_hash_search,
+ .init = tcf_ipt_init,
+ .walk = tcf_generic_walker
+};
+
+MODULE_AUTHOR("Jamal Hadi Salim(2002-4)");
+MODULE_DESCRIPTION("Iptables target actions");
+MODULE_LICENSE("GPL");
+
+static int __init
+ipt_init_module(void)
+{
+ return tcf_register_action(&act_ipt_ops);
+}
+
+static void __exit
+ipt_cleanup_module(void)
+{
+ tcf_unregister_action(&act_ipt_ops);
+}
+
+module_init(ipt_init_module);
+module_exit(ipt_cleanup_module);
--- a/net/sched/Makefile 2004-10-24 06:10:45.000000000 -0400
+++ b/net/sched/Makefile 2004-10-24 12:34:16.000000000 -0400
@@ -12,6 +12,7 @@
obj-$(CONFIG_NET_CLS_POLICE) += police.o
obj-$(CONFIG_NET_ACT_GACT) += gact.o
obj-$(CONFIG_NET_ACT_MIRRED) += mirred.o
+obj-$(CONFIG_NET_ACT_IPT) += ipt.o
obj-$(CONFIG_NET_SCH_CBQ) += sch_cbq.o
obj-$(CONFIG_NET_SCH_HTB) += sch_htb.o
obj-$(CONFIG_NET_SCH_HPFQ) += sch_hpfq.o
--- a/net/sched/Kconfig 2004-10-24 06:10:45.000000000 -0400
+++ b/net/sched/Kconfig 2004-10-24 12:33:31.000000000 -0400
@@ -406,3 +406,11 @@
---help---
requires new iproute2
This allows packets to be mirrored or redirected to netdevices
+
+config NET_ACT_IPT
+ tristate ' iptables Actions'
+ depends on NET_CLS_ACT && NETFILTER && IP_NF_IPTABLES
+ ---help---
+ requires new iproute2
+ This allows iptables targets to be used by tc filters
+
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT
2004-10-29 10:26 ` [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT Thomas Graf
2004-10-29 11:39 ` jamal
@ 2004-11-02 0:39 ` David S. Miller
1 sibling, 0 replies; 17+ messages in thread
From: David S. Miller @ 2004-11-02 0:39 UTC (permalink / raw)
To: Thomas Graf; +Cc: netdev, hadi
All 7 (of 6!) patches applied, thanks Thomas :-)
I'll add Jamal's IPT action patch right now too.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2004-11-02 0:39 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-29 0:21 [PATCHSET 0/6] PKT_SCHED: Generic classifier routines / cls_fw cleanup Thomas Graf
2004-10-29 0:22 ` [PATCH 1/6] PKT_SCHED: Add generic classifier routines Thomas Graf
2004-10-29 10:23 ` [RESEND " Thomas Graf
2004-10-29 0:23 ` [PATCH 2/6] cls_fw: Cleanup fw_classify Thomas Graf
2004-10-29 0:24 ` [PATCH 3/6] cls_fw: Use generic routines to configure action/policer Thomas Graf
2004-10-29 0:24 ` [PATCH 4/6] cls_fw: Use generic routines to dump action/policer Thomas Graf
2004-10-29 0:25 ` [PATCH 5/6] cls_fw: Whitespace/ifdef fixes Thomas Graf
2004-10-29 0:26 ` [PATCH 6/6] PKT_SCHED: break is not enough to stop walking Thomas Graf
2004-10-29 10:26 ` [PATCH 7/6] cls_fw: CONFIG_NET_CLS_IND is not dependant on CONFIG_NET_CLS_ACT Thomas Graf
2004-10-29 11:39 ` jamal
2004-10-29 11:53 ` Thomas Graf
2004-10-29 12:35 ` jamal
2004-10-29 12:53 ` Thomas Graf
2004-10-29 13:12 ` jamal
2004-10-29 14:24 ` Thomas Graf
2004-10-29 14:53 ` jamal
2004-11-02 0:39 ` David S. Miller
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).