patch: introduce simple actions

patch: introduce simple actions

[Jamal Hadi Salim]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]


Ive posted this before - dont want it to sit here rotting. 
If theres nothing glaringly wrong with it (Thomas/Patrick?) then Dave
please apply so i can start shooting other patches based on it.

cheers,
jamal 

[-- Attachment #2: p15 --]
[-- Type: text/plain, Size: 4234 bytes --]

--- /dev/null	2004-01-29 13:33:32.773091056 -0500
+++ 2611-rc3+bk3/include/net/act_generic.h	2005-02-06 15:55:36.000000000 -0500
@@ -0,0 +1,135 @@
+/*
+*/
+static inline int tcf_defact_release(struct tcf_defact *p, int bind)
+{
+	int ret = 0;
+	if (p) {
+		if (bind) {
+			p->bindcnt--;
+		}
+		p->refcnt--;
+		if (p->bindcnt <= 0 && p->refcnt <= 0) {
+			kfree(p->defdata);
+			tcf_hash_destroy(p);
+			ret = 1;
+		}
+	}
+	return ret;
+}
+
+static inline int
+alloc_defdata(struct tcf_defact *p, u32 datalen, void *defdata)
+{
+	p->defdata = kmalloc(datalen, GFP_KERNEL);
+	if (p->defdata == NULL)
+		return -ENOMEM;
+	p->datalen = datalen;
+	memcpy(p->defdata, defdata, datalen);
+	return 0;
+}
+
+static inline int
+realloc_defdata(struct tcf_defact *p, u32 datalen, void *defdata)
+{
+	/* safer to be just brute force for now */
+	kfree(p->defdata);
+	return alloc_defdata(p, datalen, defdata);
+}
+
+static inline int
+tcf_defact_init(struct rtattr *rta, struct rtattr *est,
+		struct tc_action *a, int ovr, int bind)
+{
+	struct rtattr *tb[TCA_DEF_MAX];
+	struct tc_defact *parm;
+	struct tcf_defact *p;
+	void *defdata;
+	u32 datalen = 0;
+	int ret = 0;
+
+	if (rta == NULL || rtattr_parse_nested(tb, TCA_DEF_MAX, rta) < 0)
+		return -EINVAL;
+
+	if (tb[TCA_DEF_PARMS - 1] == NULL)
+		return -EINVAL;
+	parm = RTA_DATA(tb[TCA_DEF_PARMS - 1]);
+	defdata = RTA_DATA(tb[TCA_DEF_DATA - 1]);
+	if (defdata == NULL)
+		return -EINVAL;
+
+	datalen = RTA_PAYLOAD(tb[TCA_DEF_DATA - 1]);
+	if (datalen <= 0)
+		return -EINVAL;
+
+	p = tcf_hash_check(parm->index, a, ovr, bind);
+	if (p == NULL) {
+		p = tcf_hash_create(parm->index, est, a, sizeof(*p), ovr, bind);
+		if (p == NULL)
+			return -ENOMEM;
+
+		ret = alloc_defdata(p, datalen, defdata);
+		if (ret < 0) {
+			kfree(p);
+			return ret;
+		}
+		ret = ACT_P_CREATED;
+	} else {
+		if (!ovr) {
+			tcf_defact_release(p, bind);
+			return -EEXIST;
+		}
+		realloc_defdata(p, datalen, defdata);
+	}
+
+	spin_lock_bh(&p->lock);
+	p->action = parm->action;
+	spin_unlock_bh(&p->lock);
+	if (ret == ACT_P_CREATED)
+		tcf_hash_insert(p);
+	return ret;
+}
+
+static inline int tcf_defact_cleanup(struct tc_action *a, int bind)
+{
+	struct tcf_defact *p = PRIV(a, defact);
+
+	if (p != NULL)
+		return tcf_defact_release(p, bind);
+	return 0;
+}
+
+static inline int
+tcf_defact_dump(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
+{
+	unsigned char *b = skb->tail;
+	struct tc_defact opt;
+	struct tcf_defact *p = PRIV(a, defact);
+	struct tcf_t t;
+
+	opt.index = p->index;
+	opt.refcnt = p->refcnt - ref;
+	opt.bindcnt = p->bindcnt - bind;
+	opt.action = p->action;
+	RTA_PUT(skb, TCA_DEF_PARMS, sizeof(opt), &opt);
+	RTA_PUT(skb, TCA_DEF_DATA, p->datalen, p->defdata);
+	t.install = jiffies_to_clock_t(jiffies - p->tm.install);
+	t.lastuse = jiffies_to_clock_t(jiffies - p->tm.lastuse);
+	t.expires = jiffies_to_clock_t(p->tm.expires);
+	RTA_PUT(skb, TCA_DEF_TM, sizeof(t), &t);
+	return skb->len;
+
+rtattr_failure:
+	skb_trim(skb, b - skb->data);
+	return -1;
+}
+
+#define tca_use_default_ops \
+	.dump           =       tcf_defact_dump, \
+	.cleanup        =       tcf_defact_cleanup, \
+	.init           =       tcf_defact_init, \
+	.walk           =       tcf_generic_walker, \
+
+#define tca_use_default_defines(name) \
+	static u32 idx_gen; \
+	static struct tcf_defact *tcf_##name_ht[MY_TAB_SIZE]; \
+	static DEFINE_RWLOCK(##name_lock);
--- /dev/null	2004-01-29 13:33:32.773091056 -0500
+++ 2611-rc3+bk3/include/net/tc_act/tc_defact.h	2005-02-06 15:03:05.000000000 -0500
@@ -0,0 +1,13 @@
+#ifndef __NET_TC_DEF_H
+#define __NET_TC_DEF_H
+
+#include <net/act_api.h>
+
+struct tcf_defact
+{
+	tca_gen(defact);
+	u32     datalen;
+	void    *defdata;
+};
+
+#endif
--- /dev/null	2004-01-29 13:33:32.773091056 -0500
+++ 2611-rc3+bk3/include/linux/tc_act/tc_defact.h	2005-02-06 16:47:20.039209336 -0500
@@ -0,0 +1,21 @@
+#ifndef __LINUX_TC_DEF_H
+#define __LINUX_TC_DEF_H
+
+#include <linux/pkt_cls.h>
+
+struct tc_defact
+{
+	tc_gen;
+};
+                                                                                
+enum
+{
+	TCA_DEF_UNSPEC,
+	TCA_DEF_TM,
+	TCA_DEF_PARMS,
+	TCA_DEF_DATA,
+	__TCA_DEF_MAX
+};
+#define TCA_DEF_MAX (__TCA_DEF_MAX - 1)
+
+#endif

Re: patch: introduce simple actions

[Patrick McHardy]

Jamal Hadi Salim wrote:
> Ive posted this before - dont want it to sit here rotting. 
> If theres nothing glaringly wrong with it (Thomas/Patrick?) then Dave
> please apply so i can start shooting other patches based on it.

Any reasons why all of these need to be inline functions?
One of the next things I wanted to do in this area was
moving all the large inline functions to act_generic.c,
so if possible I would prefer not to put these in a header
file.

Regards
Patrick

Re: patch: introduce simple actions

[Jamal Hadi Salim]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

On Sun, 2005-03-20 at 14:22, Patrick McHardy wrote:
> Jamal Hadi Salim wrote:
> > Ive posted this before - dont want it to sit here rotting. 
> > If theres nothing glaringly wrong with it (Thomas/Patrick?) then Dave
> > please apply so i can start shooting other patches based on it.
> 
> Any reasons why all of these need to be inline functions?


A lot of the code is very common. 
I am essentially sticking some common code as inlines to allow someone
to write some very simple code - probably just the packet processing
function - see attached simple action.
Inlines seem easy to allow this.

> One of the next things I wanted to do in this area was
> moving all the large inline functions to act_generic.c,
> so if possible I would prefer not to put these in a header
> file.
> 

These are small functions. Take a look at the simple action i attached.
If you can give me the same functionality and still move things
de_inlined to act_generic.c i would be fine with it.

cheers,
jamal

[-- Attachment #2: p16 --]
[-- Type: text/plain, Size: 2911 bytes --]

--- /dev/null	2004-01-29 13:33:32.773091056 -0500
+++ 2611-rc3+bk3/net/sched/simple.c	2005-02-06 15:55:57.000000000 -0500
@@ -0,0 +1,107 @@
+/*
+ * net/sched/simp.c	Simple example of an action
+ *
+ *		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.
+ *
+ * Authors:	Jamal Hadi Salim (2005)
+ *
+ */
+
+#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>
+
+#define TCA_ACT_SIMP 22
+
+/* XXX: Hide all these common elements under some macro 
+ * probably
+*/
+#include <linux/tc_act/tc_defact.h>
+#include <net/tc_act/tc_defact.h>
+
+/* use generic hash table with 8 buckets */
+#define MY_TAB_SIZE     8
+#define MY_TAB_MASK     (MY_TAB_SIZE - 1)
+static u32 idx_gen;
+static struct tcf_defact *tcf_simp_ht[MY_TAB_SIZE];
+static DEFINE_RWLOCK(simp_lock);
+
+/* override the defaults */
+#define tcf_st		tcf_defact
+#define tc_st		tc_defact
+#define tcf_t_lock	simp_lock
+#define tcf_ht		tcf_simp_ht
+
+#define CONFIG_NET_ACT_INIT 1
+#include <net/pkt_act.h>
+#include <net/act_generic.h>
+
+static int tcf_simp(struct sk_buff **pskb, struct tc_action *a)
+{
+	struct sk_buff *skb = *pskb;
+	struct tcf_defact *p = PRIV(a, defact);
+
+	spin_lock(&p->lock);
+	p->tm.lastuse = jiffies;
+	p->bstats.bytes += skb->len;
+	p->bstats.packets++;
+
+	/* print policy string followed by _ then packet count 
+	 * Example if this was the 3rd packet and the string was "hello" 
+	 * then it would look like "hello_3" (without quotes) 
+	 **/
+	printk("simple: %s_%d\n", (char *)p->defdata, p->bstats.packets);
+	spin_unlock(&p->lock);
+	return p->action;
+}
+
+static struct tc_action_ops act_simp_ops = {
+	.kind = "simple",
+	.type = TCA_ACT_SIMP,
+	.capab = TCA_CAP_NONE,
+	.owner = THIS_MODULE,
+	.act = tcf_simp,
+	tca_use_default_ops
+};
+
+MODULE_AUTHOR("Jamal Hadi Salim(2005)");
+MODULE_DESCRIPTION("Simple example action");
+MODULE_LICENSE("GPL");
+
+static int __init simp_init_module(void)
+{
+	int ret = tcf_register_action(&act_simp_ops);
+	if (!ret)
+		printk("Simple TC action Loaded\n");
+	return ret;
+}
+
+static void __exit simp_cleanup_module(void)
+{
+	tcf_unregister_action(&act_simp_ops);
+}
+
+module_init(simp_init_module);
+module_exit(simp_cleanup_module);

Re: patch: introduce simple actions

[Thomas Graf]

* Jamal Hadi Salim <1111345551.1095.82.camel@jzny.localdomain> 2005-03-20 14:05
> 
> Ive posted this before - dont want it to sit here rotting. 
> If theres nothing glaringly wrong with it (Thomas/Patrick?) then Dave
> please apply so i can start shooting other patches based on it.

Looks good to me, see two minor comments below.

> +static inline int
> +tcf_defact_init(struct rtattr *rta, struct rtattr *est,
> +		struct tc_action *a, int ovr, int bind)
> +{
> +	struct rtattr *tb[TCA_DEF_MAX];
> +	struct tc_defact *parm;
> +	struct tcf_defact *p;
> +	void *defdata;
> +	u32 datalen = 0;
> +	int ret = 0;
> +
> +	if (rta == NULL || rtattr_parse_nested(tb, TCA_DEF_MAX, rta) < 0)
> +		return -EINVAL;
> +
> +	if (tb[TCA_DEF_PARMS - 1] == NULL)
> +		return -EINVAL;
> +	parm = RTA_DATA(tb[TCA_DEF_PARMS - 1]);
> +	defdata = RTA_DATA(tb[TCA_DEF_DATA - 1]);

Maybe do a size sanity check here for TCA_DEF_PARMS?

> +	if (defdata == NULL)
> +		return -EINVAL;
> +
> +	datalen = RTA_PAYLOAD(tb[TCA_DEF_DATA - 1]);
> --- /dev/null	2004-01-29 13:33:32.773091056 -0500
> +++ 2611-rc3+bk3/include/net/tc_act/tc_defact.h	2005-02-06 15:03:05.000000000 -0500
> @@ -0,0 +1,13 @@
> +#ifndef __NET_TC_DEF_H
> +#define __NET_TC_DEF_H
> +
> +#include <net/act_api.h>
> +
> +struct tcf_defact
> +{
> +	tca_gen(defact);
> +	u32     datalen;
> +	void    *defdata;
> +};
> +
> +#endif
> --- /dev/null	2004-01-29 13:33:32.773091056 -0500
> +++ 2611-rc3+bk3/include/linux/tc_act/tc_defact.h	2005-02-06 16:47:20.039209336 -0500
> @@ -0,0 +1,21 @@
> +#ifndef __LINUX_TC_DEF_H
> +#define __LINUX_TC_DEF_H
> +
> +#include <linux/pkt_cls.h>
> +
> +struct tc_defact
> +{
> +	tc_gen;
> +};

tcf_defact, tc_defact, .. quite easy to get this wrong. Maybe
it would a good idea to rename tcf_defact to tcf_defact_parm?

Re: patch: introduce simple actions

[Patrick McHardy]

Jamal Hadi Salim wrote:
> On Sun, 2005-03-20 at 14:22, Patrick McHardy wrote:
>
>>One of the next things I wanted to do in this area was
>>moving all the large inline functions to act_generic.c,
>>so if possible I would prefer not to put these in a header
>>file.
> 
> These are small functions. Take a look at the simple action i attached.
> If you can give me the same functionality and still move things
> de_inlined to act_generic.c i would be fine with it.

The functions are not inlined anyway, so the question is whether any
#define magic is done as with pkt_act.h, otherwise there is no reason
to have them in a header file. Looking at your patch, it seems
tc_defact.h uses the redefined pkt_act.h functions, so it does need to
be in a header file, but frankly, I find this horrible. The patch
doesn't conflict with cleanup of pkt_act.h, but it means more work when
doing it, so I think this is what should be done first. Especially
since this is only an example, not useful for users, but it will be
possibly used to create more actions that also need to be changed
afterwards.

Regards
Patrick

Re: patch: introduce simple actions

[jamal]

On Sun, 2005-03-20 at 14:44, Thomas Graf wrote:
> * Jamal Hadi Salim <1111345551.1095.82.camel@jzny.localdomain> 2005-03-20 14:05

> > +static inline int
> > +tcf_defact_init(struct rtattr *rta, struct rtattr *est,
> > +		struct tc_action *a, int ovr, int bind)
> > +{
> > +	struct rtattr *tb[TCA_DEF_MAX];
> > +	struct tc_defact *parm;
> > +	struct tcf_defact *p;
> > +	void *defdata;
> > +	u32 datalen = 0;
> > +	int ret = 0;
> > +
> > +	if (rta == NULL || rtattr_parse_nested(tb, TCA_DEF_MAX, rta) < 0)
> > +		return -EINVAL;
> > +
> > +	if (tb[TCA_DEF_PARMS - 1] == NULL)
> > +		return -EINVAL;
> > +	parm = RTA_DATA(tb[TCA_DEF_PARMS - 1]);
> > +	defdata = RTA_DATA(tb[TCA_DEF_DATA - 1]);
> 
> Maybe do a size sanity check here for TCA_DEF_PARMS?
> 

Will do.

> > +
> > +struct tc_defact
> > +{
> > +	tc_gen;
> > +};
> 
> tcf_defact, tc_defact, .. quite easy to get this wrong. Maybe
> it would a good idea to rename tcf_defact to tcf_defact_parm?
> 

sigh. another LinuxWay(tm). Unfortunately this is all over the net/sched
code to imply something thats user specific vs kernel specific. If
you feel strongly about it i will change it - otherwise maybe we can
leave it till some day some brave person will clean up the whole thing?

cheers,
jamal

Re: patch: introduce simple actions

[Jamal Hadi Salim]

On Sun, 2005-03-20 at 14:58, Patrick McHardy wrote:
> Jamal Hadi Salim wrote:
> > On Sun, 2005-03-20 at 14:22, Patrick McHardy wrote:
> >
> >>One of the next things I wanted to do in this area was
> >>moving all the large inline functions to act_generic.c,
> >>so if possible I would prefer not to put these in a header
> >>file.
> > 
> > These are small functions. Take a look at the simple action i attached.
> > If you can give me the same functionality and still move things
> > de_inlined to act_generic.c i would be fine with it.
> 
> The functions are not inlined anyway, so the question is whether any
> #define magic is done as with pkt_act.h, otherwise there is no reason
> to have them in a header file. Looking at your patch, it seems
> tc_defact.h uses the redefined pkt_act.h functions, so it does need to
> be in a header file, but frankly, I find this horrible. The patch
> doesn't conflict with cleanup of pkt_act.h, but it means more work when
> doing it, so I think this is what should be done first. Especially
> since this is only an example, not useful for users, but it will be
> possibly used to create more actions that also need to be changed
> afterwards.

Unfortunately code "augmentation"( as opposed to inheritance - cant
think of a better word) is much easier to do this way. The benefit of it
is the user gets a very simple programming interface - which is the main
reason this was written to begin with. The standard interface exists,
and more versed people or someone wanting to write something complex can
go that path. At some point i would like to document all this stuff.

How about this:
I submit this patch (with Thomas cleanup) and the simple action. I will
hold onto the 4 other actions i already have some of them complete until
you get around to moving things around and then i will redo them to
conform. This gets it off my back and we dont have any miscommunication
of what you are trying to do. sounds fair?

cheers,
jamal

Re: patch: introduce simple actions

[Thomas Graf]

> > > +
> > > +struct tc_defact
> > > +{
> > > +	tc_gen;
> > > +};
> > 
> > tcf_defact, tc_defact, .. quite easy to get this wrong. Maybe
> > it would a good idea to rename tcf_defact to tcf_defact_parm?
> > 
> 
> sigh. another LinuxWay(tm). Unfortunately this is all over the net/sched
> code to imply something thats user specific vs kernel specific. If
> you feel strongly about it i will change it - otherwise maybe we can
> leave it till some day some brave person will clean up the whole thing?

Yes I know and I don't feel that strong about it. I just tend to dislike
it a tiny bit, I do like the struct rta_kern style better. Let's wait for
the brave knight to appear someday.

Re: patch: introduce simple actions

[Thomas Graf]

* Jamal Hadi Salim <1111349853.1093.136.camel@jzny.localdomain> 2005-03-20 15:17
> reason this was written to begin with. The standard interface exists,
> and more versed people or someone wanting to write something complex can
> go that path. At some point i would like to document all this stuff.

Yes, we do have to find some way to document all this in a proper way.
Maybe we can write some _really_ technical reference brought down to
the bare minimum required and extend the LARTC howto with new examples
to cover the practical side.

Re: patch: introduce simple actions

[jamal]

On Sun, 2005-03-20 at 15:31, Thomas Graf wrote:
> * Jamal Hadi Salim <1111349853.1093.136.camel@jzny.localdomain> 2005-03-20 15:17
> > reason this was written to begin with. The standard interface exists,
> > and more versed people or someone wanting to write something complex can
> > go that path. At some point i would like to document all this stuff.
> 
> Yes, we do have to find some way to document all this in a proper way.
> Maybe we can write some _really_ technical reference brought down to
> the bare minimum required and extend the LARTC howto with new examples
> to cover the practical side.

Agreed - this as well as the ematch etc. not sure if the current howto
will be the proper place - but somewhere close to it.

There has been a lot of very interesting new things going in and there
seems to be stability now - Lets talk offline.

cheers,
jamal

PS:- stupid mailer sometimes sticks my work email as posting address.
None of this is not work related at all ;->

Re: patch: introduce simple actions

[Patrick McHardy]

Jamal Hadi Salim wrote:
> Unfortunately code "augmentation"( as opposed to inheritance - cant
> think of a better word) is much easier to do this way. The benefit of it
> is the user gets a very simple programming interface - which is the main
> reason this was written to begin with. The standard interface exists,
> and more versed people or someone wanting to write something complex can
> go that path. At some point i would like to document all this stuff.

I don't think that a simple interface and this cleanup conflict
with each other.

> How about this:
> I submit this patch (with Thomas cleanup) and the simple action. I will
> hold onto the 4 other actions i already have some of them complete until
> you get around to moving things around and then i will redo them to
> conform. This gets it off my back and we dont have any miscommunication
> of what you are trying to do. sounds fair?

Go ahead, it will be a while before I continue with the cleanups.

Regards
Patrick

Re: patch: introduce simple actions

[David S. Miller]

On 20 Mar 2005 15:17:34 -0500
Jamal Hadi Salim <hadi@znyx.com> wrote:

> How about this:
> I submit this patch (with Thomas cleanup) and the simple action.

I'm waiting for this, send whenever it's ready.

You may want to add the standard stuff to that header
file BTW, ala:

/* foo.h: This is the foo interface header.
 *
 * Copyright (C) 2005 Jamal Hadi Salim <hadi@cyberus.ca>
 */

#ifndef _NET_FOO_H
#define _NET_FOO_H
 ...
#endif /* _NET_FOO_H */

Re: patch: introduce simple actions

[Jamal Hadi Salim]

On Tue, 2005-03-22 at 22:58, David S. Miller wrote:
> On 20 Mar 2005 15:17:34 -0500
> Jamal Hadi Salim <hadi@znyx.com> wrote:
> 
> > How about this:
> > I submit this patch (with Thomas cleanup) and the simple action.
> 
> I'm waiting for this, send whenever it's ready.
> 

I will tommorow - need to plonk tonight 

> You may want to add the standard stuff to that header
> file BTW, ala:
> 
> /* foo.h: This is the foo interface header.
>  *
>  * Copyright (C) 2005 Jamal Hadi Salim <hadi@cyberus.ca>
>  */
> 
> #ifndef _NET_FOO_H
> #define _NET_FOO_H
>  ...
> #endif /* _NET_FOO_H */

dang, yes. Thanks - will do.

cheers,
jamal

PATCH: Allow Simple actions WAS(Re: patch: introduce simple actions

[jamal]

[-- Attachment #1: Type: text/plain, Size: 585 bytes --]

On Tue, 2005-03-22 at 23:06, Jamal Hadi Salim wrote:
> On Tue, 2005-03-22 at 22:58, David S. Miller wrote:
> > I'm waiting for this, send whenever it's ready.
> > 
> 
> I will tommorow - need to plonk tonight 


That was some plonk ;->

Anyways, here it is with suggestions incorporated. 
I could probably split it into two patches one for
the base and the other for the simple action. Let me
know if you want me to do this.

description: 
-Allow simple actions
-introduce a simple action example to demonstrate usage

signed off by: Jamal Hadi Salim <hadi@cyberus.ca>

cheers,
jamal


[-- Attachment #2: simp_p --]
[-- Type: text/plain, Size: 8775 bytes --]

--- /dev/null	2005-04-06 08:24:31.814336808 -0400
+++ b/include/net/act_generic.h	2005-04-06 08:51:52.000000000 -0400
@@ -0,0 +1,142 @@
+/*
+ * include/net/act_generic.h
+ *
+*/
+#ifndef ACT_GENERIC_H
+#define ACT_GENERIC_H
+static inline int tcf_defact_release(struct tcf_defact *p, int bind)
+{
+	int ret = 0;
+	if (p) {
+		if (bind) {
+			p->bindcnt--;
+		}
+		p->refcnt--;
+		if (p->bindcnt <= 0 && p->refcnt <= 0) {
+			kfree(p->defdata);
+			tcf_hash_destroy(p);
+			ret = 1;
+		}
+	}
+	return ret;
+}
+
+static inline int
+alloc_defdata(struct tcf_defact *p, u32 datalen, void *defdata)
+{
+	p->defdata = kmalloc(datalen, GFP_KERNEL);
+	if (p->defdata == NULL)
+		return -ENOMEM;
+	p->datalen = datalen;
+	memcpy(p->defdata, defdata, datalen);
+	return 0;
+}
+
+static inline int
+realloc_defdata(struct tcf_defact *p, u32 datalen, void *defdata)
+{
+	/* safer to be just brute force for now */
+	kfree(p->defdata);
+	return alloc_defdata(p, datalen, defdata);
+}
+
+static inline int
+tcf_defact_init(struct rtattr *rta, struct rtattr *est,
+		struct tc_action *a, int ovr, int bind)
+{
+	struct rtattr *tb[TCA_DEF_MAX];
+	struct tc_defact *parm;
+	struct tcf_defact *p;
+	void *defdata;
+	u32 datalen = 0;
+	int ret = 0;
+
+	if (rta == NULL || rtattr_parse_nested(tb, TCA_DEF_MAX, rta) < 0)
+		return -EINVAL;
+
+	if (tb[TCA_DEF_PARMS - 1] == NULL || 
+	    RTA_PAYLOAD(tb[TCA_DEF_PARMS - 1]) < sizeof(*parm))
+		return -EINVAL;
+
+	parm = RTA_DATA(tb[TCA_DEF_PARMS - 1]);
+	defdata = RTA_DATA(tb[TCA_DEF_DATA - 1]);
+	if (defdata == NULL)
+		return -EINVAL;
+
+	datalen = RTA_PAYLOAD(tb[TCA_DEF_DATA - 1]);
+	if (datalen <= 0)
+		return -EINVAL;
+
+	p = tcf_hash_check(parm->index, a, ovr, bind);
+	if (p == NULL) {
+		p = tcf_hash_create(parm->index, est, a, sizeof(*p), ovr, bind);
+		if (p == NULL)
+			return -ENOMEM;
+
+		ret = alloc_defdata(p, datalen, defdata);
+		if (ret < 0) {
+			kfree(p);
+			return ret;
+		}
+		ret = ACT_P_CREATED;
+	} else {
+		if (!ovr) {
+			tcf_defact_release(p, bind);
+			return -EEXIST;
+		}
+		realloc_defdata(p, datalen, defdata);
+	}
+
+	spin_lock_bh(&p->lock);
+	p->action = parm->action;
+	spin_unlock_bh(&p->lock);
+	if (ret == ACT_P_CREATED)
+		tcf_hash_insert(p);
+	return ret;
+}
+
+static inline int tcf_defact_cleanup(struct tc_action *a, int bind)
+{
+	struct tcf_defact *p = PRIV(a, defact);
+
+	if (p != NULL)
+		return tcf_defact_release(p, bind);
+	return 0;
+}
+
+static inline int
+tcf_defact_dump(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
+{
+	unsigned char *b = skb->tail;
+	struct tc_defact opt;
+	struct tcf_defact *p = PRIV(a, defact);
+	struct tcf_t t;
+
+	opt.index = p->index;
+	opt.refcnt = p->refcnt - ref;
+	opt.bindcnt = p->bindcnt - bind;
+	opt.action = p->action;
+	RTA_PUT(skb, TCA_DEF_PARMS, sizeof(opt), &opt);
+	RTA_PUT(skb, TCA_DEF_DATA, p->datalen, p->defdata);
+	t.install = jiffies_to_clock_t(jiffies - p->tm.install);
+	t.lastuse = jiffies_to_clock_t(jiffies - p->tm.lastuse);
+	t.expires = jiffies_to_clock_t(p->tm.expires);
+	RTA_PUT(skb, TCA_DEF_TM, sizeof(t), &t);
+	return skb->len;
+
+rtattr_failure:
+	skb_trim(skb, b - skb->data);
+	return -1;
+}
+
+#define tca_use_default_ops \
+	.dump           =       tcf_defact_dump, \
+	.cleanup        =       tcf_defact_cleanup, \
+	.init           =       tcf_defact_init, \
+	.walk           =       tcf_generic_walker, \
+
+#define tca_use_default_defines(name) \
+	static u32 idx_gen; \
+	static struct tcf_defact *tcf_##name_ht[MY_TAB_SIZE]; \
+	static DEFINE_RWLOCK(##name_lock);
+#endif /* _NET_ACT_GENERIC_H */
--- /dev/null	2005-04-06 08:24:31.814336808 -0400
+++ b/include/net/tc_act/tc_defact.h	2005-04-06 08:38:47.000000000 -0400
@@ -0,0 +1,13 @@
+#ifndef __NET_TC_DEF_H
+#define __NET_TC_DEF_H
+
+#include <net/act_api.h>
+
+struct tcf_defact
+{
+	tca_gen(defact);
+	u32     datalen;
+	void    *defdata;
+};
+
+#endif
--- /dev/null	2005-04-06 08:24:31.814336808 -0400
+++ b/include/linux/tc_act/tc_defact.h	2005-04-06 08:38:47.000000000 -0400
@@ -0,0 +1,21 @@
+#ifndef __LINUX_TC_DEF_H
+#define __LINUX_TC_DEF_H
+
+#include <linux/pkt_cls.h>
+
+struct tc_defact
+{
+	tc_gen;
+};
+                                                                                
+enum
+{
+	TCA_DEF_UNSPEC,
+	TCA_DEF_TM,
+	TCA_DEF_PARMS,
+	TCA_DEF_DATA,
+	__TCA_DEF_MAX
+};
+#define TCA_DEF_MAX (__TCA_DEF_MAX - 1)
+
+#endif
--- /dev/null	2005-04-06 08:24:31.814336808 -0400
+++ b/net/sched/simple.c	2005-04-06 08:53:31.000000000 -0400
@@ -0,0 +1,107 @@
+/*
+ * net/sched/simp.c	Simple example of an action
+ *
+ *		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.
+ *
+ * Authors:	Jamal Hadi Salim (2005)
+ *
+ */
+
+#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>
+
+#define TCA_ACT_SIMP 22
+
+/* XXX: Hide all these common elements under some macro 
+ * probably
+*/
+#include <linux/tc_act/tc_defact.h>
+#include <net/tc_act/tc_defact.h>
+
+/* use generic hash table with 8 buckets */
+#define MY_TAB_SIZE     8
+#define MY_TAB_MASK     (MY_TAB_SIZE - 1)
+static u32 idx_gen;
+static struct tcf_defact *tcf_simp_ht[MY_TAB_SIZE];
+static DEFINE_RWLOCK(simp_lock);
+
+/* override the defaults */
+#define tcf_st		tcf_defact
+#define tc_st		tc_defact
+#define tcf_t_lock	simp_lock
+#define tcf_ht		tcf_simp_ht
+
+#define CONFIG_NET_ACT_INIT 1
+#include <net/pkt_act.h>
+#include <net/act_generic.h>
+
+static int tcf_simp(struct sk_buff **pskb, struct tc_action *a)
+{
+	struct sk_buff *skb = *pskb;
+	struct tcf_defact *p = PRIV(a, defact);
+
+	spin_lock(&p->lock);
+	p->tm.lastuse = jiffies;
+	p->bstats.bytes += skb->len;
+	p->bstats.packets++;
+
+	/* print policy string followed by _ then packet count 
+	 * Example if this was the 3rd packet and the string was "hello" 
+	 * then it would look like "hello_3" (without quotes) 
+	 **/
+	printk("simple: %s_%d\n", (char *)p->defdata, p->bstats.packets);
+	spin_unlock(&p->lock);
+	return p->action;
+}
+
+static struct tc_action_ops act_simp_ops = {
+	.kind = "simple",
+	.type = TCA_ACT_SIMP,
+	.capab = TCA_CAP_NONE,
+	.owner = THIS_MODULE,
+	.act = tcf_simp,
+	tca_use_default_ops
+};
+
+MODULE_AUTHOR("Jamal Hadi Salim(2005)");
+MODULE_DESCRIPTION("Simple example action");
+MODULE_LICENSE("GPL");
+
+static int __init simp_init_module(void)
+{
+	int ret = tcf_register_action(&act_simp_ops);
+	if (!ret)
+		printk("Simple TC action Loaded\n");
+	return ret;
+}
+
+static void __exit simp_cleanup_module(void)
+{
+	tcf_unregister_action(&act_simp_ops);
+}
+
+module_init(simp_init_module);
+module_exit(simp_cleanup_module);
--- b/net/sched/Kconfig	2005/04/06 12:58:03	1.1
+++ b/net/sched/Kconfig	2005/04/06 13:04:03
@@ -506,3 +506,13 @@
 	  Say Y to support traffic policing (bandwidth limits).  Needed for
 	  ingress and egress rate limiting.
 
+config NET_ACT_SIMP
+        tristate "Simple action"
+        depends on NET_CLS_ACT
+        ---help---
+        You must have new iproute2 to use this feature.
+        This adds a very simple action for demonstration purposes
+	The idea is to give action authors a basic example to look at.
+	All this action will do is print on the console the configured
+	policy string followed by _ then packet count.
+
--- b/net/sched/Makefile	2005/04/06 13:10:39	1.1
+++ b/net/sched/Makefile	2005/04/06 13:12:25
@@ -6,13 +6,14 @@
 
 obj-$(CONFIG_NET_SCHED)		+= sch_api.o sch_fifo.o
 obj-$(CONFIG_NET_CLS)		+= cls_api.o
-obj-$(CONFIG_NET_CLS_ACT)       += act_api.o
+obj-$(CONFIG_NET_CLS_ACT)	+= act_api.o
 obj-$(CONFIG_NET_ACT_POLICE)	+= police.o
 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_ACT_PEDIT)     += pedit.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_ACT_PEDIT)	+= pedit.o
+obj-$(CONFIG_NET_ACT_SIMP)	+= simple.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

Re: PATCH: Allow Simple actions WAS(Re: patch: introduce simple actions

[David S. Miller]

On 06 Apr 2005 09:29:02 -0400
jamal <hadi@cyberus.ca> wrote:

> Anyways, here it is with suggestions incorporated. 
> I could probably split it into two patches one for
> the base and the other for the simple action. Let me
> know if you want me to do this.
> 
> description: 
> -Allow simple actions
> -introduce a simple action example to demonstrate usage
> 
> signed off by: Jamal Hadi Salim <hadi@cyberus.ca>

Applied, thanks Jamal.