Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-12 Thread Patrick Schaaf
On Monday 12 January 2015 17:22:57 Patrick McHardy wrote:
> On 12.01, Patrick Schaaf wrote:
> >
> > Interfaces come and go through many different actions. There's the admin
> > downing and upping stuff like bridges or bonds. There's stuff like libvirt
> > / KVM / qemu creating and destroying interfaces. In all these cases, in
> > my practise, I give the interfaces useful names to that I can
> > prefix-match them in iptables rules.
> > 
> > Dynamically modifying the ruleset for each such creation and destruction,
> > would be a huge burden. The base ruleset would need suitable "hooks" where
> > these rules were inserted (ordering matters!). The addition would hardly
> > be
> > atomic (with traditional iptables, unless done by generating a whole new
> > ruleset and restoring). The programs (e.g. libvirt) would need to be able
> > to call out to these specially crafted rule generator scripts. The admin
> > would need to add them as pre/post actions to their static (manual)
> > interface configuration. Loading and looking at the ruleset before
> > bringing up the interface would be impossible.
> 
> devgroups seem like the best solution for this.

Could be, technically.

Is there devgroup support in libvirt, ifcfg, whatever other distros use for 
their static interface configuration? Or, do I again have to write pre/post 
scripts to set devgroups? Wouldn't bother me too much nowadays, I've automated 
that for ifcfg style stuff in my production environment a year ago, but it's 
something an admin must actively manage...

There is other stuff, apart from libvirt, that creates and destroys interfaces 
on the fly. From my production environment, there's at least keepalived, which 
creates macvlan interfaces on the fly for VRRP VMAC support. I can configure 
the name for that, but nothing else, nor can I call a script pre/post for 
that. And my iptables rules on that boxen _do_ match specially on these 
interfaces.

Gooling a bit around does not immediately turn up any good documentation on it 
at all (four year old iproute2 commits, once I give that as a search term 
too?). Looks very sketchy (although the fundamental idea is clear to me. I'm 
looking through the normal admin practise lens)

best regards
  Patrick
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-12 Thread Patrick McHardy
On 12.01, Patrick Schaaf wrote:
> On Monday 12 January 2015 08:51:54 Eric Dumazet wrote:
> > On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
> > > 
> > > Not to comment on the ifalias thing, which I think is unneccessary,
> > > too, but matching on interface names instead of only ifindex, is
> > > definitely needed, so that one can establish a full ruleset before
> > > interfaces even exist. That's good practise at boottime, but also
> > > needed for dynamic interface creation during runtime.
> > 
> > Please do not send html messages : Your reply did not reach the lists.
> 
> Sigh. Sorry...
> 
> > Then, all you mention could have been solved by proper userspace
> > support.
> > 
> > Every time you add an interface or change device name, you could change
> > firewalls rules if needed. Nothing shocking here.
> 
> That is totally impractical, IMO.
> 
> Interfaces come and go through many different actions. There's the admin 
> downing and upping stuff like bridges or bonds. There's stuff like libvirt / 
> KVM / qemu creating and destroying interfaces. In all these cases, in my 
> practise, I give the interfaces useful names to that I can prefix-match them 
> in iptables rules.
> 
> Dynamically modifying the ruleset for each such creation and destruction, 
> would be a huge burden. The base ruleset would need suitable "hooks" where 
> these rules were inserted (ordering matters!). The addition would hardly be 
> atomic (with traditional iptables, unless done by generating a whole new 
> ruleset and restoring). The programs (e.g. libvirt) would need to be able to 
> call out to these specially crafted rule generator scripts. The admin would 
> need to add them as pre/post actions to their static (manual) interface 
> configuration. Loading and looking at the ruleset before bringing up the 
> interface would be impossible.

devgroups seem like the best solution for this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-12 Thread Patrick Schaaf
On Monday 12 January 2015 08:51:54 Eric Dumazet wrote:
> On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
> > 
> > Not to comment on the ifalias thing, which I think is unneccessary,
> > too, but matching on interface names instead of only ifindex, is
> > definitely needed, so that one can establish a full ruleset before
> > interfaces even exist. That's good practise at boottime, but also
> > needed for dynamic interface creation during runtime.
> 
> Please do not send html messages : Your reply did not reach the lists.

Sigh. Sorry...

> Then, all you mention could have been solved by proper userspace
> support.
> 
> Every time you add an interface or change device name, you could change
> firewalls rules if needed. Nothing shocking here.

That is totally impractical, IMO.

Interfaces come and go through many different actions. There's the admin 
downing and upping stuff like bridges or bonds. There's stuff like libvirt / 
KVM / qemu creating and destroying interfaces. In all these cases, in my 
practise, I give the interfaces useful names to that I can prefix-match them 
in iptables rules.

Dynamically modifying the ruleset for each such creation and destruction, 
would be a huge burden. The base ruleset would need suitable "hooks" where 
these rules were inserted (ordering matters!). The addition would hardly be 
atomic (with traditional iptables, unless done by generating a whole new 
ruleset and restoring). The programs (e.g. libvirt) would need to be able to 
call out to these specially crafted rule generator scripts. The admin would 
need to add them as pre/post actions to their static (manual) interface 
configuration. Loading and looking at the ruleset before bringing up the 
interface would be impossible.

Note that I do fully agree that it's sad that iptables rules waste all that 
memory for each and every rule! I remember musing about improving that in 
talks with Harald Welte back in the 90ies. A simple match would be perfectly 
fine for me. Only having ifindex support, isn't.

best regards
  Patrick
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-12 Thread Eric Dumazet
On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
> > iptables should have used ifindex, its sad we allowed the substring
> 
> > match in first place.
> 
>  
> 
> Not to comment on the ifalias thing, which I think is unneccessary,
> too, but matching on interface names instead of only ifindex, is
> definitely needed, so that one can establish a full ruleset before
> interfaces even exist. That's good practise at boottime, but also
> needed for dynamic interface creation during runtime.
> 
>  
> 
> A pure ifindex-during-packet-inspection approach might still work, but
> the ruleset must IMO keep the interface names. Maybe register them in
> a hash, keyed by name, with values an ifindex or ifindex set (for
> wildcard names), plus a reverse mapping from active ifindices to all
> places in these hash values where an ifindex has been remembered. On
> interface creation / destruction that structure could then be updated,
> and active packet filtering rules would refer to (and keep a refcount
> on) specific hash elements.
> 
Please do not send html messages : Your reply did not reach the lists.

Then, all you mention could have been solved by proper userspace
support.

Every time you add an interface or change device name, you could change
firewalls rules if needed. Nothing shocking here.

The ruleset can indeed mention interface names, but the kernel part
really should not care about names, which are a 'human' convenient way
to represent things.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-12 Thread Eric Dumazet
On Mon, 2015-01-12 at 17:32 +0100, Jan Engelhardt wrote:
> On Monday 2015-01-12 17:04, Eric Dumazet wrote:
> >
> >iptables should have used ifindex [for interface matching],
> >it[']s sad we allowed the substring match in first place.
> 
> How would you solve interface name wildcards with ifindices?
> (They come in handy if you have something like lots of tun+/veth+
> interfaces from openvpn/lxc.)


This is what I said : "it[']s sad we allowed the substring match in
first place."

This obviously referred to wildcards, in the in/out interface match for
every _single_ rule, consuming 64 bytes of memory per rule and per cpu !
Which is absolutely crazy in term of memory usage.

Matching tun+ or whatever could easily be done by a match (-m ...),
because you can factorize this quite easily (called once for a group of
rules)



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-12 Thread Jan Engelhardt

On Monday 2015-01-12 17:04, Eric Dumazet wrote:
>
>iptables should have used ifindex [for interface matching],
>it[']s sad we allowed the substring match in first place.

How would you solve interface name wildcards with ifindices?
(They come in handy if you have something like lots of tun+/veth+
interfaces from openvpn/lxc.)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-12 Thread Richard Weinberger
Am 12.01.2015 um 17:04 schrieb Eric Dumazet:
> On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
>> Signed-off-by: Richard Weinberger 
>> ---
>>  include/linux/netfilter/x_tables.h | 22 ++
>>  net/ipv4/netfilter/arp_tables.c| 28 +---
>>  net/ipv4/netfilter/ip_tables.c | 15 +--
>>  net/ipv6/netfilter/ip6_tables.c| 18 +++---
>>  net/netfilter/xt_physdev.c |  9 ++---
>>  5 files changed, 53 insertions(+), 39 deletions(-)
> 
> Richard, I dislike this, sorry.

That's fine, the series carries the "RFC" burning mark for a reason.

> iptables is already horribly expensive, you add another expensive step
> for every rule.

Yeah, you mean the extra unlikey() check?

> device aliasing can be done from user space.

How?

I did this series because I'm not so happy with the device renaming what udev
does.
The idea was to offer udev a better kernel interface to deal with aliases.
Such that one can use the regular names form the kernel and the predictable
names generated from udev.

For block devices it was easy, we have the good old symlink.
For network interface the kernel does not offer an API.

> iptables should have used ifindex, its sad we allowed the substring
> match in first place.

Maybe nftables can do better. :-)

Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-12 Thread Eric Dumazet
On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
> Signed-off-by: Richard Weinberger 
> ---
>  include/linux/netfilter/x_tables.h | 22 ++
>  net/ipv4/netfilter/arp_tables.c| 28 +---
>  net/ipv4/netfilter/ip_tables.c | 15 +--
>  net/ipv6/netfilter/ip6_tables.c| 18 +++---
>  net/netfilter/xt_physdev.c |  9 ++---
>  5 files changed, 53 insertions(+), 39 deletions(-)

Richard, I dislike this, sorry.

iptables is already horribly expensive, you add another expensive step
for every rule.

device aliasing can be done from user space.

iptables should have used ifindex, its sad we allowed the substring
match in first place.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-12 Thread Richard Weinberger
Am 12.01.2015 um 17:04 schrieb Eric Dumazet:
 On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
 Signed-off-by: Richard Weinberger rich...@nod.at
 ---
  include/linux/netfilter/x_tables.h | 22 ++
  net/ipv4/netfilter/arp_tables.c| 28 +---
  net/ipv4/netfilter/ip_tables.c | 15 +--
  net/ipv6/netfilter/ip6_tables.c| 18 +++---
  net/netfilter/xt_physdev.c |  9 ++---
  5 files changed, 53 insertions(+), 39 deletions(-)
 
 Richard, I dislike this, sorry.

That's fine, the series carries the RFC burning mark for a reason.

 iptables is already horribly expensive, you add another expensive step
 for every rule.

Yeah, you mean the extra unlikey() check?

 device aliasing can be done from user space.

How?

I did this series because I'm not so happy with the device renaming what udev
does.
The idea was to offer udev a better kernel interface to deal with aliases.
Such that one can use the regular names form the kernel and the predictable
names generated from udev.

For block devices it was easy, we have the good old symlink.
For network interface the kernel does not offer an API.

 iptables should have used ifindex, its sad we allowed the substring
 match in first place.

Maybe nftables can do better. :-)

Thanks,
//richard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-12 Thread Jan Engelhardt

On Monday 2015-01-12 17:04, Eric Dumazet wrote:

iptables should have used ifindex [for interface matching],
it[']s sad we allowed the substring match in first place.

How would you solve interface name wildcards with ifindices?
(They come in handy if you have something like lots of tun+/veth+
interfaces from openvpn/lxc.)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-12 Thread Eric Dumazet
On Mon, 2015-01-12 at 17:32 +0100, Jan Engelhardt wrote:
 On Monday 2015-01-12 17:04, Eric Dumazet wrote:
 
 iptables should have used ifindex [for interface matching],
 it[']s sad we allowed the substring match in first place.
 
 How would you solve interface name wildcards with ifindices?
 (They come in handy if you have something like lots of tun+/veth+
 interfaces from openvpn/lxc.)


This is what I said : it[']s sad we allowed the substring match in
first place.

This obviously referred to wildcards, in the in/out interface match for
every _single_ rule, consuming 64 bytes of memory per rule and per cpu !
Which is absolutely crazy in term of memory usage.

Matching tun+ or whatever could easily be done by a match (-m ...),
because you can factorize this quite easily (called once for a group of
rules)



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-12 Thread Eric Dumazet
On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
 Signed-off-by: Richard Weinberger rich...@nod.at
 ---
  include/linux/netfilter/x_tables.h | 22 ++
  net/ipv4/netfilter/arp_tables.c| 28 +---
  net/ipv4/netfilter/ip_tables.c | 15 +--
  net/ipv6/netfilter/ip6_tables.c| 18 +++---
  net/netfilter/xt_physdev.c |  9 ++---
  5 files changed, 53 insertions(+), 39 deletions(-)

Richard, I dislike this, sorry.

iptables is already horribly expensive, you add another expensive step
for every rule.

device aliasing can be done from user space.

iptables should have used ifindex, its sad we allowed the substring
match in first place.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-12 Thread Eric Dumazet
On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
  iptables should have used ifindex, its sad we allowed the substring
 
  match in first place.
 
  
 
 Not to comment on the ifalias thing, which I think is unneccessary,
 too, but matching on interface names instead of only ifindex, is
 definitely needed, so that one can establish a full ruleset before
 interfaces even exist. That's good practise at boottime, but also
 needed for dynamic interface creation during runtime.
 
  
 
 A pure ifindex-during-packet-inspection approach might still work, but
 the ruleset must IMO keep the interface names. Maybe register them in
 a hash, keyed by name, with values an ifindex or ifindex set (for
 wildcard names), plus a reverse mapping from active ifindices to all
 places in these hash values where an ifindex has been remembered. On
 interface creation / destruction that structure could then be updated,
 and active packet filtering rules would refer to (and keep a refcount
 on) specific hash elements.
 
Please do not send html messages : Your reply did not reach the lists.

Then, all you mention could have been solved by proper userspace
support.

Every time you add an interface or change device name, you could change
firewalls rules if needed. Nothing shocking here.

The ruleset can indeed mention interface names, but the kernel part
really should not care about names, which are a 'human' convenient way
to represent things.



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-12 Thread Patrick McHardy
On 12.01, Patrick Schaaf wrote:
 On Monday 12 January 2015 08:51:54 Eric Dumazet wrote:
  On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
   
   Not to comment on the ifalias thing, which I think is unneccessary,
   too, but matching on interface names instead of only ifindex, is
   definitely needed, so that one can establish a full ruleset before
   interfaces even exist. That's good practise at boottime, but also
   needed for dynamic interface creation during runtime.
  
  Please do not send html messages : Your reply did not reach the lists.
 
 Sigh. Sorry...
 
  Then, all you mention could have been solved by proper userspace
  support.
  
  Every time you add an interface or change device name, you could change
  firewalls rules if needed. Nothing shocking here.
 
 That is totally impractical, IMO.
 
 Interfaces come and go through many different actions. There's the admin 
 downing and upping stuff like bridges or bonds. There's stuff like libvirt / 
 KVM / qemu creating and destroying interfaces. In all these cases, in my 
 practise, I give the interfaces useful names to that I can prefix-match them 
 in iptables rules.
 
 Dynamically modifying the ruleset for each such creation and destruction, 
 would be a huge burden. The base ruleset would need suitable hooks where 
 these rules were inserted (ordering matters!). The addition would hardly be 
 atomic (with traditional iptables, unless done by generating a whole new 
 ruleset and restoring). The programs (e.g. libvirt) would need to be able to 
 call out to these specially crafted rule generator scripts. The admin would 
 need to add them as pre/post actions to their static (manual) interface 
 configuration. Loading and looking at the ruleset before bringing up the 
 interface would be impossible.

devgroups seem like the best solution for this.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-12 Thread Patrick Schaaf
On Monday 12 January 2015 17:22:57 Patrick McHardy wrote:
 On 12.01, Patrick Schaaf wrote:
 
  Interfaces come and go through many different actions. There's the admin
  downing and upping stuff like bridges or bonds. There's stuff like libvirt
  / KVM / qemu creating and destroying interfaces. In all these cases, in
  my practise, I give the interfaces useful names to that I can
  prefix-match them in iptables rules.
  
  Dynamically modifying the ruleset for each such creation and destruction,
  would be a huge burden. The base ruleset would need suitable hooks where
  these rules were inserted (ordering matters!). The addition would hardly
  be
  atomic (with traditional iptables, unless done by generating a whole new
  ruleset and restoring). The programs (e.g. libvirt) would need to be able
  to call out to these specially crafted rule generator scripts. The admin
  would need to add them as pre/post actions to their static (manual)
  interface configuration. Loading and looking at the ruleset before
  bringing up the interface would be impossible.
 
 devgroups seem like the best solution for this.

Could be, technically.

Is there devgroup support in libvirt, ifcfg, whatever other distros use for 
their static interface configuration? Or, do I again have to write pre/post 
scripts to set devgroups? Wouldn't bother me too much nowadays, I've automated 
that for ifcfg style stuff in my production environment a year ago, but it's 
something an admin must actively manage...

There is other stuff, apart from libvirt, that creates and destroys interfaces 
on the fly. From my production environment, there's at least keepalived, which 
creates macvlan interfaces on the fly for VRRP VMAC support. I can configure 
the name for that, but nothing else, nor can I call a script pre/post for 
that. And my iptables rules on that boxen _do_ match specially on these 
interfaces.

Gooling a bit around does not immediately turn up any good documentation on it 
at all (four year old iproute2 commits, once I give that as a search term 
too?). Looks very sketchy (although the fundamental idea is clear to me. I'm 
looking through the normal admin practise lens)

best regards
  Patrick
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-12 Thread Patrick Schaaf
On Monday 12 January 2015 08:51:54 Eric Dumazet wrote:
 On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
  
  Not to comment on the ifalias thing, which I think is unneccessary,
  too, but matching on interface names instead of only ifindex, is
  definitely needed, so that one can establish a full ruleset before
  interfaces even exist. That's good practise at boottime, but also
  needed for dynamic interface creation during runtime.
 
 Please do not send html messages : Your reply did not reach the lists.

Sigh. Sorry...

 Then, all you mention could have been solved by proper userspace
 support.
 
 Every time you add an interface or change device name, you could change
 firewalls rules if needed. Nothing shocking here.

That is totally impractical, IMO.

Interfaces come and go through many different actions. There's the admin 
downing and upping stuff like bridges or bonds. There's stuff like libvirt / 
KVM / qemu creating and destroying interfaces. In all these cases, in my 
practise, I give the interfaces useful names to that I can prefix-match them 
in iptables rules.

Dynamically modifying the ruleset for each such creation and destruction, 
would be a huge burden. The base ruleset would need suitable hooks where 
these rules were inserted (ordering matters!). The addition would hardly be 
atomic (with traditional iptables, unless done by generating a whole new 
ruleset and restoring). The programs (e.g. libvirt) would need to be able to 
call out to these specially crafted rule generator scripts. The admin would 
need to add them as pre/post actions to their static (manual) interface 
configuration. Loading and looking at the ruleset before bringing up the 
interface would be impossible.

Note that I do fully agree that it's sad that iptables rules waste all that 
memory for each and every rule! I remember musing about improving that in 
talks with Harald Welte back in the 90ies. A simple match would be perfectly 
fine for me. Only having ifindex support, isn't.

best regards
  Patrick
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

2015-01-11 Thread Richard Weinberger
Signed-off-by: Richard Weinberger 
---
 include/linux/netfilter/x_tables.h | 22 ++
 net/ipv4/netfilter/arp_tables.c| 28 +---
 net/ipv4/netfilter/ip_tables.c | 15 +--
 net/ipv6/netfilter/ip6_tables.c| 18 +++---
 net/netfilter/xt_physdev.c |  9 ++---
 5 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h 
b/include/linux/netfilter/x_tables.h
index a3e215b..15bda23 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -351,6 +351,28 @@ static inline unsigned long ifname_compare_aligned(const 
char *_a,
return ret;
 }
 
+/*
+ * A wrapper around ifname_compare_aligned() to match against dev->name and
+ * dev->ifalias.
+ */
+static inline unsigned long ifname_compare_all(const struct net_device *dev,
+  const char *name,
+  const char *mask)
+{
+   unsigned long res = 0;
+
+   if (!dev)
+   goto out;
+
+   res = ifname_compare_aligned(dev->name, name, mask);
+   if (unlikely(dev->ifalias && res))
+   res = ifname_compare_aligned(dev->ifalias, name, mask);
+
+out:
+   return res;
+}
+
+
 struct nf_hook_ops *xt_hook_link(const struct xt_table *, nf_hookfn *);
 void xt_hook_unlink(const struct xt_table *, struct nf_hook_ops *);
 
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f95b6f9..457d4ed 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -81,19 +81,30 @@ static inline int arp_devaddr_compare(const struct 
arpt_devaddr_info *ap,
  * Some arches dont care, unrolling the loop is a win on them.
  * For other arches, we only have a 16bit alignement.
  */
-static unsigned long ifname_compare(const char *_a, const char *_b, const char 
*_mask)
+static unsigned long ifname_compare(const struct net_device *dev,
+   const char *_b, const char *_mask)
 {
 #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-   unsigned long ret = ifname_compare_aligned(_a, _b, _mask);
+   unsigned long ret = ifname_compare_all(dev, _b, _mask);
 #else
unsigned long ret = 0;
-   const u16 *a = (const u16 *)_a;
+   const u16 *a = (const u16 *)dev->name;
const u16 *b = (const u16 *)_b;
const u16 *mask = (const u16 *)_mask;
int i;
 
for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
ret |= (a[i] ^ b[i]) & mask[i];
+
+   if (likely(!(dev->ifalias && ret)))
+   goto out;
+
+   ret = 0;
+   a = (const u16 *)dev->ifalias;
+   for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
+   ret |= (a[i] ^ b[i]) & mask[i];
+
+out:
 #endif
return ret;
 }
@@ -101,8 +112,8 @@ static unsigned long ifname_compare(const char *_a, const 
char *_b, const char *
 /* Returns whether packet matches rule or not. */
 static inline int arp_packet_match(const struct arphdr *arphdr,
   struct net_device *dev,
-  const char *indev,
-  const char *outdev,
+  const struct net_device *indev,
+  const struct net_device *outdev,
   const struct arpt_arp *arpinfo)
 {
const char *arpptr = (char *)(arphdr + 1);
@@ -252,11 +263,9 @@ unsigned int arpt_do_table(struct sk_buff *skb,
   const struct net_device *out,
   struct xt_table *table)
 {
-   static const char nulldevname[IFNAMSIZ] 
__attribute__((aligned(sizeof(long;
unsigned int verdict = NF_DROP;
const struct arphdr *arp;
struct arpt_entry *e, *back;
-   const char *indev, *outdev;
void *table_base;
const struct xt_table_info *private;
struct xt_action_param acpar;
@@ -265,9 +274,6 @@ unsigned int arpt_do_table(struct sk_buff *skb,
if (!pskb_may_pull(skb, arp_hdr_len(skb->dev)))
return NF_DROP;
 
-   indev = in ? in->name : nulldevname;
-   outdev = out ? out->name : nulldevname;
-
local_bh_disable();
addend = xt_write_recseq_begin();
private = table->private;
@@ -291,7 +297,7 @@ unsigned int arpt_do_table(struct sk_buff *skb,
do {
const struct xt_entry_target *t;
 
-   if (!arp_packet_match(arp, skb->dev, indev, outdev, >arp)) {
+   if (!arp_packet_match(arp, skb->dev, in, out, >arp)) {
e = arpt_next_entry(e);
continue;
}
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 99e810f..87df9ef 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -73,8 +73,8 @@ 

[PATCH 2/3] x_tables: Use also dev-ifalias for interface matching

2015-01-11 Thread Richard Weinberger
Signed-off-by: Richard Weinberger rich...@nod.at
---
 include/linux/netfilter/x_tables.h | 22 ++
 net/ipv4/netfilter/arp_tables.c| 28 +---
 net/ipv4/netfilter/ip_tables.c | 15 +--
 net/ipv6/netfilter/ip6_tables.c| 18 +++---
 net/netfilter/xt_physdev.c |  9 ++---
 5 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h 
b/include/linux/netfilter/x_tables.h
index a3e215b..15bda23 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -351,6 +351,28 @@ static inline unsigned long ifname_compare_aligned(const 
char *_a,
return ret;
 }
 
+/*
+ * A wrapper around ifname_compare_aligned() to match against dev-name and
+ * dev-ifalias.
+ */
+static inline unsigned long ifname_compare_all(const struct net_device *dev,
+  const char *name,
+  const char *mask)
+{
+   unsigned long res = 0;
+
+   if (!dev)
+   goto out;
+
+   res = ifname_compare_aligned(dev-name, name, mask);
+   if (unlikely(dev-ifalias  res))
+   res = ifname_compare_aligned(dev-ifalias, name, mask);
+
+out:
+   return res;
+}
+
+
 struct nf_hook_ops *xt_hook_link(const struct xt_table *, nf_hookfn *);
 void xt_hook_unlink(const struct xt_table *, struct nf_hook_ops *);
 
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f95b6f9..457d4ed 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -81,19 +81,30 @@ static inline int arp_devaddr_compare(const struct 
arpt_devaddr_info *ap,
  * Some arches dont care, unrolling the loop is a win on them.
  * For other arches, we only have a 16bit alignement.
  */
-static unsigned long ifname_compare(const char *_a, const char *_b, const char 
*_mask)
+static unsigned long ifname_compare(const struct net_device *dev,
+   const char *_b, const char *_mask)
 {
 #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-   unsigned long ret = ifname_compare_aligned(_a, _b, _mask);
+   unsigned long ret = ifname_compare_all(dev, _b, _mask);
 #else
unsigned long ret = 0;
-   const u16 *a = (const u16 *)_a;
+   const u16 *a = (const u16 *)dev-name;
const u16 *b = (const u16 *)_b;
const u16 *mask = (const u16 *)_mask;
int i;
 
for (i = 0; i  IFNAMSIZ/sizeof(u16); i++)
ret |= (a[i] ^ b[i])  mask[i];
+
+   if (likely(!(dev-ifalias  ret)))
+   goto out;
+
+   ret = 0;
+   a = (const u16 *)dev-ifalias;
+   for (i = 0; i  IFNAMSIZ/sizeof(u16); i++)
+   ret |= (a[i] ^ b[i])  mask[i];
+
+out:
 #endif
return ret;
 }
@@ -101,8 +112,8 @@ static unsigned long ifname_compare(const char *_a, const 
char *_b, const char *
 /* Returns whether packet matches rule or not. */
 static inline int arp_packet_match(const struct arphdr *arphdr,
   struct net_device *dev,
-  const char *indev,
-  const char *outdev,
+  const struct net_device *indev,
+  const struct net_device *outdev,
   const struct arpt_arp *arpinfo)
 {
const char *arpptr = (char *)(arphdr + 1);
@@ -252,11 +263,9 @@ unsigned int arpt_do_table(struct sk_buff *skb,
   const struct net_device *out,
   struct xt_table *table)
 {
-   static const char nulldevname[IFNAMSIZ] 
__attribute__((aligned(sizeof(long;
unsigned int verdict = NF_DROP;
const struct arphdr *arp;
struct arpt_entry *e, *back;
-   const char *indev, *outdev;
void *table_base;
const struct xt_table_info *private;
struct xt_action_param acpar;
@@ -265,9 +274,6 @@ unsigned int arpt_do_table(struct sk_buff *skb,
if (!pskb_may_pull(skb, arp_hdr_len(skb-dev)))
return NF_DROP;
 
-   indev = in ? in-name : nulldevname;
-   outdev = out ? out-name : nulldevname;
-
local_bh_disable();
addend = xt_write_recseq_begin();
private = table-private;
@@ -291,7 +297,7 @@ unsigned int arpt_do_table(struct sk_buff *skb,
do {
const struct xt_entry_target *t;
 
-   if (!arp_packet_match(arp, skb-dev, indev, outdev, e-arp)) {
+   if (!arp_packet_match(arp, skb-dev, in, out, e-arp)) {
e = arpt_next_entry(e);
continue;
}
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 99e810f..87df9ef 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -73,8 +73,8 @@