Frederic Leroy | 7 Nov 15:26 2010

[arptables] rfc: add classify target

Hello,

I ran into an old problem with Linux [1]. I need to touch the
skb->priority field for arp packet in order to map it to vlan cos.

I wrote a new classify target. Patches for both current linux-stable and
arptables cvs are joined.

For example, if you wan't to put arp packets on vlan 100 with priority,
you can do it like this : 

vconfig set_egressmap eth0.100 7 7
arptables -A OUTPUT -o eth0.100 -j classify --set-class 0:7

[1] http://lists.openwall.net/netdev/2007/06/04/71

--

-- 
Frédéric Leroy
Attachment (arptables_classify.patch): text/x-patch, 4198 bytes
Jan Engelhardt | 7 Nov 16:18 2010
Picon

Re: [arptables] rfc: add classify target

On Sunday 2010-11-07 15:26, Frederic Leroy wrote:
>
>I wrote a new classify target. Patches for both current linux-stable and
>arptables cvs are joined.
>
>+++ b/net/ipv4/netfilter/arpt_classify.c
> <at>  <at>  -0,0 +1,41  <at>  <at> 
>+/* module that allows classification of arp packet */
>+#include <linux/module.h>
>+#include <linux/netfilter.h>
>+#include <linux/netfilter/x_tables.h>
>+#include <linux/netfilter_arp/arpt_classify.h>
>+
>+MODULE_LICENSE("GPL");
>+MODULE_AUTHOR("Frederic Leroy <fredo <at> starox.org>");
>+MODULE_DESCRIPTION("arptables arp classify target");
>+
>+static unsigned int
>+target(struct sk_buff *skb, const struct xt_action_param *par)
>+{
>+	const struct arpt_classify *classify = par->targinfo;
>+
>+	skb->priority=classify->priority;
>+
>+	return XT_CONTINUE;
>+}

Why did you not update xt_CLASSIFY instead?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
(Continue reading)

Patrick McHardy | 9 Nov 15:25 2010
Picon

Re: [arptables] rfc: add classify target

Am 07.11.2010 16:18, schrieb Jan Engelhardt:
> On Sunday 2010-11-07 15:26, Frederic Leroy wrote:
>>
>> I wrote a new classify target. Patches for both current linux-stable and
>> arptables cvs are joined.
>>
>> +++ b/net/ipv4/netfilter/arpt_classify.c
>>  <at>  <at>  -0,0 +1,41  <at>  <at> 
>> +/* module that allows classification of arp packet */
>> +#include <linux/module.h>
>> +#include <linux/netfilter.h>
>> +#include <linux/netfilter/x_tables.h>
>> +#include <linux/netfilter_arp/arpt_classify.h>
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Frederic Leroy <fredo <at> starox.org>");
>> +MODULE_DESCRIPTION("arptables arp classify target");
>> +
>> +static unsigned int
>> +target(struct sk_buff *skb, const struct xt_action_param *par)
>> +{
>> +	const struct arpt_classify *classify = par->targinfo;
>> +
>> +	skb->priority=classify->priority;
>> +
>> +	return XT_CONTINUE;
>> +}
> 
> Why did you not update xt_CLASSIFY instead?

(Continue reading)

Frederic Leroy | 9 Nov 17:10 2010

Re: [arptables] rfc: add classify target

On Tue, Nov 09, 2010 at 03:25:26PM +0100, Patrick McHardy wrote:
> Am 07.11.2010 16:18, schrieb Jan Engelhardt:
> > On Sunday 2010-11-07 15:26, Frederic Leroy wrote:
> Actually we already register for NFPROTO_UNSPEC, so simply
> adding a userspace extension should do the job.

Not really, the mangle table for arp seems to be inexistnet.
And hacking and using the filter table  I got arptables complaining about
chain INPUT missing although the command line with -A OUTPUT.

It seems there need a big work on arptables userspace side.

--

-- 
Frédéric Leroy
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patrick McHardy | 9 Nov 17:48 2010
Picon

Re: [arptables] rfc: add classify target

Am 09.11.2010 17:10, schrieb Frederic Leroy:
> On Tue, Nov 09, 2010 at 03:25:26PM +0100, Patrick McHardy wrote:
>> Am 07.11.2010 16:18, schrieb Jan Engelhardt:
>>> On Sunday 2010-11-07 15:26, Frederic Leroy wrote:
>> Actually we already register for NFPROTO_UNSPEC, so simply
>> adding a userspace extension should do the job.
> 
> Not really, the mangle table for arp seems to be inexistnet.
> And hacking and using the filter table  I got arptables complaining about
> chain INPUT missing although the command line with -A OUTPUT.
> 
> It seems there need a big work on arptables userspace side.

Actually there is no technical reason for limiting the CLASSIFY
target to the mangle table. You can simply remove this.

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bart De Schuymer | 9 Nov 18:39 2010
Picon

Re: [arptables] rfc: add classify target

Op 9/11/2010 17:10, Frederic Leroy schreef:
> On Tue, Nov 09, 2010 at 03:25:26PM +0100, Patrick McHardy wrote:
>> Am 07.11.2010 16:18, schrieb Jan Engelhardt:
>>> On Sunday 2010-11-07 15:26, Frederic Leroy wrote:
>> Actually we already register for NFPROTO_UNSPEC, so simply
>> adding a userspace extension should do the job.
> Not really, the mangle table for arp seems to be inexistnet.
> And hacking and using the filter table  I got arptables complaining about
> chain INPUT missing although the command line with -A OUTPUT.
>
> It seems there need a big work on arptables userspace side.
>
I'm not sure why you think this requires a lot of work on the userspace
side. If you get stuck, feel free to post what you already have and I'll
have a look at it. As it seems the kernel functionality is already
there, I'd be glad to submit your userspace patch.

Best regards,
Bart

--

-- 
Bart De Schuymer
www.artinalgorithms.be

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Frederic Leroy | 9 Nov 21:18 2010

Re: [arptables] rfc: add classify target

Hello Bart,

Le Tue, 09 Nov 2010 18:39:18 +0100,
Bart De Schuymer <bdschuym <at> pandora.be> a écrit :

> Op 9/11/2010 17:10, Frederic Leroy schreef:
> > On Tue, Nov 09, 2010 at 03:25:26PM +0100, Patrick McHardy wrote:
> >> Am 07.11.2010 16:18, schrieb Jan Engelhardt:
> >>> On Sunday 2010-11-07 15:26, Frederic Leroy wrote:
> >> Actually we already register for NFPROTO_UNSPEC, so simply
> >> adding a userspace extension should do the job.
> > Not really, the mangle table for arp seems to be inexistnet.
> > And hacking and using the filter table  I got arptables complaining
> > about chain INPUT missing although the command line with -A OUTPUT.
> >
> > It seems there need a big work on arptables userspace side.
> >
> I'm not sure why you think this requires a lot of work on the
> userspace side. If you get stuck, feel free to post what you already
> have and I'll have a look at it. As it seems the kernel functionality
> is already there, I'd be glad to submit your userspace patch.

It may not requires a lot of work to the userspace side, but it doesn't
seem straight for me.
By the way, I joined what I've done for the moment. My free time is
sparse, but I wan't to go until the end :)

+#include <linux/netfilter/xt_CLASSIFY.h>

I have a doubt with this include because arptables have copies of the
(Continue reading)

Jan Engelhardt | 9 Nov 21:28 2010
Picon

Re: [arptables] rfc: add classify target


On Tuesday 2010-11-09 21:18, Frederic Leroy wrote:
>
>For the kernel part,I didn't add modalias command because the
>userspace don't work yet :
>
># ./arptables -A OUTPUT -o eth0 -j CLASSIFY --set-class 0:7
>x_tables: arp_tables: CLASSIFY target: used from hooks INPUT, but only
>usable from FORWARD/OUTPUT/POSTROUTING

Here we have a perfect example of the dentrimentality of code duplication.
Hooray for NF_ARP_* not matching NF_INET_*.

Alas, when I originally coded NFPROTO_UNSPEC wildcard support,
I allowed for same-rev overloading, as in:

static struct xt_target classify_tg_reg[] __read_mostly = {
	{
		.name       = "CLASSIFY",
		.revision   = 0,
		.family     = NFPROTO_UNSPEC,
		.table      = "mangle",
		.hooks      = (1 << NF_INET_LOCAL_OUT) | (1 << NF_INET_FORWARD) |
		              (1 << NF_INET_POST_ROUTING),
		.target     = classify_tg,
		.targetsize = sizeof(struct xt_classify_target_info),
		.me         = THIS_MODULE,
	},
	{
		.name       = "CLASSIFY",
(Continue reading)

Frederic Leroy | 9 Nov 21:34 2010

Re: [arptables] rfc: add classify target

Le Tue, 9 Nov 2010 21:28:09 +0100 (CET),
Jan Engelhardt <jengelh <at> medozas.de> a écrit :

> On Tuesday 2010-11-09 21:18, Frederic Leroy wrote:
> >
> >For the kernel part,I didn't add modalias command because the
> >userspace don't work yet :
> >
> ># ./arptables -A OUTPUT -o eth0 -j CLASSIFY --set-class 0:7
> >x_tables: arp_tables: CLASSIFY target: used from hooks INPUT, but
> >only usable from FORWARD/OUTPUT/POSTROUTING
> 
> Here we have a perfect example of the dentrimentality of code
> duplication. Hooray for NF_ARP_* not matching NF_INET_*.

It was what saying me that it would be a lot of work. Move arptables to
match NF_INET_* 

--

-- 
Frédéric Leroy
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jan Engelhardt | 9 Nov 22:27 2010
Picon

Re: [arptables] rfc: add classify target


On Tuesday 2010-11-09 21:34, Frederic Leroy wrote:
>> On Tuesday 2010-11-09 21:18, Frederic Leroy wrote:
>> >
>> >For the kernel part,I didn't add modalias command because the
>> >userspace don't work yet :
>> >
>> ># ./arptables -A OUTPUT -o eth0 -j CLASSIFY --set-class 0:7
>> >x_tables: arp_tables: CLASSIFY target: used from hooks INPUT, but
>> >only usable from FORWARD/OUTPUT/POSTROUTING
>> 
>> Here we have a perfect example of the dentrimentality of code
>> duplication. Hooray for NF_ARP_* not matching NF_INET_*.
>
>It was what saying me that it would be a lot of work. Move arptables to
>match NF_INET_* 

The actual work is minimal - since you just need to change the values
of the NF_ARP_ constants. The problem is that it is shared with
userspace.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Frederic Leroy | 9 Nov 22:38 2010

Re: [arptables] rfc: add classify target

Le Tue, 9 Nov 2010 22:27:53 +0100 (CET),
Jan Engelhardt <jengelh <at> medozas.de> a écrit :

> 
> On Tuesday 2010-11-09 21:34, Frederic Leroy wrote:
> >> On Tuesday 2010-11-09 21:18, Frederic Leroy wrote:
> >> >
> >> >For the kernel part,I didn't add modalias command because the
> >> >userspace don't work yet :
> >> >
> >> ># ./arptables -A OUTPUT -o eth0 -j CLASSIFY --set-class 0:7
> >> >x_tables: arp_tables: CLASSIFY target: used from hooks INPUT, but
> >> >only usable from FORWARD/OUTPUT/POSTROUTING
> >> 
> >> Here we have a perfect example of the dentrimentality of code
> >> duplication. Hooray for NF_ARP_* not matching NF_INET_*.
> >
> >It was what saying me that it would be a lot of work. Move arptables
> >to match NF_INET_* 
> 
> The actual work is minimal - since you just need to change the values
> of the NF_ARP_ constants. The problem is that it is shared with
> userspace.

If I remember correctly what I've see sunday, there is some arrays of
size : number of NF_ARP*
There is some code with ->hook[NF_xxx]=yyy

Moreover, when I turned debug on, it outputs me warnings with overflow
on these arrays when I switched to NF_INET.
(Continue reading)

Frederic Leroy | 9 Nov 21:51 2010

Re: [arptables] rfc: add classify target

Le Tue, 9 Nov 2010 21:28:09 +0100 (CET),
Jan Engelhardt <jengelh <at> medozas.de> a écrit :

> Alas, when I originally coded NFPROTO_UNSPEC wildcard support,
> I allowed for same-rev overloading, as in:
> 
> static struct xt_target classify_tg_reg[] __read_mostly = {
> 	{
> [...]
> };
> 

Here is a patch against my previous patch with your insights.
I had time to test it. 

--

-- 
Frédéric Leroy
Patrick McHardy | 11 Nov 11:38 2010
Picon

Re: [arptables] rfc: add classify target

On 09.11.2010 21:51, Frederic Leroy wrote:
> Le Tue, 9 Nov 2010 21:28:09 +0100 (CET),
> Jan Engelhardt <jengelh <at> medozas.de> a écrit :
> 
>> Alas, when I originally coded NFPROTO_UNSPEC wildcard support,
>> I allowed for same-rev overloading, as in:
>>
>> static struct xt_target classify_tg_reg[] __read_mostly = {
>> 	{
>> [...]
>> };
>>
> 
> Here is a patch against my previous patch with your insights.
> I had time to test it. 

This seems like the best we can do for now. Does it work as intended?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Frederic Leroy | 11 Nov 12:45 2010

Re: [arptables] rfc: add classify target

Le Thu, 11 Nov 2010 11:38:41 +0100,
Patrick McHardy <kaber <at> trash.net> a écrit :

> On 09.11.2010 21:51, Frederic Leroy wrote:
> > Le Tue, 9 Nov 2010 21:28:09 +0100 (CET),
> > Jan Engelhardt <jengelh <at> medozas.de> a écrit :
> > 
> >> Alas, when I originally coded NFPROTO_UNSPEC wildcard support,
> >> I allowed for same-rev overloading, as in:
> >>
> >> static struct xt_target classify_tg_reg[] __read_mostly = {
> >> 	{
> >> [...]
> >> };
> >>
> > 
> > Here is a patch against my previous patch with your insights.
> > I had time to test it. 
> 
> This seems like the best we can do for now. Does it work as intended?

Yes, it works as intended. 

Nevertheless, I plan to update kernel and arptables to match NF_INET_*.
I should have time to do it for sunday. 

--

-- 
Frédéric Leroy
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
(Continue reading)

Patrick McHardy | 12 Nov 08:49 2010
Picon

Re: [arptables] rfc: add classify target

On 11.11.2010 12:45, Frederic Leroy wrote:
> Le Thu, 11 Nov 2010 11:38:41 +0100,
> Patrick McHardy <kaber <at> trash.net> a écrit :
> 
>> On 09.11.2010 21:51, Frederic Leroy wrote:
>>> Le Tue, 9 Nov 2010 21:28:09 +0100 (CET),
>>> Jan Engelhardt <jengelh <at> medozas.de> a écrit :
>>>
>>>> Alas, when I originally coded NFPROTO_UNSPEC wildcard support,
>>>> I allowed for same-rev overloading, as in:
>>>>
>>>> static struct xt_target classify_tg_reg[] __read_mostly = {
>>>> 	{
>>>> [...]
>>>> };
>>>>
>>>
>>> Here is a patch against my previous patch with your insights.
>>> I had time to test it. 
>>
>> This seems like the best we can do for now. Does it work as intended?
> 
> Yes, it works as intended. 
> 
> Nevertheless, I plan to update kernel and arptables to match NF_INET_*.
> I should have time to do it for sunday. 

You can't change the numerical values, that would break compatibility.
That basically leaves the option of using NF_INET_PRE_ROUTING instead
of NF_ARP_IN etc, which would make things highly confusing :)
(Continue reading)

Frederic Leroy | 13 Nov 16:29 2010

Re: [arptables] rfc: add classify target

Le Fri, 12 Nov 2010 08:49:59 +0100,
Patrick McHardy <kaber <at> trash.net> a écrit :

> On 11.11.2010 12:45, Frederic Leroy wrote:
> > Le Thu, 11 Nov 2010 11:38:41 +0100,
> > Patrick McHardy <kaber <at> trash.net> a écrit :
> 
> You can't change the numerical values, that would break compatibility.
> That basically leaves the option of using NF_INET_PRE_ROUTING instead
> of NF_ARP_IN etc, which would make things highly confusing :)

There is no much use of NF_ARP_* in the google codesearch world, but I
understand the need to not break compatibility.

So I joined my last patches to xt_CLASSIFY and arptables. 

I checked that :
- modules are autoloaded
- it works as intended for marking cos on vlan interface.

--

-- 
Frédéric Leroy
Bart De Schuymer | 14 Nov 16:36 2010
Picon

Re: [arptables] rfc: add classify target

Apart from my comments below, the userspace patch looks ok:
- I would line up the help and man page entries between arptables and
iptables so noone gets confused. Also, the man page entry refers to
set-class-mac.
- In final_check() you should make sure that the priority has been set
(similar to what's done in libxt_CLASSIFY.c).

cheers,
Bart

On 13-11-10 16:29, Frederic Leroy wrote:
> Le Fri, 12 Nov 2010 08:49:59 +0100,
> Patrick McHardy <kaber <at> trash.net> a écrit :
>
>> On 11.11.2010 12:45, Frederic Leroy wrote:
>>> Le Thu, 11 Nov 2010 11:38:41 +0100,
>>> Patrick McHardy <kaber <at> trash.net> a écrit :
>> You can't change the numerical values, that would break compatibility.
>> That basically leaves the option of using NF_INET_PRE_ROUTING instead
>> of NF_ARP_IN etc, which would make things highly confusing :)
> There is no much use of NF_ARP_* in the google codesearch world, but I
> understand the need to not break compatibility.
>
> So I joined my last patches to xt_CLASSIFY and arptables. 
>
> I checked that :
> - modules are autoloaded
> - it works as intended for marking cos on vlan interface.
>

(Continue reading)

Frederic Leroy | 15 Nov 13:32 2010

Re: [arptables] rfc: add classify target

Le Sun, 14 Nov 2010 16:36:18 +0100,
Bart De Schuymer <bdschuym <at> pandora.be> a écrit :

> Apart from my comments below, the userspace patch looks ok:
> - I would line up the help and man page entries between arptables and
> iptables so noone gets confused. Also, the man page entry refers to
> set-class-mac.
> - In final_check() you should make sure that the priority has been set
> (similar to what's done in libxt_CLASSIFY.c).

Here it is, largely borrowed from iptables.

Cheers,

--

-- 
Frédéric Leroy
Bart De Schuymer | 15 Nov 20:31 2010
Picon

Re: [arptables] rfc: add classify target

Applied,

Thanks Frederic.

Bart

On 15-11-10 13:32, Frederic Leroy wrote:
> Le Sun, 14 Nov 2010 16:36:18 +0100,
> Bart De Schuymer<bdschuym <at> pandora.be>  a écrit :
>
>> Apart from my comments below, the userspace patch looks ok:
>> - I would line up the help and man page entries between arptables and
>> iptables so noone gets confused. Also, the man page entry refers to
>> set-class-mac.
>> - In final_check() you should make sure that the priority has been set
>> (similar to what's done in libxt_CLASSIFY.c).
> Here it is, largely borrowed from iptables.
>
> Cheers,
>

--

-- 
Bart De Schuymer
www.artinalgorithms.be

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Patrick McHardy | 15 Nov 11:44 2010
Picon

Re: [arptables] rfc: add classify target

On 13.11.2010 16:29, Frederic Leroy wrote:
> Le Fri, 12 Nov 2010 08:49:59 +0100,
> Patrick McHardy <kaber <at> trash.net> a écrit :
> 
>> On 11.11.2010 12:45, Frederic Leroy wrote:
>>> Le Thu, 11 Nov 2010 11:38:41 +0100,
>>> Patrick McHardy <kaber <at> trash.net> a écrit :
>>
>> You can't change the numerical values, that would break compatibility.
>> That basically leaves the option of using NF_INET_PRE_ROUTING instead
>> of NF_ARP_IN etc, which would make things highly confusing :)
> 
> There is no much use of NF_ARP_* in the google codesearch world, but I
> understand the need to not break compatibility.
> 
> So I joined my last patches to xt_CLASSIFY and arptables. 
> 
> I checked that :
> - modules are autoloaded
> - it works as intended for marking cos on vlan interface.
> 

Thanks. Please add a Signed-off-by: line to your patch and I'll apply
it.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Frederic Leroy | 15 Nov 13:28 2010

[PATCH] netfilter: xtables: add arp support, allow CLASSIFY target on any table


Signed-off-by: Frédéric Leroy <fredo <at> starox.org>
---
 net/netfilter/xt_CLASSIFY.c |   36 ++++++++++++++++++++++++------------
 1 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/xt_CLASSIFY.c b/net/netfilter/xt_CLASSIFY.c
index c2c0e4a..af9c4da 100644
--- a/net/netfilter/xt_CLASSIFY.c
+++ b/net/netfilter/xt_CLASSIFY.c
 <at>  <at>  -19,12 +19,14  <at>  <at> 
 #include <linux/netfilter_ipv6.h>
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_CLASSIFY.h>
+#include <linux/netfilter_arp.h>

 MODULE_AUTHOR("Patrick McHardy <kaber <at> trash.net>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Xtables: Qdisc classification");
 MODULE_ALIAS("ipt_CLASSIFY");
 MODULE_ALIAS("ip6t_CLASSIFY");
+MODULE_ALIAS("arpt_CLASSIFY");

 static unsigned int
 classify_tg(struct sk_buff *skb, const struct xt_action_param *par)
 <at>  <at>  -35,26 +37,36  <at>  <at>  classify_tg(struct sk_buff *skb, const struct xt_action_param *par)
 	return XT_CONTINUE;
 }

-static struct xt_target classify_tg_reg __read_mostly = {
(Continue reading)

Patrick McHardy | 15 Nov 13:59 2010
Picon

Re: [PATCH] netfilter: xtables: add arp support, allow CLASSIFY target on any table

On 15.11.2010 13:28, Frederic Leroy wrote:
> Signed-off-by: Frédéric Leroy <fredo <at> starox.org>

Applied, thanks Frédéric.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Gmane