[Patch net-next v2 1/2] netpoll: use ipv6_addr_equal() to compare ipv6 addr

[Patch net-next v2 1/2] netpoll: use ipv6_addr_equal() to compare ipv6 addr

[Cong Wang]

From: Cong Wang <amwang@redhat.com>

ipv6_addr_equal() is faster.

Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
 net/core/netpoll.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index e2f79a1..a6f39b6 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -635,7 +635,7 @@ static void netpoll_neigh_reply(struct sk_buff *skb, struct netpoll_info *npinfo
 
 		spin_lock_irqsave(&npinfo->rx_lock, flags);
 		list_for_each_entry_safe(np, tmp, &npinfo->rx_np, rx) {
-			if (memcmp(daddr, &np->local_ip, sizeof(*daddr)))
+			if (!ipv6_addr_equal(daddr, &np->local_ip.in6))
 				continue;
 
 			hlen = LL_RESERVED_SPACE(np->dev);
@@ -828,9 +828,9 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
 		if (udp6_csum_init(skb, uh, IPPROTO_UDP))
 			goto out;
 		list_for_each_entry_safe(np, tmp, &npinfo->rx_np, rx) {
-			if (memcmp(&np->local_ip.in6, &ip6h->daddr, sizeof(struct in6_addr)) != 0)
+			if (!ipv6_addr_equal(&np->local_ip.in6, &ip6h->daddr))
 				continue;
-			if (memcmp(&np->remote_ip.in6, &ip6h->saddr, sizeof(struct in6_addr)) != 0)
+			if (!ipv6_addr_equal(&np->remote_ip.in6, &ip6h->saddr))
 				continue;
 			if (np->local_port && np->local_port != ntohs(uh->dest))
 				continue;
-- 
1.7.7.6

[Patch net-next v2 2/2] netpoll: use the net namespace of current process instead of init_net

[Cong Wang]

From: Cong Wang <amwang@redhat.com>

This will allow us to setup netconsole in a different namespace
rather than where init_net is.

Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index a6f39b6..331ccb9 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -1049,8 +1049,10 @@ int netpoll_setup(struct netpoll *np)
 	int err;
 
 	rtnl_lock();
-	if (np->dev_name)
-		ndev = __dev_get_by_name(&init_net, np->dev_name);
+	if (np->dev_name) {
+		struct net *net = current->nsproxy->net_ns;
+		ndev = __dev_get_by_name(net, np->dev_name);
+	}
 	if (!ndev) {
 		np_err(np, "%s doesn't exist, aborting\n", np->dev_name);
 		err = -ENODEV;

Re: [Patch net-next v2 1/2] netpoll: use ipv6_addr_equal() to compare ipv6 addr

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Mon, 28 Jan 2013 09:55:20 +0800

> From: Cong Wang <amwang@redhat.com>
> 
> ipv6_addr_equal() is faster.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>

Applied.

Re: [Patch net-next v2 2/2] netpoll: use the net namespace of current process instead of init_net

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Mon, 28 Jan 2013 09:55:21 +0800

> From: Cong Wang <amwang@redhat.com>
> 
> This will allow us to setup netconsole in a different namespace
> rather than where init_net is.
> 
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>

Applied but:

> +	if (np->dev_name) {
> +		struct net *net = current->nsproxy->net_ns;
> +		ndev = __dev_get_by_name(net, np->dev_name);
> +	}

I wonder if you should be using task_nsproxy() here.

Re: [Patch net-next v2 2/2] netpoll: use the net namespace of current process instead of init_net

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> From: Cong Wang <amwang@redhat.com>
> Date: Mon, 28 Jan 2013 09:55:21 +0800
>
>> From: Cong Wang <amwang@redhat.com>
>> 
>> This will allow us to setup netconsole in a different namespace
>> rather than where init_net is.
>> 
>> Cc: Eric W. Biederman <ebiederm@xmission.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Signed-off-by: Cong Wang <amwang@redhat.com>
>
> Applied but:
>
>> +	if (np->dev_name) {
>> +		struct net *net = current->nsproxy->net_ns;
>> +		ndev = __dev_get_by_name(net, np->dev_name);
>> +	}
>
> I wonder if you should be using task_nsproxy() here.

At a practical level using nsproxy straight is what we want here,
as nsproxy can not be changed by anything other than the current
process.  So we have an implicit lock just by being the current
process.

Now there might be some set of rules for error checking that
I am not up to speed on that says because some paths that do
the rcu dance:

	rcu_read_lock();
	nsproxy =  rcu_dereference(tsk->nsproxy);
        net = get_net(nsproxy->net_ns);
        rcu_read_lock();

That all paths have to do a companion rcu dance to say this is a
place where we don't need rcu protection because this is the
current process and we don't need an rcu_lock here.

If there is such a set of rules we have at least 24 other places
in the kernel that need to be fixed.

Eric