* Re: [PATCH v2.39 0/7] MPLS actions and matches
From: Ben Pfaff @ 2013-09-12 22:54 UTC (permalink / raw)
To: Simon Horman
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA, Ravi K,
Isaku Yamahata
In-Reply-To: <20130912225614.GB30229-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
On Fri, Sep 13, 2013 at 07:56:14AM +0900, Simon Horman wrote:
> On Thu, Sep 12, 2013 at 12:06:36PM -0700, Ben Pfaff wrote:
> > I've totally lost track of the status of this patch series. I assume it
> > needs Jesse's review. Jesse, if I'm wrong about that, let me know and
> > I'll take a pass at it.
>
> My understanding is that you have looked over the approach
> taken for the non-datapath code and were happy with it in
> the context that it needed review from Jesse along with the
> datapath code.
>
> I believe it was a few revisions ago that you looked over
> the series but I don't believe the non-datapath code has changed
> in a meaningful way since then.
That sounds plausible, thanks for refreshing my memory.
^ permalink raw reply
* Re: [PATCH RFC] net: neighbour: use source address of last enqueued packet for solicitation
From: Hannes Frederic Sowa @ 2013-09-12 22:20 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev, davem
In-Reply-To: <alpine.LFD.2.00.1309092208460.1655@ja.ssi.bg>
On Mon, Sep 09, 2013 at 11:17:45PM +0300, Julian Anastasov wrote:
> arp_queue has packets only in NUD_INCOMPLETE state
> (mcast_solicit=3 secs by default). And __neigh_event_send()
> now can keep many packets, 64KB from recent changes. So the
> 1st place is not guaranteed but now it is more difficult
> to kick the first packet compared to the old limit of just
> 3 packets.
>
> The change can give chance for 2nd and 3th
> probe if the 1st probe is not replied, so it should be
> better to apply it:
>
> Reviewed-by: Julian Anastasov <ja@ssi.bg>
Thanks for the review. I resend the patch as soon as net-next opens.
>
> Still, I think such problems should be addressed
> with conf/{DEV,all}/arp_announce=1 or 2.
*nod*
I use this knob if I have such problems. But this patch improves
connectivity in the default configuration and we actually don't care much
about the source address in either ipv4 or ipv6. So it seemed legitimate
and simple to me.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH 10/11] ipv6: move route updating for redirect to ndisc layer
From: Hannes Frederic Sowa @ 2013-09-12 22:04 UTC (permalink / raw)
To: Duan Jiong; +Cc: davem, netdev
In-Reply-To: <52319CC5.10902@cn.fujitsu.com>
On Thu, Sep 12, 2013 at 06:51:49PM +0800, Duan Jiong wrote:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> And when dealing with redirect message, the err shoud
> be assigned to 0.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
> net/ipv6/raw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
> index 58916bb..6138199 100644
> --- a/net/ipv6/raw.c
> +++ b/net/ipv6/raw.c
> @@ -336,7 +336,7 @@ static void rawv6_err(struct sock *sk, struct sk_buff *skb,
> harderr = (np->pmtudisc == IPV6_PMTUDISC_DO);
> }
> if (type == NDISC_REDIRECT)
> - ip6_sk_redirect(skb, sk);
> + err = 0;
> if (np->recverr) {
> u8 *payload = skb->data;
> if (!inet->hdrincl)
Hm, I don't like the cirumstance that we first call icmpv6_err_convert at
first and get back a bogus error value and later on convert it to something
meaningful.
Either:
a) Don't call icmpv6_err_convert at all for redirects. Then we could
place a WARN_ON(type == NDISC_REDIRECT) into this function to find future
missuse of this function with redirects or
b) handle the update of the NDISC_REDIRECT error code directly in
icmpv6_err_convert.
Also that you used the same headings for some commits shows that you could
perhaps squash them into one patch.
Otherwise I'm fine with the changes, thanks.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH 1/1] net: race condition when removing virtual net_device
From: Francesco Ruggeri @ 2013-09-12 22:02 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
Cong Wang, netdev
In-Reply-To: <CA+HUmGgyCsc-er6iB_mqg9whMyaquGWinkHOQEuEHbFCYVrXBw@mail.gmail.com>
On Thu, Sep 12, 2013 at 2:48 PM, Francesco Ruggeri
<fruggeri@aristanetworks.com> wrote:
> On Thu, Sep 12, 2013 at 1:06 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Francesco Ruggeri <fruggeri@aristanetworks.com> writes:
...
> netdev_run_todo. This guarantees that no deadlocks are introduced,
> since the relative order of net_devices within each namespace is
> preserved, and only non-loopback_devs take and release the extra
> reference.
I should have said "since in net_todo_list the relative order of
net_devices within each namespace is the same as before, ...".
Sorry for any confusion.
Francesco
^ permalink raw reply
* Re: [PATCH 01/11] ipv6: Do route updating for redirect in ndisc layer
From: Hannes Frederic Sowa @ 2013-09-12 21:54 UTC (permalink / raw)
To: Duan Jiong; +Cc: davem, netdev
In-Reply-To: <52319AEC.1010206@cn.fujitsu.com>
On Thu, Sep 12, 2013 at 06:43:56PM +0800, Duan Jiong wrote:
> -
> -void ip6_sk_redirect(struct sk_buff *skb, struct sock *sk)
> -{
> - ip6_redirect(skb, sock_net(sk), sk->sk_bound_dev_if, sk->sk_mark);
> -}
> -EXPORT_SYMBOL_GPL(ip6_sk_redirect);
> +EXPORT_SYMBOL_GPL(ip6_redirect);
>
> static unsigned int ip6_default_advmss(const struct dst_entry *dst)
> {
This breaks bisectability. Please remove the function after you cleared up all
references to it. Otherwise git bisect would throw compiler errors.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH 1/1] net: race condition when removing virtual net_device
From: Francesco Ruggeri @ 2013-09-12 21:48 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
Cong Wang, netdev
In-Reply-To: <87txhp249u.fsf@xmission.com>
On Thu, Sep 12, 2013 at 1:06 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Francesco Ruggeri <fruggeri@aristanetworks.com> writes:
>
>> Resending.
>
> To summarize:
>
> In netdev_run_todo, netdev_wait_allrefs, call_netdevice_notifiers you
> have observed a situation where dev_net(dev) was an invalid pointer.
>
Correct.
> My first impression is that we probably just want to remove the repeated
> call to call_netdevice_notifiers, that happens after 1 second of
> waiting.
>
> Your suggested patch below doesn't have a prayer of working, as it only
> decreases the device reference count after the loop to wait for the
> reference count to go to zero.
>
Actually it did work. In 3.4 it avoided the crash in
inetpeer_invalidate_tree and it did not cause any visible adverse
effects after running it several days on multiple servers that create
and destroy namespaces on a regular basis.
The extra refcount that is taken and released is on the loopback_dev
of the namespace that the net_device is in. The assumption is that
loopback_dev is always the last net_device to be destroyed in a
namespace. So each net_device in a namespace (except for the
loopback_dev itself) takes such reference when it is unlisted, and it
releases it after it is destroyed (after netdev_wait_allrefs).
The patch also makes sure that any loopback_devs are at the tail of
net_todo_list within every namespace in every instance of
netdev_run_todo. This guarantees that no deadlocks are introduced,
since the relative order of net_devices within each namespace is
preserved, and only non-loopback_devs take and release the extra
reference.
> Your patch modified to grab a count on the network namespace the device
> references and not the device itself might make sense, but that runs the
> risk of incrementing the network namespace counts after the network
> namespace is down.
Right.
>
> Simply not rerunning call_netdevice_notifiers seems like the proper
> approach to fix this.
>
That would be great. There would still be one scenario to take care of though:
- veth interfaces v0 and v1 are in namespaces ns0 and ns1.
- process p0 unregisters v0, which also causes v1 to be unregistered.
When p0 enters netdev_run_todo both v0 and v1 are in net_todo_list and
have been unlisted from their namespaces.
- then in p0's netdev_run_todo:
void netdev_run_todo(void)
{
struct list_head list;
/* Snapshot list, allow later requests */
list_replace_init(&net_todo_list, &list);
__rtnl_unlock();
/* Wait for rcu callbacks to finish before next phase */
if (!list_empty(&list))
rcu_barrier();
********* Assume ns1 is destroyed by another process here ************
while (!list_empty(&list)) {
struct net_device *dev
= list_first_entry(&list, struct net_device, todo_list);
list_del(&dev->todo_list);
rtnl_lock();
call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
********* dst_dev_event/dst_ifdown is invoked here, and it tries to access
dev_net(dev)->loopback_dev.
In case of v1 this would be an invalid pointer. *****************
__rtnl_unlock();
Similar scenarios apply if v1 is a vlan or macvlan interface, and v0
is its real device.
Francesco
^ permalink raw reply
* Re: [PATCH net v2] net: sctp: fix ipv6 ipsec encryption bug in sctp_v6_xmit
From: David Miller @ 2013-09-12 21:26 UTC (permalink / raw)
To: vyasevich
Cc: dborkman, netdev, linux-sctp, adobriyan, steffen.klassert, hannes
In-Reply-To: <52308C88.10604@gmail.com>
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Wed, 11 Sep 2013 11:30:16 -0400
> On 09/11/2013 10:58 AM, Daniel Borkmann wrote:
>> Alan Chester reported an issue with IPv6 on SCTP that IPsec traffic is
>> not
>> being encrypted, whereas on IPv4 it is. Setting up an AH + ESP
>> transport
>> does not seem to have the desired effect:
...
>> Reported-by: Alan Chester <alan.chester@tekelec.com>
>> Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> Cc: Steffen Klassert <steffen.klassert@secunet.com>
>> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Applied and queued up for -stable, thanks.
Yeah it is tricky to share a common transmit path with the
way SCTP works.
^ permalink raw reply
* Re: [PATCH net V2] tuntap: correctly handle error in tun_set_iff()
From: David Miller @ 2013-09-12 21:22 UTC (permalink / raw)
To: mst; +Cc: jasowang, netdev, linux-kernel, wannes.rombouts
In-Reply-To: <20130911104350.GA25316@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Wed, 11 Sep 2013 13:43:50 +0300
> On Wed, Sep 11, 2013 at 06:09:48PM +0800, Jason Wang wrote:
>> Commit c8d68e6be1c3b242f1c598595830890b65cea64a
>> (tuntap: multiqueue support) only call free_netdev() on error in
>> tun_set_iff(). This causes several issues:
>>
>> - memory of tun security were leaked
>> - use after free since the flow gc timer was not deleted and the tfile
>> were not detached
>>
>> This patch solves the above issues.
>>
>> Reported-by: Wannes Rombouts <wannes.rombouts@epitech.eu>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] xen-netback: fix possible format string flaw
From: David Miller @ 2013-09-12 21:20 UTC (permalink / raw)
To: Ian.Campbell; +Cc: keescook, linux-kernel, xen-devel, netdev
In-Reply-To: <1378883893.3057.60.camel@dagon.hellion.org.uk>
From: Ian Campbell <Ian.Campbell@citrix.com>
Date: Wed, 11 Sep 2013 08:18:13 +0100
> On Tue, 2013-09-10 at 21:39 -0700, Kees Cook wrote:
>> This makes sure a format string cannot accidentally leak into the
>> kthread_run() call.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] bug[#43]: kgdboe: netpoll: Should handle ETH_P_ARP other than ETH_P_IP in netpoll_neigh_reply
From: David Miller @ 2013-09-12 21:17 UTC (permalink / raw)
To: sonic.adi; +Cc: netdev, adi-buildroot-devel, sonic.zhang, amwang
In-Reply-To: <1378870313-14161-1-git-send-email-sonic.adi@gmail.com>
From: Sonic Zhang <sonic.adi@gmail.com>
Date: Wed, 11 Sep 2013 11:31:53 +0800
> From: Sonic Zhang <sonic.zhang@analog.com>
>
> The received ARP request type in the Ethernet packet head is ETH_P_ARP other than ETH_P_IP.
>
> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
This bug was introduced by commit b7394d2429c198b1da3d46ac39192e891029ec0f
("netpoll: prepare for ipv6")
I'll apply this, thank you.
^ permalink raw reply
* Re: [PATCH v2 1/1] net: fec: fix phy reset operation to let imx6sl evk work
From: David Miller @ 2013-09-12 21:12 UTC (permalink / raw)
To: B38611; +Cc: shawn.guo, netdev, bhutchings, stephen, b20596, s.hauer
In-Reply-To: <1378865937-12055-1-git-send-email-B38611@freescale.com>
From: Fugang Duan <B38611@freescale.com>
Date: Wed, 11 Sep 2013 10:18:57 +0800
> Current driver only do phy reset in probe function, which is
> not right. Since some phy clock is disabled after module probe,
> the phy enter abnormal status, which needs do reset to recovery
> the phy. And do ifconfig ethx up/down test, the phy also enter
> abnormal status.
What would disable the PHY clock after the probe?
You have to explain these kinds of things in your changelog
message.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] bonding: Make alb learning packet interval configurable
From: Andy Gospodarek @ 2013-09-12 21:06 UTC (permalink / raw)
To: David Miller; +Cc: nhorman, netdev, vfalico, nhorman, fubar
In-Reply-To: <20130912.164948.1364121695106020281.davem@davemloft.net>
On Thu, Sep 12, 2013 at 04:49:48PM -0400, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Tue, 10 Sep 2013 16:39:03 -0400
>
> > From: Neil Horman <nhorman@redhat.com>
> >
> > running bonding in ALB mode requires that learning packets be sent periodically,
> > so that the switch knows where to send responding traffic. However, depending
> > on switch configuration, there may not be any need to send traffic at the
> > default rate of 3 packets per second, which represents little more than wasted
> > data. Allow the ALB learning packet interval to be made configurable via sysfs
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>
> I hate to be a stickler, but I'd like you to make the default value
> documented both in the code and in the documentation.
>
> Use some macro for the code "#define BOND_ALB_DEFAULT_LP_INTERVAL 1" and
> mention the default in the bonding.txt changes.
>
Agree with DaveM on this. You can just keep the one that was there and
it should be pretty easy.
^ permalink raw reply
* [PATCH net] tg3: Expand led off fix to include 5720
From: Nithin Nayak Sujir @ 2013-09-12 21:01 UTC (permalink / raw)
To: davem; +Cc: netdev, Nithin Nayak Sujir, stable, Michael Chan
Commit 989038e217e94161862a959e82f9a1ecf8dda152 ("tg3: Don't turn off
led on 5719 serdes port 0") added code to skip turning led off on port
0 of the 5719 since it powered down other ports. This workaround needs
to be enabled on the 5720 as well.
Cc: stable@vger.kernel.org
Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/ethernet/broadcom/tg3.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 5701f3d..9011ea0 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -3034,6 +3034,7 @@ static bool tg3_phy_led_bug(struct tg3 *tp)
{
switch (tg3_asic_rev(tp)) {
case ASIC_REV_5719:
+ case ASIC_REV_5720:
if ((tp->phy_flags & TG3_PHYFLG_MII_SERDES) &&
!tp->pci_fn)
return true;
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH net 1/4] bridge: Don't use VID 0 and 4095 in vlan filtering
From: Vlad Yasevich @ 2013-09-12 20:57 UTC (permalink / raw)
To: David Miller; +Cc: makita.toshiaki, netdev
In-Reply-To: <20130912.155533.521690436399073964.davem@davemloft.net>
On 09/12/2013 03:55 PM, David Miller wrote:
> From: Vlad Yasevich <vyasevic@redhat.com>
> Date: Tue, 10 Sep 2013 10:22:26 -0400
>
>> On 09/10/2013 06:32 AM, Toshiaki Makita wrote:
>>> IEEE 802.1Q says that:
>>> - VID 0 shall not be configured as a PVID, or configured in any
>>> - Filtering
>>> Database entry.
>>> - VID 4095 shall not be configured as a PVID, or transmitted in a tag
>>> header. This VID value may be used to indicate a wildcard match for
>>> the VID
>>> in management operations or Filtering Database entries.
>>> (See IEEE 802.1Q-2005 6.7.1 and Table 9-2)
>>>
>>> Don't accept adding these VIDs in the vlan_filtering implementation.
>>>
>>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>
>> Reviewed-by: Vlad Yasevich <vyasevich@redhat.com>
>
> Vlad, as far as I can see your redhat.com email ends just with a "c"
> not a "ch". It's your gmail account that ends in a "ch".
>
> I'm fixing this up while applying these patches, if you have something
> automatically generating these reviewed-by strings please fix it up
> although I note that some are correct and some are not so maybe you
> do this by hand :-)
>
Apologies. Yes, I usually ack things by hand and sometimes fingers just
keep going...
-vlad
^ permalink raw reply
* [PATCH] net/irda/mcs7780: fix memory leaks in mcs_net_open()
From: Alexey Khoroshilov @ 2013-09-12 20:51 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Alexey Khoroshilov, David S . Miller, netdev, linux-kernel,
ldv-project
If rx_urb allocation fails in mcs_setup_urbs(), tx_urb leaks.
If mcs_receive_start() fails in mcs_net_open(), the both urbs are not deallocated.
The patch fixes the issues and by the way fixes label indentation.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/net/irda/mcs7780.c | 40 +++++++++++++++++++++++-----------------
1 file changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/net/irda/mcs7780.c b/drivers/net/irda/mcs7780.c
index f07c340..3f138ca 100644
--- a/drivers/net/irda/mcs7780.c
+++ b/drivers/net/irda/mcs7780.c
@@ -191,8 +191,8 @@ static inline int mcs_setup_transceiver_vishay(struct mcs_cb *mcs)
goto error;
ret = 0;
- error:
- return ret;
+error:
+ return ret;
}
/* Setup a communication between mcs7780 and agilent chip. */
@@ -501,8 +501,11 @@ static inline int mcs_setup_urbs(struct mcs_cb *mcs)
return 0;
mcs->rx_urb = usb_alloc_urb(0, GFP_KERNEL);
- if (!mcs->rx_urb)
+ if (!mcs->rx_urb) {
+ usb_free_urb(mcs->tx_urb);
+ mcs->tx_urb = NULL;
return 0;
+ }
return 1;
}
@@ -643,9 +646,9 @@ static int mcs_speed_change(struct mcs_cb *mcs)
ret = mcs_set_reg(mcs, MCS_MODE_REG, rval);
mcs->speed = mcs->new_speed;
- error:
- mcs->new_speed = 0;
- return ret;
+error:
+ mcs->new_speed = 0;
+ return ret;
}
/* Ioctl calls not supported at this time. Can be an area of future work. */
@@ -738,17 +741,20 @@ static int mcs_net_open(struct net_device *netdev)
ret = mcs_receive_start(mcs);
if (ret)
- goto error3;
+ goto error4;
netif_start_queue(netdev);
return 0;
- error3:
- irlap_close(mcs->irlap);
- error2:
- kfree_skb(mcs->rx_buff.skb);
- error1:
- return ret;
+error4:
+ usb_free_urb(mcs->rx_urb);
+ usb_free_urb(mcs->tx_urb);
+error3:
+ irlap_close(mcs->irlap);
+error2:
+ kfree_skb(mcs->rx_buff.skb);
+error1:
+ return ret;
}
/* Receive callback function. */
@@ -946,11 +952,11 @@ static int mcs_probe(struct usb_interface *intf,
usb_set_intfdata(intf, mcs);
return 0;
- error2:
- free_netdev(ndev);
+error2:
+ free_netdev(ndev);
- error1:
- return ret;
+error1:
+ return ret;
}
/* The current device is removed, the USB layer tells us to shut down. */
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v2] bonding: Make alb learning packet interval configurable
From: David Miller @ 2013-09-12 20:49 UTC (permalink / raw)
To: nhorman; +Cc: netdev, vfalico, nhorman, fubar, andy
In-Reply-To: <1378845543-14876-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 10 Sep 2013 16:39:03 -0400
> From: Neil Horman <nhorman@redhat.com>
>
> running bonding in ALB mode requires that learning packets be sent periodically,
> so that the switch knows where to send responding traffic. However, depending
> on switch configuration, there may not be any need to send traffic at the
> default rate of 3 packets per second, which represents little more than wasted
> data. Allow the ALB learning packet interval to be made configurable via sysfs
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
I hate to be a stickler, but I'd like you to make the default value
documented both in the code and in the documentation.
Use some macro for the code "#define BOND_ALB_DEFAULT_LP_INTERVAL 1" and
mention the default in the bonding.txt changes.
Thanks.
^ permalink raw reply
* Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called
From: Thomas Gleixner @ 2013-09-12 20:41 UTC (permalink / raw)
To: John Stultz
Cc: Herbert Xu, Fan Du, Steffen Klassert, David Miller,
Daniel Borkmann, LKML, netdev
In-Reply-To: <CANcMJZCvoynPh4rCkKfnGTHeG2evpKwS0gj_VLmFtmbRusmEog@mail.gmail.com>
On Thu, 12 Sep 2013, John Stultz wrote:
> On Thu, Sep 12, 2013 at 6:44 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > On Thu, Sep 12, 2013 at 03:21:24PM +0200, Thomas Gleixner wrote:
> >>
> >> > (3): http://www.spinics.net/lists/netdev/msg245169.html
> >>
> >> Thanks for the explanation so far.
> >>
> >> What's still unclear to me is why these timeouts are bound to wall
> >> time in the first place.
> >>
> >> Is there any real reason why the key life time can't simply be
> >> expressed in monotonic time, e.g. N seconds after creation or M
> >> seconds after usage? Looking at the relevant RFCs I can't find any
> >> requirement for binding the life time to wall time.
> >>
> >> A life time of 10 minutes does not change when the wall clock is
> >> adjusted for whatever reasons. It's still 10 minutes and not some
> >> random result of the wall clock adjustments. But I might be wrong as
> >> usual :)
> >
> > Well we started out with straight timers. It was changed because
> > people wanted IPsec SAs to expire after a suspect/resume which
> > AFAIK does not touch normal timers.
>
> I'm not sure I've totally groked the specific need here, but if you're
> wanting a monotonic clockbase that includes suspend time, then you
> might checkout CLOCK_BOOTTIME.
Duh, completely forgot about that one. Sure that would avoid the whole
business.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called
From: John Stultz @ 2013-09-12 20:35 UTC (permalink / raw)
To: Herbert Xu
Cc: Thomas Gleixner, Fan Du, Steffen Klassert, David Miller,
Daniel Borkmann, LKML, netdev
In-Reply-To: <20130912134409.GB21212@gondor.apana.org.au>
On Thu, Sep 12, 2013 at 6:44 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Thu, Sep 12, 2013 at 03:21:24PM +0200, Thomas Gleixner wrote:
>>
>> > (3): http://www.spinics.net/lists/netdev/msg245169.html
>>
>> Thanks for the explanation so far.
>>
>> What's still unclear to me is why these timeouts are bound to wall
>> time in the first place.
>>
>> Is there any real reason why the key life time can't simply be
>> expressed in monotonic time, e.g. N seconds after creation or M
>> seconds after usage? Looking at the relevant RFCs I can't find any
>> requirement for binding the life time to wall time.
>>
>> A life time of 10 minutes does not change when the wall clock is
>> adjusted for whatever reasons. It's still 10 minutes and not some
>> random result of the wall clock adjustments. But I might be wrong as
>> usual :)
>
> Well we started out with straight timers. It was changed because
> people wanted IPsec SAs to expire after a suspect/resume which
> AFAIK does not touch normal timers.
I'm not sure I've totally groked the specific need here, but if you're
wanting a monotonic clockbase that includes suspend time, then you
might checkout CLOCK_BOOTTIME.
thanks
-john
^ permalink raw reply
* Re: [net 2/2] igb: Read flow control for i350 from correct EEPROM section
From: David Miller @ 2013-09-12 20:25 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: todd.fujinaka, netdev, gospo, sassmann
In-Reply-To: <1378839437-10965-2-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 10 Sep 2013 11:57:17 -0700
> From: "Fujinaka, Todd" <todd.fujinaka@intel.com>
>
> Flow control is defined in the four EEPROM sections but the driver only reads
> from section 0.
>
> Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net 1/2] igb: Add additional get_phy_id call for i354 devices
From: David Miller @ 2013-09-12 20:25 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: carolyn.wyborny, netdev, gospo, sassmann
In-Reply-To: <1378839437-10965-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 10 Sep 2013 11:57:16 -0700
> From: Carolyn Wyborny <carolyn.wyborny@intel.com>
>
> This patch fixes a problem where some ports can fail to initialize on a
> cold boot. This patch adds an additional call to read the PHY id for i354
> devices in order workaround the hardware problem.
>
> 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>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1] net: race condition when removing virtual net_device
From: Eric W. Biederman @ 2013-09-12 20:06 UTC (permalink / raw)
To: Francesco Ruggeri
Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
Cong Wang, netdev
In-Reply-To: <1379008796-2121-1-git-send-email-fruggeri@aristanetworks.com>
Francesco Ruggeri <fruggeri@aristanetworks.com> writes:
> Resending.
To summarize:
In netdev_run_todo, netdev_wait_allrefs, call_netdevice_notifiers you
have observed a situation where dev_net(dev) was an invalid pointer.
My first impression is that we probably just want to remove the repeated
call to call_netdevice_notifiers, that happens after 1 second of
waiting.
Your suggested patch below doesn't have a prayer of working, as it only
decreases the device reference count after the loop to wait for the
reference count to go to zero.
Your patch modified to grab a count on the network namespace the device
references and not the device itself might make sense, but that runs the
risk of incrementing the network namespace counts after the network
namespace is down.
Simply not rerunning call_netdevice_notifiers seems like the proper
approach to fix this.
So I think the fix will probably just be:
David, Eric, do any of you know why we have this netdevice notfier
rebroadcast?
diff --git a/net/core/dev.c b/net/core/dev.c
index a3d8d44..e6bd828 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5556,42 +5556,15 @@ EXPORT_SYMBOL(netdev_refcnt_read);
*/
static void netdev_wait_allrefs(struct net_device *dev)
{
- unsigned long rebroadcast_time, warning_time;
+ unsigned long warning_time;
int refcnt;
linkwatch_forget_dev(dev);
- rebroadcast_time = warning_time = jiffies;
+ warning_time = jiffies;
refcnt = netdev_refcnt_read(dev);
while (refcnt != 0) {
- if (time_after(jiffies, rebroadcast_time + 1 * HZ)) {
- rtnl_lock();
-
- /* Rebroadcast unregister notification */
- call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
-
- __rtnl_unlock();
- rcu_barrier();
- rtnl_lock();
-
- call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
- if (test_bit(__LINK_STATE_LINKWATCH_PENDING,
- &dev->state)) {
- /* We must not have linkwatch events
- * pending on unregister. If this
- * happens, we simply run the queue
- * unscheduled, resulting in a noop
- * for this device.
- */
- linkwatch_run_queue();
- }
-
- __rtnl_unlock();
-
- rebroadcast_time = jiffies;
- }
-
msleep(250);
refcnt = netdev_refcnt_read(dev);
> There is a race condition when removing a net_device while its net namespace
> is also being removed.
> This can result in a crash or other unpredictable behavior.
> This is a sample scenario with veth, but the same applies to other virtual
> devices such as vlan and macvlan.
> veth pair v0-v1 is created, with v0 in namespace ns0 and v1 in ns1.
> Process p0 deletes v0. v1 is also unregistered (in veth_dellink), so when p0
> gets to netdev_run_todo both v0 and v1 are in net_todo_list, and they have both
> been unlisted from their respective namespaces. If all references to v1 have not
> already been dropped then netdev_run_todo/netdev_wait_allrefs will call netdev
> notifiers for v1, releasing the rtnl lock between calls.
> Now process p1 removes namespace ns1. v1 will not prevent this from happening
> (in default_device_exit_batch) since it was already unlisted by p0.
> Next time p0 invokes the notifiers for v1 any notifiers that use dev_net(v1)
> will get a pointer to a namespace that has been or is being removed.
> Similar scenarios apply with v1 as a vlan or macvlan interface and v0 as its
> real device.
> We hit this problem in 3.4 with sequence fib_netdev_event, fib_disable_ip,
> rt_cache_flush, rt_cache_invalidate, inetpeer_invalidate_tree. That sequence no
> longer applies in later kernels, but unless we can guarantee that no
> NETDEV_UNREGISTER or NETDEV_UNREGISTER_FINAL handler accesses a net_device's
> dev_net(dev) then there is a vulnerability (this happens for example with
> NETDEV_UNREGISTER_FINAL in dst_dev_event/dst_ifdown).
> Commit 0115e8e3 later made things better by reducing the chances of this
> happening, but the underlying problem still seems to be there.
> I would like to get some feedback on this patch.
> The idea is to take a reference to the loopback_dev of a net_device's
> namespace (which is always the last net_device to be removed when a namespace
> is destroyed) when the net_device is unlisted, and release it when the
> net_device is disposed of.
> To avoid deadlocks in cleanup_net all loopback_devs are queued at the end
> of net_todo_list.
>
> Tested on Linux 3.4.
>
> Signed-off-by: Francesco Ruggeri <fruggeri@aristanetworks.com>
> ---
> net/core/dev.c | 30 +++++++++++++++++++++++++++++-
> 1 files changed, 29 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 26755dd..da2fd78 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -225,10 +225,14 @@ static void list_netdevice(struct net_device *dev)
> }
>
> /* Device list removal
> + * Take a reference to dev_net(dev)->loopback_dev, so dev_net(dev)
> + * will not be freed until we are done with dev.
> * caller must respect a RCU grace period before freeing/reusing dev
> */
> static void unlist_netdevice(struct net_device *dev)
> {
> + struct net_device *loopback_dev = dev_net(dev)->loopback_dev;
> +
> ASSERT_RTNL();
>
> /* Unlink dev from the device chain */
> @@ -238,9 +242,23 @@ static void unlist_netdevice(struct net_device *dev)
> hlist_del_rcu(&dev->index_hlist);
> write_unlock_bh(&dev_base_lock);
>
> + if (dev != loopback_dev)
> + dev_hold(loopback_dev);
> +
> dev_base_seq_inc(dev_net(dev));
> }
>
> +/**
> + * Called when a net_device that has been previously unlisted from a net
> + * namespace is disposed of.
> + */
> +static inline void unlist_netdevice_done(struct net_device *dev)
> +{
> + struct net_device *loopback_dev = dev_net(dev)->loopback_dev;
> + if (dev != loopback_dev)
> + dev_put(loopback_dev);
> +}
> +
> /*
> * Our notifier list
> */
> @@ -5009,10 +5027,17 @@ static int dev_new_index(struct net *net)
>
> /* Delayed registration/unregisteration */
> static LIST_HEAD(net_todo_list);
> +static struct list_head *first_loopback_dev = &net_todo_list;
>
> static void net_set_todo(struct net_device *dev)
> {
> - list_add_tail(&dev->todo_list, &net_todo_list);
> + /* All loopback_devs go to end of net_todo_list. */
> + if (dev == dev_net(dev)->loopback_dev) {
> + list_add_tail(&dev->todo_list, &net_todo_list);
> + if (first_loopback_dev == &net_todo_list)
> + first_loopback_dev = &dev->todo_list;
> + } else
> + list_add_tail(&dev->todo_list, first_loopback_dev);
> }
>
> static void rollback_registered_many(struct list_head *head)
> @@ -5641,6 +5666,7 @@ void netdev_run_todo(void)
>
> /* Snapshot list, allow later requests */
> list_replace_init(&net_todo_list, &list);
> + first_loopback_dev = &net_todo_list;
>
> __rtnl_unlock();
>
> @@ -5670,6 +5696,7 @@ void netdev_run_todo(void)
> on_each_cpu(flush_backlog, dev, 1);
>
> netdev_wait_allrefs(dev);
> + unlist_netdevice_done(dev);
>
> /* paranoia */
> BUG_ON(netdev_refcnt_read(dev));
> @@ -6076,6 +6103,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
> kobject_uevent(&dev->dev.kobj, KOBJ_REMOVE);
>
> /* Actually switch the network namespace */
> + unlist_netdevice_done(dev);
> dev_net_set(dev, net);
>
> /* If there is an ifindex conflict assign a new one */
^ permalink raw reply related
* Re: [PATCH net 0/4] bridge: Fix problems around the PVID
From: David Miller @ 2013-09-12 20:00 UTC (permalink / raw)
To: makita.toshiaki; +Cc: vyasevic, netdev
In-Reply-To: <1378808874.3988.2.camel@ubuntu-vm-makita>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Tue, 10 Sep 2013 19:27:54 +0900
> There seem to be some undesirable behaviors related with PVID.
> 1. It has no effect assigning PVID to a port. PVID cannot be applied
> to any frame regardless of whether we set it or not.
> 2. FDB entries learned via frames applied PVID are registered with
> VID 0 rather than VID value of PVID.
> 3. We can set 0 or 4095 as a PVID that are not allowed in IEEE 802.1Q.
> This leads interoperational problems such as sending frames with VID
> 4095, which is not allowed in IEEE 802.1Q, and treating frames with VID
> 0 as they belong to VLAN 0, which is expected to be handled as they have
> no VID according to IEEE 802.1Q.
>
> Note: 2nd and 3rd problems are potential and not exposed unless 1st problem
> is fixed, because we cannot activate PVID due to it.
Please work out the issues in patch #2 with Vlad and resubmit this
series.
Thank you.
^ permalink raw reply
* Re: [PATCH net 1/4] bridge: Don't use VID 0 and 4095 in vlan filtering
From: David Miller @ 2013-09-12 19:55 UTC (permalink / raw)
To: vyasevic; +Cc: makita.toshiaki, netdev
In-Reply-To: <522F2B22.5010007@redhat.com>
From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 10 Sep 2013 10:22:26 -0400
> On 09/10/2013 06:32 AM, Toshiaki Makita wrote:
>> IEEE 802.1Q says that:
>> - VID 0 shall not be configured as a PVID, or configured in any
>> - Filtering
>> Database entry.
>> - VID 4095 shall not be configured as a PVID, or transmitted in a tag
>> header. This VID value may be used to indicate a wildcard match for
>> the VID
>> in management operations or Filtering Database entries.
>> (See IEEE 802.1Q-2005 6.7.1 and Table 9-2)
>>
>> Don't accept adding these VIDs in the vlan_filtering implementation.
>>
>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>
> Reviewed-by: Vlad Yasevich <vyasevich@redhat.com>
Vlad, as far as I can see your redhat.com email ends just with a "c"
not a "ch". It's your gmail account that ends in a "ch".
I'm fixing this up while applying these patches, if you have something
automatically generating these reviewed-by strings please fix it up
although I note that some are correct and some are not so maybe you
do this by hand :-)
^ permalink raw reply
* Re: [PATCH 2/7] sysfs: make attr namespace interface less convoluted
From: David Miller @ 2013-09-12 19:46 UTC (permalink / raw)
To: tj; +Cc: gregkh, linux-kernel, kay, ebiederm, netdev, lizefan
In-Reply-To: <1378952949-7900-3-git-send-email-tj@kernel.org>
From: Tejun Heo <tj@kernel.org>
Date: Wed, 11 Sep 2013 22:29:04 -0400
> sysfs ns (namespace) implementation became more convoluted than
> necessary while trying to hide ns information from visible interface.
> The relatively recent attr ns support is a good example.
>
> * attr ns tag is determined by sysfs_ops->namespace() callback while
> dir tag is determined by kobj_type->namespace(). The placement is
> arbitrary.
>
> * Instead of performing operations with explicit ns tag, the namespace
> callback is routed through sysfs_attr_ns(), sysfs_ops->namespace(),
> class_attr_namespace(), class_attr->namespace(). It's not simpler
> in any sense. The only thing this convolution does is traversing
> the whole stack backwards.
>
> The namespace callbacks are unncessary because the operations involved
> are inherently synchronous. The information can be provided in in
> straight-forward top-down direction and reversing that direction is
> unnecessary and against basic design principles.
>
> This backward interface is unnecessarily convoluted and hinders
> properly separating out sysfs from driver model / kobject for proper
> layering. This patch updates attr ns support such that
>
> * sysfs_ops->namespace() and class_attr->namespace() are dropped.
>
> * sysfs_{create|remove}_file_ns(), which take explicit @ns param, are
> added and sysfs_{create|remove}_file() are now simple wrappers
> around the ns aware functions.
>
> * ns handling is dropped from sysfs_chmod_file(). Nobody uses it at
> this point. sysfs_chmod_file_ns() can be added later if necessary.
>
> * Explicit @ns is propagated through class_{create|remove}_file_ns()
> and netdev_class_{create|remove}_file_ns().
>
> * driver/net/bonding which is currently the only user of attr
> namespace is updated to use netdev_class_{create|remove}_file_ns()
> with @bh->net as the ns tag instead of using the namespace callback.
>
> This patch should be an equivalent conversion without any functional
> difference. It makes the code easier to follow, reduces lines of code
> a bit and helps proper separation and layering.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
For networking bits:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH] sh_eth: r8a7790: Handle the RFE (Receive FIFO overflow Error) interrupt
From: Sergei Shtylyov @ 2013-09-12 19:45 UTC (permalink / raw)
To: Laurent Pinchart, Simon Horman; +Cc: netdev, linux-sh, Magnus Damm
In-Reply-To: <4760883.hI1JN8gopV@avalon>
Hello.
On 07/30/2013 06:20 PM, Laurent Pinchart wrote:
>>> The RFE interrupt is enabled for the r8a7790 but isn't handled,
>>> resulting in the interrupts core noticing unhandled interrupts, and
>>> eventually disabling the ethernet IRQ.
>>> Fix it by adding RFE to the bitmask of error interrupts to be handled
>>> for r8a7790.
>> So, Simon hasn't synced his patch to my late bug fix in 3.10... Did this
>> patch help you with your NFS boot issue, Laurent?
> Yes, it fixes the "disabling interrupt, nobody cared" problem. I still have
> intermittent NFS issues, but at least I can now boot.
Looks like the reason for them is the same I had to fix up for the BOCK-W:
the bouncing LINK signal. The PHY used is the same as on BOCK-W, however, its
LED seems to be configured differently: for LINK and ACTIVE LEDs, this is
non-default PHY configuration which AFAIK gets reset to default when the PHY
gets reset. What I saw when I added orintk() for the interrupt enable/mask
tracing was the LINK signal behaving normally at first but after some time ECI
(M-Port in the manuals) interrupts started to behave the way well known from
BOCK-W, i.e. bouncing on and off after each packet; I was also getting endless
RFE (Rx FIFO overflow) interrupts and NFS was unable to mount at all in this
traced mode. The fix was the same as for BOCK-W: to set 'no_ether_link' field
of the platfrom data to 1. After that I've no more seen NFS timeouts and RFE
interrupts. I'm going to continue testing but thought I let everybody know of
my currct findings and the remedy for the NFS issue.
WBR, Sergei
^ 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