[PATCH net-next] bridge: ebtables: Avoid resetting limit rule state

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-11-25  7:44 ` Linus Lüssing
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-11-25  7:44 UTC (permalink / raw)
  To: netfilter-devel
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Stephen Hemminger, David S . Miller, coreteam, bridge, netdev,
	linux-kernel, Linus Lüssing

So far any changes with ebtables will reset the state of limit rules,
leading to spikes in traffic. This is especially noticeable if changes
are done frequently, for instance via a daemon.

This patch fixes this by bailing out from (re)setting if the limit
rule was initialized before.

When sending packets every 250ms for 600s, with a
"--limit 1/sec --limit-burst 50" rule and a command like this
in the background:

$ ebtables -N VOIDCHAIN
$ while true; do ebtables -F VOIDCHAIN; sleep 30; done

The results are:

Before: ~1600 packets
After: 650 packets

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/bridge/netfilter/ebt_limit.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
index 61a9f1be1263..f74b48633feb 100644
--- a/net/bridge/netfilter/ebt_limit.c
+++ b/net/bridge/netfilter/ebt_limit.c
@@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
 {
 	struct ebt_limit_info *info = par->matchinfo;
 
+	/* Do not reset state on unrelated table changes */
+	if (info->prev)
+		return 0;
+
 	/* Check for overflow. */
 	if (info->burst == 0 ||
 	    user2credits(info->avg * info->burst) < user2credits(info->avg)) {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Bridge] [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-11-25  7:44 ` Linus Lüssing
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-11-25  7:44 UTC (permalink / raw)
  To: netfilter-devel
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	Jozsef Kadlecsik, David S . Miller, Pablo Neira Ayuso

So far any changes with ebtables will reset the state of limit rules,
leading to spikes in traffic. This is especially noticeable if changes
are done frequently, for instance via a daemon.

This patch fixes this by bailing out from (re)setting if the limit
rule was initialized before.

When sending packets every 250ms for 600s, with a
"--limit 1/sec --limit-burst 50" rule and a command like this
in the background:

$ ebtables -N VOIDCHAIN
$ while true; do ebtables -F VOIDCHAIN; sleep 30; done

The results are:

Before: ~1600 packets
After: 650 packets

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/bridge/netfilter/ebt_limit.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
index 61a9f1be1263..f74b48633feb 100644
--- a/net/bridge/netfilter/ebt_limit.c
+++ b/net/bridge/netfilter/ebt_limit.c
@@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
 {
 	struct ebt_limit_info *info = par->matchinfo;
 
+	/* Do not reset state on unrelated table changes */
+	if (info->prev)
+		return 0;
+
 	/* Check for overflow. */
 	if (info->burst == 0 ||
 	    user2credits(info->avg * info->burst) < user2credits(info->avg)) {
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
  2017-11-25  7:44 ` [Bridge] " Linus Lüssing
@ 2017-11-27 23:30   ` Pablo Neira Ayuso
  -1 siblings, 0 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2017-11-27 23:30 UTC (permalink / raw)
  To: Linus Lüssing
  Cc: netfilter-devel, Jozsef Kadlecsik, Florian Westphal,
	Stephen Hemminger, David S . Miller, coreteam, bridge, netdev,
	linux-kernel

Hi Linus,

On Sat, Nov 25, 2017 at 08:44:18AM +0100, Linus Lüssing wrote:
> So far any changes with ebtables will reset the state of limit rules,
> leading to spikes in traffic. This is especially noticeable if changes
> are done frequently, for instance via a daemon.
> 
> This patch fixes this by bailing out from (re)setting if the limit
> rule was initialized before.
> 
> When sending packets every 250ms for 600s, with a
> "--limit 1/sec --limit-burst 50" rule and a command like this
> in the background:
> 
> $ ebtables -N VOIDCHAIN
> $ while true; do ebtables -F VOIDCHAIN; sleep 30; done
> 
> The results are:
> 
> Before: ~1600 packets
> After: 650 packets
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  net/bridge/netfilter/ebt_limit.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
> index 61a9f1be1263..f74b48633feb 100644
> --- a/net/bridge/netfilter/ebt_limit.c
> +++ b/net/bridge/netfilter/ebt_limit.c
> @@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
>  {
>  	struct ebt_limit_info *info = par->matchinfo;
>  
> +	/* Do not reset state on unrelated table changes */
> +	if (info->prev)
> +		return 0;

What kernel version are you using? I suspect you don't have this
applied?

commit ec23189049651b16dc2ffab35a4371dc1f491aca
Author: Willem de Bruijn <willemb@google.com>
Date:   Mon Jan 2 17:19:46 2017 -0500

    xtables: extend matches and targets with .usersize

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bridge] [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-11-27 23:30   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2017-11-27 23:30 UTC (permalink / raw)
  To: Linus Lüssing
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

Hi Linus,

On Sat, Nov 25, 2017 at 08:44:18AM +0100, Linus Lüssing wrote:
> So far any changes with ebtables will reset the state of limit rules,
> leading to spikes in traffic. This is especially noticeable if changes
> are done frequently, for instance via a daemon.
> 
> This patch fixes this by bailing out from (re)setting if the limit
> rule was initialized before.
> 
> When sending packets every 250ms for 600s, with a
> "--limit 1/sec --limit-burst 50" rule and a command like this
> in the background:
> 
> $ ebtables -N VOIDCHAIN
> $ while true; do ebtables -F VOIDCHAIN; sleep 30; done
> 
> The results are:
> 
> Before: ~1600 packets
> After: 650 packets
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  net/bridge/netfilter/ebt_limit.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
> index 61a9f1be1263..f74b48633feb 100644
> --- a/net/bridge/netfilter/ebt_limit.c
> +++ b/net/bridge/netfilter/ebt_limit.c
> @@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
>  {
>  	struct ebt_limit_info *info = par->matchinfo;
>  
> +	/* Do not reset state on unrelated table changes */
> +	if (info->prev)
> +		return 0;

What kernel version are you using? I suspect you don't have this
applied?

commit ec23189049651b16dc2ffab35a4371dc1f491aca
Author: Willem de Bruijn <willemb@google.com>
Date:   Mon Jan 2 17:19:46 2017 -0500

    xtables: extend matches and targets with .usersize

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
  2017-11-27 23:30   ` [Bridge] " Pablo Neira Ayuso
  (?)
@ 2017-12-04  4:53     ` Linus Lüssing
  -1 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-12-04  4:53 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netfilter-devel, Jozsef Kadlecsik, Florian Westphal,
	Stephen Hemminger, David S . Miller, coreteam, bridge, netdev,
	linux-kernel

Hi Pablo,

Thanks for your reply!

On Tue, Nov 28, 2017 at 12:30:08AM +0100, Pablo Neira Ayuso wrote:
> [...]
> > diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
> > index 61a9f1be1263..f74b48633feb 100644
> > --- a/net/bridge/netfilter/ebt_limit.c
> > +++ b/net/bridge/netfilter/ebt_limit.c
> > @@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
> >  {
> >  	struct ebt_limit_info *info = par->matchinfo;
> >  
> > +	/* Do not reset state on unrelated table changes */
> > +	if (info->prev)
> > +		return 0;
> 
> What kernel version are you using? I suspect you don't have this
> applied?

I'm indeed using a 4.4.102 kernel, as LEDE is still in the process
of updating to 4.14. So 4.4 with LEDE is where I got the measurement
results from.

> 
> commit ec23189049651b16dc2ffab35a4371dc1f491aca
> Author: Willem de Bruijn <willemb@google.com>
> Date:   Mon Jan 2 17:19:46 2017 -0500
> 
>     xtables: extend matches and targets with .usersize

And so, no I do not have this patch. I looked at it now, but it
does not seem to have any relation with .matchinfo, does it?

I also had a quick look at a 4.15-rc1 kernel in a VM now. I still
end up in ebt_limit_mt_check() with the variables being reset
when editing the table somewhere.

Regards, Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bridge] [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-12-04  4:53     ` Linus Lüssing
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-12-04  4:53 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

Hi Pablo,

Thanks for your reply!

On Tue, Nov 28, 2017 at 12:30:08AM +0100, Pablo Neira Ayuso wrote:
> [...]
> > diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
> > index 61a9f1be1263..f74b48633feb 100644
> > --- a/net/bridge/netfilter/ebt_limit.c
> > +++ b/net/bridge/netfilter/ebt_limit.c
> > @@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
> >  {
> >  	struct ebt_limit_info *info = par->matchinfo;
> >  
> > +	/* Do not reset state on unrelated table changes */
> > +	if (info->prev)
> > +		return 0;
> 
> What kernel version are you using? I suspect you don't have this
> applied?

I'm indeed using a 4.4.102 kernel, as LEDE is still in the process
of updating to 4.14. So 4.4 with LEDE is where I got the measurement
results from.

> 
> commit ec23189049651b16dc2ffab35a4371dc1f491aca
> Author: Willem de Bruijn <willemb@google.com>
> Date:   Mon Jan 2 17:19:46 2017 -0500
> 
>     xtables: extend matches and targets with .usersize

And so, no I do not have this patch. I looked at it now, but it
does not seem to have any relation with .matchinfo, does it?

I also had a quick look at a 4.15-rc1 kernel in a VM now. I still
end up in ebt_limit_mt_check() with the variables being reset
when editing the table somewhere.

Regards, Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-12-04  4:53     ` Linus Lüssing
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-12-04  4:53 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

Hi Pablo,

Thanks for your reply!

On Tue, Nov 28, 2017 at 12:30:08AM +0100, Pablo Neira Ayuso wrote:
> [...]
> > diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
> > index 61a9f1be1263..f74b48633feb 100644
> > --- a/net/bridge/netfilter/ebt_limit.c
> > +++ b/net/bridge/netfilter/ebt_limit.c
> > @@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
> >  {
> >  	struct ebt_limit_info *info = par->matchinfo;
> >  
> > +	/* Do not reset state on unrelated table changes */
> > +	if (info->prev)
> > +		return 0;
> 
> What kernel version are you using? I suspect you don't have this
> applied?

I'm indeed using a 4.4.102 kernel, as LEDE is still in the process
of updating to 4.14. So 4.4 with LEDE is where I got the measurement
results from.

> 
> commit ec23189049651b16dc2ffab35a4371dc1f491aca
> Author: Willem de Bruijn <willemb@google.com>
> Date:   Mon Jan 2 17:19:46 2017 -0500
> 
>     xtables: extend matches and targets with .usersize

And so, no I do not have this patch. I looked at it now, but it
does not seem to have any relation with .matchinfo, does it?

I also had a quick look at a 4.15-rc1 kernel in a VM now. I still
end up in ebt_limit_mt_check() with the variables being reset
when editing the table somewhere.

Regards, Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bridge] [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
  2017-12-04  4:53     ` Linus Lüssing
  (?)
  (?)
@ 2017-12-04  5:20     ` Linus Lüssing
  2017-12-04 10:13       ` Pablo Neira Ayuso
  -1 siblings, 1 reply; 17+ messages in thread
From: Linus Lüssing @ 2017-12-04  5:20 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

On Mon, Dec 04, 2017 at 05:53:35AM +0100, Linus Lüssing wrote:
> And so, no I do not have this patch. I looked at it now, but it
> does not seem to have any relation with .matchinfo, does it?

Relation between .usersize and .checkentry I ment, not
.usersize and .matchinfo.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bridge] [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
  2017-12-04  5:20     ` [Bridge] " Linus Lüssing
@ 2017-12-04 10:13       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2017-12-04 10:13 UTC (permalink / raw)
  To: Linus Lüssing
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

On Mon, Dec 04, 2017 at 06:20:06AM +0100, Linus Lüssing wrote:
> On Mon, Dec 04, 2017 at 05:53:35AM +0100, Linus Lüssing wrote:
> > And so, no I do not have this patch. I looked at it now, but it
> > does not seem to have any relation with .matchinfo, does it?
> 
> Relation between .usersize and .checkentry I ment, not
> .usersize and .matchinfo.

In your patch, info->prev comes set to a value from userspace, right?

commit 324318f0248c31be8a08984146e7e4dd7cdd091d
Author: Willem de Bruijn <willemb@google.com>
Date:   Tue May 9 16:17:37 2017 -0400

    netfilter: xtables: zero padding in data_to_user

Since that patch above, the data area is zero'ed before dumped to
userspace, so we would get a null info->prev, hence defeating the
trick your patch relies on.

Am I missing anything?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
  2017-12-04  4:53     ` Linus Lüssing
  (?)
@ 2017-12-07  0:26       ` Pablo Neira Ayuso
  -1 siblings, 0 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2017-12-07  0:26 UTC (permalink / raw)
  To: Linus Lüssing
  Cc: netfilter-devel, Jozsef Kadlecsik, Florian Westphal,
	Stephen Hemminger, David S . Miller, coreteam, bridge, netdev,
	linux-kernel

Hi Linus,

On Mon, Dec 04, 2017 at 05:53:35AM +0100, Linus Lüssing wrote:
> Hi Pablo,
> 
> Thanks for your reply!
> 
> On Tue, Nov 28, 2017 at 12:30:08AM +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
> > > index 61a9f1be1263..f74b48633feb 100644
> > > --- a/net/bridge/netfilter/ebt_limit.c
> > > +++ b/net/bridge/netfilter/ebt_limit.c
> > > @@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
> > >  {
> > >  	struct ebt_limit_info *info = par->matchinfo;
> > >  
> > > +	/* Do not reset state on unrelated table changes */
> > > +	if (info->prev)
> > > +		return 0;
> > 
> > What kernel version are you using? I suspect you don't have this
> > applied?
> 
> I'm indeed using a 4.4.102 kernel, as LEDE is still in the process
> of updating to 4.14. So 4.4 with LEDE is where I got the measurement
> results from.
> 
> > 
> > commit ec23189049651b16dc2ffab35a4371dc1f491aca
> > Author: Willem de Bruijn <willemb@google.com>
> > Date:   Mon Jan 2 17:19:46 2017 -0500
> > 
> >     xtables: extend matches and targets with .usersize
> 
> And so, no I do not have this patch. I looked at it now, but it
> does not seem to have any relation with .matchinfo, does it?
> 
> I also had a quick look at a 4.15-rc1 kernel in a VM now. I still
> end up in ebt_limit_mt_check() with the variables being reset
> when editing the table somewhere.

My question is if your fix would work with 4.15-rc1.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bridge] [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-12-07  0:26       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2017-12-07  0:26 UTC (permalink / raw)
  To: Linus Lüssing
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

Hi Linus,

On Mon, Dec 04, 2017 at 05:53:35AM +0100, Linus Lüssing wrote:
> Hi Pablo,
> 
> Thanks for your reply!
> 
> On Tue, Nov 28, 2017 at 12:30:08AM +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
> > > index 61a9f1be1263..f74b48633feb 100644
> > > --- a/net/bridge/netfilter/ebt_limit.c
> > > +++ b/net/bridge/netfilter/ebt_limit.c
> > > @@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
> > >  {
> > >  	struct ebt_limit_info *info = par->matchinfo;
> > >  
> > > +	/* Do not reset state on unrelated table changes */
> > > +	if (info->prev)
> > > +		return 0;
> > 
> > What kernel version are you using? I suspect you don't have this
> > applied?
> 
> I'm indeed using a 4.4.102 kernel, as LEDE is still in the process
> of updating to 4.14. So 4.4 with LEDE is where I got the measurement
> results from.
> 
> > 
> > commit ec23189049651b16dc2ffab35a4371dc1f491aca
> > Author: Willem de Bruijn <willemb@google.com>
> > Date:   Mon Jan 2 17:19:46 2017 -0500
> > 
> >     xtables: extend matches and targets with .usersize
> 
> And so, no I do not have this patch. I looked at it now, but it
> does not seem to have any relation with .matchinfo, does it?
> 
> I also had a quick look at a 4.15-rc1 kernel in a VM now. I still
> end up in ebt_limit_mt_check() with the variables being reset
> when editing the table somewhere.

My question is if your fix would work with 4.15-rc1.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-12-07  0:26       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2017-12-07  0:26 UTC (permalink / raw)
  To: Linus Lüssing
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

Hi Linus,

On Mon, Dec 04, 2017 at 05:53:35AM +0100, Linus Lüssing wrote:
> Hi Pablo,
> 
> Thanks for your reply!
> 
> On Tue, Nov 28, 2017 at 12:30:08AM +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
> > > index 61a9f1be1263..f74b48633feb 100644
> > > --- a/net/bridge/netfilter/ebt_limit.c
> > > +++ b/net/bridge/netfilter/ebt_limit.c
> > > @@ -69,6 +69,10 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
> > >  {
> > >  	struct ebt_limit_info *info = par->matchinfo;
> > >  
> > > +	/* Do not reset state on unrelated table changes */
> > > +	if (info->prev)
> > > +		return 0;
> > 
> > What kernel version are you using? I suspect you don't have this
> > applied?
> 
> I'm indeed using a 4.4.102 kernel, as LEDE is still in the process
> of updating to 4.14. So 4.4 with LEDE is where I got the measurement
> results from.
> 
> > 
> > commit ec23189049651b16dc2ffab35a4371dc1f491aca
> > Author: Willem de Bruijn <willemb@google.com>
> > Date:   Mon Jan 2 17:19:46 2017 -0500
> > 
> >     xtables: extend matches and targets with .usersize
> 
> And so, no I do not have this patch. I looked at it now, but it
> does not seem to have any relation with .matchinfo, does it?
> 
> I also had a quick look at a 4.15-rc1 kernel in a VM now. I still
> end up in ebt_limit_mt_check() with the variables being reset
> when editing the table somewhere.

My question is if your fix would work with 4.15-rc1.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
  2017-12-07  0:26       ` Pablo Neira Ayuso
@ 2017-12-08  5:46         ` Linus Lüssing
  -1 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-12-08  5:46 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netfilter-devel, Jozsef Kadlecsik, Florian Westphal,
	Stephen Hemminger, David S . Miller, coreteam, bridge, netdev,
	linux-kernel

On Thu, Dec 07, 2017 at 01:26:19AM +0100, Pablo Neira Ayuso wrote:
> > I also had a quick look at a 4.15-rc1 kernel in a VM now. I still
> > end up in ebt_limit_mt_check() with the variables being reset
> > when editing the table somewhere.
> 
> My question is if your fix would work with 4.15-rc1.

You are absoluetly right, it's not working anymore since the
commit you mentioned initially :-(
("xtables: extend matches and targets with .usersize").
info->prev is always 0 since exactly this commit.

That means, trying tricks in ebt_limit_mt_check() is too
late now, the old values are already overwritten? (or is there
some commit scheme which installs the ebt_limit_info provided
by ebt_limit_check() some time after its call?)

Extending the usersize to include info->prev would probably be too
hackish/ugly, right?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bridge] [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-12-08  5:46         ` Linus Lüssing
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-12-08  5:46 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

On Thu, Dec 07, 2017 at 01:26:19AM +0100, Pablo Neira Ayuso wrote:
> > I also had a quick look at a 4.15-rc1 kernel in a VM now. I still
> > end up in ebt_limit_mt_check() with the variables being reset
> > when editing the table somewhere.
> 
> My question is if your fix would work with 4.15-rc1.

You are absoluetly right, it's not working anymore since the
commit you mentioned initially :-(
("xtables: extend matches and targets with .usersize").
info->prev is always 0 since exactly this commit.

That means, trying tricks in ebt_limit_mt_check() is too
late now, the old values are already overwritten? (or is there
some commit scheme which installs the ebt_limit_info provided
by ebt_limit_check() some time after its call?)

Extending the usersize to include info->prev would probably be too
hackish/ugly, right?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
  2017-12-08  5:46         ` [Bridge] " Linus Lüssing
  (?)
@ 2017-12-08  5:49           ` Linus Lüssing
  -1 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-12-08  5:49 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netfilter-devel, Jozsef Kadlecsik, Florian Westphal,
	Stephen Hemminger, David S . Miller, coreteam, bridge, netdev,
	linux-kernel

On Fri, Dec 08, 2017 at 06:46:06AM +0100, Linus Lüssing wrote:
> Extending the usersize to include info->prev would probably be too
> hackish/ugly, right?

And wouldn't be enough anyway, since
info->{credit,credit_cap,cost} would still be zeroed... Hm.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bridge] [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-12-08  5:49           ` Linus Lüssing
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-12-08  5:49 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

On Fri, Dec 08, 2017 at 06:46:06AM +0100, Linus Lüssing wrote:
> Extending the usersize to include info->prev would probably be too
> hackish/ugly, right?

And wouldn't be enough anyway, since
info->{credit,credit_cap,cost} would still be zeroed... Hm.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state
@ 2017-12-08  5:49           ` Linus Lüssing
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Lüssing @ 2017-12-08  5:49 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, bridge, Florian Westphal, linux-kernel, coreteam,
	netfilter-devel, Jozsef Kadlecsik, David S . Miller

On Fri, Dec 08, 2017 at 06:46:06AM +0100, Linus Lüssing wrote:
> Extending the usersize to include info->prev would probably be too
> hackish/ugly, right?

And wouldn't be enough anyway, since
info->{credit,credit_cap,cost} would still be zeroed... Hm.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-12-08  5:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-25  7:44 [PATCH net-next] bridge: ebtables: Avoid resetting limit rule state Linus Lüssing
2017-11-25  7:44 ` [Bridge] " Linus Lüssing
2017-11-27 23:30 ` Pablo Neira Ayuso
2017-11-27 23:30   ` [Bridge] " Pablo Neira Ayuso
2017-12-04  4:53   ` Linus Lüssing
2017-12-04  4:53     ` [Bridge] " Linus Lüssing
2017-12-04  4:53     ` Linus Lüssing
2017-12-04  5:20     ` [Bridge] " Linus Lüssing
2017-12-04 10:13       ` Pablo Neira Ayuso
2017-12-07  0:26     ` Pablo Neira Ayuso
2017-12-07  0:26       ` [Bridge] " Pablo Neira Ayuso
2017-12-07  0:26       ` Pablo Neira Ayuso
2017-12-08  5:46       ` Linus Lüssing
2017-12-08  5:46         ` [Bridge] " Linus Lüssing
2017-12-08  5:49         ` Linus Lüssing
2017-12-08  5:49           ` [Bridge] " Linus Lüssing
2017-12-08  5:49           ` Linus Lüssing

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.