[Patch] netpoll: allow spaces in its parameter

[Patch] netpoll: allow spaces in its parameter

[Amerigo Wang]

It would be nice if we allow spaces in netconsole parameters,
otherwise we get weird results if there are spaces in it.

After this patch, we will allow things like:
 "netconsole= @192.168.0.1/eth0 , 66666@192.168.0.2/".

Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: David Miller <davem@davemloft.net>

---
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 7aa6972..bf3b2f0 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -629,6 +629,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
 	char *cur=opt, *delim;
 
 	if (*cur != '@') {
+		cur = skip_spaces(cur);
 		if ((delim = strchr(cur, '@')) == NULL)
 			goto parse_failed;
 		*delim = 0;
@@ -651,12 +652,14 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
 		if ((delim = strchr(cur, ',')) == NULL)
 			goto parse_failed;
 		*delim = 0;
+		strim(cur);
 		strlcpy(np->dev_name, cur, sizeof(np->dev_name));
 		cur = delim;
 	}
 	cur++;
 
 	if (*cur != '@') {
+		cur = skip_spaces(cur);
 		/* dst port */
 		if ((delim = strchr(cur, '@')) == NULL)
 			goto parse_failed;

Re: [Patch] netpoll: allow spaces in its parameter

[David Miller]

From: Amerigo Wang <amwang@redhat.com>
Date: Wed, 17 Mar 2010 01:53:26 -0400

> 
> It would be nice if we allow spaces in netconsole parameters,
> otherwise we get weird results if there are spaces in it.
> 
> After this patch, we will allow things like:
>  "netconsole= @192.168.0.1/eth0 , 66666@192.168.0.2/".
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Cc: David Miller <davem@davemloft.net>

That kernel command line looks absolutely awful, not
better.

Sorry I'm not applying this.

Re: [Patch] netpoll: allow spaces in its parameter

[Cong Wang]

David Miller wrote:
> From: Amerigo Wang <amwang@redhat.com>
> Date: Wed, 17 Mar 2010 01:53:26 -0400
> 
>> It would be nice if we allow spaces in netconsole parameters,
>> otherwise we get weird results if there are spaces in it.
>>
>> After this patch, we will allow things like:
>>  "netconsole= @192.168.0.1/eth0 , 66666@192.168.0.2/".
>>
>> Signed-off-by: WANG Cong <amwang@redhat.com>
>> Cc: David Miller <davem@davemloft.net>
> 
> That kernel command line looks absolutely awful, not
> better.
> 
> Sorry I'm not applying this.

The problem is that the dst port is 0 when I even use:

"netconsole=@192.168.0.1/eth0, 66666@192.168.0.2/"

This is awful too? I don't think so...

Re: [Patch] netpoll: allow spaces in its parameter

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Wed, 17 Mar 2010 14:01:06 +0800

> The problem is that the dst port is 0 when I even use:
> 
> "netconsole=@192.168.0.1/eth0, 66666@192.168.0.2/"
> 
> This is awful too? I don't think so...

It's very simple, don't use spaces.  Is it so difficult
to follow this rule?

"ipconfig" does it as do a host of other kernel command
line options.

There is no real reason to make netconsle= special in
this regard, really.

Re: [Patch] netpoll: allow spaces in its parameter

[Cong Wang]

David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Wed, 17 Mar 2010 14:01:06 +0800
> 
>> The problem is that the dst port is 0 when I even use:
>>
>> "netconsole=@192.168.0.1/eth0, 66666@192.168.0.2/"
>>
>> This is awful too? I don't think so...
> 
> It's very simple, don't use spaces.  Is it so difficult
> to follow this rule?
> 

No, but silently accepting it as 0 is not correct, why don't
reject it if it is not allowed?

Re: [Patch] netpoll: allow spaces in its parameter

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Wed, 17 Mar 2010 14:07:57 +0800

> No, but silently accepting it as 0 is not correct, why don't
> reject it if it is not allowed?

You have to be careful even with that.  For example, if two
netconsoles are specified, seeing the space shouldn't
kill the first netconsole specification we parsed.

A warning perhaps, but outright rejection of all specifications is
really bad behavior.

Re: [Patch] netpoll: allow spaces in its parameter

[Cong Wang]

David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Wed, 17 Mar 2010 14:07:57 +0800
> 
>> No, but silently accepting it as 0 is not correct, why don't
>> reject it if it is not allowed?
> 
> You have to be careful even with that.  For example, if two
> netconsoles are specified, seeing the space shouldn't
> kill the first netconsole specification we parsed.
> 
> A warning perhaps, but outright rejection of all specifications is
> really bad behavior.

OK, I will put a warning there instead.

Thanks, David!