* Re: [PATCH net] xfrm: Release dst if this dst is improper for vti tunnel
From: David Miller @ 2013-11-19 20:51 UTC (permalink / raw)
To: fan.du; +Cc: steffen.klassert, saurabh.mohan, netdev
In-Reply-To: <1384851208-30353-1-git-send-email-fan.du@windriver.com>
From: Fan Du <fan.du@windriver.com>
Date: Tue, 19 Nov 2013 16:53:28 +0800
> After searching rt by the vti tunnel dst/src parameter,
> if this rt has neither attached to any transformation
> nor the transformation is not tunnel oriented, this rt
> should be released back to ip layer.
>
> otherwise causing dst memory leakage.
>
> Signed-off-by: Fan Du <fan.du@windriver.com>
Good catch, applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Michael S. Tsirkin @ 2013-11-19 20:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: Michael Dalton, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <1384869828.8604.97.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 19, 2013 at 06:03:48AM -0800, Eric Dumazet wrote:
> On Tue, 2013-11-19 at 16:05 +0800, Jason Wang wrote:
> > We need to drop the refcnt of page when we fail to allocate an skb for frag
> > list, otherwise it will be leaked. The bug was introduced by commit
> > 2613af0ed18a11d5c566a81f9a6510b73180660a ("virtio_net: migrate mergeable rx
> > buffers to page frag allocators").
> >
> > Cc: Michael Dalton <mwdalton@google.com>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> > The patch was needed for 3.12 stable.
>
> Good catch, but if we return from receive_mergeable() in the 'middle'
> of the frags we would need for the current skb, who will
> call the virtqueue_get_buf() to flush the remaining frags ?
>
> Don't we also need to call virtqueue_get_buf() like
>
> while (--num_buf) {
> buf = virtqueue_get_buf(rq->vq, &len);
> if (!buf)
> break;
> put_page(virt_to_head_page(buf));
> }
>
> ?
>
>
Let me explain what worries me in your suggestion:
struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
if (unlikely(!nskb)) {
head_skb->dev->stats.rx_dropped++;
return -ENOMEM;
}
is this the failure case we are talking about?
I think this is a symprom of a larger problem
introduced by 2613af0ed18a11d5c566a81f9a6510b73180660a,
namely that we now need to allocate memory in the
middle of processing a packet.
I think discarding a completely valid and well-formed
packet from the receive queue because we are unable
to allocate new memory with GFP_ATOMIC
for future packets is not a good idea.
It certainly violates the principle of least surprize:
when one sees host pass packet to guest, one expects
the packet to get into the networking stack, not get
dropped by the driver internally.
Guest stack can do with the packet what it sees fit.
We actually wake up a thread if we can't fill up the queue,
that will fill it up in GFP_KERNEL context.
So I think we should find a way to pre-allocate if necessary and avoid
error paths where allocating new memory is a required to avoid drops.
--
MST
^ permalink raw reply
* Re: [PATCH RFC] net: rework recvmsg handler msg_name and msg_namelen logic
From: David Miller @ 2013-11-19 20:40 UTC (permalink / raw)
To: hannes; +Cc: netdev
In-Reply-To: <20131119052759.GR16541@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 19 Nov 2013 06:27:59 +0100
> This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
> set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
> to return msg_name to the user.
>
> This prevents numerous uninitialized memory leaks we had in the
> recvmsg handlers and makes it harder for new code to accidentally leak
> uninitialized memory.
>
> Optimize for the case recvfrom is called with NULL as address. We don't
> need to copy the address at all, so set it to NULL before invoking the
> recvmsg handler. We can do so, because all the recvmsg handlers must
> cope with the case a plain read() is called on them. read() also sets
> msg_name to NULL.
>
> Also document these changes in include/linux/net.h as suggested by David
> Miller.
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Looks good to me.
The only other invoker of ->recvmsg is vhost.c but it sets msg_name to NULL
and msg_namelen to 0 so should be fine.
^ permalink raw reply
* Re: [PATCH net v4 3/4] r8152: support stopping/waking tx queue
From: David Miller @ 2013-11-19 20:25 UTC (permalink / raw)
To: hayeswang-Rasf1IRRPZFBDgjK7y7TUQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1384831511-1625-4-git-send-email-hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
From: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
Date: Tue, 19 Nov 2013 11:25:10 +0800
> The maximum packet number which a tx aggregation buffer could contain
> is the buffer size / (packet size + descriptor size).
>
> If the tx buffer is empty and the tx queue length is more than the
> maximum value which is defined above, stop the tx queue. Wake the tx
> queue after any queued packet is filled in a available tx buffer.
>
> Signed-off-by: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
This is racy.
You have nothing which synchronizes r8152_tx_agg_fill() and rtl8152_start_xmit(),
therefore:
> + if (netif_queue_stopped(tp->netdev))
> + netif_wake_queue(tp->netdev);
> +
A netif_stop_queue() can occur right after the netif_queue_stopped() check,
meaning you can end up with the queue being stopped forever and the TX queue
stuck.
I am finding this patch series _very_ tiring, every single time I take
the time to review it, I find major problems.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: [PATCH] netlink: fix documentation typo in netlink_set_err()
From: David Miller @ 2013-11-19 20:07 UTC (permalink / raw)
To: johannes; +Cc: netdev, johannes.berg
In-Reply-To: <1384853740-21404-1-git-send-email-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 19 Nov 2013 10:35:40 +0100
> From: Johannes Berg <johannes.berg@intel.com>
>
> The parameter is just 'group', not 'groups', fix the documentation typo.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Michael S. Tsirkin @ 2013-11-19 18:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: Michael Dalton, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <1384869828.8604.97.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 19, 2013 at 06:03:48AM -0800, Eric Dumazet wrote:
> On Tue, 2013-11-19 at 16:05 +0800, Jason Wang wrote:
> > We need to drop the refcnt of page when we fail to allocate an skb for frag
> > list, otherwise it will be leaked. The bug was introduced by commit
> > 2613af0ed18a11d5c566a81f9a6510b73180660a ("virtio_net: migrate mergeable rx
> > buffers to page frag allocators").
> >
> > Cc: Michael Dalton <mwdalton@google.com>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> > The patch was needed for 3.12 stable.
>
> Good catch, but if we return from receive_mergeable() in the 'middle'
> of the frags we would need for the current skb, who will
> call the virtqueue_get_buf() to flush the remaining frags ?
>
> Don't we also need to call virtqueue_get_buf() like
>
> while (--num_buf) {
> buf = virtqueue_get_buf(rq->vq, &len);
> if (!buf)
> break;
> put_page(virt_to_head_page(buf));
> }
>
> ?
>
>
>
virtqueue_get_buf only gives you back a buffer that has been DMA-ed
to by hardware. ATM there's no way to get back a buffer once you
gave it to hardware without doing a NIC reset.
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-19 18:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: Thomas Petazzoni, Florian Fainelli, simon.guinot, netdev,
Arnaud Ebalard, edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <1384885910.8604.110.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 19, 2013 at 10:31:50AM -0800, Eric Dumazet wrote:
> On Tue, 2013-11-19 at 18:43 +0100, Willy Tarreau wrote:
>
> > - #define MVNETA_TX_DONE_TIMER_PERIOD 10
> > + #define MVNETA_TX_DONE_TIMER_PERIOD (1000/HZ)
> >
>
> I suggested this in a prior mail :
>
> #define MVNETA_TX_DONE_TIMER_PERIOD 1
Ah sorry, I remember now.
> But apparently it was triggering strange crashes...
Ah, when a bug hides another one, it's the situation I prefer, because
by working on one, you end up fixing two :-)
Cheers,
Willy
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Eric Dumazet @ 2013-11-19 18:31 UTC (permalink / raw)
To: Willy Tarreau
Cc: Arnaud Ebalard, Thomas Petazzoni, netdev, edumazet, Cong Wang,
linux-arm-kernel, Florian Fainelli, simon.guinot
In-Reply-To: <20131119174323.GH913@1wt.eu>
On Tue, 2013-11-19 at 18:43 +0100, Willy Tarreau wrote:
> - #define MVNETA_TX_DONE_TIMER_PERIOD 10
> + #define MVNETA_TX_DONE_TIMER_PERIOD (1000/HZ)
>
I suggested this in a prior mail :
#define MVNETA_TX_DONE_TIMER_PERIOD 1
But apparently it was triggering strange crashes...
> This can only have any effect if you run at 250 or 1000 Hz, but not at 100
> of course. It should reduce the time to first IRQ.
^ permalink raw reply
* Re: [PATCH] tcp: don't update snd_nxt, when a socket is switched from repair mode
From: Eric Dumazet @ 2013-11-19 18:29 UTC (permalink / raw)
To: Andrey Vagin
Cc: netdev, linux-kernel, criu, Pavel Emelyanov, Eric Dumazet,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <1384884606-9978-1-git-send-email-avagin@openvz.org>
On Tue, 2013-11-19 at 22:10 +0400, Andrey Vagin wrote:
> snd_nxt must be updated synchronously with sk_send_head. Otherwise
> tp->packets_out may be updated incorrectly, what may bring a kernel panic.
>
> Here is a kernel panic from my host.
> [ 103.043194] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
> [ 103.044025] IP: [<ffffffff815aaaaf>] tcp_rearm_rto+0xcf/0x150
> ...
> [ 146.301158] Call Trace:
> [ 146.301158] [<ffffffff815ab7f0>] tcp_ack+0xcc0/0x12c0
>
> Before this panic a tcp socket was restored. This socket had sent and
> unsent data in the write queue. Sent data was restored in repair mode,
> then the socket was switched from reapair mode and unsent data was
> restored. After that the socket was switched back into repair mode.
>
> In that moment we had a socket where write queue looks like this:
> snd_una snd_nxt write_seq
> |_________|________|
> |
> sk_send_head
>
> After a second switching from repair mode the state of socket was
> changed:
>
> snd_una snd_nxt, write_seq
> |_________ ________|
> |
> sk_send_head
>
> This state is inconsistent, because snd_nxt and sk_send_head are not
> synchronized.
>
> Bellow you can find a call trace, how packets_out can be incremented
> twice for one skb, if snd_nxt and sk_send_head are not synchronized.
> In this case packets_out will be always positive, even when
> sk_write_queue is empty.
>
> tcp_write_wakeup
> skb = tcp_send_head(sk);
> tcp_fragment
> if (!before(tp->snd_nxt, TCP_SKB_CB(buff)->end_seq))
> tcp_adjust_pcount(sk, skb, diff);
> tcp_event_new_data_sent
> tp->packets_out += tcp_skb_pcount(skb);
>
> I think update of snd_nxt isn't required, when a socket is switched from
> repair mode. Because it's initialized in tcp_connect_init. Then when a
> write queue is restored, snd_nxt is incremented in tcp_event_new_data_sent,
> so it's always is in consistent state.
>
> I have checked, that the bug is not reproduced with this patch and
> all tests about restoring tcp connections work fine.
>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: James Morris <jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
> net/ipv4/tcp_output.c | 1 -
> 1 file changed, 1 deletion(-)
Fixes: c0e88ff0f256 ("tcp: Repair socket queues")
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH] tcp: don't update snd_nxt, when a socket is switched from repair mode
From: Pavel Emelyanov @ 2013-11-19 18:20 UTC (permalink / raw)
To: Andrey Vagin, David S. Miller
Cc: netdev, linux-kernel, criu, Eric Dumazet, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <1384884606-9978-1-git-send-email-avagin@openvz.org>
On 11/19/2013 10:10 PM, Andrey Vagin wrote:
>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: James Morris <jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
Acked-by: Pavel Emelyanov <xemul@parallels.com>
> ---
> net/ipv4/tcp_output.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 6728546..fa9f57d 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -3093,7 +3093,6 @@ void tcp_send_window_probe(struct sock *sk)
> {
> if (sk->sk_state == TCP_ESTABLISHED) {
> tcp_sk(sk)->snd_wl1 = tcp_sk(sk)->rcv_nxt - 1;
> - tcp_sk(sk)->snd_nxt = tcp_sk(sk)->write_seq;
> tcp_xmit_probe_skb(sk, 0);
> }
> }
>
^ permalink raw reply
* [PATCH] tcp: don't update snd_nxt, when a socket is switched from repair mode
From: Andrey Vagin @ 2013-11-19 18:10 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, criu, Andrey Vagin, Pavel Emelyanov, Eric Dumazet,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
snd_nxt must be updated synchronously with sk_send_head. Otherwise
tp->packets_out may be updated incorrectly, what may bring a kernel panic.
Here is a kernel panic from my host.
[ 103.043194] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
[ 103.044025] IP: [<ffffffff815aaaaf>] tcp_rearm_rto+0xcf/0x150
...
[ 146.301158] Call Trace:
[ 146.301158] [<ffffffff815ab7f0>] tcp_ack+0xcc0/0x12c0
Before this panic a tcp socket was restored. This socket had sent and
unsent data in the write queue. Sent data was restored in repair mode,
then the socket was switched from reapair mode and unsent data was
restored. After that the socket was switched back into repair mode.
In that moment we had a socket where write queue looks like this:
snd_una snd_nxt write_seq
|_________|________|
|
sk_send_head
After a second switching from repair mode the state of socket was
changed:
snd_una snd_nxt, write_seq
|_________ ________|
|
sk_send_head
This state is inconsistent, because snd_nxt and sk_send_head are not
synchronized.
Bellow you can find a call trace, how packets_out can be incremented
twice for one skb, if snd_nxt and sk_send_head are not synchronized.
In this case packets_out will be always positive, even when
sk_write_queue is empty.
tcp_write_wakeup
skb = tcp_send_head(sk);
tcp_fragment
if (!before(tp->snd_nxt, TCP_SKB_CB(buff)->end_seq))
tcp_adjust_pcount(sk, skb, diff);
tcp_event_new_data_sent
tp->packets_out += tcp_skb_pcount(skb);
I think update of snd_nxt isn't required, when a socket is switched from
repair mode. Because it's initialized in tcp_connect_init. Then when a
write queue is restored, snd_nxt is incremented in tcp_event_new_data_sent,
so it's always is in consistent state.
I have checked, that the bug is not reproduced with this patch and
all tests about restoring tcp connections work fine.
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
net/ipv4/tcp_output.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 6728546..fa9f57d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3093,7 +3093,6 @@ void tcp_send_window_probe(struct sock *sk)
{
if (sk->sk_state == TCP_ESTABLISHED) {
tcp_sk(sk)->snd_wl1 = tcp_sk(sk)->rcv_nxt - 1;
- tcp_sk(sk)->snd_nxt = tcp_sk(sk)->write_seq;
tcp_xmit_probe_skb(sk, 0);
}
}
--
1.8.3.1
^ permalink raw reply related
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-19 17:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: Thomas Petazzoni, Florian Fainelli, simon.guinot, netdev,
Arnaud Ebalard, edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <1384869194.8604.92.camel@edumazet-glaptop2.roam.corp.google.com>
Hi Eric,
On Tue, Nov 19, 2013 at 05:53:14AM -0800, Eric Dumazet wrote:
> These strange results you have tend to show that if you have a big TCP
> send window, the web server pushes a lot of bytes per system call and
> might stall the ACK clocking or TX refills.
It's tx refills which are not done in this case from what I think I
understood in the driver. IIRC, the refill is done once at the beginning
of xmit and in the tx timer callback. So if you have too large a window
that fills the descriptors during a few tx calls during which no desc was
released, you could end up having to wait for the timer since you're not
allowed to send anymore.
> > Then, I started playing w/ tcp_limit_output_bytes (default is 131072),
> > w/ TCP send window set to 256KB:
> >
> > tcp_limit_output_bytes set to 512KB: 59.3 MB/s
> > tcp_limit_output_bytes set to 256KB: 58.5 MB/s
> > tcp_limit_output_bytes set to 128KB: 56.2 MB/s
> > tcp_limit_output_bytes set to 64KB: 32.1 MB/s
> > tcp_limit_output_bytes set to 32KB: 4.76 MB/s
> >
> > As a side note, during the test, I sometimes gets peak for some seconds
> > at the beginning at 90MB/s which tend to confirm what WIlly wrote,
> > i.e. that the hardware can do more.
>
> I would also check the receiver. I suspect packets drops because of a
> bad driver doing skb->truesize overshooting.
When I first observed the issue, at first I suspected my laptop's driver
when I saw this problem, so I tried with a dockstar instead and the issue
disappeared... until I increased the tcp_rmem on it to match my laptop :-)
Arnaud, you might be interested in trying checking if the following change
does something for you in mvneta.c :
- #define MVNETA_TX_DONE_TIMER_PERIOD 10
+ #define MVNETA_TX_DONE_TIMER_PERIOD (1000/HZ)
This can only have any effect if you run at 250 or 1000 Hz, but not at 100
of course. It should reduce the time to first IRQ.
Willy
^ permalink raw reply
* Re: [net 1/3] net: allow netdev_all_upper_get_next_dev_rcu with rtnl lock held
From: John Fastabend @ 2013-11-19 17:18 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem, John Fastabend, netdev, gospo, sassmann, Veaceslav Falico
In-Reply-To: <1384875654-7795-2-git-send-email-jeffrey.t.kirsher@intel.com>
On 11/19/2013 07:40 AM, Jeff Kirsher wrote:
> From: John Fastabend <john.r.fastabend@intel.com>
>
> It is useful to be able to walk all upper devices when bringing
> a device online where the RTNL lock is held. In this case it
> is safe to walk the all_adj_list because the RTNL lock is used
> to protect the write side as well.
>
> Here we rearrange the netdev_all_upper_get_next_dev_rcu into three
> routines:
>
> netdev_all_upper_get_next_dev_rcu()
> netdev_all_upper_get_next_dev_rtnl()
> netdev_all_upper_get_next_dev()
>
> One for RCU callers, one for RTNL callers and a final routine
> to implement the work. Both the _rcu and _rtnl variants are
> exposed. Where the net/ethernet/intel/ixgbe driver is a consumer
> of the _rtnl variant. netdev_all_upper_get_next_dev() is static.
>
> CC: Veaceslav Falico <vfalico@redhat.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
[...]
NAK, lockdep_rtnl_is_held() is wrapped in CONFIG_PROVE_LOCKING,
#ifdef CONFIG_PROVE_LOCKING
extern int lockdep_rtnl_is_held(void);
#endif /* #ifdef CONFIG_PROVE_LOCKING */
[...]
> +struct net_device *netdev_all_upper_get_next_dev_rtnl(struct net_device *dev,
> + struct list_head **iter)
> +{
> + WARN_ON_ONCE(!lockdep_rtnl_is_held());
so this can cause a build error without CONFIG_PROVE_LOCKING. I can
either wrap this in a CONFIG_PROVE_LOCKING or do something like this,
extern int rtnl_is_locked(void);
#ifdef CONFIG_PROVE_LOCKING
extern int lockdep_rtnl_is_held(void);
+#else
+static inline int lockdep_rtnl_is_held(void)
+{
+ return 0;
+}
#endif /* #ifdef CONFIG_PROVE_LOCKING */
The lazy way to do this is to wrap the call site in rcu_read_{un}lock
but I would prefer not to do that.
> + return netdev_all_upper_get_next_dev(dev, iter);
> +}
Sorry for the noise I'll spin a new version.
John
--
John Fastabend Intel Corporation
^ permalink raw reply
* [PATCH 1/1] IPv6: Fixed support for blackhole and prohibit routes
From: Kamala R @ 2013-11-19 16:49 UTC (permalink / raw)
To: Hannes Frederic Sowa, netdev, David Miller; +Cc: linux-kernel, Kamala R
From: Kamala R <kamala@aristanetworks.com>
The behaviour of blackhole and prohibit routes has been corrected by setting the input and output
function pointers of the dst variable appropriately. For blackhole routes, they are set to
dst_discard and for prohibit routes they are set to ip6_pkt_prohibit and ip6_pkt_prohbit_out
respectively.
Signed-off-by: Kamala R <kamala@aristanetworks.com>
---
net/ipv6/route.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f54e3a1..f2289fd 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1564,21 +1564,25 @@ int ip6_route_add(struct fib6_config *cfg)
goto out;
}
}
- rt->dst.output = ip6_pkt_discard_out;
- rt->dst.input = ip6_pkt_discard;
+
rt->rt6i_flags = RTF_REJECT|RTF_NONEXTHOP;
+ rt->dst.error = (cfg->fc_type == RTN_BLACKHOLE) ? -EINVAL
+ : ((cfg->fc_type == RTN_PROHIBIT ? -EACCES
+ : ((cfg->fc_type == RTN_THROW ? -EAGAIN
+ : -ENETUNREACH))));
switch (cfg->fc_type) {
case RTN_BLACKHOLE:
- rt->dst.error = -EINVAL;
+ rt->dst.output = dst_discard;
+ rt->dst.input = dst_discard;
break;
case RTN_PROHIBIT:
- rt->dst.error = -EACCES;
+ rt->dst.output = ip6_pkt_prohibit_out;
+ rt->dst.input = ip6_pkt_prohibit;
break;
case RTN_THROW:
- rt->dst.error = -EAGAIN;
- break;
default:
- rt->dst.error = -ENETUNREACH;
+ rt->dst.output = ip6_pkt_discard_out;
+ rt->dst.input = ip6_pkt_discard;
break;
}
goto install_route;
--
1.7.9.5
^ permalink raw reply related
* RE: net/usb/ax88179_178a driver broken in linux-3.12
From: David Laight @ 2013-11-19 16:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Mark Lord, Ming Lei, davem, netdev, Linux Kernel, stable
In-Reply-To: <1384877400.8604.107.camel@edumazet-glaptop2.roam.corp.google.com>
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> On Tue, 2013-11-19 at 14:43 +0000, David Laight wrote:
>
> > It isn't directly a TSO problem.
> > There has always been a bug in the xhci driver for fragmented buffers.
> > TSO just means it is given a lot of fragmented buffers.
> >
> > As well as user-supplied fragmented buffers, the bug affects
> > internal fragmentation that happens whenever a buffer crosses a 64k
> > byte boundary (please hw engineers - stop doing this!)
>
> TCP stack uses order-3 allocations.
>
> This means 32KB for x86 (PAGE_SIZE=4096)
>
> What is PAGE_SIZE for your arches ?
>
> If there is a 6KB limit, we might adapt TCP to make sure we do not cross
> a 64KB boundary.
>
> Other strategy would be to detect this case in the driver and split a
> problematic segment into two parts.
The xhci code does all the checks for buffer fragments crossing 64k boundaries.
I've posted a patch that uses a conservative upper limit for the
number of fragments: 2 * number_of_sg_buffs + xfer_len/65536.
This saves scanning the sg list twice.
For those not reading linux-usb:
The xhci 'bug' is that an SG list may only cross the end of a ring
segment at an aligned length.
For USB2 devices this will be 512 bytes. For USB3 the documented alignment
could be 16k (depends on a burst size), but might only be 1k.
(This is another place where the hw engineers haven't made life easy.)
The only simple solution is to ensure that a SG list doesn't cross
the end of a ring segment.
David
^ permalink raw reply
* RE: net/usb/ax88179_178a driver broken in linux-3.12
From: Eric Dumazet @ 2013-11-19 16:10 UTC (permalink / raw)
To: David Laight; +Cc: Mark Lord, Ming Lei, davem, netdev, Linux Kernel, stable
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7434@saturn3.aculab.com>
On Tue, 2013-11-19 at 14:43 +0000, David Laight wrote:
> It isn't directly a TSO problem.
> There has always been a bug in the xhci driver for fragmented buffers.
> TSO just means it is given a lot of fragmented buffers.
>
> As well as user-supplied fragmented buffers, the bug affects
> internal fragmentation that happens whenever a buffer crosses a 64k
> byte boundary (please hw engineers - stop doing this!)
TCP stack uses order-3 allocations.
This means 32KB for x86 (PAGE_SIZE=4096)
What is PAGE_SIZE for your arches ?
If there is a 6KB limit, we might adapt TCP to make sure we do not cross
a 64KB boundary.
Other strategy would be to detect this case in the driver and split a
problematic segment into two parts.
>
> I'm not sure whether usbnet would ever pass buffers that cross 64k
> boundaries. I've not seen one - even with TSO. But the rx buffers
> are 20k (doesn't seem ideal!) and could also be problematical.
>
> USB mass storage has used SG for ages, the buffers must all be
> adequately aligned for the hardware - they won't meet the constraint
> for USB3 itself, but the documented restriction may be more severe
> than the actual one.
^ permalink raw reply
* Re: [PATCH net RESEND] bridge: flush br's address entry in fdb when remove the bridge dev
From: Vlad Yasevich @ 2013-11-19 15:52 UTC (permalink / raw)
To: Ding Tianhong, Stephen Hemminger, David S. Miller, bridge, Netdev
In-Reply-To: <528AD37E.1050105@huawei.com>
On 11/18/2013 09:57 PM, Ding Tianhong wrote:
> When the following commands are executed:
>
> brctl addbr br0
> ifconfig br0 hw ether <addr>
> rmmod bridge
>
> The calltrace will occur:
>
> [ 563.312114] device eth1 left promiscuous mode
> [ 563.312188] br0: port 1(eth1) entered disabled state
> [ 563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
> [ 563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G O 3.12.0-0.7-default+ #9
> [ 563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
> [ 563.468200] 0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
> [ 563.468204] ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
> [ 563.468206] ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
> [ 563.468209] Call Trace:
> [ 563.468218] [<ffffffff814d1c92>] dump_stack+0x6a/0x78
> [ 563.468234] [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
> [ 563.468242] [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
> [ 563.468247] [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
> [ 563.468254] [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
> [ 563.468259] [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
> [ 570.377958] Bridge firewalling registered
>
> ------------------------------------------------
>
> The reason is that when the bridge dev's address is changed, the
> br_fdb_change_mac_address() will add new address in fdb, but when
> the bridge was removed, the address entry in the fdb did not free,
> the bridge_fdb_cache still has objects when destroy the cache, Fix
> this by flushing the bridge address entry when removing the bridge.
>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
> net/bridge/br_if.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 6e6194f..98084d8 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
> del_nbp(p);
> }
>
> + spin_lock_bh(&br->hash_lock);
> + fdb_delete_by_addr(br, br->dev->dev_addr, 0);
> + spin_unlock_bh(&br->hash_lock);
> +
> br_vlan_flush(br);
> del_timer_sync(&br->gc_timer);
>
>
I think you need to use fdb_delete_by_port(br, NULL, 1); here.
The reason is that if the bridge had vlan filtering configured, when the
bridge mac address was changed, an fdb would have been created for
for ever vlan configured on the bridge. You delete only vlan0, so you
would still have a leak.
-vlad
^ permalink raw reply
* Re: [PATCH net] net: sctp: fix copying sk_v6_rcv_saddr in sctp_v6_create_accept_sk
From: Daniel Borkmann @ 2013-11-19 15:46 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: davem, netdev, linux-sctp, Wang Weidong, Eric Dumazet
In-Reply-To: <528B85E2.5080403@gmail.com>
On 11/19/2013 04:38 PM, Vlad Yasevich wrote:
> On 11/19/2013 05:51 AM, Daniel Borkmann wrote:
>> Wang reported an issue that lksctp's test_getname_v6 seems to fail.
>>
>> The issue is that we do not copy sk_v6_rcv_saddr over to the new
>> socket, although the comment above says so regarding rcv_saddr.
>>
>> Commit 914e1c8b6980 ("sctp: Inherit all socket options from parent
>> correctly.") originally moved that over to sctp_copy_sock(), but
>> after commit efe4208f47f9 ("ipv6: make lookups simpler and faster")
>> this no longer holds and the actual value of sk_v6_rcv_saddr was
>> no longer being migrated.
>>
>> With this patch, the lksctp test suite passes again for IPv6.
>>
>> Fixes: efe4208f47f9 ("ipv6: make lookups simpler and faster")
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> Cc: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
>> net/sctp/ipv6.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
>> index 7567e6f..be08592 100644
>> --- a/net/sctp/ipv6.c
>> +++ b/net/sctp/ipv6.c
>> @@ -661,6 +661,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
>> * and getpeername().
>> */
>> sctp_v6_to_sk_daddr(&asoc->peer.primary_addr, newsk);
>> + newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
>>
>> sk_refcnt_debug_inc(newsk);
>>
>>
>
> This fixes the issue for the accept() case, but the bug is still there
> in the peeloff case.
>
> I think you should make sctp_copy_sock() use sock_copy() for now. It
> looks like it will catch all the cases.
I tried that and my machine froze when running lksctp. ;-)
Currently a bit busy, so I need to get back to this later tonight.
> We can then look at possibly getting rid of sctp_copy_sock() for net-next.
>
> -vlad
^ permalink raw reply
* Re: [PATCH net RESEND] bridge: flush br's address entry in fdb when remove the bridge dev
From: Toshiaki Makita @ 2013-11-19 15:46 UTC (permalink / raw)
To: Ding Tianhong; +Cc: Stephen Hemminger, David S. Miller, bridge, Netdev
In-Reply-To: <528AD37E.1050105@huawei.com>
On Tue, 2013-11-19 at 10:57 +0800, Ding Tianhong wrote:
> When the following commands are executed:
>
> brctl addbr br0
> ifconfig br0 hw ether <addr>
> rmmod bridge
>
> The calltrace will occur:
>
> [ 563.312114] device eth1 left promiscuous mode
> [ 563.312188] br0: port 1(eth1) entered disabled state
> [ 563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
> [ 563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G O 3.12.0-0.7-default+ #9
> [ 563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
> [ 563.468200] 0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
> [ 563.468204] ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
> [ 563.468206] ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
> [ 563.468209] Call Trace:
> [ 563.468218] [<ffffffff814d1c92>] dump_stack+0x6a/0x78
> [ 563.468234] [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
> [ 563.468242] [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
> [ 563.468247] [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
> [ 563.468254] [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
> [ 563.468259] [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
> [ 570.377958] Bridge firewalling registered
>
> ------------------------------------------------
>
> The reason is that when the bridge dev's address is changed, the
> br_fdb_change_mac_address() will add new address in fdb, but when
> the bridge was removed, the address entry in the fdb did not free,
> the bridge_fdb_cache still has objects when destroy the cache, Fix
> this by flushing the bridge address entry when removing the bridge.
>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
> net/bridge/br_if.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 6e6194f..98084d8 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
> del_nbp(p);
> }
>
> + spin_lock_bh(&br->hash_lock);
> + fdb_delete_by_addr(br, br->dev->dev_addr, 0);
> + spin_unlock_bh(&br->hash_lock);
You are not deleting entries with vid other than 0.
If we have added some vid, there might be fdb entries with that vid
whose dst is NULL, which also should be cleaned up.
How about deleting all fdb entries whose dst is NULL?
br_fdb_delete_by_port() looks able to be called with p == NULL as well.
Thanks,
Toshiaki Makita
> +
> br_vlan_flush(br);
> del_timer_sync(&br->gc_timer);
>
^ permalink raw reply
* [patch iproute2] ipaddrlabel: use uint32_t instead of int32_t
From: Hangbin Liu @ 2013-11-19 15:46 UTC (permalink / raw)
To: network dev; +Cc: Hangbin Liu
As both linux kernel and function ipaddrlabel_modify use unsigned int for
label. We should also use unsigned int value when print addrlabel in case of
misunderstanding.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
ip/ipaddrlabel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ip/ipaddrlabel.c b/ip/ipaddrlabel.c
index 1789d9c..68a94dd 100644
--- a/ip/ipaddrlabel.c
+++ b/ip/ipaddrlabel.c
@@ -86,10 +86,10 @@ int print_addrlabel(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg
if (ifal->ifal_index)
fprintf(fp, "dev %s ", ll_index_to_name(ifal->ifal_index));
- if (tb[IFAL_LABEL] && RTA_PAYLOAD(tb[IFAL_LABEL]) == sizeof(int32_t)) {
- int32_t label;
+ if (tb[IFAL_LABEL] && RTA_PAYLOAD(tb[IFAL_LABEL]) == sizeof(uint32_t)) {
+ uint32_t label;
memcpy(&label, RTA_DATA(tb[IFAL_LABEL]), sizeof(label));
- fprintf(fp, "label %d ", label);
+ fprintf(fp, "label %u ", label);
}
fprintf(fp, "\n");
--
1.8.1.4
^ permalink raw reply related
* [net 3/3] igb: Update queue reinit function to call dev_close when init of queues fails
From: Jeff Kirsher @ 2013-11-19 15:40 UTC (permalink / raw)
To: davem; +Cc: Carolyn Wyborny, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1384875654-7795-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Carolyn Wyborny <carolyn.wyborny@intel.com>
This patch adds a call to dev_close if the queue reinit fails in order
to make clearer to the user that the device is down.
Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index ebe6370..40320bd 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7847,6 +7847,7 @@ int igb_reinit_queues(struct igb_adapter *adapter)
if (igb_init_interrupt_scheme(adapter, true)) {
dev_err(&pdev->dev, "Unable to allocate memory for queues\n");
+ dev_close(netdev);
return -ENOMEM;
}
--
1.8.3.1
^ permalink raw reply related
* [net 2/3] igb: Fixed Wake On Lan support
From: Jeff Kirsher @ 2013-11-19 15:40 UTC (permalink / raw)
To: davem; +Cc: Akeem G Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1384875654-7795-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
This patch fixes Wake on Lan being reported as supported on some Ethernet
ports, in contrary to Hardware capability.
Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index b0f3666..c3143da 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2062,14 +2062,15 @@ static void igb_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct igb_adapter *adapter = netdev_priv(netdev);
- wol->supported = WAKE_UCAST | WAKE_MCAST |
- WAKE_BCAST | WAKE_MAGIC |
- WAKE_PHY;
wol->wolopts = 0;
if (!(adapter->flags & IGB_FLAG_WOL_SUPPORTED))
return;
+ wol->supported = WAKE_UCAST | WAKE_MCAST |
+ WAKE_BCAST | WAKE_MAGIC |
+ WAKE_PHY;
+
/* apply any specific unsupported masks here */
switch (adapter->hw.device_id) {
default:
--
1.8.3.1
^ permalink raw reply related
* [net 1/3] net: allow netdev_all_upper_get_next_dev_rcu with rtnl lock held
From: Jeff Kirsher @ 2013-11-19 15:40 UTC (permalink / raw)
To: davem
Cc: John Fastabend, netdev, gospo, sassmann, Veaceslav Falico,
Jeff Kirsher
In-Reply-To: <1384875654-7795-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: John Fastabend <john.r.fastabend@intel.com>
It is useful to be able to walk all upper devices when bringing
a device online where the RTNL lock is held. In this case it
is safe to walk the all_adj_list because the RTNL lock is used
to protect the write side as well.
Here we rearrange the netdev_all_upper_get_next_dev_rcu into three
routines:
netdev_all_upper_get_next_dev_rcu()
netdev_all_upper_get_next_dev_rtnl()
netdev_all_upper_get_next_dev()
One for RCU callers, one for RTNL callers and a final routine
to implement the work. Both the _rcu and _rtnl variants are
exposed. Where the net/ethernet/intel/ixgbe driver is a consumer
of the _rtnl variant. netdev_all_upper_get_next_dev() is static.
CC: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++--
include/linux/netdevice.h | 9 ++++++
net/core/dev.c | 43 ++++++++++++++++++++-------
3 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bd8f523..9eeb6f0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4336,7 +4336,7 @@ static void ixgbe_configure_dfwd(struct ixgbe_adapter *adapter)
struct list_head *iter;
int err;
- netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) {
+ netdev_for_each_all_upper_dev_rtnl(adapter->netdev, upper, iter) {
if (netif_is_macvlan(upper)) {
struct macvlan_dev *dfwd = netdev_priv(upper);
struct ixgbe_fwd_adapter *vadapter = dfwd->fwd_priv;
@@ -4601,7 +4601,7 @@ static void ixgbe_up_complete(struct ixgbe_adapter *adapter)
netif_tx_start_all_queues(adapter->netdev);
/* enable any upper devices */
- netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) {
+ netdev_for_each_all_upper_dev_rtnl(adapter->netdev, upper, iter) {
if (netif_is_macvlan(upper)) {
struct macvlan_dev *vlan = netdev_priv(upper);
@@ -4803,7 +4803,7 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
netif_tx_disable(netdev);
/* disable any upper devices */
- netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) {
+ netdev_for_each_all_upper_dev_rtnl(adapter->netdev, upper, iter) {
if (netif_is_macvlan(upper)) {
struct macvlan_dev *vlan = netdev_priv(upper);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8b3de7c..59872b2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2843,6 +2843,8 @@ bool netdev_has_upper_dev(struct net_device *dev, struct net_device *upper_dev);
bool netdev_has_any_upper_dev(struct net_device *dev);
struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
struct list_head **iter);
+struct net_device *netdev_all_upper_get_next_dev_rtnl(struct net_device *dev,
+ struct list_head **iter);
/* iterate through upper list, must be called under RCU read lock */
#define netdev_for_each_all_upper_dev_rcu(dev, updev, iter) \
@@ -2851,6 +2853,13 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
updev; \
updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)))
+/* iterate through upper list, must be called under RTNL */
+#define netdev_for_each_all_upper_dev_rtnl(dev, updev, iter) \
+ for (iter = &(dev)->all_adj_list.upper, \
+ updev = netdev_all_upper_get_next_dev_rtnl(dev, &(iter)); \
+ updev; \
+ updev = netdev_all_upper_get_next_dev_rtnl(dev, &(iter)))
+
void *netdev_lower_get_next_private(struct net_device *dev,
struct list_head **iter);
void *netdev_lower_get_next_private_rcu(struct net_device *dev,
diff --git a/net/core/dev.c b/net/core/dev.c
index 7e00a73..79c4c1e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4487,6 +4487,21 @@ void *netdev_adjacent_get_private(struct list_head *adj_list)
}
EXPORT_SYMBOL(netdev_adjacent_get_private);
+static struct net_device *netdev_all_upper_get_next_dev(struct net_device *dev,
+ struct list_head **iter)
+{
+ struct netdev_adjacent *upper;
+
+ upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
+
+ if (&upper->list == &dev->all_adj_list.upper)
+ return NULL;
+
+ *iter = &upper->list;
+
+ return upper->dev;
+}
+
/**
* netdev_all_upper_get_next_dev_rcu - Get the next dev from upper list
* @dev: device
@@ -4498,22 +4513,28 @@ EXPORT_SYMBOL(netdev_adjacent_get_private);
struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
struct list_head **iter)
{
- struct netdev_adjacent *upper;
-
WARN_ON_ONCE(!rcu_read_lock_held());
-
- upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
-
- if (&upper->list == &dev->all_adj_list.upper)
- return NULL;
-
- *iter = &upper->list;
-
- return upper->dev;
+ return netdev_all_upper_get_next_dev(dev, iter);
}
EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu);
/**
+ * netdev_all_upper_get_next_dev_rtnl - Get the next dev from upper list
+ * @dev: device
+ * @iter: list_head ** of the current position
+ *
+ * Gets the next device from the dev's upper list, starting from iter
+ * position. The caller must hold RTNL.
+ */
+struct net_device *netdev_all_upper_get_next_dev_rtnl(struct net_device *dev,
+ struct list_head **iter)
+{
+ WARN_ON_ONCE(!lockdep_rtnl_is_held());
+ return netdev_all_upper_get_next_dev(dev, iter);
+}
+EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rtnl);
+
+/**
* netdev_lower_get_next_private - Get the next ->private from the
* lower neighbour list
* @dev: device
--
1.8.3.1
^ permalink raw reply related
* [net 0/3][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-11-19 15:40 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann
This series contains updates to net and igb.
John provides a patch against net to be able to walk all upper devices
when bringing a device online where the RTNL lock is held.
Akeem provides a igb fix where WOL was being reported as supported on
some ethernet devices which did not have that capability.
Carolyn provides a igb fix to add a call to dev_close if the queue
reinit fails in order to make it clear to the user that the device is
down.
The following are changes since commit d11a347de3f521af62da25e74156ea39e3774f19:
be2net: Delete secondary unicast MAC addresses during be_close
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master
Akeem G Abodunrin (1):
igb: Fixed Wake On Lan support
Carolyn Wyborny (1):
igb: Update queue reinit function to call dev_close when init of
queues fails
John Fastabend (1):
net: allow netdev_all_upper_get_next_dev_rcu with rtnl lock held
drivers/net/ethernet/intel/igb/igb_ethtool.c | 7 +++--
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++--
include/linux/netdevice.h | 9 ++++++
net/core/dev.c | 43 ++++++++++++++++++++-------
5 files changed, 49 insertions(+), 17 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH net] net: sctp: fix copying sk_v6_rcv_saddr in sctp_v6_create_accept_sk
From: Vlad Yasevich @ 2013-11-19 15:38 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: netdev, linux-sctp, Wang Weidong, Eric Dumazet
In-Reply-To: <1384858284-2246-1-git-send-email-dborkman@redhat.com>
On 11/19/2013 05:51 AM, Daniel Borkmann wrote:
> Wang reported an issue that lksctp's test_getname_v6 seems to fail.
>
> The issue is that we do not copy sk_v6_rcv_saddr over to the new
> socket, although the comment above says so regarding rcv_saddr.
>
> Commit 914e1c8b6980 ("sctp: Inherit all socket options from parent
> correctly.") originally moved that over to sctp_copy_sock(), but
> after commit efe4208f47f9 ("ipv6: make lookups simpler and faster")
> this no longer holds and the actual value of sk_v6_rcv_saddr was
> no longer being migrated.
>
> With this patch, the lksctp test suite passes again for IPv6.
>
> Fixes: efe4208f47f9 ("ipv6: make lookups simpler and faster")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/sctp/ipv6.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 7567e6f..be08592 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -661,6 +661,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
> * and getpeername().
> */
> sctp_v6_to_sk_daddr(&asoc->peer.primary_addr, newsk);
> + newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
>
> sk_refcnt_debug_inc(newsk);
>
>
This fixes the issue for the accept() case, but the bug is still there
in the peeloff case.
I think you should make sctp_copy_sock() use sock_copy() for now. It
looks like it will catch all the cases.
We can then look at possibly getting rid of sctp_copy_sock() for net-next.
-vlad
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox