* Re: [PATCH v2 2/4] net: ethoc: don't advertise gigabit speed on attached PHY
From: Max Filippov @ 2014-01-29 18:32 UTC (permalink / raw)
To: Florian Fainelli
Cc: Marc Gauthier, Ben Hutchings, LKML, David S. Miller, Chris Zankel,
linux-xtensa@linux-xtensa.org, netdev
In-Reply-To: <CAGVrzcboHp8-qHZseGOVm14u1-cTcOjRZGExFxNu_nbK__XCSA@mail.gmail.com>
On Wed, Jan 29, 2014 at 9:12 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On Jan 28, 2014 11:01 PM, "Max Filippov" <jcmvbkbc@gmail.com> wrote:
>>
>> On Wed, Jan 29, 2014 at 10:47 AM, Florian Fainelli <f.fainelli@gmail.com>
>> wrote:
>> > Hi Max,
>> >
>> > Le 28/01/2014 22:00, Max Filippov a écrit :
>> >
>> >> OpenCores 10/100 Mbps MAC does not support speeds above 100 Mbps, but
>> >> does
>> >> not disable advertisement when PHY supports them. This results in
>> >> non-functioning network when the MAC is connected to a gigabit PHY
>> >> connected
>> >> to a gigabit switch.
>> >>
>> >> The fix is to disable gigabit speed advertisement on attached PHY
>> >> unconditionally.
>> >>
>> >> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>> >> ---
>> >> Changes v1->v2:
>> >> - disable both gigabit advertisement and support.
>> >>
>> >> drivers/net/ethernet/ethoc.c | 8 ++++++++
>> >> 1 file changed, 8 insertions(+)
>> >>
>> >> diff --git a/drivers/net/ethernet/ethoc.c
>> >> b/drivers/net/ethernet/ethoc.c
>> >> index 4de8cfd..5643b2d 100644
>> >> --- a/drivers/net/ethernet/ethoc.c
>> >> +++ b/drivers/net/ethernet/ethoc.c
>> >> @@ -688,6 +688,14 @@ static int ethoc_mdio_probe(struct net_device
>> >> *dev)
>> >> }
>> >>
>> >> priv->phy = phy;
>> >> + phy_update_advert(phy,
>> >> + ADVERTISED_1000baseT_Full |
>> >> + ADVERTISED_1000baseT_Half, 0);
>> >> + phy_start_aneg(phy);
>> >
>> >
>> > This does not look necessary, you should not have to call
>> > phy_start_aneg()
>> > because the PHY state machine is not yet started, at best this calls
>> > does
>> > nothing.
>>
>> This call actually makes the whole thing work, because otherwise once
>> gigabit
>> support is cleared from the supported mask genphy_config_advert does not
>> update gigabit advertisement register, leaving it enabled.
>
> OK, then we need to figure out what is wrong with ethoc since this is
> unusual.
Maybe they boot up with gigabit advertisement disabled in their PHY
and thus they don't see the problem?
> Other drivers do the following:
>
> - connect to the PHY
> - phydev->supported = PHY_BASIC_FEATURES
> - phydev->advertising &= phydev->supported
> - start the PHY state machine
>
> And they work just fine. Is the PHY driver you are bound to the "Generic
> PHY" or something else which does something funky in config_aneg()?
It's marvell 88E1111 from the KC-705 board, but the behaviour doesn't
change if I disable it and the generic phy is used.
--
Thanks.
-- Max
^ permalink raw reply
* [rfc] b43: possible missing break in b43_wa_all
From: Dave Jones @ 2014-01-29 17:35 UTC (permalink / raw)
To: netdev; +Cc: stefano.brivio
I'm going through all the 'missing break in switch' bugs in coverity, and
most of them look like intentional/false positives, but this one looks odd to me.
It seems the intent is to B43_WARN_ON only if we don't know the phy, so it seems
strange to do it after we've just initialized type 7.
If this is agreed to be a bug, I'll submit the below patch properly,
but I want to be sure this isn't something like "type 7 is work in progress"
It's been this way since 2007 though. I guess no-one runs with CONFIG_B43_DEBUG on.
Dave
diff --git a/drivers/net/wireless/b43/wa.c b/drivers/net/wireless/b43/wa.c
index 9b1a038be08b..47f0ec887e1f 100644
--- a/drivers/net/wireless/b43/wa.c
+++ b/drivers/net/wireless/b43/wa.c
@@ -587,6 +587,7 @@ void b43_wa_all(struct b43_wldev *dev)
b43_wa_txpuoff_rxpuon(dev);
b43_wa_txlna_gain(dev);
b43_wa_rssi_adc(dev);
+ break;
default:
B43_WARN_ON(1);
}
^ permalink raw reply related
* Re: [PATCH net] net/ipv4: Use non-atomic allocation of udp offloads structure instance
From: Eric Dumazet @ 2014-01-29 17:31 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, netdev, dan.carpenter, Shlomo Pongratz
In-Reply-To: <52E92F92.9080307@mellanox.com>
On Wed, 2014-01-29 at 18:42 +0200, Or Gerlitz wrote:
> On 29/01/2014 18:20, Eric Dumazet wrote:
> > Could you also fix all the rcu_dereference() calls ?
> >
> > Try CONFIG_PROVE_RCU=y
>
> Thanks Eric, I tried this out now and got one alarm on udp_add_offload,
> which I will loop up how to fix,
> do you see anything else?
>
>
You should have same issue in udp_del_offload()
(Lockdep triggers once only)
> ===============================
> [ INFO: suspicious RCU usage. ]
> 3.13.0+ #109 Not tainted
> -------------------------------
> net/ipv4/udp_offload.c:112 suspicious rcu_dereference_check() usage!
>
> other info that might help us debug this:
>
>
> rcu_scheduler_active = 1, debug_locks = 0
> 4 locks held by kworker/2:2/2752:
> #0: (vxlan){.+.+.+}, at: [<ffffffff8108bd5a>]
> process_one_work+0x15a/0x710
> #1: ((&vxlan->sock_work)){+.+.+.}, at: [<ffffffff8108bd5a>]
> process_one_work+0x15a/0x710
> #2: (&(&vn->sock_lock)->rlock){+.+...}, at: [<ffffffffa03caae3>]
> vxlan_sock_add+0x323/0xb70 [vxlan]
> #3: (udp_offload_lock){+.+...}, at: [<ffffffff814d22e1>]
> udp_add_offload+0x41/0xd0
>
> stack backtrace:
> CPU: 2 PID: 2752 Comm: kworker/2:2 Not tainted 3.13.0+ #109
> Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2007
> Workqueue: vxlan vxlan_sock_work [vxlan]
> 0000000000000001 ffff8800d1949bf8 ffffffff81528412 0000000000000002
> ffff88021ef2e060 ffff8800d1949c28 ffffffff810b8373 ffff880218f59160
> 0000000000000000 ffffffff81aa3540 ffff88021dcaf000 ffff8800d1949c48
> Call Trace:
> [<ffffffff81528412>] dump_stack+0x51/0x77
> [<ffffffff810b8373>] lockdep_rcu_suspicious+0x103/0x140
> [<ffffffff814d234a>] udp_add_offload+0xaa/0xd0
> [<ffffffffa03cb0ad>] vxlan_sock_add+0x8ed/0xb70 [vxlan]
> [<ffffffffa03caa77>] ? vxlan_sock_add+0x2b7/0xb70 [vxlan]
> [<ffffffffa03ce390>] ? vxlan_xmit+0x650/0x650 [vxlan]
> [<ffffffffa03cb330>] ? vxlan_sock_add+0xb70/0xb70 [vxlan]
> [<ffffffff81291f7c>] ? delay_tsc+0x6c/0xd0
> [<ffffffffa03cb439>] vxlan_sock_work+0x109/0x260 [vxlan]
> [<ffffffffa03cb330>] ? vxlan_sock_add+0xb70/0xb70 [vxlan]
> [<ffffffff8108bdd1>] process_one_work+0x1d1/0x710
> [<ffffffff8108bd5a>] ? process_one_work+0x15a/0x710
> [<ffffffff8108c432>] worker_thread+0x122/0x400
> [<ffffffff8108c310>] ? process_one_work+0x710/0x710
> [<ffffffff810943fe>] kthread+0xde/0x100
> [<ffffffff81094320>] ? __init_kthread_worker+0x70/0x70
> [<ffffffff8152f3bc>] ret_from_fork+0x7c/0xb0
> [<ffffffff81094320>] ? __init_kthread_worker+0x70/0x70
>
> --
> 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
* Re: [PATCH 0/2] [BUG FIXES - 3.10.27] sit: More backports
From: Willem de Bruijn @ 2014-01-29 17:21 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: Steven Rostedt, linux-kernel, netdev, stable, Clark Williams,
Luis Claudio R. Goncalves, John Kacur
In-Reply-To: <52E9267C.90403@6wind.com>
> From d101450583c3a472a2a94904cfe13fd4e7d2f519 Mon Sep 17 00:00:00 2001
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Wed, 29 Jan 2014 16:40:30 +0100
> Subject: [PATCH] sit: fix double free of fb_tunnel_dev on exit
>
> This problem was fixed upstream by commit 9434266f2c64 ("sit: fix use after free
> of fb_tunnel_dev").
> The upstream patch depends on upstream commit 5e6700b3bf98 ("sit: add support of
> x-netns"), which was not backported into 3.10 branch.
>
> First, explain the problem: when the sit module is unloaded, sit_cleanup() is
> called.
> rmmod sit
> => sit_cleanup()
> => rtnl_link_unregister()
> => __rtnl_kill_links()
> => for_each_netdev(net, dev) {
> if (dev->rtnl_link_ops == ops)
> ops->dellink(dev, &list_kill);
> }
> At this point, the FB device is deleted (and all sit tunnels).
> => unregister_pernet_device()
> => unregister_pernet_operations()
> => ops_exit_list()
> => sit_exit_net()
> => sit_destroy_tunnels()
> In this function, no tunnel is found.
> => unregister_netdevice_queue(sitn->fb_tunnel_dev, &list);
> We delete the FB device a second time here!
>
> Because we cannot simply remove the second deletion (sit_exit_net() must remove
> the FB device when a netns is deleted), we add an rtnl ops which delete all sit
> device excepting the FB device and thus we can keep the explicit deletion in
> sit_exit_net().
>
> CC: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> net/ipv6/sit.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index 0491264b8bfc..620d326e8fdd 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -1507,6 +1507,15 @@ static const struct nla_policy ipip6_policy[IFLA_IPTUN_MAX + 1] = {
> #endif
> };
>
> +static void ipip6_dellink(struct net_device *dev, struct list_head *head)
> +{
> + struct net *net = dev_net(dev);
> + struct sit_net *sitn = net_generic(net, sit_net_id);
> +
> + if (dev != sitn->fb_tunnel_dev)
> + unregister_netdevice_queue(dev, head);
> +}
> +
> static struct rtnl_link_ops sit_link_ops __read_mostly = {
> .kind = "sit",
> .maxtype = IFLA_IPTUN_MAX,
> @@ -1517,6 +1526,7 @@ static struct rtnl_link_ops sit_link_ops __read_mostly = {
> .changelink = ipip6_changelink,
> .get_size = ipip6_get_size,
> .fill_info = ipip6_fill_info,
> + .dellink = ipip6_dellink,
> };
>
> static struct xfrm_tunnel sit_handler __read_mostly = {
> --
> 1.8.4.1
This looks good to me. It is the same as the backport "sit: fix use
after free of fb_tunnel_dev" (9434266f2c64), minus the small code
cleanup at the end of that patch that is not relevant to 3.10.27 (do
not define sit_net *sitn in sit_exit_net if it is not used there. But in
3.10.27 it is used, in unregister_netdevice_queue).
^ permalink raw reply
* Re: [PATCH v2 1/4] phy: provide accessors for 'advertising' and 'supported' fields
From: Florian Fainelli @ 2014-01-29 17:14 UTC (permalink / raw)
To: Max Filippov
Cc: linux-xtensa@linux-xtensa.org, netdev,
linux-kernel@vger.kernel.org, Chris Zankel, Marc Gauthier,
David S. Miller, Ben Hutchings
In-Reply-To: <1390975218-13863-2-git-send-email-jcmvbkbc@gmail.com>
2014-01-28 Max Filippov <jcmvbkbc@gmail.com>:
> Many network drivers directly modify phy_device::advertising and
> phy_device::supported. Provide accessors to these fields to better
> isolate phylib from its users.
>
> Suggested-by: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
After giving some more thought to this patch, I am not sure this
really adds anything, struct phy_device is already exposed to drivers,
and those drivers have been able to modify phydev->supported and
phydev->advertising to suit their needs.
> ---
> Changes v1->v2:
> - new patch
>
> include/linux/phy.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 48a4dc3..2ae58f8 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -559,6 +559,18 @@ static inline int phy_read_status(struct phy_device *phydev) {
> return phydev->drv->read_status(phydev);
> }
>
> +static inline void phy_update_advert(struct phy_device *phydev, u32 clear,
> + u32 set)
> +{
> + phydev->advertising = (phydev->advertising & ~clear) | set;
> +}
> +
> +static inline void phy_update_supported(struct phy_device *phydev, u32 clear,
> + u32 set)
> +{
> + phydev->supported = (phydev->supported & ~clear) | set;
> +}
> +
> int genphy_setup_forced(struct phy_device *phydev);
> int genphy_restart_aneg(struct phy_device *phydev);
> int genphy_config_aneg(struct phy_device *phydev);
> --
> 1.8.1.4
>
--
Florian
^ permalink raw reply
* Re: [PATCH net] net/ipv4: Use non-atomic allocation of udp offloads structure instance
From: Or Gerlitz @ 2014-01-29 16:42 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev, dan.carpenter, Shlomo Pongratz
In-Reply-To: <1391012411.28432.55.camel@edumazet-glaptop2.roam.corp.google.com>
On 29/01/2014 18:20, Eric Dumazet wrote:
> Could you also fix all the rcu_dereference() calls ?
>
> Try CONFIG_PROVE_RCU=y
Thanks Eric, I tried this out now and got one alarm on udp_add_offload,
which I will loop up how to fix,
do you see anything else?
===============================
[ INFO: suspicious RCU usage. ]
3.13.0+ #109 Not tainted
-------------------------------
net/ipv4/udp_offload.c:112 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 0
4 locks held by kworker/2:2/2752:
#0: (vxlan){.+.+.+}, at: [<ffffffff8108bd5a>]
process_one_work+0x15a/0x710
#1: ((&vxlan->sock_work)){+.+.+.}, at: [<ffffffff8108bd5a>]
process_one_work+0x15a/0x710
#2: (&(&vn->sock_lock)->rlock){+.+...}, at: [<ffffffffa03caae3>]
vxlan_sock_add+0x323/0xb70 [vxlan]
#3: (udp_offload_lock){+.+...}, at: [<ffffffff814d22e1>]
udp_add_offload+0x41/0xd0
stack backtrace:
CPU: 2 PID: 2752 Comm: kworker/2:2 Not tainted 3.13.0+ #109
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2007
Workqueue: vxlan vxlan_sock_work [vxlan]
0000000000000001 ffff8800d1949bf8 ffffffff81528412 0000000000000002
ffff88021ef2e060 ffff8800d1949c28 ffffffff810b8373 ffff880218f59160
0000000000000000 ffffffff81aa3540 ffff88021dcaf000 ffff8800d1949c48
Call Trace:
[<ffffffff81528412>] dump_stack+0x51/0x77
[<ffffffff810b8373>] lockdep_rcu_suspicious+0x103/0x140
[<ffffffff814d234a>] udp_add_offload+0xaa/0xd0
[<ffffffffa03cb0ad>] vxlan_sock_add+0x8ed/0xb70 [vxlan]
[<ffffffffa03caa77>] ? vxlan_sock_add+0x2b7/0xb70 [vxlan]
[<ffffffffa03ce390>] ? vxlan_xmit+0x650/0x650 [vxlan]
[<ffffffffa03cb330>] ? vxlan_sock_add+0xb70/0xb70 [vxlan]
[<ffffffff81291f7c>] ? delay_tsc+0x6c/0xd0
[<ffffffffa03cb439>] vxlan_sock_work+0x109/0x260 [vxlan]
[<ffffffffa03cb330>] ? vxlan_sock_add+0xb70/0xb70 [vxlan]
[<ffffffff8108bdd1>] process_one_work+0x1d1/0x710
[<ffffffff8108bd5a>] ? process_one_work+0x15a/0x710
[<ffffffff8108c432>] worker_thread+0x122/0x400
[<ffffffff8108c310>] ? process_one_work+0x710/0x710
[<ffffffff810943fe>] kthread+0xde/0x100
[<ffffffff81094320>] ? __init_kthread_worker+0x70/0x70
[<ffffffff8152f3bc>] ret_from_fork+0x7c/0xb0
[<ffffffff81094320>] ? __init_kthread_worker+0x70/0x70
^ permalink raw reply
* Re: [PATCH net] net/ipv4: Use non-atomic allocation of udp offloads structure instance
From: Eric Dumazet @ 2014-01-29 16:20 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, netdev, dan.carpenter
In-Reply-To: <1391011739-8597-1-git-send-email-ogerlitz@mellanox.com>
On Wed, 2014-01-29 at 18:08 +0200, Or Gerlitz wrote:
> Since udp_add_offload() can be called from non-sleepable context e.g
> under this call tree from the vxlan driver use case:
>
> vxlan_socket_create() <-- holds the spinlock
> -> vxlan_notify_add_rx_port()
> -> udp_add_offload() <-- schedules
>
> we should allocate the udp_offloads structure in atomic manner.
>
> Fixes: b582ef0 ('net: Add GRO support for UDP encapsulating protocols')
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> net/ipv4/udp_offload.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 25f5cee..2ffea6f 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -101,7 +101,7 @@ out:
> int udp_add_offload(struct udp_offload *uo)
> {
> struct udp_offload_priv __rcu **head = &udp_offload_base;
> - struct udp_offload_priv *new_offload = kzalloc(sizeof(*new_offload), GFP_KERNEL);
> + struct udp_offload_priv *new_offload = kzalloc(sizeof(*new_offload), GFP_ATOMIC);
>
> if (!new_offload)
> return -ENOMEM;
Could you also fix all the rcu_dereference() calls ?
Try CONFIG_PROVE_RCU=y
^ permalink raw reply
* Re: [PATCH stable 3.11+] can: use private sk reference to detect originating socket
From: Eric Dumazet @ 2014-01-29 16:12 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: David Miller, Linux Netdev List, Andre Naujoks
In-Reply-To: <52E905C8.1060408@hartkopp.net>
On Wed, 2014-01-29 at 14:44 +0100, Oliver Hartkopp wrote:
> ps. I would suggest to move the BUG() in WARN_ON_ONCE() to fulfill the
> requirement to detect a possible misuse but not kill the entire machine.
> BUG() is intended to be used when there's no way to continue any reasonable
> operation. Detecting and fixing this issue half a year after the "temporary
> sanity check" has been integrated is a bad example for using BUG() IMO.
I find worrying it took 6 months for CAN developers to hit this problem.
A WARN_ON_ONCE() has almost no chance being noticed in the embedded
world, and makes no pressure on developers to fix the bugs.
Patch is 6 months old, and will be reverted when we have confidence that
all offenders are fixed. Your feedback shows that we shall wait 10 years
just to make sure, and maybe we wont revert it.
All BUG() in the kernel have the property to potentially halt a
perfectly working machine, at a bad time, yes. But they make us
build a better kernel.
^ permalink raw reply
* [PATCH net] net/vxlan: Go over all candidate streams for GRO matching
From: Or Gerlitz @ 2014-01-29 16:10 UTC (permalink / raw)
To: davem; +Cc: netdev, therbert, edumazet, Or Gerlitz
The loop in vxlan_gro_receive() over the current set of candidates for
coalescing was wrongly aborted once a match was found. In rare cases,
this can cause a false-positives matching in the next layer GRO checks.
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/vxlan.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 026a313..3d6cd1c 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -596,10 +596,8 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff
NAPI_GRO_CB(p)->same_flow = 0;
continue;
}
- goto found;
}
-found:
type = eh->h_proto;
rcu_read_lock();
--
1.7.1
^ permalink raw reply related
* [PATCH net] net/ipv4: Use non-atomic allocation of udp offloads structure instance
From: Or Gerlitz @ 2014-01-29 16:08 UTC (permalink / raw)
To: davem; +Cc: netdev, dan.carpenter, Or Gerlitz
Since udp_add_offload() can be called from non-sleepable context e.g
under this call tree from the vxlan driver use case:
vxlan_socket_create() <-- holds the spinlock
-> vxlan_notify_add_rx_port()
-> udp_add_offload() <-- schedules
we should allocate the udp_offloads structure in atomic manner.
Fixes: b582ef0 ('net: Add GRO support for UDP encapsulating protocols')
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
net/ipv4/udp_offload.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 25f5cee..2ffea6f 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -101,7 +101,7 @@ out:
int udp_add_offload(struct udp_offload *uo)
{
struct udp_offload_priv __rcu **head = &udp_offload_base;
- struct udp_offload_priv *new_offload = kzalloc(sizeof(*new_offload), GFP_KERNEL);
+ struct udp_offload_priv *new_offload = kzalloc(sizeof(*new_offload), GFP_ATOMIC);
if (!new_offload)
return -ENOMEM;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 0/2] [BUG FIXES - 3.10.27] sit: More backports
From: Nicolas Dichtel @ 2014-01-29 16:04 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, netdev, stable, Clark Williams,
Luis Claudio R. Goncalves, John Kacur
In-Reply-To: <20140129075745.4227a2ed@gandalf.local.home>
Le 29/01/2014 13:57, Steven Rostedt a écrit :
> On Wed, 29 Jan 2014 12:04:05 +0100
> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
>> Le 28/01/2014 21:57, Steven Rostedt a écrit :
>>> At Red Hat we base our real-time kernel off of 3.10.27 and do lots of
>>> stress testing on that kernel. This has discovered some bugs that we
>>> can hit with the vanilla 3.10.27 kernel (no -rt patches applied).
>>>
>>> I sent out a bug fix that can cause a crash with the current 3.10.27
>>> when you add and then remove the sit module. That patch is obsoleted by
>>> these patches, as that patch was not enough.
>> Can you explain a bit more which problem remains after that patch?
>> I wonder if a problem remains also with ip6_tunnel.ko (net/ipv6/ip6_tunnel.c),
>> the same problem was spotted into this module.
>
> Hmm, OK it may only need the first version of the patch. It was one of
> those cases where it didn't fix the second bug, so we added the full
> patch as well. Then we found the second bug but never tried all the
> tests with the smaller version of the patch. I'll put back the first
> patch and then ask our QA to retest it with the older version and the
> other patch.
>
>>
>>>
>>> A previous patch that was backported:
>>>
>>> Upstream commit 205983c43700ac3a81e7625273a3fa83cd2759b5
>>> sit: allow to use rtnl ops on fb tunnel
>>>
>>> Had a depenency on commit 5e6700b3bf98 ("sit: add support of x-netns")
>>> which was not backported. The dependency was only on part of that
>>> commit which is what I backported.
>> I cannot comment directly the patch, it was an attachement, hence I put my
>
> It's not really an attachment. It was sent with quilt mail, which only
> makes it look like one. What mail client are you using? mutt, alpine,
> evolution, claws-mail, and thunderbird all show it as a normal patch.
> I do know that k9-mail on android makes it into an attachment.
Thunderbird. The patch was show as a normal aptch, but when I do "reply all", I
get an empty mail.
>
>> comments here.
>> In patch 0001-sit-Unregister-sit-devices-with-rtnl_link_ops.patch, I wonder how
>> 'if (dev_net(t->dev) != net)' can be wrong. If commit 5e6700b3bf98 ("sit: add
>> support of x-netns") has not been backported, this test is always true.
>
> Should it just be removed then? This has fixed our bugs, but perhaps it
> opened new ones we haven't detected yet. I can remove the if and
> unregister and see if it still works. Or perhaps calling unregister all
> the time isn't bad.
Ok, I've think a bit more to this problem. First, let's explain it.
rmmod sit
=> sit_cleanup()
=> rtnl_link_unregister()
=> __rtnl_kill_links()
=> for_each_netdev(net, dev) {
if (dev->rtnl_link_ops == ops)
ops->dellink(dev, &list_kill);
}
**at this point, the FB device is deleted (and all sit tunnels)**
=> unregister_pernet_device()
=> unregister_pernet_operations()
=> ops_exit_list()
=> sit_exit_net()
=> sit_destroy_tunnels()
in this function, no tunnel is found
unregister_netdevice_queue(sitn->fb_tunnel_dev, &list);
** second deletion, here is the bug **
Now, what happen on netns deletion with the previous patch? Only sit_exit_net()
will be called, hence with the previous patch, the fb device will not be
destroyed, netns will leak.
Your patch serie seems to be the good way to go (note that patch 1/2 does not
compile) but I think the fix is smaller because we don't have x-netns.
Here is my proposal, if you agree, I will send the same patch for ip6_tunnnel,
which have the netns leak.
From d101450583c3a472a2a94904cfe13fd4e7d2f519 Mon Sep 17 00:00:00 2001
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed, 29 Jan 2014 16:40:30 +0100
Subject: [PATCH] sit: fix double free of fb_tunnel_dev on exit
This problem was fixed upstream by commit 9434266f2c64 ("sit: fix use after free
of fb_tunnel_dev").
The upstream patch depends on upstream commit 5e6700b3bf98 ("sit: add support of
x-netns"), which was not backported into 3.10 branch.
First, explain the problem: when the sit module is unloaded, sit_cleanup() is
called.
rmmod sit
=> sit_cleanup()
=> rtnl_link_unregister()
=> __rtnl_kill_links()
=> for_each_netdev(net, dev) {
if (dev->rtnl_link_ops == ops)
ops->dellink(dev, &list_kill);
}
At this point, the FB device is deleted (and all sit tunnels).
=> unregister_pernet_device()
=> unregister_pernet_operations()
=> ops_exit_list()
=> sit_exit_net()
=> sit_destroy_tunnels()
In this function, no tunnel is found.
=> unregister_netdevice_queue(sitn->fb_tunnel_dev, &list);
We delete the FB device a second time here!
Because we cannot simply remove the second deletion (sit_exit_net() must remove
the FB device when a netns is deleted), we add an rtnl ops which delete all sit
device excepting the FB device and thus we can keep the explicit deletion in
sit_exit_net().
CC: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
net/ipv6/sit.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 0491264b8bfc..620d326e8fdd 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1507,6 +1507,15 @@ static const struct nla_policy
ipip6_policy[IFLA_IPTUN_MAX + 1] = {
#endif
};
+static void ipip6_dellink(struct net_device *dev, struct list_head *head)
+{
+ struct net *net = dev_net(dev);
+ struct sit_net *sitn = net_generic(net, sit_net_id);
+
+ if (dev != sitn->fb_tunnel_dev)
+ unregister_netdevice_queue(dev, head);
+}
+
static struct rtnl_link_ops sit_link_ops __read_mostly = {
.kind = "sit",
.maxtype = IFLA_IPTUN_MAX,
@@ -1517,6 +1526,7 @@ static struct rtnl_link_ops sit_link_ops __read_mostly = {
.changelink = ipip6_changelink,
.get_size = ipip6_get_size,
.fill_info = ipip6_fill_info,
+ .dellink = ipip6_dellink,
};
static struct xfrm_tunnel sit_handler __read_mostly = {
--
1.8.4.1
^ permalink raw reply related
* Re: [PATCH stable 3.11+] can: bcm: add skb destructor
From: Eric Dumazet @ 2014-01-29 15:48 UTC (permalink / raw)
To: Andre Naujoks; +Cc: David Miller, socketcan, netdev
In-Reply-To: <52E91FD8.90402@gmail.com>
On Wed, 2014-01-29 at 16:35 +0100, Andre Naujoks wrote:
> I am not resisting to anything. I was just *irritated* about the way
> this was handled. Since Oliver is already trying to fix this, any
> further discussion here is meaningless anyway.
I sent a patch, because I thought the fix was obvious, but last attempt
from Oliver was way too complex for a backport.
^ permalink raw reply
* Re: [PATCH stable 3.11+] can: use private sk reference to detect originating socket
From: Eric Dumazet @ 2014-01-29 15:47 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: David Miller, Linux Netdev List, Andre Naujoks
In-Reply-To: <1391009406.28432.38.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, 2014-01-29 at 07:30 -0800, Eric Dumazet wrote:
> On Wed, 2014-01-29 at 07:02 -0800, Eric Dumazet wrote:
>
> > Thats how every protocol does this, with variants.
> >
> > Check l2tp_sock_wfree()/l2tp_skb_set_owner_w() for an example
>
> Also keep in mind this patch is needed for all kernels, not only 3.11+
>
> So better keep it as simple as possible, to ease backports.
Please try the following patch.
diff --git a/net/can/af_can.c b/net/can/af_can.c
index d249874a366d..98d74bd6030f 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -290,7 +290,8 @@ int can_send(struct sk_buff *skb, int loop)
return -ENOMEM;
}
- newskb->sk = skb->sk;
+ if (skb->sk)
+ can_skb_set_owner(newskb, skb->sk);
newskb->ip_summed = CHECKSUM_UNNECESSARY;
newskb->pkt_type = PACKET_BROADCAST;
}
diff --git a/net/can/af_can.h b/net/can/af_can.h
index 6de58b40535c..e9cd045ba1cc 100644
--- a/net/can/af_can.h
+++ b/net/can/af_can.h
@@ -45,6 +45,7 @@
#include <linux/list.h>
#include <linux/rcupdate.h>
#include <linux/can.h>
+#include <net/sock.h>
/* af_can rx dispatcher structures */
@@ -118,4 +119,17 @@ extern struct s_stats can_stats; /* packet statistics */
extern struct s_pstats can_pstats; /* receive list statistics */
extern struct hlist_head can_rx_dev_list; /* rx dispatcher structures */
+static inline void can_skb_destructor(struct sk_buff *skb)
+{
+ sock_put(skb->sk);
+}
+
+static inline void can_skb_set_owner(struct sk_buff *skb, struct sock *sk)
+{
+ skb_orphan(skb);
+ skb->destructor = can_skb_destructor;
+ skb->sk = sk;
+ sock_hold(sk);
+}
+
#endif /* AF_CAN_H */
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 3fc737b214c7..b31480f3a97c 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -59,6 +59,7 @@
#include <linux/slab.h>
#include <net/sock.h>
#include <net/net_namespace.h>
+#include "af_can.h"
/*
* To send multiple CAN frame content within TX_SETUP or to filter
@@ -268,7 +269,8 @@ static void bcm_can_tx(struct bcm_op *op)
/* send with loopback */
skb->dev = dev;
- skb->sk = op->sk;
+ if (op->sk)
+ can_skb_set_owner(skb, op->sk);
can_send(skb, 1);
/* update statistics */
^ permalink raw reply related
* Re: [PATCH stable 3.11+] can: bcm: add skb destructor
From: Andre Naujoks @ 2014-01-29 15:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, socketcan, netdev
In-Reply-To: <1391007225.28432.29.camel@edumazet-glaptop2.roam.corp.google.com>
On 29.01.2014 15:53, schrieb Eric Dumazet:
> On Wed, 2014-01-29 at 09:47 +0100, Andre Naujoks wrote:
>> On 29.01.2014 08:46, schrieb David Miller:
>>> From: Andre Naujoks <nautsch2@gmail.com>
>>> Date: Wed, 29 Jan 2014 08:40:03 +0100
>>>
>>>> Even if this is a bug in the CAN BCM implementation. Your "fix" just
>>>> enabled a user space application to shut down any machine with a kernel
>>>> containing the BUG_ON patch.
>>>
>>> Rather, he detected a potential stray pointer reference to freed data
>>> that was caused by the CAN code which would difficult if not
>>> impossible to detect otherwise.
>>>
>>> That's even more dangerous, and you should be thanking him.
>>
>> "potential" is the keyword here. But its a definite kernel crash as it
>> is right now with a standard use case for the BCM.
>>
>> Don't get me wrong. If there are bugs in the code, they should be fixed,
>> but I don't think breaking a working (even if flawed) part of the kernel
>> is the right thing to do here.
>
> Shall I remember you this patch was suggested by David Miller, our
> beloved network maintainer ?
no, but thank you.
>
> Really this is quite silly, I'll tell you.
Totally with you on that.
>
> I can send a patch to mark CAN as BROKEN if you want, or you can send an
> appropriate patch.
>
> Your resistance is futile.
I am not resisting to anything. I was just *irritated* about the way
this was handled. Since Oliver is already trying to fix this, any
further discussion here is meaningless anyway.
Regards
Andre
>
>
^ permalink raw reply
* Re: [PATCH stable 3.11+] can: use private sk reference to detect originating socket
From: Eric Dumazet @ 2014-01-29 15:30 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: David Miller, Linux Netdev List, Andre Naujoks
In-Reply-To: <1391007745.28432.36.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, 2014-01-29 at 07:02 -0800, Eric Dumazet wrote:
> Thats how every protocol does this, with variants.
>
> Check l2tp_sock_wfree()/l2tp_skb_set_owner_w() for an example
Also keep in mind this patch is needed for all kernels, not only 3.11+
So better keep it as simple as possible, to ease backports.
^ permalink raw reply
* Re: [PATCH stable 3.11+] can: use private sk reference to detect originating socket
From: Eric Dumazet @ 2014-01-29 15:02 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: David Miller, Linux Netdev List, Andre Naujoks
In-Reply-To: <52E905C8.1060408@hartkopp.net>
On Wed, 2014-01-29 at 14:44 +0100, Oliver Hartkopp wrote:
> Commit 376c7311bdb6 (net: add a temporary sanity check in skb_orphan())
> leads to a BUG in can_put_echo_skb() when skb_orphan() is executed.
>
> The use of skb->sk to detect the originating socket can lead to side effects
> in net/core and net/sched. Therefore the reference needed in net/can/raw.c is
> now stored in a CAN private skbuff space in struct can_skb_priv.
>
> The reference which comes with the CAN frame is only needed in raw_rcv() and
> checked against his own sock pointer and does only special things if it
> matches. So removing the socket does not hurt as the in-flight skbs are
> removed and the receive filters are purged anyway when the socket disappears.
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>
> ---
>
> Here's my idea to fix up the original skb->sk handling concept for detecting
> the originating socket instance. The patch applies properly down to 3.11.
>
> Eric please take a look if I used sock_alloc_send_skb() correctly in bcm.c.
> Andre please check if you see any functional differences on your machine.
I think this is opening yet another can of bugs.
CAN calls dev_queue_xmit() : This layer can zap skb->cb[] totally.
Please hold a reference on the socket instead. Thats a 10 lines patch,
instead of yet another complex patch.
void can_destructor(struct sk_buff *skb)
{
sock_put(skb->sk);
}
void can_set_owner(struct sk_buff *skb, struct sock *sk)
{
skb_orphan(skb);
skb->destructor = can_skb_destructor;
skb->sk = sk;
sock_hold(sk);
}
Thats how every protocol does this, with variants.
Check l2tp_sock_wfree()/l2tp_skb_set_owner_w() for an example
^ permalink raw reply
* Re: [PATCH 3/3] net: via-rhine: add OF bus binding
From: Rob Herring @ 2014-01-29 14:59 UTC (permalink / raw)
To: Alexey Charkov
Cc: Tony Prisk, netdev,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Roger Luethi,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CABjd4Yy2pFr=qr1_e3XqupHngp_mF0GcDT0hCM46v=tNNDY1hw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Jan 28, 2014 at 11:20 PM, Alexey Charkov <alchark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2014/1/29 Tony Prisk <linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>:
>> On 29/01/14 07:27, Alexey Charkov wrote:
>>>
>>> 2014/1/27 Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>>>
>>>> On Mon, Jan 27, 2014 at 5:51 AM, Alexey Charkov <alchark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> wrote:
>>>>>
>>>>> This should make the driver usable with VIA/WonderMedia ARM-based
>>>>> Systems-on-Chip integrated Rhine III adapters. Note that these
>>>>> are always in MMIO mode, and don't have any known EEPROM.
>>>>>
>>>>> Signed-off-by: Alexey Charkov <alchark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> Signed-off-by: Roger Luethi <rl-7uj+XXdSDtwfv37vnLkPlQ@public.gmane.org>
>>>>> ---
>>>>> .../devicetree/bindings/net/via-rhine.txt | 18 ++
>>>>> drivers/net/ethernet/via/Kconfig | 2 +-
>>>>> drivers/net/ethernet/via/via-rhine.c | 293
>>>>> +++++++++++++--------
>>>>> 3 files changed, 200 insertions(+), 113 deletions(-)
>>>>> create mode 100644 Documentation/devicetree/bindings/net/via-rhine.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/via-rhine.txt
>>>>> b/Documentation/devicetree/bindings/net/via-rhine.txt
>>>>> new file mode 100644
>>>>> index 0000000..684dd3a
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/via-rhine.txt
>>>>> @@ -0,0 +1,18 @@
>>>>> +* VIA Rhine 10/100 Network Controller
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible : Should be "via,rhine"
>>>>
>>>> This should be more specific rather than...
>>>>
>>>>> +- reg : Address and length of the io space
>>>>> +- interrupts : Should contain the controller interrupt line
>>>>> +- rhine,revision : Rhine core revision, used to inform the
>>>>> + driver of quirks and capabilities to expect from
>>>>> + the device. Mimics the respective PCI attribute.
>>>>
>>>> having this property. The OF match table can then have the quirks set
>>>> based on compatible strings.
>>>
>>> Sounds fair. Do you think something like the following would fly?
>>>
>>> Required properties:
>>> - compatible : Should be "via,rhine-soc-vt8500" for integrated Rhine
>>> cores found in SoC's such as VIA VT8500, WonderMedia WM8950 and
>>> possibly others. These are listed as 1106:3106 rev. 0x84 on the
>>> virtual PCI bus under vendor-provided kernels.
>>
>> Does it need a special name? I would have assumed they are using their own
>> IP for the VT6105 or VT6106S.
>> Then you can use it to add quirks later on if needed.
>
> The problem is that I have no reliable source for the exact name of
> the IP block. The lookup table within the driver says that rev. 0x83
> is VT6105_B0, and rev. 0x8A is VT6105L (and it doesn't contain any
> entry for 0x84, so our case gets treated as 0x83 currently).
>
> If we only differentiate them by the compatible string, I would be
> reluctant to call it vt6105 or vt6106 or whatever else without knowing
> for sure. Otherwise, if at some point we need to add any quirks
> specific to rev. 0x84, we wouldn't be able to do that without
> side-effects.
If you don't know the IP rev, then you should assume it is different
and you should certainly know which chip it is in. This is why the soc
name is typically in the compatible string. You can always have
multiple compatible strings. You want to have a unique compatible
string so you can add quirks later if needed.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 resend nf] ipvs: fix AF assignment in ip_vs_conn_new()
From: Michal Kubecek @ 2014-01-29 14:54 UTC (permalink / raw)
To: netfilter-devel
Cc: netdev, lvs-devel, Wensong Zhang, Simon Horman, Julian Anastasov,
Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller
If a fwmark is passed to ip_vs_conn_new(), it is passed in
vaddr, not daddr. Therefore we should set AF to AF_UNSPEC in
vaddr assignment (like we do in ip_vs_ct_in_get()).
Signed-off-by: Bogdano Arendartchuk <barendartchuk@suse.com>
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
(first time it didn't reach all recipients due to a malformed header)
---
net/netfilter/ipvs/ip_vs_conn.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 59a1a85..282b39b 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -871,11 +871,11 @@ ip_vs_conn_new(const struct ip_vs_conn_param *p,
cp->protocol = p->protocol;
ip_vs_addr_set(p->af, &cp->caddr, p->caddr);
cp->cport = p->cport;
- ip_vs_addr_set(p->af, &cp->vaddr, p->vaddr);
- cp->vport = p->vport;
- /* proto should only be IPPROTO_IP if d_addr is a fwmark */
+ /* proto should only be IPPROTO_IP if p->vaddr is a fwmark */
ip_vs_addr_set(p->protocol == IPPROTO_IP ? AF_UNSPEC : p->af,
- &cp->daddr, daddr);
+ &cp->vaddr, vaddr);
+ cp->vport = p->vport;
+ ip_vs_addr_set(p->af, &cp->daddr, p->daddr);
cp->dport = dport;
cp->flags = flags;
cp->fwmark = fwmark;
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH stable 3.11+] can: bcm: add skb destructor
From: Eric Dumazet @ 2014-01-29 14:53 UTC (permalink / raw)
To: Andre Naujoks; +Cc: David Miller, socketcan, netdev
In-Reply-To: <52E8C03A.4030906@gmail.com>
On Wed, 2014-01-29 at 09:47 +0100, Andre Naujoks wrote:
> On 29.01.2014 08:46, schrieb David Miller:
> > From: Andre Naujoks <nautsch2@gmail.com>
> > Date: Wed, 29 Jan 2014 08:40:03 +0100
> >
> >> Even if this is a bug in the CAN BCM implementation. Your "fix" just
> >> enabled a user space application to shut down any machine with a kernel
> >> containing the BUG_ON patch.
> >
> > Rather, he detected a potential stray pointer reference to freed data
> > that was caused by the CAN code which would difficult if not
> > impossible to detect otherwise.
> >
> > That's even more dangerous, and you should be thanking him.
>
> "potential" is the keyword here. But its a definite kernel crash as it
> is right now with a standard use case for the BCM.
>
> Don't get me wrong. If there are bugs in the code, they should be fixed,
> but I don't think breaking a working (even if flawed) part of the kernel
> is the right thing to do here.
Shall I remember you this patch was suggested by David Miller, our
beloved network maintainer ?
Really this is quite silly, I'll tell you.
I can send a patch to mark CAN as BROKEN if you want, or you can send an
appropriate patch.
Your resistance is futile.
^ permalink raw reply
* Re: [PATCH stable 3.11+] can: use private sk reference to detect originating socket
From: Marc Kleine-Budde @ 2014-01-29 14:38 UTC (permalink / raw)
To: Oliver Hartkopp, Eric Dumazet, David Miller
Cc: Linux Netdev List, Andre Naujoks, linux-can@vger.kernel.org
In-Reply-To: <52E905C8.1060408@hartkopp.net>
[-- Attachment #1: Type: text/plain, Size: 10518 bytes --]
Please put linux-can on Cc for CAN related patches.
Marc
On 01/29/2014 02:44 PM, Oliver Hartkopp wrote:
> Commit 376c7311bdb6 (net: add a temporary sanity check in skb_orphan())
> leads to a BUG in can_put_echo_skb() when skb_orphan() is executed.
>
> The use of skb->sk to detect the originating socket can lead to side effects
> in net/core and net/sched. Therefore the reference needed in net/can/raw.c is
> now stored in a CAN private skbuff space in struct can_skb_priv.
>
> The reference which comes with the CAN frame is only needed in raw_rcv() and
> checked against his own sock pointer and does only special things if it
> matches. So removing the socket does not hurt as the in-flight skbs are
> removed and the receive filters are purged anyway when the socket disappears.
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>
> ---
>
> Here's my idea to fix up the original skb->sk handling concept for detecting
> the originating socket instance. The patch applies properly down to 3.11.
>
> Eric please take a look if I used sock_alloc_send_skb() correctly in bcm.c.
> Andre please check if you see any functional differences on your machine.
>
> Tnx & regards,
> Oliver
>
> ps. I would suggest to move the BUG() in WARN_ON_ONCE() to fulfill the
> requirement to detect a possible misuse but not kill the entire machine.
> BUG() is intended to be used when there's no way to continue any reasonable
> operation. Detecting and fixing this issue half a year after the "temporary
> sanity check" has been integrated is a bad example for using BUG() IMO.
>
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 13a9098..6d51fb7 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -323,7 +323,6 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
> }
>
> if (!priv->echo_skb[idx]) {
> - struct sock *srcsk = skb->sk;
>
> if (atomic_read(&skb->users) != 1) {
> struct sk_buff *old_skb = skb;
> @@ -335,8 +334,6 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
> } else
> skb_orphan(skb);
>
> - skb->sk = srcsk;
> -
> /* make settings for echo to reduce code in irq context */
> skb->protocol = htons(ETH_P_CAN);
> skb->pkt_type = PACKET_BROADCAST;
> @@ -513,6 +510,7 @@ struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf)
>
> can_skb_reserve(skb);
> can_skb_prv(skb)->ifindex = dev->ifindex;
> + can_skb_prv(skb)->src_sk = NULL;
>
> *cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
> memset(*cf, 0, sizeof(struct can_frame));
> diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
> index e24e669..d27a1c9 100644
> --- a/drivers/net/can/janz-ican3.c
> +++ b/drivers/net/can/janz-ican3.c
> @@ -1133,8 +1133,6 @@ static void ican3_handle_message(struct ican3_dev *mod, struct ican3_msg *msg)
> */
> static void ican3_put_echo_skb(struct ican3_dev *mod, struct sk_buff *skb)
> {
> - struct sock *srcsk = skb->sk;
> -
> if (atomic_read(&skb->users) != 1) {
> struct sk_buff *old_skb = skb;
>
> @@ -1142,11 +1140,8 @@ static void ican3_put_echo_skb(struct ican3_dev *mod, struct sk_buff *skb)
> kfree_skb(old_skb);
> if (!skb)
> return;
> - } else {
> + } else
> skb_orphan(skb);
> - }
> -
> - skb->sk = srcsk;
>
> /* save this skb for tx interrupt echo handling */
> skb_queue_tail(&mod->echoq, skb);
> diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
> index 3fcdae2..44766c18 100644
> --- a/drivers/net/can/slcan.c
> +++ b/drivers/net/can/slcan.c
> @@ -215,6 +215,7 @@ static void slc_bump(struct slcan *sl)
>
> can_skb_reserve(skb);
> can_skb_prv(skb)->ifindex = sl->dev->ifindex;
> + can_skb_prv(skb)->src_sk = NULL;
>
> memcpy(skb_put(skb, sizeof(struct can_frame)),
> &cf, sizeof(struct can_frame));
> diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
> index 0a2a5ee..1a89116 100644
> --- a/drivers/net/can/vcan.c
> +++ b/drivers/net/can/vcan.c
> @@ -116,14 +116,11 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
> /* perform standard echo handling for CAN network interfaces */
>
> if (loop) {
> - struct sock *srcsk = skb->sk;
> -
> skb = skb_share_check(skb, GFP_ATOMIC);
> if (!skb)
> return NETDEV_TX_OK;
>
> /* receive with packet counting */
> - skb->sk = srcsk;
> vcan_rx(skb, dev);
> } else {
> /* no looped packets => no counting */
> diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
> index 2f0543f..a8dc078 100644
> --- a/include/linux/can/skb.h
> +++ b/include/linux/can/skb.h
> @@ -25,10 +25,12 @@
> /**
> * struct can_skb_priv - private additional data inside CAN sk_buffs
> * @ifindex: ifindex of the first interface the CAN frame appeared on
> + * @src_sk: pointer to the originating sock to detect self generated skbs
> * @cf: align to the following CAN frame at skb->data
> */
> struct can_skb_priv {
> int ifindex;
> + struct sock *src_sk;
> struct can_frame cf[0];
> };
>
> diff --git a/net/can/af_can.c b/net/can/af_can.c
> index d249874..6ed6a23 100644
> --- a/net/can/af_can.c
> +++ b/net/can/af_can.c
> @@ -270,15 +270,6 @@ int can_send(struct sk_buff *skb, int loop)
> /* indication for the CAN driver: do loopback */
> skb->pkt_type = PACKET_LOOPBACK;
>
> - /*
> - * The reference to the originating sock may be required
> - * by the receiving socket to check whether the frame is
> - * its own. Example: can_raw sockopt CAN_RAW_RECV_OWN_MSGS
> - * Therefore we have to ensure that skb->sk remains the
> - * reference to the originating sock by restoring skb->sk
> - * after each skb_clone() or skb_orphan() usage.
> - */
> -
> if (!(skb->dev->flags & IFF_ECHO)) {
> /*
> * If the interface is not capable to do loopback
> @@ -290,7 +281,6 @@ int can_send(struct sk_buff *skb, int loop)
> return -ENOMEM;
> }
>
> - newskb->sk = skb->sk;
> newskb->ip_summed = CHECKSUM_UNNECESSARY;
> newskb->pkt_type = PACKET_BROADCAST;
> }
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 3fc737b..f10f521 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -245,7 +245,9 @@ static void bcm_can_tx(struct bcm_op *op)
> {
> struct sk_buff *skb;
> struct net_device *dev;
> + struct sock *sk = op->sk;
> struct can_frame *cf = &op->frames[op->currframe];
> + int err;
>
> /* no target device? => exit */
> if (!op->ifindex)
> @@ -257,18 +259,23 @@ static void bcm_can_tx(struct bcm_op *op)
> return;
> }
>
> - skb = alloc_skb(CFSIZ + sizeof(struct can_skb_priv), gfp_any());
> + /*
> + * we are probably in softirq context (timer, net-rx):
> + * get an unblocked skb
> + */
> + sk->sk_allocation = GFP_ATOMIC;
> + skb = sock_alloc_send_skb(sk, CFSIZ + sizeof(struct can_skb_priv),
> + 1, &err);
> if (!skb)
> goto out;
>
> can_skb_reserve(skb);
> - can_skb_prv(skb)->ifindex = dev->ifindex;
> -
> memcpy(skb_put(skb, CFSIZ), cf, CFSIZ);
>
> /* send with loopback */
> + can_skb_prv(skb)->ifindex = dev->ifindex;
> + can_skb_prv(skb)->src_sk = sk;
> skb->dev = dev;
> - skb->sk = op->sk;
> can_send(skb, 1);
>
> /* update statistics */
> @@ -1203,12 +1210,13 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk)
> if (!ifindex)
> return -ENODEV;
>
> - skb = alloc_skb(CFSIZ + sizeof(struct can_skb_priv), GFP_KERNEL);
> + /* get an unblocked skb */
> + skb = sock_alloc_send_skb(sk, CFSIZ + sizeof(struct can_skb_priv),
> + 1, &err);
> if (!skb)
> return -ENOMEM;
>
> can_skb_reserve(skb);
> -
> err = memcpy_fromiovec(skb_put(skb, CFSIZ), msg->msg_iov, CFSIZ);
> if (err < 0) {
> kfree_skb(skb);
> @@ -1221,10 +1229,11 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk)
> return -ENODEV;
> }
>
> - can_skb_prv(skb)->ifindex = dev->ifindex;
> + /* send with loopback */
> + can_skb_prv(skb)->ifindex = ifindex;
> + can_skb_prv(skb)->src_sk = sk;
> skb->dev = dev;
> - skb->sk = sk;
> - err = can_send(skb, 1); /* send with loopback */
> + err = can_send(skb, 1);
> dev_put(dev);
>
> if (err)
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 07d72d8..7e67560 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -113,12 +113,13 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
> {
> struct sock *sk = (struct sock *)data;
> struct raw_sock *ro = raw_sk(sk);
> + struct sock *src_sk = can_skb_prv(oskb)->src_sk;
> struct sockaddr_can *addr;
> struct sk_buff *skb;
> unsigned int *pflags;
>
> /* check the received tx sock reference */
> - if (!ro->recv_own_msgs && oskb->sk == sk)
> + if (!ro->recv_own_msgs && src_sk == sk)
> return;
>
> /* do not pass frames with DLC > 8 to a legacy socket */
> @@ -150,9 +151,9 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
> /* add CAN specific message flags for raw_recvmsg() */
> pflags = raw_flags(skb);
> *pflags = 0;
> - if (oskb->sk)
> + if (src_sk)
> *pflags |= MSG_DONTROUTE;
> - if (oskb->sk == sk)
> + if (src_sk == sk)
> *pflags |= MSG_CONFIRM;
>
> if (sock_queue_rcv_skb(sk, skb) < 0)
> @@ -705,17 +706,15 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
> goto put_dev;
>
> can_skb_reserve(skb);
> - can_skb_prv(skb)->ifindex = dev->ifindex;
> -
> err = memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size);
> if (err < 0)
> goto free_skb;
>
> sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
>
> + can_skb_prv(skb)->ifindex = dev->ifindex;
> + can_skb_prv(skb)->src_sk = sk;
> skb->dev = dev;
> - skb->sk = sk;
> -
> err = can_send(skb, ro->loopback);
>
> dev_put(dev);
>
>
> --
> 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
>
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* RE: Many USB ethernet devices are broken over xhci
From: David Laight @ 2014-01-29 14:26 UTC (permalink / raw)
To: 'Greg KH'
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sarah Sharp
In-Reply-To: <20140127233326.GB652-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
From: Greg KH
> On Mon, Jan 27, 2014 at 04:06:22PM +0000, David Laight wrote:
> > Many of the net/usb ethernet drivers (including common ones like
> > the smsc95xx) will fail to transmit all packet length properly
> > when connected to a USB3 port (ie using the xhci driver).
>
> That's odd, as I've never had a problem with my USB 3.0 ethernet device
> (sorry, don't remember what driver it uses), and I stress it all the
> time.
If it is the ax88179_178a driver then it carefully pads packets
that are multiples of the message size.
> I've also run two different USB 2 ethernet devices on xhci with no
> issues at all, so perhaps the "many" statement might not be true?
Dunno, quite a few will send down requests with URB_SEND_SERO set.
Whether this causes a problem depends on exactly how the ethernet
packets are encapsulated in usb ones.
The ax88179_178a hardware gets very confused if you fail to send
a ZLP - which it why it uses the 'bodge' in usbnet to append a byte.
> > The underlying problem is that they assume the host controller
> > will honour the URB_ZERO_PACKET flag - which usbnet sets because
> > they specify FLAG_SEND_ZLP.
> >
> > However no one has ever added support for URB_ZERO_PACKET to
> > the xhci driver - so packets that need padding (probably 512
> > bytes after any header is added) will not be sent correctly
> > and may have very adverse effects on the usb target.
>
> Really? How has things been working so well up to now without this
> support? Doesn't the hardware automatically add this padding with no
> explicit need for the xhci driver to do something?
Nope - things must work by luck!
Try ping -n $((512-0x32)) on an smsc95xx card - it should need a ZLP.
> > I don't think anything significant has changed (in the main kernel
> > sources) since I wrote the patch.
>
> Then resubmit it, don't post links to it on the web, there's nothing we
> can do with them, sorry.
I've resubmitted it as a v3 patch (to linux-usb) in small steps so that
the changes are easier to verify.
The final result is slightly different as I spotted a couple of minor
issues when re-reading the final version.
Mostly due to another 2 months of looking at this code.
David
--
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 3/3] net: via-rhine: add OF bus binding
From: Rob Herring @ 2014-01-29 13:46 UTC (permalink / raw)
To: Alexey Charkov
Cc: netdev, Tony Prisk,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Roger Luethi,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CABjd4Yxo0Pp+QtTuXhKBT9KdVhUk12x2L8+FBL_ZWmPYopG2JQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Jan 28, 2014 at 12:27 PM, Alexey Charkov <alchark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2014/1/27 Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> On Mon, Jan 27, 2014 at 5:51 AM, Alexey Charkov <alchark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> This should make the driver usable with VIA/WonderMedia ARM-based
>>> Systems-on-Chip integrated Rhine III adapters. Note that these
>>> are always in MMIO mode, and don't have any known EEPROM.
>>>
>>> Signed-off-by: Alexey Charkov <alchark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Signed-off-by: Roger Luethi <rl-7uj+XXdSDtwfv37vnLkPlQ@public.gmane.org>
>>> ---
>>> .../devicetree/bindings/net/via-rhine.txt | 18 ++
>>> drivers/net/ethernet/via/Kconfig | 2 +-
>>> drivers/net/ethernet/via/via-rhine.c | 293 +++++++++++++--------
>>> 3 files changed, 200 insertions(+), 113 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/net/via-rhine.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/via-rhine.txt b/Documentation/devicetree/bindings/net/via-rhine.txt
>>> new file mode 100644
>>> index 0000000..684dd3a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/via-rhine.txt
>>> @@ -0,0 +1,18 @@
>>> +* VIA Rhine 10/100 Network Controller
>>> +
>>> +Required properties:
>>> +- compatible : Should be "via,rhine"
>>
>> This should be more specific rather than...
>>
>>> +- reg : Address and length of the io space
>>> +- interrupts : Should contain the controller interrupt line
>>> +- rhine,revision : Rhine core revision, used to inform the
>>> + driver of quirks and capabilities to expect from
>>> + the device. Mimics the respective PCI attribute.
>>
>> having this property. The OF match table can then have the quirks set
>> based on compatible strings.
>
> Sounds fair. Do you think something like the following would fly?
>
> Required properties:
> - compatible : Should be "via,rhine-soc-vt8500" for integrated Rhine
> cores found in SoC's such as VIA VT8500, WonderMedia WM8950 and
> possibly others. These are listed as 1106:3106 rev. 0x84 on the
> virtual PCI bus under vendor-provided kernels.
Yes, although the "soc" part seems a bit redundant. The usual pattern
is <vendor>,<chip/soc>-<device>. I would go with "via,vt8500-rhine".
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 stable 3.11+] can: use private sk reference to detect originating socket
From: Oliver Hartkopp @ 2014-01-29 13:44 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: Linux Netdev List, Andre Naujoks
Commit 376c7311bdb6 (net: add a temporary sanity check in skb_orphan())
leads to a BUG in can_put_echo_skb() when skb_orphan() is executed.
The use of skb->sk to detect the originating socket can lead to side effects
in net/core and net/sched. Therefore the reference needed in net/can/raw.c is
now stored in a CAN private skbuff space in struct can_skb_priv.
The reference which comes with the CAN frame is only needed in raw_rcv() and
checked against his own sock pointer and does only special things if it
matches. So removing the socket does not hurt as the in-flight skbs are
removed and the receive filters are purged anyway when the socket disappears.
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
Here's my idea to fix up the original skb->sk handling concept for detecting
the originating socket instance. The patch applies properly down to 3.11.
Eric please take a look if I used sock_alloc_send_skb() correctly in bcm.c.
Andre please check if you see any functional differences on your machine.
Tnx & regards,
Oliver
ps. I would suggest to move the BUG() in WARN_ON_ONCE() to fulfill the
requirement to detect a possible misuse but not kill the entire machine.
BUG() is intended to be used when there's no way to continue any reasonable
operation. Detecting and fixing this issue half a year after the "temporary
sanity check" has been integrated is a bad example for using BUG() IMO.
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 13a9098..6d51fb7 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -323,7 +323,6 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
}
if (!priv->echo_skb[idx]) {
- struct sock *srcsk = skb->sk;
if (atomic_read(&skb->users) != 1) {
struct sk_buff *old_skb = skb;
@@ -335,8 +334,6 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
} else
skb_orphan(skb);
- skb->sk = srcsk;
-
/* make settings for echo to reduce code in irq context */
skb->protocol = htons(ETH_P_CAN);
skb->pkt_type = PACKET_BROADCAST;
@@ -513,6 +510,7 @@ struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf)
can_skb_reserve(skb);
can_skb_prv(skb)->ifindex = dev->ifindex;
+ can_skb_prv(skb)->src_sk = NULL;
*cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
memset(*cf, 0, sizeof(struct can_frame));
diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
index e24e669..d27a1c9 100644
--- a/drivers/net/can/janz-ican3.c
+++ b/drivers/net/can/janz-ican3.c
@@ -1133,8 +1133,6 @@ static void ican3_handle_message(struct ican3_dev *mod, struct ican3_msg *msg)
*/
static void ican3_put_echo_skb(struct ican3_dev *mod, struct sk_buff *skb)
{
- struct sock *srcsk = skb->sk;
-
if (atomic_read(&skb->users) != 1) {
struct sk_buff *old_skb = skb;
@@ -1142,11 +1140,8 @@ static void ican3_put_echo_skb(struct ican3_dev *mod, struct sk_buff *skb)
kfree_skb(old_skb);
if (!skb)
return;
- } else {
+ } else
skb_orphan(skb);
- }
-
- skb->sk = srcsk;
/* save this skb for tx interrupt echo handling */
skb_queue_tail(&mod->echoq, skb);
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index 3fcdae2..44766c18 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -215,6 +215,7 @@ static void slc_bump(struct slcan *sl)
can_skb_reserve(skb);
can_skb_prv(skb)->ifindex = sl->dev->ifindex;
+ can_skb_prv(skb)->src_sk = NULL;
memcpy(skb_put(skb, sizeof(struct can_frame)),
&cf, sizeof(struct can_frame));
diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
index 0a2a5ee..1a89116 100644
--- a/drivers/net/can/vcan.c
+++ b/drivers/net/can/vcan.c
@@ -116,14 +116,11 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
/* perform standard echo handling for CAN network interfaces */
if (loop) {
- struct sock *srcsk = skb->sk;
-
skb = skb_share_check(skb, GFP_ATOMIC);
if (!skb)
return NETDEV_TX_OK;
/* receive with packet counting */
- skb->sk = srcsk;
vcan_rx(skb, dev);
} else {
/* no looped packets => no counting */
diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 2f0543f..a8dc078 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -25,10 +25,12 @@
/**
* struct can_skb_priv - private additional data inside CAN sk_buffs
* @ifindex: ifindex of the first interface the CAN frame appeared on
+ * @src_sk: pointer to the originating sock to detect self generated skbs
* @cf: align to the following CAN frame at skb->data
*/
struct can_skb_priv {
int ifindex;
+ struct sock *src_sk;
struct can_frame cf[0];
};
diff --git a/net/can/af_can.c b/net/can/af_can.c
index d249874..6ed6a23 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -270,15 +270,6 @@ int can_send(struct sk_buff *skb, int loop)
/* indication for the CAN driver: do loopback */
skb->pkt_type = PACKET_LOOPBACK;
- /*
- * The reference to the originating sock may be required
- * by the receiving socket to check whether the frame is
- * its own. Example: can_raw sockopt CAN_RAW_RECV_OWN_MSGS
- * Therefore we have to ensure that skb->sk remains the
- * reference to the originating sock by restoring skb->sk
- * after each skb_clone() or skb_orphan() usage.
- */
-
if (!(skb->dev->flags & IFF_ECHO)) {
/*
* If the interface is not capable to do loopback
@@ -290,7 +281,6 @@ int can_send(struct sk_buff *skb, int loop)
return -ENOMEM;
}
- newskb->sk = skb->sk;
newskb->ip_summed = CHECKSUM_UNNECESSARY;
newskb->pkt_type = PACKET_BROADCAST;
}
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 3fc737b..f10f521 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -245,7 +245,9 @@ static void bcm_can_tx(struct bcm_op *op)
{
struct sk_buff *skb;
struct net_device *dev;
+ struct sock *sk = op->sk;
struct can_frame *cf = &op->frames[op->currframe];
+ int err;
/* no target device? => exit */
if (!op->ifindex)
@@ -257,18 +259,23 @@ static void bcm_can_tx(struct bcm_op *op)
return;
}
- skb = alloc_skb(CFSIZ + sizeof(struct can_skb_priv), gfp_any());
+ /*
+ * we are probably in softirq context (timer, net-rx):
+ * get an unblocked skb
+ */
+ sk->sk_allocation = GFP_ATOMIC;
+ skb = sock_alloc_send_skb(sk, CFSIZ + sizeof(struct can_skb_priv),
+ 1, &err);
if (!skb)
goto out;
can_skb_reserve(skb);
- can_skb_prv(skb)->ifindex = dev->ifindex;
-
memcpy(skb_put(skb, CFSIZ), cf, CFSIZ);
/* send with loopback */
+ can_skb_prv(skb)->ifindex = dev->ifindex;
+ can_skb_prv(skb)->src_sk = sk;
skb->dev = dev;
- skb->sk = op->sk;
can_send(skb, 1);
/* update statistics */
@@ -1203,12 +1210,13 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk)
if (!ifindex)
return -ENODEV;
- skb = alloc_skb(CFSIZ + sizeof(struct can_skb_priv), GFP_KERNEL);
+ /* get an unblocked skb */
+ skb = sock_alloc_send_skb(sk, CFSIZ + sizeof(struct can_skb_priv),
+ 1, &err);
if (!skb)
return -ENOMEM;
can_skb_reserve(skb);
-
err = memcpy_fromiovec(skb_put(skb, CFSIZ), msg->msg_iov, CFSIZ);
if (err < 0) {
kfree_skb(skb);
@@ -1221,10 +1229,11 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk)
return -ENODEV;
}
- can_skb_prv(skb)->ifindex = dev->ifindex;
+ /* send with loopback */
+ can_skb_prv(skb)->ifindex = ifindex;
+ can_skb_prv(skb)->src_sk = sk;
skb->dev = dev;
- skb->sk = sk;
- err = can_send(skb, 1); /* send with loopback */
+ err = can_send(skb, 1);
dev_put(dev);
if (err)
diff --git a/net/can/raw.c b/net/can/raw.c
index 07d72d8..7e67560 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -113,12 +113,13 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
{
struct sock *sk = (struct sock *)data;
struct raw_sock *ro = raw_sk(sk);
+ struct sock *src_sk = can_skb_prv(oskb)->src_sk;
struct sockaddr_can *addr;
struct sk_buff *skb;
unsigned int *pflags;
/* check the received tx sock reference */
- if (!ro->recv_own_msgs && oskb->sk == sk)
+ if (!ro->recv_own_msgs && src_sk == sk)
return;
/* do not pass frames with DLC > 8 to a legacy socket */
@@ -150,9 +151,9 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
/* add CAN specific message flags for raw_recvmsg() */
pflags = raw_flags(skb);
*pflags = 0;
- if (oskb->sk)
+ if (src_sk)
*pflags |= MSG_DONTROUTE;
- if (oskb->sk == sk)
+ if (src_sk == sk)
*pflags |= MSG_CONFIRM;
if (sock_queue_rcv_skb(sk, skb) < 0)
@@ -705,17 +706,15 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
goto put_dev;
can_skb_reserve(skb);
- can_skb_prv(skb)->ifindex = dev->ifindex;
-
err = memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size);
if (err < 0)
goto free_skb;
sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
+ can_skb_prv(skb)->ifindex = dev->ifindex;
+ can_skb_prv(skb)->src_sk = sk;
skb->dev = dev;
- skb->sk = sk;
-
err = can_send(skb, ro->loopback);
dev_put(dev);
^ permalink raw reply related
* STAKEHOLDER NEEDED!!!
From: Yung kyu kim @ 2014-01-29 12:21 UTC (permalink / raw)
Hello,
The Project is about the exportation of 100,000 barrels of Light Crude
Oil daily out from Iraq to Turkey through my client's company in Iraq
at the rate of $92.00 per barrel. This amount to $9,200,000 daily. I ask
for your support as a foreigner to handle this business project with my
client and you are not expected to invest in Iraq
If yes, let me know and we will discuss this project proper.
Kim.
Email :2784295239@qq.com
^ permalink raw reply
* Re: [PATCH 0/2] [BUG FIXES - 3.10.27] sit: More backports
From: Steven Rostedt @ 2014-01-29 12:59 UTC (permalink / raw)
To: David Miller
Cc: linux-kernel, netdev, nicolas.dichtel, stable, williams, lclaudio,
jkacur
In-Reply-To: <20140128.235214.881810600024309551.davem@davemloft.net>
On Tue, 28 Jan 2014 23:52:14 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> Greg, these have my blessing, please apply these to 3.10 -stable,
> thanks a lot!
Hi David,
Thanks for the blessing :-) But I want to work with Nicolas a bit more
to make sure that I have a correct backport. Just because it fixed some
of our bugs doesn't mean that I didn't make new ones.
Thanks,
-- Steve
^ 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