[PATCH net-next? 1/1] netlabel: return msg overflow error from netlbl_cipsov4_list faster

[PATCH net-next? 1/1] netlabel: return msg overflow error from netlbl_cipsov4_list faster

[Denis V. Lunev]

Currently, we are trying to place the information from the kernel to
1, 2, 3 and 4 pages sequentially. These pages are allocated via slab.
Though, from the slab point of view steps 3 and 4 are equivalent on
most architectures. So, lets skip 3 pages attempt.

By the way, should we switch from .doit to .dumpit interface here?
The amount of data seems quite big for me.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 net/netlabel/netlabel_cipso_v4.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
--- a/net/netlabel/netlabel_cipso_v4.c
+++ b/net/netlabel/netlabel_cipso_v4.c
@@ -591,7 +591,7 @@ list_retry:
 	if (nlsze_mult < 4) {
 		rcu_read_unlock();
 		kfree_skb(ans_skb);
-		nlsze_mult++;
+		nlsze_mult *= 2;
 		goto list_start;
 	}
 list_failure_lock:
-- 
1.5.3.rc5

Re: [PATCH net-next? 1/1] netlabel: return msg overflow error from netlbl_cipsov4_list faster

[Paul Moore]

On Friday 11 July 2008 6:45:49 am Denis V. Lunev wrote:
> Currently, we are trying to place the information from the kernel to
> 1, 2, 3 and 4 pages sequentially. These pages are allocated via slab.
> Though, from the slab point of view steps 3 and 4 are equivalent on
> most architectures. So, lets skip 3 pages attempt.
>
> By the way, should we switch from .doit to .dumpit interface here?
> The amount of data seems quite big for me.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>

I'll add my ack to this patch because your logic sounds reasonable and 
I'm glad to have more eyes on the code :)

However, I not a big fan of converting this from a .doit to a .dumpit 
interface because this would cause userspace breakage.  We could 
potentially do both for backwards compatibility but I have no idea if 
it is possible to have both interfaces available at the same time.  One 
thing to keep in mind before diving into your favorite editor is that 
I've yet to hear of there being a problem with this NetLabel command 
and the .doit interface, we may want to hold off to see if it becomes 
an issue.

Acked-by: Paul Moore <paul.moore@hp.com>

> ---
>  net/netlabel/netlabel_cipso_v4.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/netlabel/netlabel_cipso_v4.c
> b/net/netlabel/netlabel_cipso_v4.c ---
> a/net/netlabel/netlabel_cipso_v4.c
> +++ b/net/netlabel/netlabel_cipso_v4.c
> @@ -591,7 +591,7 @@ list_retry:
>  	if (nlsze_mult < 4) {
>  		rcu_read_unlock();
>  		kfree_skb(ans_skb);
> -		nlsze_mult++;
> +		nlsze_mult *= 2;
>  		goto list_start;
>  	}
>  list_failure_lock:



-- 
paul moore
linux @ hp

Re: [PATCH net-next? 1/1] netlabel: return msg overflow error from netlbl_cipsov4_list faster

[Denis V. Lunev]

On Fri, 2008-07-11 at 07:01 -0400, Paul Moore wrote:
> On Friday 11 July 2008 6:45:49 am Denis V. Lunev wrote:
> > Currently, we are trying to place the information from the kernel to
> > 1, 2, 3 and 4 pages sequentially. These pages are allocated via slab.
> > Though, from the slab point of view steps 3 and 4 are equivalent on
> > most architectures. So, lets skip 3 pages attempt.
> >
> > By the way, should we switch from .doit to .dumpit interface here?
> > The amount of data seems quite big for me.
> >
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> 
> I'll add my ack to this patch because your logic sounds reasonable and 
> I'm glad to have more eyes on the code :)
cool :)

> However, I not a big fan of converting this from a .doit to a .dumpit 
> interface because this would cause userspace breakage.  We could 
> potentially do both for backwards compatibility but I have no idea if 
> it is possible to have both interfaces available at the same time.  One 
> thing to keep in mind before diving into your favorite editor is that 
> I've yet to hear of there being a problem with this NetLabel command 
> and the .doit interface, we may want to hold off to see if it becomes 
> an issue.
> 
> Acked-by: Paul Moore <paul.moore@hp.com>

for the dumpip interface. 2-order page is a real pain on the heavy
loaded systems. Yes, the allocation will not fail, but it can take a
significant amount of time to perform the allocation.

I understand your logic with keeping this interface, but such allocation
can take > 1 sec for sure.

Re: [PATCH net-next? 1/1] netlabel: return msg overflow error from netlbl_cipsov4_list faster

[Paul Moore]

On Friday 11 July 2008 8:13:12 am Denis V. Lunev wrote:
> On Fri, 2008-07-11 at 07:01 -0400, Paul Moore wrote:
> > On Friday 11 July 2008 6:45:49 am Denis V. Lunev wrote:
> > > Currently, we are trying to place the information from the kernel
> > > to 1, 2, 3 and 4 pages sequentially. These pages are allocated
> > > via slab. Though, from the slab point of view steps 3 and 4 are
> > > equivalent on most architectures. So, lets skip 3 pages attempt.
> > >
> > > By the way, should we switch from .doit to .dumpit interface
> > > here? The amount of data seems quite big for me.
> > >
> > > Signed-off-by: Denis V. Lunev <den@openvz.org>
> >
> > I'll add my ack to this patch because your logic sounds reasonable
> > and I'm glad to have more eyes on the code :)
>
> cool :)
>
> > However, I not a big fan of converting this from a .doit to a
> > .dumpit interface because this would cause userspace breakage.  We
> > could potentially do both for backwards compatibility but I have no
> > idea if it is possible to have both interfaces available at the
> > same time.  One thing to keep in mind before diving into your
> > favorite editor is that I've yet to hear of there being a problem
> > with this NetLabel command and the .doit interface, we may want to
> > hold off to see if it becomes an issue.
> >
> > Acked-by: Paul Moore <paul.moore@hp.com>
>
> for the dumpip interface. 2-order page is a real pain on the heavy
> loaded systems. Yes, the allocation will not fail, but it can take a
> significant amount of time to perform the allocation.
>
> I understand your logic with keeping this interface, but such
> allocation can take > 1 sec for sure.

Okay, that is good to know.  If it ever starts to become a problem I'll 
look at leveraging the .dumpit interface.

Thanks.

-- 
paul moore
linux @ hp

Re: [PATCH net-next? 1/1] netlabel: return msg overflow error from netlbl_cipsov4_list faster

[David Miller]

From: Paul Moore <paul.moore@hp.com>
Date: Fri, 11 Jul 2008 07:01:05 -0400

> On Friday 11 July 2008 6:45:49 am Denis V. Lunev wrote:
> > Currently, we are trying to place the information from the kernel to
> > 1, 2, 3 and 4 pages sequentially. These pages are allocated via slab.
> > Though, from the slab point of view steps 3 and 4 are equivalent on
> > most architectures. So, lets skip 3 pages attempt.
> >
> > By the way, should we switch from .doit to .dumpit interface here?
> > The amount of data seems quite big for me.
> >
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> 
> I'll add my ack to this patch because your logic sounds reasonable and 
> I'm glad to have more eyes on the code :)
> 
> However, I not a big fan of converting this from a .doit to a .dumpit 
> interface because this would cause userspace breakage.  We could 
> potentially do both for backwards compatibility but I have no idea if 
> it is possible to have both interfaces available at the same time.  One 
> thing to keep in mind before diving into your favorite editor is that 
> I've yet to hear of there being a problem with this NetLabel command 
> and the .doit interface, we may want to hold off to see if it becomes 
> an issue.
> 
> Acked-by: Paul Moore <paul.moore@hp.com>

I've applied Denis's patch.

Also, I agree with him that a .dumpit conversion might be
entirely appropriate.

Re: [PATCH net-next? 1/1] netlabel: return msg overflow error from netlbl_cipsov4_list faster

[Paul Moore]

On Tuesday 15 July 2008 1:29:15 am David Miller wrote:
> From: Paul Moore <paul.moore@hp.com>
> Date: Fri, 11 Jul 2008 07:01:05 -0400
>
> > On Friday 11 July 2008 6:45:49 am Denis V. Lunev wrote:
> > > Currently, we are trying to place the information from the kernel
> > > to 1, 2, 3 and 4 pages sequentially. These pages are allocated
> > > via slab. Though, from the slab point of view steps 3 and 4 are
> > > equivalent on most architectures. So, lets skip 3 pages attempt.
> > >
> > > By the way, should we switch from .doit to .dumpit interface
> > > here? The amount of data seems quite big for me.
> > >
> > > Signed-off-by: Denis V. Lunev <den@openvz.org>
> >
> > I'll add my ack to this patch because your logic sounds reasonable
> > and I'm glad to have more eyes on the code :)
> >
> > However, I not a big fan of converting this from a .doit to a
> > .dumpit interface because this would cause userspace breakage.  We
> > could potentially do both for backwards compatibility but I have no
> > idea if it is possible to have both interfaces available at the
> > same time.  One thing to keep in mind before diving into your
> > favorite editor is that I've yet to hear of there being a problem
> > with this NetLabel command and the .doit interface, we may want to
> > hold off to see if it becomes an issue.
> >
> > Acked-by: Paul Moore <paul.moore@hp.com>
>
> I've applied Denis's patch.
>
> Also, I agree with him that a .dumpit conversion might be
> entirely appropriate.

After Dennis' mail I looked into a bit more and there doesn't appear to 
be any reason why you couldn't have both a .doit and a .dumpit 
interface for the same command.  I'll throw this on the ToDo list, but 
if anyone already has patches I'd be more than happy to review them ;)

-- 
paul moore
linux @ hp