netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/2] r8152: random MAC address
From: Hayes Wang @ 2014-09-03  2:59 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-26-Taiwan-albertk@realtek.com>

If the interface has invalid MAC address, it couldn't
be used. In order to let it work normally, give a
random one.

v2:
  Use "%pM" format specifier for printing a MAC address.

Hayes Wang (2):
  r8152: change the location of rtl8152_set_mac_address
  r8152: use eth_hw_addr_random

 drivers/net/usb/r8152.c | 65 ++++++++++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 28 deletions(-)

-- 
1.9.3

^ permalink raw reply

* Re: [PATCH] net: Validate frames going through the direct_xmit path
From: Alexander Duyck @ 2014-09-03  2:46 UTC (permalink / raw)
  To: Eric Dumazet, Alexander Duyck; +Cc: netdev, davem
In-Reply-To: <1409700648.26422.21.camel@edumazet-glaptop2.roam.corp.google.com>

On 09/02/2014 04:30 PM, Eric Dumazet wrote:
> On Tue, 2014-09-02 at 18:55 -0400, Alexander Duyck wrote:
>> In commit 50cbe9ab5f8d92d2d4a327b56e96559d8f63a1fa "net: Validate xmit SKBs
>> right when we pull them out of the qdisc" the validation code was moved out
>> of dev_hard_start_xmit and into dequeue_skb.  However this overlooked the
>> fact that we do not always enqueue the skb onto a qdisc.
>>
>> As a result I was seeing issues trying to connect to a vhost_net interface
>> after this patch was applied.  To resolve the issue I have added a call to
>> validate_xmit_skb in sched_direct_xmit and this seems to have resolved the
>> issue by restoring the validation to this xmit path.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>>  net/sched/sch_generic.c |    9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
>> index a8bf9f9..203ee65 100644
>> --- a/net/sched/sch_generic.c
>> +++ b/net/sched/sch_generic.c
>> @@ -128,8 +128,13 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
>>  	spin_unlock(root_lock);
>>  
>>  	HARD_TX_LOCK(dev, txq, smp_processor_id());
>> -	if (!netif_xmit_frozen_or_stopped(txq))
>> -		skb = dev_hard_start_xmit(skb, dev, txq, &ret);
>> +	if (!netif_xmit_frozen_or_stopped(txq)) {
>> +		skb = validate_xmit_skb(skb, dev);
>> +		if (!skb)
>> +			ret = NETDEV_TX_OK;
>> +		else
>> +			skb = dev_hard_start_xmit(skb, dev, txq, &ret);
>> +	}
>>  
>>  	HARD_TX_UNLOCK(dev, txq);
>>  
> 
> This looks very weird.

It's ugly, I will admit it.  It was a quick hack to fix the issue I had
been seeing as it was in my way.

> Calling validate_xmit_skb() twice per packet is not needed in the case
> sch_direct_xmit() is called from qdisc_restart()

My bad, I overlooked that sch_direct_xmit is called by qdisc_restart.

> This will add bad branch prediction at very minimum.
> 
> This is a TCQ_F_CAN_BYPASS issue that should be fixed there.

Actually it looks like there are several issues.  One is the bypass
problem which is the major issue. Another side effect of the original
patch is that a bad frame will cause us to exit __qdisc_run prematurely
even if other frames are still in the qdisc.

Alternative patches always welcome. :-)  My goal at this point is to
just have my vhost_net interface work so I can get back to my other
development work.  I will submit a v2 in the morning if I don't see
anything.

Alex

^ permalink raw reply

* [PATCH rfc] sock: return error when sk_error_queue is not empty
From: Willem de Bruijn @ 2014-09-03  2:39 UTC (permalink / raw)
  To: netdev; +Cc: davem, hannes, eric.dumazet, Willem de Bruijn

[rfc related to http://patchwork.ozlabs.org/patch/384606/]

Sockets can have two types of errors: those set in sk->sk_err and
those queued onto sk->sk_error_queue. When either is non-zero,
regular socket syscall processing must aborted and the error handled.

Ensure consistent behavior by introducing two new helper functions:

  sock_has_error:    test for both types of errors
  sock_peek_err_skb: extract the first errno from sk_error_queue

and update the existing codepaths to use this. In particular, change
sock_error to call sock_peek_err_skb.

Previously, errnos of queued errors would be mirrored onto sk->sk_err
in some cases to return them. Now that the queue is checked directly,
remove this assignment.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/net/sock.h           | 18 ++++++++++++++++++
 net/bluetooth/af_bluetooth.c |  2 +-
 net/core/datagram.c          |  2 +-
 net/core/skbuff.c            | 21 +++++++++++++++------
 net/core/sock.c              |  2 +-
 net/core/stream.c            |  2 +-
 net/ipv4/tcp.c               | 17 ++++++++---------
 net/iucv/af_iucv.c           |  2 +-
 net/nfc/llcp_sock.c          |  2 +-
 net/rxrpc/ar-output.c        |  2 +-
 net/sctp/socket.c            |  2 +-
 net/unix/af_unix.c           |  2 +-
 12 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 3fde613..c27e2cd 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2041,8 +2041,22 @@ void sk_stop_timer(struct sock *sk, struct timer_list *timer);
 int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
 
 int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb);
+int sock_dequeue_and_peek_err_skb(struct sock *sk, struct sk_buff **skb);
 struct sk_buff *sock_dequeue_err_skb(struct sock *sk);
 
+static inline bool sock_has_error(struct sock *sk)
+{
+	return sk->sk_err || !skb_queue_empty(&sk->sk_error_queue);
+}
+
+static inline int sock_peek_err_skb(struct sock *sk)
+{
+	if (unlikely(!skb_queue_empty(&sk->sk_error_queue)))
+		return sock_dequeue_and_peek_err_skb(sk, NULL);
+
+	return 0;
+}
+
 /*
  *	Recover an error report and clear atomically
  */
@@ -2050,6 +2064,10 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk);
 static inline int sock_error(struct sock *sk)
 {
 	int err;
+
+	err = sock_peek_err_skb(sk);
+	if (err)
+		return err;
 	if (likely(!sk->sk_err))
 		return 0;
 	err = xchg(&sk->sk_err, 0);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 4dca029..345dbc7 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -415,7 +415,7 @@ unsigned int bt_sock_poll(struct file *file, struct socket *sock,
 	if (sk->sk_state == BT_LISTEN)
 		return bt_accept_poll(sk);
 
-	if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
+	if (sock_has_error(sk))
 		mask |= POLLERR |
 			(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0);
 
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 488dd1a..2d585e9 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -851,7 +851,7 @@ unsigned int datagram_poll(struct file *file, struct socket *sock,
 	mask = 0;
 
 	/* exceptional events? */
-	if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
+	if (sock_has_error(sk))
 		mask |= POLLERR |
 			(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0);
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 53ce536..170a076 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3491,20 +3491,29 @@ int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(sock_queue_err_skb);
 
-struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
+int sock_dequeue_and_peek_err_skb(struct sock *sk, struct sk_buff **skb)
 {
 	struct sk_buff_head *q = &sk->sk_error_queue;
-	struct sk_buff *skb, *skb_next;
+	struct sk_buff *skb_next;
 	int err = 0;
 
 	spin_lock_bh(&q->lock);
-	skb = __skb_dequeue(q);
-	if (skb && (skb_next = skb_peek(q)))
+	if (skb)
+		*skb = __skb_dequeue(q);
+	skb_next = skb_peek(q);
+	if (skb_next)
 		err = SKB_EXT_ERR(skb_next)->ee.ee_errno;
 	spin_unlock_bh(&q->lock);
 
-	sk->sk_err = err;
-	if (err)
+	return err;
+}
+EXPORT_SYMBOL(sock_dequeue_and_peek_err_skb);
+
+struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
+{
+	struct sk_buff *skb;
+
+	if (sock_dequeue_and_peek_err_skb(sk, &skb))
 		sk->sk_error_report(sk);
 
 	return skb;
diff --git a/net/core/sock.c b/net/core/sock.c
index f1a638e..b5a7baa 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1739,7 +1739,7 @@ static long sock_wait_for_wmem(struct sock *sk, long timeo)
 			break;
 		if (sk->sk_shutdown & SEND_SHUTDOWN)
 			break;
-		if (sk->sk_err)
+		if (sock_has_error(sk))
 			break;
 		timeo = schedule_timeout(timeo);
 	}
diff --git a/net/core/stream.c b/net/core/stream.c
index 301c05f..1554648 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -129,7 +129,7 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
 
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 
-		if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
+		if (sock_has_error(sk) || (sk->sk_shutdown & SEND_SHUTDOWN))
 			goto do_error;
 		if (!*timeo_p)
 			goto do_nonblock;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 541f26a..4723ff9 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -534,7 +534,7 @@ unsigned int tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
 	}
 	/* This barrier is coupled with smp_wmb() in tcp_reset() */
 	smp_rmb();
-	if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
+	if (sock_has_error(sk))
 		mask |= POLLERR;
 
 	return mask;
@@ -757,10 +757,9 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 				break;
 			if (sock_flag(sk, SOCK_DONE))
 				break;
-			if (sk->sk_err) {
-				ret = sock_error(sk);
+			ret = sock_error(sk);
+			if (ret)
 				break;
-			}
 			if (sk->sk_shutdown & RCV_SHUTDOWN)
 				break;
 			if (sk->sk_state == TCP_CLOSE) {
@@ -791,7 +790,7 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 		release_sock(sk);
 		lock_sock(sk);
 
-		if (sk->sk_err || sk->sk_state == TCP_CLOSE ||
+		if (sock_has_error(sk) || sk->sk_state == TCP_CLOSE ||
 		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
 		    signal_pending(current))
 			break;
@@ -914,7 +913,7 @@ static ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
 	copied = 0;
 
 	err = -EPIPE;
-	if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
+	if (sock_has_error(sk) || (sk->sk_shutdown & SEND_SHUTDOWN))
 		goto out_err;
 
 	while (size > 0) {
@@ -1142,7 +1141,7 @@ int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	copied = 0;
 
 	err = -EPIPE;
-	if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
+	if (sock_has_error(sk) || (sk->sk_shutdown & SEND_SHUTDOWN))
 		goto out_err;
 
 	sg = !!(sk->sk_route_caps & NETIF_F_SG);
@@ -1739,7 +1738,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 			break;
 
 		if (copied) {
-			if (sk->sk_err ||
+			if (sock_has_error(sk) ||
 			    sk->sk_state == TCP_CLOSE ||
 			    (sk->sk_shutdown & RCV_SHUTDOWN) ||
 			    !timeo ||
@@ -1749,7 +1748,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 			if (sock_flag(sk, SOCK_DONE))
 				break;
 
-			if (sk->sk_err) {
+			if (sock_has_error(sk)) {
 				copied = sock_error(sk);
 				break;
 			}
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index a089b6b..5ef473c 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1465,7 +1465,7 @@ unsigned int iucv_sock_poll(struct file *file, struct socket *sock,
 	if (sk->sk_state == IUCV_LISTEN)
 		return iucv_accept_poll(sk);
 
-	if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
+	if (sock_has_error(sk))
 		mask |= POLLERR |
 			(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0);
 
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index 51f077a..06e0411 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -553,7 +553,7 @@ static unsigned int llcp_sock_poll(struct file *file, struct socket *sock,
 	if (sk->sk_state == LLCP_LISTEN)
 		return llcp_accept_poll(sk);
 
-	if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
+	if (sock_has_error(sk))
 		mask |= POLLERR |
 			(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0);
 
diff --git a/net/rxrpc/ar-output.c b/net/rxrpc/ar-output.c
index 0b4b9a7..d663ed8 100644
--- a/net/rxrpc/ar-output.c
+++ b/net/rxrpc/ar-output.c
@@ -544,7 +544,7 @@ static int rxrpc_send_data(struct kiocb *iocb,
 	/* this should be in poll */
 	clear_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
 
-	if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
+	if (sock_has_error(sk) || (sk->sk_shutdown & SEND_SHUTDOWN))
 		return -EPIPE;
 
 	iov = msg->msg_iov;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index eb71d49..6609f17 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6437,7 +6437,7 @@ unsigned int sctp_poll(struct file *file, struct socket *sock, poll_table *wait)
 	mask = 0;
 
 	/* Is there any exceptional events?  */
-	if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
+	if (sock_has_error(sk))
 		mask |= POLLERR |
 			(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0);
 	if (sk->sk_shutdown & RCV_SHUTDOWN)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e968843..e9957fd 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2220,7 +2220,7 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock,
 	mask = 0;
 
 	/* exceptional events? */
-	if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
+	if (sock_has_error(sk))
 		mask |= POLLERR |
 			(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0);
 
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* Re: [PATCH net-next] sock: consistent errqueue errors and signals
From: Willem de Bruijn @ 2014-09-03  2:34 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: Network Development, David Miller
In-Reply-To: <1409692691.15984.16.camel@localhost>

>> I agree, in that it is hard to verify that this does not overwrite
>> an existing error. This patch only makes the behavior
>> consistent between enqueue and dequeue, but perhaps a
>> better way to achieve that is to change the dequeue side:
>> remove the assignment to sk->sk_err there. If so, then all
>> locations that currently check the state of sk->sk_err should
>> be changed to also check the qlen of the error queue and
>> if non-zero return the embedded error of the first skb. I'll
>> take a look whether that is feasible without adding locks
>> or atomics in the common path.
>
> That would be great.

This looks feasible, though the patch is considerably longer.
If we can be sure that in skb_dequeue_err_skb in this patch

+       sk->sk_err = SKB_EXT_ERR(skb)->ee.ee_errno;

never overwrites an existing error, I think that this short
patch is preferable.

I wrote a preliminary alternative patch that modifies
sock_error to check the queue and converts other
code that manually tests sk->sk_err to instead call a new
function sock_has_error that similarly tests both. The patch
is rough and minimally tested, but I will send it out as rfc.

>>
>> > It also depends on socket state bits (e.g. np->recverr) if
>> > the update happens. So we still cannot get rid of the protocol dependent
>> > sk->sk_err updates.
>> >
>> > It looks like we have to check all error handling functions in the
>> > protocols. Maybe timestamp code needs to adapt?
>>
>> Does the above sound okay, or did you mean something else?
>
> Best thing would be to not keep the error status two times per socket.

I agree in principle, but this may be hard to achieve.

> Maybe it would make sense to always synchronize on the error queue and
> don't check for sk->sk_err at all?

sk->sk_err is set in many locations. I don't think that we can change all
those sites.

>It seems to get very hairy without
> taking any locks though.

To fix this particular issue, I prefer to leave it sk->sk_err as is, instead
separate the error queue logic from it and make sure that that has
consistent semantics.

> I even don't know what the semantics for sk_err should be.

Neither do I. That's one reason I'd rather not touch it :)

> Should we
> leave the oldest error in place until it got fetched? Then we could use
> cmpxchg in slow path with 0 as the old value. They could easily become
> unsynchronized if the user switches off recverr setsockopt. But I don't
> think we need to handle that.
>
> I think best effort should would be ok, too. Not having locked
> instructions in fast path is much more important.

We can optimistically test the qlen without a lock and only take the
lock and peek into the queue in the unlikely case that qlen is non-zero.
This is racy, but already done in various poll routines. It seems to be
protected some other way, or the race is deemed benign. Either way,
the patch does not change these callers. For recv/send, which it does
change, best effort seems acceptable to me, too. Perhaps we can do
better, but I'll send out the patch as is for now.

On a related note, a comment in net/core/sock.c states that
sk->sk_err is only changed under lock. This is not currently
true. skb_dequeue_err_skb does not take the socket lock,
for one.

> Thanks,
> Hannes

Thanks for having a look at the patch!
>
>

^ permalink raw reply

* Re: [RFC] net: ipv4: drop unicast encapsulated in L2 multicast
From: YOSHIFUJI Hideaki @ 2014-09-03  1:59 UTC (permalink / raw)
  To: Johannes Berg, Hannes Frederic Sowa
  Cc: hideaki.yoshifuji, linux-wireless, netdev, YOSHIFUJI Hideaki
In-Reply-To: <1409650573.1808.11.camel@jlt4.sipsolutions.net>

Johannes Berg wrote:
> As long as IPv6 doesn't mandate it in the RFCs I'm not really sure we
> should just drop it, even if we think it won't cause any problems?
>
> CLUSTERIP seems like a special configuration, but I'm not sure it can be
> detected and automatically allowed?

Please do not "drop" L2 multicast/broadcast for L3 unicast and
vice versa, unless it is explicitly specified by RFC.

Upper-layer needs to cope eith situation of seeing packets with
"incorrect" L2 header anyway (e.g., in promiscous mode).
I do not see much advantage to drop them here.

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

^ permalink raw reply

* Re: [PATCH net-next 3/4] ipv6: coding style - convert printk
From: YOSHIFUJI Hideaki @ 2014-09-03  1:44 UTC (permalink / raw)
  To: Ian Morris, netdev; +Cc: hideaki.yoshifuji
In-Reply-To: <1409685364-4327-4-git-send-email-ipm@chirality.org.uk>

Hello

Ian Morris wrote:
> No change in the object output is detected by the objdiff script.
> 
> Signed-off-by: Ian Morris <ipm@chirality.org.uk>

Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

^ permalink raw reply

* Re: [PATCH net-next 1/4] ipv6: coding style - no assignment in if statements
From: YOSHIFUJI Hideaki @ 2014-09-03  1:44 UTC (permalink / raw)
  To: Ian Morris, netdev; +Cc: hideaki.yoshifuji, YOSHIFUJI Hideaki
In-Reply-To: <1409685364-4327-2-git-send-email-ipm@chirality.org.uk>

Hi,

Ian Morris wrote:
> This patch makes no changes to the logic of the ipv6 stack however
> it addresses some coding style issues by removing assignments made
> in if statements.
> 
> No change in the object output is detected by the objdiff script.
> 
> Signed-off-by: Ian Morris <ipm@chirality.org.uk>

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index b7a3e7b..86fc687 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -754,9 +754,8 @@ slow_path:
>   		/*
>   		 *	Allocate buffer.
>   		 */
> -
> -		if ((frag = alloc_skb(len + hlen + sizeof(struct frag_hdr) +
> -				      hroom + troom, GFP_ATOMIC)) == NULL) {
> +		frag = alloc_skb(len + hlen + sizeof(struct frag_hdr) + hroom + troom, GFP_ATOMIC);
> +		if (frag == NULL) {

This line is too long.
Otherwise, it seems okay.

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

^ permalink raw reply

* Re: [PATCH net-next 4/4] ipv6: coding style - cleanse bracing
From: YOSHIFUJI Hideaki/吉藤英明 @ 2014-09-03  1:31 UTC (permalink / raw)
  To: Ian Morris, netdev; +Cc: hideaki.yoshifuji, YOSHIFUJI Hideaki
In-Reply-To: <1409685364-4327-5-git-send-email-ipm@chirality.org.uk>

Hi,

Ian Morris wrote:
> Tidy up braces in a few places (mainly if statements):
> * remove unnecessary braces
> * add braces for single line statements where part of an "else" where the other branch has braces

Hmm, I am not for this change.

If "if" (or "else") statement occupies multiple lines, I want to see
braces even if it contains a single statement.
On the other hand, especiallly if the statement occupiles only one line
(and the logic do not require those braces), no braces.

e.g. 1
    if (cond)
            statement();
    else {
            statement2(argument,
                       argument2);
    }
e.g. 2
    if (cond) {
            if (cond2)
                    statement();
    } else
            statement2(argument, argument2);

e.g. 3
    if (cond) {
            /* comment */
            statement();
    }

> 
> No change in the object output is detected by the objdiff script.
> 
> Signed-off-by: Ian Morris <ipm@chirality.org.uk>
> ---
>   net/ipv6/addrconf.c      |   11 +++++------
>   net/ipv6/addrconf_core.c |    3 +--
>   net/ipv6/exthdrs_core.c  |    4 ++--
>   net/ipv6/ip6_fib.c       |    4 ++--
>   net/ipv6/ip6_flowlabel.c |    3 +--
>   net/ipv6/ip6_gre.c       |    4 ++--
>   net/ipv6/ip6_input.c     |   13 +++++--------
>   net/ipv6/ip6_output.c    |   15 +++++++--------
>   net/ipv6/ip6_tunnel.c    |    6 +++---
>   net/ipv6/ip6mr.c         |   13 +++++++------
>   net/ipv6/ipv6_sockglue.c |    7 ++++---
>   net/ipv6/mcast.c         |   48 +++++++++++++++++++++++++++-------------------
>   net/ipv6/ndisc.c         |   18 ++++++-----------
>   net/ipv6/ping.c          |    8 ++++----
>   net/ipv6/route.c         |   25 +++++++++++-------------
>   net/ipv6/tcp_ipv6.c      |   10 ++++++----
>   net/ipv6/udp.c           |   20 +++++++++----------
>   net/ipv6/xfrm6_output.c  |    4 ++--
>   net/ipv6/xfrm6_tunnel.c  |    3 ++-
>   19 files changed, 108 insertions(+), 111 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 23d8493..f31a204 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -895,9 +895,9 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
>   out2:
>   	rcu_read_unlock_bh();
>   
> -	if (likely(err == 0))
> +	if (likely(err == 0)) {
>   		inet6addr_notifier_call_chain(NETDEV_UP, ifa);
> -	else {
> +	} else {
>   		kfree(ifa);
>   		ifa = ERR_PTR(err);
>   	}
> @@ -979,9 +979,9 @@ cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long expires, bool del_r
>   				       ifp->idev->dev,
>   				       0, RTF_GATEWAY | RTF_DEFAULT);
>   	if (rt) {
> -		if (del_rt)
> +		if (del_rt) {
>   			ip6_del_rt(rt);
> -		else {
> +		} else {
>   			if (!(rt->rt6i_flags & RTF_EXPIRES))
>   				rt6_set_expires(rt, expires);
>   			ip6_rt_put(rt);
> @@ -1037,10 +1037,9 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>   
>   	inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);
>   
> -	if (action != CLEANUP_PREFIX_RT_NOP) {
> +	if (action != CLEANUP_PREFIX_RT_NOP)
>   		cleanup_prefix_route(ifp, expires,
>   			action == CLEANUP_PREFIX_RT_DEL);
> -	}
>   
>   	/* clean up prefsrc entries */
>   	rt6_remove_prefsrc(ifp);
> diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
> index e696045..cfa6739 100644
> --- a/net/ipv6/addrconf_core.c
> +++ b/net/ipv6/addrconf_core.c
> @@ -40,12 +40,11 @@ int __ipv6_addr_type(const struct in6_addr *addr)
>   		return (IPV6_ADDR_UNICAST |
>   			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));
>   
> -	if ((st & htonl(0xFF000000)) == htonl(0xFF000000)) {
> +	if ((st & htonl(0xFF000000)) == htonl(0xFF000000))
>   		/* multicast */
>   		/* addr-select 3.1 */
>   		return (IPV6_ADDR_MULTICAST |
>   			ipv6_addr_scope2type(IPV6_ADDR_MC_SCOPE(addr)));
> -	}
>   
>   	if ((st & htonl(0xFFC00000)) == htonl(0xFE800000))
>   		return (IPV6_ADDR_LINKLOCAL | IPV6_ADDR_UNICAST |
> diff --git a/net/ipv6/exthdrs_core.c b/net/ipv6/exthdrs_core.c
> index 2f2945d..4ffc04f 100644
> --- a/net/ipv6/exthdrs_core.c
> +++ b/net/ipv6/exthdrs_core.c
> @@ -264,9 +264,9 @@ int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
>   			if (flags && (*flags & IP6_FH_F_AUTH) && (target < 0))
>   				break;
>   			hdrlen = (hp->hdrlen + 2) << 2;
> -		} else
> +		} else {
>   			hdrlen = ipv6_optlen(hp);
> -
> +		}
>   		if (!found) {
>   			nexthdr = hp->nexthdr;
>   			len -= hdrlen;
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index 76b7f5e..17aceb6 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -334,9 +334,9 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
>   			w->state = FWS_INIT;
>   			w->node = w->root;
>   			w->skip = w->count;
> -		} else
> +		} else {
>   			w->skip = 0;
> -
> +		}
>   		read_lock_bh(&table->tb6_lock);
>   		res = fib6_walk_continue(w);
>   		read_unlock_bh(&table->tb6_lock);
> diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
> index 36da541..c99a031 100644
> --- a/net/ipv6/ip6_flowlabel.c
> +++ b/net/ipv6/ip6_flowlabel.c
> @@ -162,9 +162,8 @@ static void ip6_fl_gc(unsigned long dummy)
>   	}
>   	if (!sched && atomic_read(&fl_size))
>   		sched = now + FL_MAX_LINGER;
> -	if (sched) {
> +	if (sched)
>   		mod_timer(&ip6_fl_gc_timer, sched);
> -	}
>   	spin_unlock(&ip6_fl_lock);
>   }
>   
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 5f19dfb..7f9283f 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -1124,8 +1124,9 @@ static int ip6gre_tunnel_ioctl(struct net_device *dev,
>   			ip6gre_tnl_parm_to_user(&p, &t->parms);
>   			if (copy_to_user(ifr->ifr_ifru.ifru_data, &p, sizeof(p)))
>   				err = -EFAULT;
> -		} else
> +		} else {
>   			err = (cmd == SIOCADDTUNNEL ? -ENOBUFS : -ENOENT);
> +		}
>   		break;
>   
>   	case SIOCDELTUNNEL:
> @@ -1339,7 +1340,6 @@ static int __net_init ip6gre_init_net(struct net *net)
>   	 */
>   	ign->fb_tunnel_dev->features |= NETIF_F_NETNS_LOCAL;
>   
> -
>   	ip6gre_fb_tunnel_init(ign->fb_tunnel_dev);
>   	ign->fb_tunnel_dev->rtnl_link_ops = &ip6gre_link_ops;
>   
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index aacdcb4..ba3a7f8 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -338,25 +338,22 @@ int ip6_mc_input(struct sk_buff *skb)
>   			/* unknown RA - process it normally */
>   		}
>   
> -		if (deliver)
> +		if (deliver) {
>   			skb2 = skb_clone(skb, GFP_ATOMIC);
> -		else {
> +		} else {
>   			skb2 = skb;
>   			skb = NULL;
>   		}
>   
> -		if (skb2) {
> +		if (skb2)
>   			ip6_mr_input(skb2);
> -		}
>   	}
>   out:
>   #endif
>   	if (likely(deliver))
>   		ip6_input(skb);
> -	else {
> -		/* discard */
> -		kfree_skb(skb);
> -	}
> +	else
> +		kfree_skb(skb); /* discard */
>   
>   	return 0;
>   }
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 86fc687..5e520f2 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -275,9 +275,9 @@ static int ip6_forward_proxy_check(struct sk_buff *skb)
>   		offset = ipv6_skip_exthdr(skb, sizeof(*hdr), &nexthdr, &frag_off);
>   		if (offset < 0)
>   			return 0;
> -	} else
> +	} else {
>   		offset = sizeof(struct ipv6hdr);
> -
> +	}
>   	if (nexthdr == IPPROTO_ICMPV6) {
>   		struct icmp6hdr *icmp6;
>   
> @@ -388,7 +388,7 @@ int ip6_forward(struct sk_buff *skb)
>   	/*
>   	 *	We DO NOT make any processing on
>   	 *	RA packets, pushing them to user level AS IS
> -	 *	without ane WARRANTY that application will be able
> +	 *	without any WARRANTY that application will be able
>   	 *	to interpret them. The reason is that we
>   	 *	cannot make anything clever here.
>   	 *
> @@ -741,16 +741,15 @@ slow_path:
>   	/*
>   	 *	Keep copying data until we run out.
>   	 */
> -	while (left > 0)	{
> +	while (left > 0) {
>   		len = left;
>   		/* IF: it doesn't fit, use 'mtu' - the data space left */
>   		if (len > mtu)
>   			len = mtu;
>   		/* IF: we are not sending up to and including the packet end
>   		   then align the next start on an eight byte boundary */
> -		if (len < left)	{
> +		if (len < left)
>   			len &= ~7;
> -		}
>   		/*
>   		 *	Allocate buffer.
>   		 */
> @@ -795,9 +794,9 @@ slow_path:
>   		if (!frag_id) {
>   			ipv6_select_ident(fh, rt);
>   			frag_id = fh->identification;
> -		} else
> +		} else {
>   			fh->identification = frag_id;
> -
> +		}
>   		/*
>   		 *	Copy a block of the IP datagram.
>   		 */
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index b38cc18..488f0dc 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -1362,9 +1362,8 @@ ip6_tnl_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>   			memset(&p, 0, sizeof(p));
>   		}
>   		ip6_tnl_parm_to_user(&p, &t->parms);
> -		if (copy_to_user(ifr->ifr_ifru.ifru_data, &p, sizeof(p))) {
> +		if (copy_to_user(ifr->ifr_ifru.ifru_data, &p, sizeof(p)))
>   			err = -EFAULT;
> -		}
>   		break;
>   	case SIOCADDTUNNEL:
>   	case SIOCCHGTUNNEL:
> @@ -1640,8 +1639,9 @@ static int ip6_tnl_changelink(struct net_device *dev, struct nlattr *tb[],
>   	if (t) {
>   		if (t->dev != dev)
>   			return -EEXIST;
> -	} else
> +	} else {
>   		t = netdev_priv(dev);
> +	}
>   
>   	return ip6_tnl_update(t, &p);
>   }
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index 0171f08..ea054d5 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -853,8 +853,9 @@ static void ip6mr_destroy_unres(struct mr6_table *mrt, struct mfc6_cache *c)
>   			skb_trim(skb, nlh->nlmsg_len);
>   			((struct nlmsgerr *)nlmsg_data(nlh))->error = -ETIMEDOUT;
>   			rtnl_unicast(skb, net, NETLINK_CB(skb).portid);
> -		} else
> +		} else {
>   			kfree_skb(skb);
> +		}
>   	}
>   
>   	ip6mr_cache_free(c);
> @@ -1116,8 +1117,9 @@ static void ip6mr_cache_resolve(struct net *net, struct mr6_table *mrt,
>   				((struct nlmsgerr *)nlmsg_data(nlh))->error = -EMSGSIZE;
>   			}
>   			rtnl_unicast(skb, net, NETLINK_CB(skb).portid);
> -		} else
> +		} else {
>   			ip6_mr_forward(net, mrt, skb, c);
> +		}
>   	}
>   }
>   
> @@ -1591,9 +1593,9 @@ static int ip6mr_sk_init(struct mr6_table *mrt, struct sock *sk)
>   		inet6_netconf_notify_devconf(net, NETCONFA_MC_FORWARDING,
>   					     NETCONFA_IFINDEX_ALL,
>   					     net->ipv6.devconf_all);
> -	}
> -	else
> +	} else {
>   		err = -EADDRINUSE;
> +	}
>   	write_unlock_bh(&mrt_lock);
>   
>   	rtnl_unlock();
> @@ -2406,8 +2408,7 @@ static int mr6_msgsize(bool unresolved, int maxvif)
>   		      + nla_total_size(0)	/* RTA_MULTIPATH */
>   		      + maxvif * NLA_ALIGN(sizeof(struct rtnexthop))
>   						/* RTA_MFC_STATS */
> -		      + nla_total_size(sizeof(struct rta_mfc_stats))
> -		;
> +		      + nla_total_size(sizeof(struct rta_mfc_stats));
>   
>   	return len;
>   }
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index e1a9583..8811f84 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -129,14 +129,15 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
>   	int val, valbool;
>   	int retv = -ENOPROTOOPT;
>   
> -	if (optval == NULL)
> +	if (optval == NULL) {
>   		val = 0;
> -	else {
> +	} else {
>   		if (optlen >= sizeof(int)) {
>   			if (get_user(val, (int __user *) optval))
>   				return -EFAULT;
> -		} else
> +		} else {
>   			val = 0;
> +		}
>   	}
>   
>   	valbool = (val != 0);
> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index 7088179..2eb811e 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -180,9 +180,9 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
>   			dev = rt->dst.dev;
>   			ip6_rt_put(rt);
>   		}
> -	} else
> +	} else {
>   		dev = dev_get_by_index_rcu(net, ifindex);
> -
> +	}
>   	if (dev == NULL) {
>   		rcu_read_unlock();
>   		sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
> @@ -277,9 +277,9 @@ static struct inet6_dev *ip6_mc_find_dev_rcu(struct net *net,
>   			dev = rt->dst.dev;
>   			ip6_rt_put(rt);
>   		}
> -	} else
> +	} else {
>   		dev = dev_get_by_index_rcu(net, ifindex);
> -
> +	}
>   	if (!dev)
>   		return NULL;
>   	idev = __in6_dev_get(dev);
> @@ -318,8 +318,9 @@ void ipv6_sock_mc_close(struct sock *sk)
>   			(void) ip6_mc_leave_src(sk, mc_lst, idev);
>   			if (idev)
>   				__ipv6_dev_mc_dec(idev, &mc_lst->addr);
> -		} else
> +		} else {
>   			(void) ip6_mc_leave_src(sk, mc_lst, NULL);
> +		}
>   		rcu_read_unlock();
>   
>   		atomic_sub(sizeof(*mc_lst), &sk->sk_omem_alloc);
> @@ -537,8 +538,9 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf)
>   		(void) ip6_mc_del_src(idev, group, pmc->sfmode,
>   			psl->sl_count, psl->sl_addr, 0);
>   		sock_kfree_s(sk, psl, IP6_SFLSIZE(psl->sl_max));
> -	} else
> +	} else {
>   		(void) ip6_mc_del_src(idev, group, pmc->sfmode, 0, NULL, 0);
> +	}
>   	pmc->sflist = newpsl;
>   	pmc->sfmode = gsf->gf_fmode;
>   	write_unlock(&pmc->sflock);
> @@ -1359,9 +1361,9 @@ int igmp6_event_query(struct sk_buff *skb)
>   			mlh2 = (struct mld2_query *)skb_transport_header(skb);
>   			mark = 1;
>   		}
> -	} else
> +	} else {
>   		return -EINVAL;
> -
> +	}
>   	read_lock_bh(&idev->lock);
>   	if (group_type == IPV6_ADDR_ANY) {
>   		for (ma = idev->mc_list; ma; ma = ma->next) {
> @@ -1832,8 +1834,9 @@ static void mld_clear_zeros(struct ip6_sf_list **ppsf)
>   			else
>   				*ppsf = psf->sf_next;
>   			kfree(psf);
> -		} else
> +		} else {
>   			psf_prev = psf;
> +		}
>   	}
>   }
>   
> @@ -1875,8 +1878,9 @@ static void mld_send_cr(struct inet6_dev *idev)
>   				idev->mc_tomb = pmc_next;
>   			in6_dev_put(pmc->idev);
>   			kfree(pmc);
> -		} else
> +		} else {
>   			pmc_prev = pmc;
> +		}
>   	}
>   	spin_unlock(&idev->mc_lock);
>   
> @@ -1960,9 +1964,9 @@ static void igmp6_send(struct in6_addr *addr, struct net_device *dev, int type)
>   		 * when a valid link-local address is not available.
>   		 */
>   		saddr = &in6addr_any;
> -	} else
> +	} else {
>   		saddr = &addr_buf;
> -
> +	}
>   	ip6_mc_hdr(sk, skb, dev, saddr, snd_addr, NEXTHDR_HOP, payload_len);
>   
>   	memcpy(skb_put(skb, sizeof(ra)), ra, sizeof(ra));
> @@ -1996,9 +2000,9 @@ out:
>   		ICMP6MSGOUT_INC_STATS(net, idev, type);
>   		ICMP6_INC_STATS(net, idev, ICMP6_MIB_OUTMSGS);
>   		IP6_UPD_PO_STATS(net, idev, IPSTATS_MIB_OUTMCAST, full_len);
> -	} else
> +	} else {
>   		IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);
> -
> +	}
>   	rcu_read_unlock();
>   	return;
>   
> @@ -2087,8 +2091,9 @@ static int ip6_mc_del1_src(struct ifmcaddr6 *pmc, int sfmode,
>   			psf->sf_next = pmc->mca_tomb;
>   			pmc->mca_tomb = psf;
>   			rv = 1;
> -		} else
> +		} else {
>   			kfree(psf);
> +		}
>   	}
>   	return rv;
>   }
> @@ -2143,8 +2148,9 @@ static int ip6_mc_del_src(struct inet6_dev *idev, const struct in6_addr *pmca,
>   		for (psf = pmc->mca_sources; psf; psf = psf->sf_next)
>   			psf->sf_crcount = 0;
>   		mld_ifc_event(pmc->idev);
> -	} else if (sf_setstate(pmc) || changerec)
> +	} else if (sf_setstate(pmc) || changerec) {
>   		mld_ifc_event(pmc->idev);
> +	}
>   	spin_unlock_bh(&pmc->mca_lock);
>   	read_unlock_bh(&idev->lock);
>   	return err;
> @@ -2170,9 +2176,9 @@ static int ip6_mc_add1_src(struct ifmcaddr6 *pmc, int sfmode,
>   			return -ENOBUFS;
>   
>   		psf->sf_addr = *psfsrc;
> -		if (psf_prev) {
> +		if (psf_prev)
>   			psf_prev->sf_next = psf;
> -		} else
> +		else
>   			pmc->mca_sources = psf;
>   	}
>   	psf->sf_count[sfmode]++;
> @@ -2189,8 +2195,9 @@ static void sf_markstate(struct ifmcaddr6 *pmc)
>   			psf->sf_oldin = mca_xcount ==
>   				psf->sf_count[MCAST_EXCLUDE] &&
>   				!psf->sf_count[MCAST_INCLUDE];
> -		} else
> +		} else {
>   			psf->sf_oldin = psf->sf_count[MCAST_INCLUDE] != 0;
> +		}
>   }
>   
>   static int sf_setstate(struct ifmcaddr6 *pmc)
> @@ -2311,8 +2318,9 @@ static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *pmca,
>   		for (psf = pmc->mca_sources; psf; psf = psf->sf_next)
>   			psf->sf_crcount = 0;
>   		mld_ifc_event(idev);
> -	} else if (sf_setstate(pmc))
> +	} else if (sf_setstate(pmc)) {
>   		mld_ifc_event(idev);
> +	}
>   	spin_unlock_bh(&pmc->mca_lock);
>   	read_unlock_bh(&idev->lock);
>   	return err;
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index d0232b2..a38d730 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -303,9 +303,8 @@ static int ndisc_constructor(struct neighbour *neigh)
>   	bool is_multicast = ipv6_addr_is_multicast(addr);
>   
>   	in6_dev = in6_dev_get(dev);
> -	if (in6_dev == NULL) {
> +	if (in6_dev == NULL)
>   		return -EINVAL;
> -	}
>   
>   	parms = in6_dev->nd_parms;
>   	__neigh_parms_put(neigh->parms);
> @@ -604,9 +603,8 @@ void ndisc_send_rs(struct net_device *dev, const struct in6_addr *saddr,
>   		struct inet6_ifaddr *ifp = ipv6_get_ifaddr(dev_net(dev), saddr,
>   							   dev, 1);
>   		if (ifp) {
> -			if (ifp->flags & IFA_F_OPTIMISTIC)  {
> +			if (ifp->flags & IFA_F_OPTIMISTIC)
>   				send_sllao = 0;
> -			}
>   			in6_ifa_put(ifp);
>   		} else {
>   			send_sllao = 0;
> @@ -805,8 +803,9 @@ static void ndisc_recv_ns(struct sk_buff *skb)
>   					pneigh_enqueue(&nd_tbl, idev->nd_parms, n);
>   				goto out;
>   			}
> -		} else
> +		} else {
>   			goto out;
> +		}
>   	}
>   
>   	if (is_router < 0)
> @@ -865,18 +864,15 @@ static void ndisc_recv_na(struct sk_buff *skb)
>   		ND_PRINTK(2, warn, "NA: packet too short\n");
>   		return;
>   	}
> -
>   	if (ipv6_addr_is_multicast(&msg->target)) {
>   		ND_PRINTK(2, warn, "NA: target address is multicast\n");
>   		return;
>   	}
> -
>   	if (ipv6_addr_is_multicast(daddr) &&
>   	    msg->icmph.icmp6_solicited) {
>   		ND_PRINTK(2, warn, "NA: solicited NA is multicasted\n");
>   		return;
>   	}
> -
>   	if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) {
>   		ND_PRINTK(2, warn, "NS: invalid ND option\n");
>   		return;
> @@ -1026,9 +1022,8 @@ static void ndisc_ra_useropt(struct sk_buff *ra, struct nd_opt_hdr *opt)
>   	}
>   
>   	nlh = nlmsg_put(skb, 0, 0, RTM_NEWNDUSEROPT, base_size, 0);
> -	if (nlh == NULL) {
> +	if (nlh == NULL)
>   		goto nla_put_failure;
> -	}
>   
>   	ndmsg = nlmsg_data(nlh);
>   	ndmsg->nduseropt_family = AF_INET6;
> @@ -1376,9 +1371,8 @@ skip_routeinfo:
>   		}
>   	}
>   
> -	if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh) {
> +	if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh)
>   		ND_PRINTK(2, warn, "RA: invalid RA options\n");
> -	}
>   out:
>   	ip6_rt_put(rt);
>   	if (neigh)
> diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
> index 5b7a1ed..b1274f4 100644
> --- a/net/ipv6/ping.c
> +++ b/net/ipv6/ping.c
> @@ -103,13 +103,13 @@ int ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
>   	if (msg->msg_name) {
>   		DECLARE_SOCKADDR(struct sockaddr_in6 *, u, msg->msg_name);
>   		if (msg->msg_namelen < sizeof(struct sockaddr_in6) ||
> -		    u->sin6_family != AF_INET6) {
> +		    u->sin6_family != AF_INET6)
>   			return -EINVAL;
> -		}
> +
>   		if (sk->sk_bound_dev_if &&
> -		    sk->sk_bound_dev_if != u->sin6_scope_id) {
> +		    sk->sk_bound_dev_if != u->sin6_scope_id)
>   			return -EINVAL;
> -		}
> +
>   		daddr = &(u->sin6_addr);
>   		iif = u->sin6_scope_id;
>   	} else {
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f74b041..0b924e3 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -708,9 +708,8 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
>   	unsigned long lifetime;
>   	struct rt6_info *rt;
>   
> -	if (len < sizeof(struct route_info)) {
> +	if (len < sizeof(struct route_info))
>   		return -EINVAL;
> -	}
>   
>   	/* Sanity check for prefix_len and length */
>   	if (rinfo->length > 3) {
> @@ -718,13 +717,11 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
>   	} else if (rinfo->prefix_len > 128) {
>   		return -EINVAL;
>   	} else if (rinfo->prefix_len > 64) {
> -		if (rinfo->length < 2) {
> +		if (rinfo->length < 2)
>   			return -EINVAL;
> -		}
>   	} else if (rinfo->prefix_len > 0) {
> -		if (rinfo->length < 1) {
> +		if (rinfo->length < 1)
>   			return -EINVAL;
> -		}
>   	}
>   
>   	pref = rinfo->route_pref;
> @@ -733,9 +730,9 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
>   
>   	lifetime = addrconf_timeout_fixup(ntohl(rinfo->lifetime), HZ);
>   
> -	if (rinfo->length == 3)
> +	if (rinfo->length == 3) {
>   		prefix = (struct in6_addr *)rinfo->prefix;
> -	else {
> +	} else {
>   		/* this function is safe */
>   		ipv6_addr_prefix(&prefix_buf,
>   				 (struct in6_addr *)rinfo->prefix,
> @@ -2578,13 +2575,13 @@ static int rt6_fill_node(struct net *net,
>   			rtm->rtm_type = RTN_UNREACHABLE;
>   			break;
>   		}
> -	}
> -	else if (rt->rt6i_flags & RTF_LOCAL)
> +	} else if (rt->rt6i_flags & RTF_LOCAL) {
>   		rtm->rtm_type = RTN_LOCAL;
> -	else if (rt->dst.dev && (rt->dst.dev->flags & IFF_LOOPBACK))
> +	} else if (rt->dst.dev && (rt->dst.dev->flags & IFF_LOOPBACK)) {
>   		rtm->rtm_type = RTN_LOCAL;
> -	else
> +	} else {
>   		rtm->rtm_type = RTN_UNICAST;
> +	}
>   	rtm->rtm_flags = 0;
>   	rtm->rtm_scope = RT_SCOPE_UNIVERSE;
>   	rtm->rtm_protocol = rt->rt6i_protocol;
> @@ -2682,9 +2679,9 @@ int rt6_dump_route(struct rt6_info *rt, void *p_arg)
>   	if (nlmsg_len(arg->cb->nlh) >= sizeof(struct rtmsg)) {
>   		struct rtmsg *rtm = nlmsg_data(arg->cb->nlh);
>   		prefix = (rtm->rtm_flags & RTM_F_PREFIX) != 0;
> -	} else
> +	} else {
>   		prefix = 0;
> -
> +	}
>   	return rt6_fill_node(arg->net,
>   		     arg->skb, rt, NULL, NULL, 0, RTM_NEWROUTE,
>   		     NETLINK_CB(arg->cb->skb).portid, arg->cb->nlh->nlmsg_seq,
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 29964c3..2790300 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -454,16 +454,18 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>   			sk->sk_error_report(sk);		/* Wake people up to see the error (see connect in sock.c) */
>   
>   			tcp_done(sk);
> -		} else
> +		} else {
>   			sk->sk_err_soft = err;
> +		}
>   		goto out;
>   	}
>   
>   	if (!sock_owned_by_user(sk) && np->recverr) {
>   		sk->sk_err = err;
>   		sk->sk_error_report(sk);
> -	} else
> +	} else {
>   		sk->sk_err_soft = err;
> +	}
>   
>   out:
>   	bh_unlock_sock(sk);
> @@ -1323,9 +1325,9 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
>   				__kfree_skb(opt_skb);
>   			return 0;
>   		}
> -	} else
> +	} else {
>   		sock_rps_save_rxhash(sk, skb);
> -
> +	}
>   	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len))
>   		goto reset;
>   	if (opt_skb)
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index e06d460..d59c723 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -424,10 +424,10 @@ try_again:
>   			goto csum_copy_err;
>   	}
>   
> -	if (skb_csum_unnecessary(skb))
> +	if (skb_csum_unnecessary(skb)) {
>   		err = skb_copy_datagram_iovec(skb, sizeof(struct udphdr),
>   					      msg->msg_iov, copied);
> -	else {
> +	} else {
>   		err = skb_copy_and_csum_datagram_iovec(skb, sizeof(struct udphdr), msg->msg_iov);
>   		if (err == -EINVAL)
>   			goto csum_copy_err;
> @@ -488,7 +488,6 @@ try_again:
>   		if (np->rxopt.all)
>   			ip6_datagram_recv_specific_ctl(sk, msg, skb);
>   	}
> -
>   	err = copied;
>   	if (flags & MSG_TRUNC)
>   		err = ulen;
> @@ -819,11 +818,10 @@ start_lookup:
>   		goto start_lookup;
>   	}
>   
> -	if (count) {
> +	if (count)
>   		flush_stack(stack, count, skb, count - 1);
> -	} else {
> +	else
>   		kfree_skb(skb);
> -	}
>   	return 0;
>   }
>   
> @@ -1061,9 +1059,10 @@ send:
>   					    UDP_MIB_SNDBUFERRORS, is_udplite);
>   			err = 0;
>   		}
> -	} else
> +	} else {
>   		UDP6_INC_STATS_USER(sock_net(sk),
>   				    UDP_MIB_OUTDATAGRAMS, is_udplite);
> +	}
>   out:
>   	up->len = 0;
>   	up->pending = 0;
> @@ -1119,9 +1118,9 @@ int udpv6_sendmsg(struct kiocb *iocb, struct sock *sk,
>   		if (sk->sk_state != TCP_ESTABLISHED)
>   			return -EDESTADDRREQ;
>   		daddr = &sk->sk_v6_daddr;
> -	} else
> +	} else {
>   		daddr = NULL;
> -
> +	}
>   	if (daddr) {
>   		if (ipv6_addr_v4mapped(daddr)) {
>   			struct sockaddr_in sin;
> @@ -1453,7 +1452,8 @@ int __net_init udp6_proc_init(struct net *net)
>   	return udp_proc_register(net, &udp6_seq_afinfo);
>   }
>   
> -void udp6_proc_exit(struct net *net) {
> +void udp6_proc_exit(struct net *net)
> +{
>   	udp_proc_unregister(net, &udp6_seq_afinfo);
>   }
>   #endif /* CONFIG_PROC_FS */
> diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
> index ca3f29b..f9f745d 100644
> --- a/net/ipv6/xfrm6_output.c
> +++ b/net/ipv6/xfrm6_output.c
> @@ -159,9 +159,9 @@ static int __xfrm6_output(struct sk_buff *skb)
>   
>   	if (x->props.mode == XFRM_MODE_TUNNEL &&
>   	    ((skb->len > mtu && !skb_is_gso(skb)) ||
> -		dst_allfrag(skb_dst(skb)))) {
> +		dst_allfrag(skb_dst(skb))))
>   			return ip6_fragment(skb, x->outer_mode->afinfo->output_finish);
> -	}
> +
>   	return x->outer_mode->afinfo->output_finish(skb);
>   }
>   
> diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c
> index 5743044..2965098 100644
> --- a/net/ipv6/xfrm6_tunnel.c
> +++ b/net/ipv6/xfrm6_tunnel.c
> @@ -180,8 +180,9 @@ __be32 xfrm6_tunnel_alloc_spi(struct net *net, xfrm_address_t *saddr)
>   	if (x6spi) {
>   		atomic_inc(&x6spi->refcnt);
>   		spi = x6spi->spi;
> -	} else
> +	} else {
>   		spi = __xfrm6_tunnel_alloc_spi(net, saddr);
> +	}
>   	spin_unlock_bh(&xfrm6_tunnel_spi_lock);
>   
>   	return htonl(spi);
> 

-- 
吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
ミラクル・リナックス株式会社 技術本部 サポート部

^ permalink raw reply

* Re: [PATCH net-next 2/4] ipv6: coding style - min to min_t conversion
From: YOSHIFUJI Hideaki @ 2014-09-03  1:29 UTC (permalink / raw)
  To: Ian Morris, netdev; +Cc: hideaki.yoshifuji, YOSHIFUJI Hideaki
In-Reply-To: <1409685364-4327-3-git-send-email-ipm@chirality.org.uk>

Hi,

Ian Morris wrote:
> This patch changes a min to min_t as per checkpatch recomendation.
> 
> No change in the object output is detected by the objdiff script.
> 
> Signed-off-by: Ian Morris <ipm@chirality.org.uk>
> ---
>   net/ipv6/addrconf.c |    7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 509c53e..23d8493 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2320,8 +2320,11 @@ ok:
>   			else
>   				stored_lft = 0;
>   			if (!update_lft && !create && stored_lft) {
> -				const u32 minimum_lft = min(
> -					stored_lft, (u32)MIN_VALID_LIFETIME);
> +				const u32 minimum_lft = min_t(
> +							  u32,
> +							  stored_lft,
> +							  MIN_VALID_LIFETIME);
> +
>   				valid_lft = max(valid_lft, minimum_lft);
>   
>   				/* RFC4862 Section 5.5.3e:
> 

Please put 1st argument(u32) at the first line and make arguments aligned.

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

^ permalink raw reply

* [PATCH net-next v5] lib/rhashtable: allow user to set the minimum shifts of shrinking
From: Ying Xue @ 2014-09-03  1:22 UTC (permalink / raw)
  To: tgraf; +Cc: davem, netdev

Although rhashtable library allows user to specify a quiet big size
for user's created hash table, the table may be shrunk to a
very small size - HASH_MIN_SIZE(4) after object is removed from
the table at the first time. Subsequently, even if the total amount
of objects saved in the table is quite lower than user's initial
setting in a long time, the hash table size is still dynamically
adjusted by rhashtable_shrink() or rhashtable_expand() each time
object is inserted or removed from the table. However, as
synchronize_rcu() has to be called when table is shrunk or
expanded by the two functions, we should permit user to set the
minimum table size through configuring the minimum number of shifts
according to user specific requirement, avoiding these expensive
actions of shrinking or expanding because of calling synchronize_rcu().

Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
 include/linux/rhashtable.h |    2 ++
 lib/rhashtable.c           |   12 ++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 36826c0..fb298e9d 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -44,6 +44,7 @@ struct rhashtable;
  * @head_offset: Offset of rhash_head in struct to be hashed
  * @hash_rnd: Seed to use while hashing
  * @max_shift: Maximum number of shifts while expanding
+ * @min_shift: Minimum number of shifts while shrinking
  * @hashfn: Function to hash key
  * @obj_hashfn: Function to hash object
  * @grow_decision: If defined, may return true if table should expand
@@ -57,6 +58,7 @@ struct rhashtable_params {
 	size_t			head_offset;
 	u32			hash_rnd;
 	size_t			max_shift;
+	size_t			min_shift;
 	rht_hashfn_t		hashfn;
 	rht_obj_hashfn_t	obj_hashfn;
 	bool			(*grow_decision)(const struct rhashtable *ht,
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index a2c7881..8dfec3f 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -298,7 +298,7 @@ int rhashtable_shrink(struct rhashtable *ht, gfp_t flags)
 
 	ASSERT_RHT_MUTEX(ht);
 
-	if (tbl->size <= HASH_MIN_SIZE)
+	if (ht->shift <= ht->p.min_shift)
 		return 0;
 
 	ntbl = bucket_table_alloc(tbl->size / 2, flags);
@@ -506,9 +506,10 @@ void *rhashtable_lookup_compare(const struct rhashtable *ht, u32 hash,
 }
 EXPORT_SYMBOL_GPL(rhashtable_lookup_compare);
 
-static size_t rounded_hashtable_size(unsigned int nelem)
+static size_t rounded_hashtable_size(struct rhashtable_params *params)
 {
-	return max(roundup_pow_of_two(nelem * 4 / 3), HASH_MIN_SIZE);
+	return max(roundup_pow_of_two(params->nelem_hint * 4 / 3),
+		   1UL << params->min_shift);
 }
 
 /**
@@ -566,8 +567,11 @@ int rhashtable_init(struct rhashtable *ht, struct rhashtable_params *params)
 	    (!params->key_len && !params->obj_hashfn))
 		return -EINVAL;
 
+	params->min_shift = max_t(size_t, params->min_shift,
+				  ilog2(HASH_MIN_SIZE));
+
 	if (params->nelem_hint)
-		size = rounded_hashtable_size(params->nelem_hint);
+		size = rounded_hashtable_size(params);
 
 	tbl = bucket_table_alloc(size, GFP_KERNEL);
 	if (tbl == NULL)
-- 
1.7.9.5

^ permalink raw reply related

* Re: [net-next PATCH 3/3] qdisc: sysctl to adjust bulk dequeue limit
From: Tom Herbert @ 2014-09-03  0:12 UTC (permalink / raw)
  To: Cong Wang
  Cc: Jesper Dangaard Brouer, David S. Miller, netdev, Florian Westphal,
	Hannes Frederic Sowa, Daniel Borkmann
In-Reply-To: <CAHA+R7MT-HjOwit2pvkeWT2_pYHOupg=0YbTSTVJ8LbC+yhrDg@mail.gmail.com>

On Tue, Sep 2, 2014 at 2:20 PM, Cong Wang <cwang@twopensource.com> wrote:
>
> On Tue, Sep 2, 2014 at 7:36 AM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
> > Allow userspace to adjust how many packet the qdisc is allowed to
> > bulk dequeue.
> >
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >
> > ---
> > Question should we allow this to be adjusted?
> >
>
> A sysctl is ugly and seems not fit well with Qdisc which always uses netlink,
> so I think a netlink flag might be better if we can find a generic one.
>
You could also make this a device specific attribute easily enough.

> Also, you forgot to document it.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net-next] net: gso_skb is now a list, needs a proper destructor
From: Eric Dumazet @ 2014-09-02 23:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

gso_skb can now be a list of skbs.

Qdisc destructors have to take care of it, or we can leak skbs.

Fixes: ce93718fb7cd ("net: Don't keep around original SKB when we software segment GSO frames.")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_generic.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 5b261e91bdbd..19696ebe9ebc 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -621,7 +621,7 @@ void qdisc_reset(struct Qdisc *qdisc)
 		ops->reset(qdisc);
 
 	if (qdisc->gso_skb) {
-		kfree_skb(qdisc->gso_skb);
+		kfree_skb_list(qdisc->gso_skb);
 		qdisc->gso_skb = NULL;
 		qdisc->q.qlen = 0;
 	}
@@ -657,7 +657,7 @@ void qdisc_destroy(struct Qdisc *qdisc)
 	module_put(ops->owner);
 	dev_put(qdisc_dev(qdisc));
 
-	kfree_skb(qdisc->gso_skb);
+	kfree_skb_list(qdisc->gso_skb);
 	/*
 	 * gen_estimator est_timer() might access qdisc->q.lock,
 	 * wait a RCU grace period before freeing qdisc.

^ permalink raw reply related

* iproute2 util: Change signature of rtnl dump filters to use struct params
From: Vadim Kochan @ 2014-09-02 23:46 UTC (permalink / raw)
  To: netdev

Hi all,

I am thinking about to make some refactoring and change the signature
of all rtnl dump filters to form:

diff --git a/include/libnetlink.h b/include/libnetlink.h
index fe7d5d3..9514e67 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -39,8 +39,14 @@ extern int rtnl_dump_request(struct rtnl_handle
*rth, int type, void *req,
                             int len)
        __attribute__((warn_unused_result));

-typedef int (*rtnl_filter_t)(const struct sockaddr_nl *,
-                            struct nlmsghdr *n, void *);
+struct nl_param
+{
+       struct sockaddr_nl *who;
+       struct nlmsghdr *n;
+       void *arg;
+};
+
+typedef int (*rtnl_filter_t)(struct nl_param *p);

 struct rtnl_dump_filter_arg
 {

What do you think?

Regards,

^ permalink raw reply related

* [Patch net v2] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join()
From: Cong Wang @ 2014-09-02 23:31 UTC (permalink / raw)
  To: netdev
  Cc: tt.rantala, Cong Wang, David S. Miller, Hannes Frederic Sowa,
	Sabrina Dubroca

Tommi reported the following RTNL lock assertion failure:

[   77.297196] RTNL: assertion failed at net/ipv6/addrconf.c (1699)
[   77.298080] CPU: 0 PID: 4842 Comm: trinity-main Not tainted 3.17.0-rc2+ #30
[   77.299039] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   77.299789]  ffff88003d76a618 ffff880026133c50 ffffffff8238ba79
ffff880037c84520
[   77.300829]  ffff880026133c90 ffffffff820bd52b 0000000000000000
ffffffff82d86c40
[   77.301869]  0000000000000000 00000000f76fd1e1 ffff8800382d8000
ffff8800382d8220
[   77.302906] Call Trace:
[   77.303246]  [<ffffffff8238ba79>] dump_stack+0x4d/0x66
[   77.303928]  [<ffffffff820bd52b>] addrconf_join_solict+0x4b/0xb0
[   77.304731]  [<ffffffff820b031b>] ipv6_dev_ac_inc+0x2bb/0x330
[   77.305498]  [<ffffffff820b0060>] ? ac6_seq_start+0x260/0x260
[   77.306257]  [<ffffffff820b05fe>] ipv6_sock_ac_join+0x26e/0x360
[   77.307046]  [<ffffffff820b0429>] ? ipv6_sock_ac_join+0x99/0x360
[   77.307798]  [<ffffffff820cdd60>] do_ipv6_setsockopt.isra.5+0xa70/0xf20

This is due to we don't hold rtnl lock when calling addrconf_join_solict()
in ipv6_sock_ac_join(). So hold rtnl lock instead of RCU lock here,
after all it is not a hot path.

Although this warning was directly introduced by commit c15b1ccadb323ea
(ipv6: move DAD and addrconf_verify processing to workqueue), it doesn't
mean older kernels are fine since they may need rtnl lock as well,
but we don't take the risk since no one complains yet.

BTW, mcast _might_ have similar problem, but I don't want to touch it
as no one reports a bug so far.

Reported-by: Tommi Rantala <tt.rantala@gmail.com>
Tested-by: Sabrina Dubroca <sd@queasysnail.net>
Fixes: commit c15b1ccadb323ea ("ipv6: move DAD and addrconf_verify processing to workqueue")
Cc: David S. Miller <davem@davemloft.net>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/linux/netdevice.h |  4 ++--
 net/core/dev.c            | 13 +++++++------
 net/ipv6/anycast.c        | 24 ++++++++++++------------
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 38377392..71838bb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2074,8 +2074,8 @@ void __dev_remove_pack(struct packet_type *pt);
 void dev_add_offload(struct packet_offload *po);
 void dev_remove_offload(struct packet_offload *po);
 
-struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short flags,
-					unsigned short mask);
+struct net_device *__dev_get_by_flags(struct net *net, unsigned short flags,
+				      unsigned short mask);
 struct net_device *dev_get_by_name(struct net *net, const char *name);
 struct net_device *dev_get_by_name_rcu(struct net *net, const char *name);
 struct net_device *__dev_get_by_name(struct net *net, const char *name);
diff --git a/net/core/dev.c b/net/core/dev.c
index ab9a165..343847a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -897,23 +897,24 @@ struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type)
 EXPORT_SYMBOL(dev_getfirstbyhwtype);
 
 /**
- *	dev_get_by_flags_rcu - find any device with given flags
+ *	__dev_get_by_flags - find any device with given flags
  *	@net: the applicable net namespace
  *	@if_flags: IFF_* values
  *	@mask: bitmask of bits in if_flags to check
  *
  *	Search for any interface with the given flags. Returns NULL if a device
  *	is not found or a pointer to the device. Must be called inside
- *	rcu_read_lock(), and result refcount is unchanged.
+ *	rtnl_lock(), and result refcount is unchanged.
  */
 
-struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags,
-				    unsigned short mask)
+struct net_device *__dev_get_by_flags(struct net *net, unsigned short if_flags,
+				      unsigned short mask)
 {
 	struct net_device *dev, *ret;
 
+	ASSERT_RTNL();
 	ret = NULL;
-	for_each_netdev_rcu(net, dev) {
+	for_each_netdev(net, dev) {
 		if (((dev->flags ^ if_flags) & mask) == 0) {
 			ret = dev;
 			break;
@@ -921,7 +922,7 @@ struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags
 	}
 	return ret;
 }
-EXPORT_SYMBOL(dev_get_by_flags_rcu);
+EXPORT_SYMBOL(__dev_get_by_flags);
 
 /**
  *	dev_valid_name - check if name is okay for network device
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 2101832..a1eac55 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -77,7 +77,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 	pac->acl_next = NULL;
 	pac->acl_addr = *addr;
 
-	rcu_read_lock();
+	rtnl_lock();
 	if (ifindex == 0) {
 		struct rt6_info *rt;
 
@@ -90,11 +90,11 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 			goto error;
 		} else {
 			/* router, no matching interface: just pick one */
-			dev = dev_get_by_flags_rcu(net, IFF_UP,
-						   IFF_UP | IFF_LOOPBACK);
+			dev = __dev_get_by_flags(net, IFF_UP,
+						 IFF_UP | IFF_LOOPBACK);
 		}
 	} else
-		dev = dev_get_by_index_rcu(net, ifindex);
+		dev = __dev_get_by_index(net, ifindex);
 
 	if (dev == NULL) {
 		err = -ENODEV;
@@ -136,7 +136,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 	}
 
 error:
-	rcu_read_unlock();
+	rtnl_unlock();
 	if (pac)
 		sock_kfree_s(sk, pac, sizeof(*pac));
 	return err;
@@ -171,11 +171,11 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
 
 	spin_unlock_bh(&ipv6_sk_ac_lock);
 
-	rcu_read_lock();
-	dev = dev_get_by_index_rcu(net, pac->acl_ifindex);
+	rtnl_lock();
+	dev = __dev_get_by_index(net, pac->acl_ifindex);
 	if (dev)
 		ipv6_dev_ac_dec(dev, &pac->acl_addr);
-	rcu_read_unlock();
+	rtnl_unlock();
 
 	sock_kfree_s(sk, pac, sizeof(*pac));
 	return 0;
@@ -198,12 +198,12 @@ void ipv6_sock_ac_close(struct sock *sk)
 	spin_unlock_bh(&ipv6_sk_ac_lock);
 
 	prev_index = 0;
-	rcu_read_lock();
+	rtnl_lock();
 	while (pac) {
 		struct ipv6_ac_socklist *next = pac->acl_next;
 
 		if (pac->acl_ifindex != prev_index) {
-			dev = dev_get_by_index_rcu(net, pac->acl_ifindex);
+			dev = __dev_get_by_index(net, pac->acl_ifindex);
 			prev_index = pac->acl_ifindex;
 		}
 		if (dev)
@@ -211,7 +211,7 @@ void ipv6_sock_ac_close(struct sock *sk)
 		sock_kfree_s(sk, pac, sizeof(*pac));
 		pac = next;
 	}
-	rcu_read_unlock();
+	rtnl_unlock();
 }
 
 static void aca_put(struct ifacaddr6 *ac)
@@ -331,7 +331,7 @@ int __ipv6_dev_ac_dec(struct inet6_dev *idev, const struct in6_addr *addr)
 	return 0;
 }
 
-/* called with rcu_read_lock() */
+/* called with rtnl_lock() */
 static int ipv6_dev_ac_dec(struct net_device *dev, const struct in6_addr *addr)
 {
 	struct inet6_dev *idev = __in6_dev_get(dev);
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] net: Validate frames going through the direct_xmit path
From: Eric Dumazet @ 2014-09-02 23:30 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: netdev, davem
In-Reply-To: <20140902225548.885.79277.stgit@ahduyck-bv4.jf.intel.com>

On Tue, 2014-09-02 at 18:55 -0400, Alexander Duyck wrote:
> In commit 50cbe9ab5f8d92d2d4a327b56e96559d8f63a1fa "net: Validate xmit SKBs
> right when we pull them out of the qdisc" the validation code was moved out
> of dev_hard_start_xmit and into dequeue_skb.  However this overlooked the
> fact that we do not always enqueue the skb onto a qdisc.
> 
> As a result I was seeing issues trying to connect to a vhost_net interface
> after this patch was applied.  To resolve the issue I have added a call to
> validate_xmit_skb in sched_direct_xmit and this seems to have resolved the
> issue by restoring the validation to this xmit path.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>  net/sched/sch_generic.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index a8bf9f9..203ee65 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -128,8 +128,13 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
>  	spin_unlock(root_lock);
>  
>  	HARD_TX_LOCK(dev, txq, smp_processor_id());
> -	if (!netif_xmit_frozen_or_stopped(txq))
> -		skb = dev_hard_start_xmit(skb, dev, txq, &ret);
> +	if (!netif_xmit_frozen_or_stopped(txq)) {
> +		skb = validate_xmit_skb(skb, dev);
> +		if (!skb)
> +			ret = NETDEV_TX_OK;
> +		else
> +			skb = dev_hard_start_xmit(skb, dev, txq, &ret);
> +	}
>  
>  	HARD_TX_UNLOCK(dev, txq);
>  

This looks very weird.

Calling validate_xmit_skb() twice per packet is not needed in the case
sch_direct_xmit() is called from qdisc_restart()

This will add bad branch prediction at very minimum.

This is a TCQ_F_CAN_BYPASS issue that should be fixed there.

^ permalink raw reply

* Re: [Patch net] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join()
From: Cong Wang @ 2014-09-02 23:29 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: Cong Wang, netdev, David S. Miller, Sabrina Dubroca
In-Reply-To: <1409693381.15984.19.camel@localhost>

On Tue, Sep 2, 2014 at 2:29 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hi Cong,
>
> On Di, 2014-09-02 at 11:07 -0700, Cong Wang wrote:
>> @@ -198,7 +198,7 @@ void ipv6_sock_ac_close(struct sock *sk)
>>       spin_unlock_bh(&ipv6_sk_ac_lock);
>>
>>       prev_index = 0;
>> -     rcu_read_lock();
>> +     rtnl_lock();
>>       while (pac) {
>>               struct ipv6_ac_socklist *next = pac->acl_next;
>>
>> @@ -211,7 +211,7 @@ void ipv6_sock_ac_close(struct sock *sk)
>>               sock_kfree_s(sk, pac, sizeof(*pac));
>>               pac = next;
>>       }
>> -     rcu_read_unlock();
>> +     rtnl_unlock();
>>  }
>>
>
> Nit:
> You forgot to convert a dev_get_by_index_rcu to __dev_get_by_index in
> ipv6_sock_ac_close.
>

Good catch! Will update this patch together with "Fixes:" tag.

Thanks.

^ permalink raw reply

* [PATCH] net: Validate frames going through the direct_xmit path
From: Alexander Duyck @ 2014-09-02 22:55 UTC (permalink / raw)
  To: netdev, davem

In commit 50cbe9ab5f8d92d2d4a327b56e96559d8f63a1fa "net: Validate xmit SKBs
right when we pull them out of the qdisc" the validation code was moved out
of dev_hard_start_xmit and into dequeue_skb.  However this overlooked the
fact that we do not always enqueue the skb onto a qdisc.

As a result I was seeing issues trying to connect to a vhost_net interface
after this patch was applied.  To resolve the issue I have added a call to
validate_xmit_skb in sched_direct_xmit and this seems to have resolved the
issue by restoring the validation to this xmit path.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 net/sched/sch_generic.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index a8bf9f9..203ee65 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -128,8 +128,13 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
 	spin_unlock(root_lock);
 
 	HARD_TX_LOCK(dev, txq, smp_processor_id());
-	if (!netif_xmit_frozen_or_stopped(txq))
-		skb = dev_hard_start_xmit(skb, dev, txq, &ret);
+	if (!netif_xmit_frozen_or_stopped(txq)) {
+		skb = validate_xmit_skb(skb, dev);
+		if (!skb)
+			ret = NETDEV_TX_OK;
+		else
+			skb = dev_hard_start_xmit(skb, dev, txq, &ret);
+	}
 
 	HARD_TX_UNLOCK(dev, txq);
 

^ permalink raw reply related

* Re: [PATCH] net: export pkt_type_offset() helper
From: Alexei Starovoitov @ 2014-09-02 22:59 UTC (permalink / raw)
  To: Denis Kirjanov
  Cc: netdev, Markos Chandras, Martin Schwidefsky, Daniel Borkmann
In-Reply-To: <1409688238-3152-1-git-send-email-kda@linux-powerpc.org>

On Wed, Sep 03, 2014 at 12:03:58AM +0400, Denis Kirjanov wrote:
> Currently we have 2 pkt_type_offset functions doing
> the same thing and spread across the architecture files.
> Let's use the generic helper routine.

that would be good cleanup.
Please tag your subject as '[PATCH net-next]...'

> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> Cc: Markos Chandras <markos.chandras@imgtec.com>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Daniel Borkmann <dborkman@redhat.com>
> ---
>  arch/mips/net/bpf_jit.c      | 21 ---------------------
>  arch/s390/net/bpf_jit_comp.c | 38 ++++----------------------------------
>  include/linux/filter.h       |  7 +++++++
>  net/core/filter.c            |  7 +------
>  4 files changed, 12 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
> -#define PKT_TYPE_MAX 0xe0
> -static int pkt_type_offset;
...
> -}
> -device_initcall(bpf_pkt_type_offset_init);

may be make sense to optimize common case via initcall() as well
instead of dropping it for s390?

> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index a5227ab..97e0549 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -424,6 +424,13 @@ static inline void *bpf_load_pointer(const struct sk_buff *skb, int k,
>  	return bpf_internal_load_pointer_neg_helper(skb, k, size);
>  }
>  
> +#ifdef __BIG_ENDIAN_BITFIELD
> +#define PKT_TYPE_MAX	(7 << 5)
> +#else
> +#define PKT_TYPE_MAX    7
> +#endif

these defines don't need to move. keep them in filter.c

^ permalink raw reply

* Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Alexei Starovoitov @ 2014-09-02 22:08 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: Eric Dumazet, Network Development, LKML, Brad Spengler,
	Daniel Borkmann, Kees Cook
In-Reply-To: <1409694182.1034977.162882841.29AA6A80@webmail.messagingengine.com>

On Tue, Sep 2, 2014 at 2:43 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Tue, Sep 2, 2014, at 23:40, Eric Dumazet wrote:
>> On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
>>
>> > > +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
>> > > +{
>> > > +       set_memory_rw((unsigned long)fp, fp->pages);
>> >
>> > why rw is needed?
>> > since fp is allocated with vmalloc, vfree doesn't need
>> > to touch the pages to free them, no?
>>
>> That assumes that vmalloc() do not have any debugging features, like
>> poisoning content before freeing, to catch some use after free.
>>
>> Lets be clean and safe, and give back same memory permission we had
>> after vmalloc()
>
> Yes, I agree. I just went down the kmemleak codepaths and we certainly
> don't want to cause issues in there if the implementation changes one
> day.

agree.
I asked, because skipping set_memory_rw() would have
removed the need for 'struct bpf_work_struct' and complexity
around it.

Simple testing looks good, so:
Acked-by: Alexei Starovoitov <ast@plumgrid.com>

will rebase with all of my stuff and do some more tests.

^ permalink raw reply

* Fw: [Bug 83661] New: CPU hangs on pppd disconnect
From: Stephen Hemminger @ 2014-09-02 22:05 UTC (permalink / raw)
  To: netdev



Begin forwarded message:

Date: Mon, 1 Sep 2014 02:43:35 -0700
From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
To: "stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: [Bug 83661] New: CPU hangs on pppd disconnect


https://bugzilla.kernel.org/show_bug.cgi?id=83661

            Bug ID: 83661
           Summary: CPU hangs on pppd disconnect
           Product: Networking
           Version: 2.5
    Kernel Version: 3.16.1-1-ARCH
          Hardware: x86-64
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: high
          Priority: P1
         Component: Other
          Assignee: shemminger@linux-foundation.org
          Reporter: alex@kurilo.me
        Regression: No

The following appears in the system log when l2tp-connection terminates:


Sep 01 12:16:17 localhost kernel: BUG: soft lockup - CPU#0 stuck for 23s!
[pppd:2320]
Sep 01 12:16:17 localhost kernel: Modules linked in: authenc xfrm_user ah6 ah4
esp6 esp4 xfrm4_mode_beet xfrm4_tunnel xfrm4_mode_tunnel xfrm4_mode_transport
xfrm6_mode_transport xfrm6_mode_ro xfrm6_mode_beet xfrm6_mode_tunnel ipcomp
ipcomp6 xfrm6_tunnel tunnel6 xfrm_ipcomp af_key l2tp_ppp l2tp_netlink l2tp_core
pppoe pppox ppp_generic slhc cmac rmd160 sha1_ssse3 sha1_generic hmac
crypto_null tunnel4 rng_core xfrm_algo camellia_generic camellia_aesni_avx2
camellia_aesni_avx_x86_64 camellia_x86_64 cast6_avx_x86_64 cast6_generic
cast5_avx_x86_64 cast5_generic cast_common deflate cts ctr gcm ccm serpent_avx2
serpent_avx_x86_64 serpent_sse2_x86_64 serpent_generic blowfish_generic
blowfish_x86_64 blowfish_common twofish_generic twofish_avx_x86_64
twofish_x86_64_3way xts twofish_x86_64 twofish_common xcbc cbc md5 sha256_ssse3
Sep 01 12:16:17 localhost kernel:  sha256_generic sha512_ssse3 sha512_generic
des_generic fuse bnep snd_hda_codec_hdmi snd_hda_codec_idt
snd_hda_codec_generic uvcvideo ecb btusb videobuf2_vmalloc videobuf2_memops
videobuf2_core v4l2_common bluetooth videodev 6lowpan_iphc media wl(PO)
snd_hda_intel rtsx_pci_ms i915 snd_hda_controller snd_hda_codec coretemp
rtsx_pci_sdmmc hwmon mmc_core intel_rapl x86_pkg_temp_thermal memstick joydev
mousedev iTCO_wdt iTCO_vendor_support snd_hwdep intel_powerclamp drm_kms_helper
cfg80211 hp_wmi sparse_keymap rfkill ppdev drm snd_pcm kvm_intel rtsx_pci kvm
snd_timer pcspkr evdev crct10dif_pclmul crc32_pclmul crc32c_intel
ghash_clmulni_intel aesni_intel tpm_infineon aes_x86_64 lrw gf128mul
glue_helper ablk_helper cryptd microcode e1000e snd soundcore ptp mac_hid
psmouse serio_raw pps_core
Sep 01 12:16:17 localhost kernel:  mei_me mei hp_accel led_class lis3lv02d
intel_gtt i2c_algo_bit i2c_i801 input_polldev i2c_core ac lpc_ich tpm_tis
battery tpm thermal wmi hp_wireless shpchp parport_pc parport video button
processor ext4 crc16 mbcache jbd2 sd_mod crc_t10dif crct10dif_common atkbd
libps2 ahci libahci libata scsi_mod ehci_pci ehci_hcd i8042 serio xhci_hcd
usbcore usb_common [last unloaded: tunnel6]
Sep 01 12:16:17 localhost kernel: CPU: 0 PID: 2320 Comm: pppd Tainted: P
   O  3.16.1-1-ARCH #1
Sep 01 12:16:17 localhost kernel: Hardware name: Hewlett-Packard HP ProBook 650
G1/1993, BIOS L77 Ver. 01.05 04/29/2014
Sep 01 12:16:17 localhost kernel: task: ffff8804197d8a30 ti: ffff880412f44000
task.ti: ffff880412f44000
Sep 01 12:16:17 localhost kernel: RIP: 0010:[<ffffffff815303a7>]
[<ffffffff815303a7>] _raw_spin_lock_bh+0x47/0x50
Sep 01 12:16:17 localhost kernel: RSP: 0018:ffff880412f47a58  EFLAGS: 00000206
Sep 01 12:16:17 localhost kernel: RAX: 00000000000057dd RBX: 0000000812f47b90
RCX: 0000000000000073
Sep 01 12:16:17 localhost kernel: RDX: 0000000000000072 RSI: 0000000000000002
RDI: ffff880412c20ea0
Sep 01 12:16:17 localhost kernel: RBP: ffff880412f47a58 R08: 0000000000000246
R09: ffff8800af918000
Sep 01 12:16:17 localhost kernel: R10: 000000000000ffff R11: 0000000000000096
R12: ffff880412f47a00
Sep 01 12:16:17 localhost kernel: R13: ffffffffa0f42e43 R14: 0000000000000008
R15: ffff880412f479c0
Sep 01 12:16:17 localhost kernel: FS:  00007f267c484700(0000)
GS:ffff88043fa00000(0000) knlGS:0000000000000000
Sep 01 12:16:17 localhost kernel: CS:  0010 DS: 0000 ES: 0000 CR0:
0000000080050033
Sep 01 12:16:17 localhost kernel: CR2: 0000000002d10000 CR3: 0000000414353000
CR4: 00000000001407f0
Sep 01 12:16:17 localhost kernel: Stack:
Sep 01 12:16:17 localhost kernel:  ffff880412f47af0 ffffffffa0f5c71c
ffff8800af91ada0 ffff8800af91ad10
Sep 01 12:16:17 localhost kernel:  ffff880412cd8a00 ffff880412f47aa0
ffffffff81268e96 ffff880412cd8a60
Sep 01 12:16:17 localhost kernel:  ffff8800af91ad90 ffff880412f47ab0
ffff8800af843900 ffff880412f47af0
Sep 01 12:16:17 localhost kernel: Call Trace:
Sep 01 12:16:17 localhost kernel:  [<ffffffffa0f5c71c>] ppp_push+0x11c/0x600
[ppp_generic]
Sep 01 12:16:17 localhost kernel:  [<ffffffff81268e96>] ?
shash_finup_unaligned+0x26/0x30
Sep 01 12:16:17 localhost kernel:  [<ffffffffa0f32331>] ? hmac_finup+0xa1/0xb0
[hmac]
Sep 01 12:16:17 localhost kernel:  [<ffffffffa0f5d03f>]
ppp_xmit_process+0x43f/0x650 [ppp_generic]
Sep 01 12:16:17 localhost kernel:  [<ffffffff81269686>] ?
shash_ahash_finup+0x56/0xc0
Sep 01 12:16:17 localhost kernel:  [<ffffffffa0f5d359>]
ppp_start_xmit+0x109/0x1d0 [ppp_generic]
Sep 01 12:16:17 localhost kernel:  [<ffffffff81438957>]
dev_hard_start_xmit+0x2f7/0x630
Sep 01 12:16:17 localhost kernel:  [<ffffffff8145a546>]
sch_direct_xmit+0xa6/0x210
Sep 01 12:16:17 localhost kernel:  [<ffffffff81438e6d>]
__dev_queue_xmit+0x1dd/0x4e0
Sep 01 12:16:17 localhost kernel:  [<ffffffff81439180>]
dev_queue_xmit+0x10/0x20
Sep 01 12:16:17 localhost kernel:  [<ffffffff81440401>]
neigh_direct_output+0x11/0x20
Sep 01 12:16:17 localhost kernel:  [<ffffffff8147536c>]
ip_finish_output+0x2ec/0x8e0
Sep 01 12:16:17 localhost kernel:  [<ffffffff814770c8>] ip_output+0x58/0x90
Sep 01 12:16:17 localhost kernel:  [<ffffffff814d0bca>]
xfrm_output_resume+0x39a/0x3c0
Sep 01 12:16:17 localhost kernel:  [<ffffffff814d0c52>] xfrm_output+0x42/0x100
Sep 01 12:16:17 localhost kernel:  [<ffffffff814c5eb2>]
xfrm4_output_finish+0x32/0x40
Sep 01 12:16:17 localhost kernel:  [<ffffffff814c5d6b>]
__xfrm4_output+0x2b/0x50
Sep 01 12:16:17 localhost kernel:  [<ffffffff814c5ef0>] xfrm4_output+0x30/0x70
Sep 01 12:16:17 localhost kernel:  [<ffffffff81476870>]
ip_local_out_sk+0x30/0x40
Sep 01 12:16:17 localhost kernel:  [<ffffffff81476bf3>]
ip_queue_xmit+0x143/0x390
Sep 01 12:16:17 localhost kernel:  [<ffffffffa0f738f6>]
l2tp_xmit_skb+0x2a6/0x540 [l2tp_core]
Sep 01 12:16:17 localhost kernel:  [<ffffffff81423e1c>] ?
skb_free_head+0x6c/0x80
Sep 01 12:16:17 localhost kernel:  [<ffffffffa0f883c7>]
pppol2tp_xmit+0x117/0x230 [l2tp_ppp]
Sep 01 12:16:17 localhost kernel:  [<ffffffffa0f5d470>]
ppp_channel_push+0x50/0xd0 [ppp_generic]
Sep 01 12:16:17 localhost kernel:  [<ffffffffa0f5d5b0>] ppp_write+0xa0/0xf0
[ppp_generic]
Sep 01 12:16:17 localhost kernel:  [<ffffffff811c1f87>] vfs_write+0xb7/0x200
Sep 01 12:16:17 localhost kernel:  [<ffffffff811c2bf9>] SyS_write+0x59/0xd0
Sep 01 12:16:17 localhost kernel:  [<ffffffff81530be9>]
system_call_fastpath+0x16/0x1b
Sep 01 12:16:17 localhost kernel: Code: 0f c1 07 0f b6 cc 38 c1 75 0a 5d c3 0f
1f 84 00 00 00 00 00 b8 00 80 00 00 eb 10 66 0f 1f 84 00 00 00 00 00 f3 90 83
e8 01 74 e9 <0f> b6 17 38 ca 75 f2 5d c3 0f 1f 44 00 00 55 48 89 e5 80 07 01
Sep 01 12:16:27 localhost xl2tpd[1342]: xl2tpd[1342]: Maximum retries exceeded
for tunnel 35511.  Closing.
Sep 01 12:16:27 localhost xl2tpd[1342]: xl2tpd[1342]: Terminating pppd: sending
TERM signal to pid 2320
Sep 01 12:16:27 localhost xl2tpd[1342]: xl2tpd[1342]: Connection 1043 closed to
VPN_SERVER, port 1701 (Timeout)
Sep 01 12:16:32 localhost xl2tpd[1342]: xl2tpd[1342]: Unable to deliver closing
message for tunnel 35511. Destroying anyway.
Sep 01 12:16:45 localhost kernel: BUG: soft lockup - CPU#0 stuck for 23s!
[pppd:2320]

Not sure what's the real cause but circumstances make me thing that it's
network-related.
IP address of the VPN server is replaced with VPN_SERVER and machine's hostname
is changed to localhost.

Please, advice whether this bug should live here or be submitted anywhere else.
I'm not even close to kernel- or C-development, but can provide some additional
information (well, if I understand how to get it).

--
You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* Re: [RFC] net: ipv4: drop unicast encapsulated in L2 multicast
From: David Miller @ 2014-09-02 22:03 UTC (permalink / raw)
  To: hideaki.yoshifuji-GmhWrQMWH5w7YuNMryXyOw
  Cc: johannes-cdvu00un1VgdHxzADdlk8Q,
	hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, yoshfuji-VfPWfsRibaP+Ru+s062T9g
In-Reply-To: <540675F2.1030308-GmhWrQMWH5w7YuNMryXyOw@public.gmane.org>

From: YOSHIFUJI Hideaki <hideaki.yoshifuji-GmhWrQMWH5w7YuNMryXyOw@public.gmane.org>
Date: Wed, 03 Sep 2014 10:59:14 +0900

> Upper-layer needs to cope eith situation of seeing packets with
> "incorrect" L2 header anyway (e.g., in promiscous mode).
> I do not see much advantage to drop them here.

It's required to prevent wireless nodes from using the shared wireless
group keys (used for multicast transmission) to inject unicast frames.

The RFCs really do specify this at least on the ipv4 side.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net-next v4 2/3] ethtool: Add generic options for tunables
From: Govindarajulu Varadarajan @ 2014-09-02 21:47 UTC (permalink / raw)
  To: davem, netdev; +Cc: ssujith, ben, Govindarajulu Varadarajan
In-Reply-To: <1409694441-3843-1-git-send-email-_govind@gmx.com>

This patch adds new ethtool cmd, ETHTOOL_GTUNABLE & ETHTOOL_STUNABLE for getting
tunable values from driver.

Add get_tunable and set_tunable to ethtool_ops. Driver implements these
functions for getting/setting tunable value.

Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
 include/linux/ethtool.h      |  4 +++
 include/uapi/linux/ethtool.h | 28 +++++++++++++++
 net/core/ethtool.c           | 81 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index e658229..c1a2d60 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -257,6 +257,10 @@ struct ethtool_ops {
 				     struct ethtool_eeprom *, u8 *);
 	int	(*get_eee)(struct net_device *, struct ethtool_eee *);
 	int	(*set_eee)(struct net_device *, struct ethtool_eee *);
+	int	(*get_tunable)(struct net_device *,
+			       const struct ethtool_tunable *, void *);
+	int	(*set_tunable)(struct net_device *,
+			       const struct ethtool_tunable *, const void *);
 
 
 };
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index e3c7a71..7a364f2 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -209,6 +209,32 @@ struct ethtool_value {
 	__u32	data;
 };
 
+enum tunable_id {
+	ETHTOOL_ID_UNSPEC,
+	ETHTOOL_RX_COPYBREAK,
+};
+
+enum tunable_type_id {
+	ETHTOOL_TUNABLE_UNSPEC,
+	ETHTOOL_TUNABLE_U8,
+	ETHTOOL_TUNABLE_U16,
+	ETHTOOL_TUNABLE_U32,
+	ETHTOOL_TUNABLE_U64,
+	ETHTOOL_TUNABLE_STRING,
+	ETHTOOL_TUNABLE_S8,
+	ETHTOOL_TUNABLE_S16,
+	ETHTOOL_TUNABLE_S32,
+	ETHTOOL_TUNABLE_S64,
+};
+
+struct ethtool_tunable {
+	__u32	cmd;
+	__u32	id;
+	__u32	type_id;
+	__u32	len;
+	void	*data[0];
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -1152,6 +1178,8 @@ enum ethtool_sfeatures_retval_bits {
 
 #define ETHTOOL_GRSSH		0x00000046 /* Get RX flow hash configuration */
 #define ETHTOOL_SRSSH		0x00000047 /* Set RX flow hash configuration */
+#define ETHTOOL_GTUNABLE	0x00000048 /* Get tunable configuration */
+#define ETHTOOL_STUNABLE	0x00000049 /* Set tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 17cb912..27e61b8 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1621,6 +1621,80 @@ static int ethtool_get_module_eeprom(struct net_device *dev,
 				      modinfo.eeprom_len);
 }
 
+static int ethtool_tunable_valid(const struct ethtool_tunable *tuna)
+{
+	switch (tuna->id) {
+	case ETHTOOL_RX_COPYBREAK:
+		if (tuna->len != sizeof(u32) ||
+		    tuna->type_id != ETHTOOL_TUNABLE_U32)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ethtool_get_tunable(struct net_device *dev, void __user *useraddr)
+{
+	int ret;
+	struct ethtool_tunable tuna;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	void *data;
+
+	if (!ops->get_tunable)
+		return -EOPNOTSUPP;
+	if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+		return -EFAULT;
+	ret = ethtool_tunable_valid(&tuna);
+	if (ret)
+		return ret;
+	data = kmalloc(tuna.len, GFP_USER);
+	if (!data)
+		return -ENOMEM;
+	ret = ops->get_tunable(dev, &tuna, data);
+	if (ret)
+		goto out;
+	useraddr += sizeof(tuna);
+	ret = -EFAULT;
+	if (copy_to_user(useraddr, data, tuna.len))
+		goto out;
+	ret = 0;
+
+out:
+	kfree(data);
+	return ret;
+}
+
+static int ethtool_set_tunable(struct net_device *dev, void __user *useraddr)
+{
+	int ret;
+	struct ethtool_tunable tuna;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	void *data;
+
+	if (!ops->set_tunable)
+		return -EOPNOTSUPP;
+	if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+		return -EFAULT;
+	ret = ethtool_tunable_valid(&tuna);
+	if (ret)
+		return ret;
+	data = kmalloc(tuna.len, GFP_USER);
+	if (!data)
+		return -ENOMEM;
+	useraddr += sizeof(tuna);
+	ret = -EFAULT;
+	if (copy_from_user(data, useraddr, tuna.len))
+		goto out;
+	ret = ops->set_tunable(dev, &tuna, data);
+
+out:
+	kfree(data);
+	return ret;
+}
+
 /* The main entry point in this file.  Called from net/core/dev_ioctl.c */
 
 int dev_ethtool(struct net *net, struct ifreq *ifr)
@@ -1670,6 +1744,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GCHANNELS:
 	case ETHTOOL_GET_TS_INFO:
 	case ETHTOOL_GEEE:
+	case ETHTOOL_GTUNABLE:
 		break;
 	default:
 		if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
@@ -1857,6 +1932,12 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GMODULEEEPROM:
 		rc = ethtool_get_module_eeprom(dev, useraddr);
 		break;
+	case ETHTOOL_GTUNABLE:
+		rc = ethtool_get_tunable(dev, useraddr);
+		break;
+	case ETHTOOL_STUNABLE:
+		rc = ethtool_set_tunable(dev, useraddr);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}
-- 
2.1.0

^ permalink raw reply related

* [PATCH net-next v4 3/3] enic: Add tunable_ops support for rx_copybreak
From: Govindarajulu Varadarajan @ 2014-09-02 21:47 UTC (permalink / raw)
  To: davem, netdev; +Cc: ssujith, ben, Govindarajulu Varadarajan
In-Reply-To: <1409694441-3843-1-git-send-email-_govind@gmx.com>

This patch adds support for setting/getting rx_copybreak using
generic ethtool tunable.

Defines enic_get_tunable() & enic_set_tunable() to get/set rx_copybreak.
As of now, these two function supports only rx_copybreak.

Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
 drivers/net/ethernet/cisco/enic/enic_ethtool.c | 39 ++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/net/ethernet/cisco/enic/enic_ethtool.c b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
index 523c9ce..85173d6 100644
--- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
+++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
@@ -379,6 +379,43 @@ static int enic_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
 	return ret;
 }
 
+static int enic_get_tunable(struct net_device *dev,
+			    const struct ethtool_tunable *tuna, void *data)
+{
+	struct enic *enic = netdev_priv(dev);
+	int ret = 0;
+
+	switch (tuna->id) {
+	case ETHTOOL_RX_COPYBREAK:
+		*(u32 *)data = enic->rx_copybreak;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static int enic_set_tunable(struct net_device *dev,
+			    const struct ethtool_tunable *tuna,
+			    const void *data)
+{
+	struct enic *enic = netdev_priv(dev);
+	int ret = 0;
+
+	switch (tuna->id) {
+	case ETHTOOL_RX_COPYBREAK:
+		enic->rx_copybreak = *(u32 *)data;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
 static const struct ethtool_ops enic_ethtool_ops = {
 	.get_settings = enic_get_settings,
 	.get_drvinfo = enic_get_drvinfo,
@@ -391,6 +428,8 @@ static const struct ethtool_ops enic_ethtool_ops = {
 	.get_coalesce = enic_get_coalesce,
 	.set_coalesce = enic_set_coalesce,
 	.get_rxnfc = enic_get_rxnfc,
+	.get_tunable = enic_get_tunable,
+	.set_tunable = enic_set_tunable,
 };
 
 void enic_set_ethtool_ops(struct net_device *netdev)
-- 
2.1.0

^ permalink raw reply related

* [PATCH net-next v4 1/3] enic: implement rx_copybreak
From: Govindarajulu Varadarajan @ 2014-09-02 21:47 UTC (permalink / raw)
  To: davem, netdev; +Cc: ssujith, ben, Govindarajulu Varadarajan
In-Reply-To: <1409694441-3843-1-git-send-email-_govind@gmx.com>

Calling dma_map_single()/dma_unmap_single() is quite expensive compared
to copying a small packet. So let's copy short frames and keep the buffers
mapped.

Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
 drivers/net/ethernet/cisco/enic/enic.h      |  1 +
 drivers/net/ethernet/cisco/enic/enic_main.c | 50 +++++++++++++++++++++++++++--
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 962510f..5ba5ad0 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -186,6 +186,7 @@ struct enic {
 	____cacheline_aligned struct vnic_cq cq[ENIC_CQ_MAX];
 	unsigned int cq_count;
 	struct enic_rfs_flw_tbl rfs_h;
+	u32 rx_copybreak;
 };
 
 static inline struct device *enic_get_dev(struct enic *enic)
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index c8832bc..929bfe7 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -66,6 +66,8 @@
 #define PCI_DEVICE_ID_CISCO_VIC_ENET_DYN     0x0044  /* enet dynamic vnic */
 #define PCI_DEVICE_ID_CISCO_VIC_ENET_VF      0x0071  /* enet SRIOV VF */
 
+#define RX_COPYBREAK_DEFAULT		256
+
 /* Supported devices */
 static const struct pci_device_id enic_id_table[] = {
 	{ PCI_VDEVICE(CISCO, PCI_DEVICE_ID_CISCO_VIC_ENET) },
@@ -924,6 +926,7 @@ static void enic_free_rq_buf(struct vnic_rq *rq, struct vnic_rq_buf *buf)
 	pci_unmap_single(enic->pdev, buf->dma_addr,
 		buf->len, PCI_DMA_FROMDEVICE);
 	dev_kfree_skb_any(buf->os_buf);
+	buf->os_buf = NULL;
 }
 
 static int enic_rq_alloc_buf(struct vnic_rq *rq)
@@ -934,7 +937,24 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
 	unsigned int len = netdev->mtu + VLAN_ETH_HLEN;
 	unsigned int os_buf_index = 0;
 	dma_addr_t dma_addr;
+	struct vnic_rq_buf *buf = rq->to_use;
+
+	if (buf->os_buf) {
+		buf = buf->next;
+		rq->to_use = buf;
+		rq->ring.desc_avail--;
+		if ((buf->index & VNIC_RQ_RETURN_RATE) == 0) {
+			/* Adding write memory barrier prevents compiler and/or
+			 * CPU reordering, thus avoiding descriptor posting
+			 * before descriptor is initialized. Otherwise, hardware
+			 * can read stale descriptor fields.
+			 */
+			wmb();
+			iowrite32(buf->index, &rq->ctrl->posted_index);
+		}
 
+		return 0;
+	}
 	skb = netdev_alloc_skb_ip_align(netdev, len);
 	if (!skb)
 		return -ENOMEM;
@@ -957,6 +977,25 @@ static void enic_intr_update_pkt_size(struct vnic_rx_bytes_counter *pkt_size,
 		pkt_size->small_pkt_bytes_cnt += pkt_len;
 }
 
+static bool enic_rxcopybreak(struct net_device *netdev, struct sk_buff **skb,
+			     struct vnic_rq_buf *buf, u16 len)
+{
+	struct enic *enic = netdev_priv(netdev);
+	struct sk_buff *new_skb;
+
+	if (len > enic->rx_copybreak)
+		return false;
+	new_skb = netdev_alloc_skb_ip_align(netdev, len);
+	if (!new_skb)
+		return false;
+	pci_dma_sync_single_for_cpu(enic->pdev, buf->dma_addr, len,
+				    DMA_FROM_DEVICE);
+	memcpy(new_skb->data, (*skb)->data, len);
+	*skb = new_skb;
+
+	return true;
+}
+
 static void enic_rq_indicate_buf(struct vnic_rq *rq,
 	struct cq_desc *cq_desc, struct vnic_rq_buf *buf,
 	int skipped, void *opaque)
@@ -978,9 +1017,6 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
 		return;
 
 	skb = buf->os_buf;
-	prefetch(skb->data - NET_IP_ALIGN);
-	pci_unmap_single(enic->pdev, buf->dma_addr,
-		buf->len, PCI_DMA_FROMDEVICE);
 
 	cq_enet_rq_desc_dec((struct cq_enet_rq_desc *)cq_desc,
 		&type, &color, &q_number, &completed_index,
@@ -1011,6 +1047,13 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
 		/* Good receive
 		 */
 
+		if (!enic_rxcopybreak(netdev, &skb, buf, bytes_written)) {
+			buf->os_buf = NULL;
+			pci_unmap_single(enic->pdev, buf->dma_addr, buf->len,
+					 PCI_DMA_FROMDEVICE);
+		}
+		prefetch(skb->data - NET_IP_ALIGN);
+
 		skb_put(skb, bytes_written);
 		skb->protocol = eth_type_trans(skb, netdev);
 		skb_record_rx_queue(skb, q_number);
@@ -2531,6 +2574,7 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev_err(dev, "Cannot register net device, aborting\n");
 		goto err_out_dev_deinit;
 	}
+	enic->rx_copybreak = RX_COPYBREAK_DEFAULT;
 
 	return 0;
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH net-next v4 0/3] enic: Add support for rx_copybreak
From: Govindarajulu Varadarajan @ 2014-09-02 21:47 UTC (permalink / raw)
  To: davem, netdev; +Cc: ssujith, ben, Govindarajulu Varadarajan

The following series implements rx_copybreak.

dma_map_single()/dma_unmap_single() is more expensive than alloc_skb & memcpy
for smaller packets. By doing this we can reuse the dma buff which is already
mapped. This is very useful when iommu is on. The default skb copybreak value
is 256.

When iommu is on, we can go much higher than 256. All the drivers that supports
rx_copybreak provides module parameter to change this value. Since module
parameter is the least preferred way for changing driver values, this series
adds ethtool support for setting rx_copybreak.

v4:
Validate tunable length in ethtool_get_tunable, not in driver implemented
function.

Loose tunable_ops array for each tunable type. Define one function and let the
driver use switch case for each type.

Use double underscore for data type in UAPI headers.
Use const qualifier where possible.

v3:
Add tunable namespace to ethtool. Use new ethtool cmd ETHTOOL_S/GTUNABLE to
set/get rx_copybreak from userspace.

v2:
Add new ethtool_cmd for DMA buffer parameters, instead of adding new members to
existing ethtool_ringparam.

Govindarajulu Varadarajan (3):
  enic: implement rx_copybreak
  ethtool: Add generic options for tunables
  enic: Add tunable_ops support for rx_copybreak

 drivers/net/ethernet/cisco/enic/enic.h         |  1 +
 drivers/net/ethernet/cisco/enic/enic_ethtool.c | 39 +++++++++++++
 drivers/net/ethernet/cisco/enic/enic_main.c    | 50 +++++++++++++++-
 include/linux/ethtool.h                        |  4 ++
 include/uapi/linux/ethtool.h                   | 28 +++++++++
 net/core/ethtool.c                             | 81 ++++++++++++++++++++++++++
 6 files changed, 200 insertions(+), 3 deletions(-)

-- 
2.1.0

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).