* [PATCH net-next] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock
@ 2015-08-18 16:17 Nikolay Aleksandrov
2015-08-18 18:15 ` [PATCH net-next v2] " Nikolay Aleksandrov
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Nikolay Aleksandrov @ 2015-08-18 16:17 UTC (permalink / raw)
To: netdev; +Cc: dsa, shm, davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
While running net-next I hit this:
[ 634.073119] ===============================
[ 634.073150] [ INFO: suspicious RCU usage. ]
[ 634.073182] 4.2.0-rc6+ #45 Not tainted
[ 634.073213] -------------------------------
[ 634.073244] include/net/vrf.h:38 suspicious rcu_dereference_check()
usage!
[ 634.073274]
other info that might help us debug this:
[ 634.073307]
rcu_scheduler_active = 1, debug_locks = 1
[ 634.073338] 2 locks held by swapper/0/0:
[ 634.073369] #0: (((&n->timer))){+.-...}, at: [<ffffffff8112bc35>]
call_timer_fn+0x5/0x480
[ 634.073412] #1: (slock-AF_INET){+.-...}, at: [<ffffffff8174f0f5>]
icmp_send+0x155/0x5f0
[ 634.073450]
stack backtrace:
[ 634.073483] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc6+ #45
[ 634.073514] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
VirtualBox 12/01/2006
[ 634.073545] 0000000000000000 0593ba8242d9ace4 ffff88002fc03b48
ffffffff81803f1b
[ 634.073612] 0000000000000000 ffffffff81e12500 ffff88002fc03b78
ffffffff811003c5
[ 634.073642] 0000000000000000 ffff88002ec4e600 ffffffff81f00f80
ffff88002fc03cf0
[ 634.073669] Call Trace:
[ 634.073694] <IRQ> [<ffffffff81803f1b>] dump_stack+0x4c/0x65
[ 634.073728] [<ffffffff811003c5>] lockdep_rcu_suspicious+0xc5/0x100
[ 634.073763] [<ffffffff8174eb56>] icmp_route_lookup+0x176/0x5c0
[ 634.073793] [<ffffffff8174f2fb>] ? icmp_send+0x35b/0x5f0
[ 634.073818] [<ffffffff8174f274>] ? icmp_send+0x2d4/0x5f0
[ 634.073844] [<ffffffff8174f3ce>] icmp_send+0x42e/0x5f0
[ 634.073873] [<ffffffff8170b662>] ipv4_link_failure+0x22/0xa0
[ 634.073899] [<ffffffff8174bdda>] arp_error_report+0x3a/0x80
[ 634.073926] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.073952] [<ffffffff816d396e>] neigh_invalidate+0x8e/0x110
[ 634.073984] [<ffffffff816d62ae>] neigh_timer_handler+0x1ae/0x290
[ 634.074013] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.074013] [<ffffffff8112bce3>] call_timer_fn+0xb3/0x480
[ 634.074013] [<ffffffff8112bc35>] ? call_timer_fn+0x5/0x480
[ 634.074013] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.074013] [<ffffffff8112c2bc>] run_timer_softirq+0x20c/0x430
[ 634.074013] [<ffffffff810af50e>] __do_softirq+0xde/0x630
[ 634.074013] [<ffffffff810afc97>] irq_exit+0x117/0x120
[ 634.074013] [<ffffffff81810976>] smp_apic_timer_interrupt+0x46/0x60
[ 634.074013] [<ffffffff8180e950>] apic_timer_interrupt+0x70/0x80
[ 634.074013] <EOI> [<ffffffff8106b9d6>] ? native_safe_halt+0x6/0x10
[ 634.074013] [<ffffffff81101d8d>] ? trace_hardirqs_on+0xd/0x10
[ 634.074013] [<ffffffff81027d43>] default_idle+0x23/0x200
[ 634.074013] [<ffffffff8102852f>] arch_cpu_idle+0xf/0x20
[ 634.074013] [<ffffffff810f89ba>] default_idle_call+0x2a/0x40
[ 634.074013] [<ffffffff810f8dcc>] cpu_startup_entry+0x39c/0x4c0
[ 634.074013] [<ffffffff817f9cad>] rest_init+0x13d/0x150
[ 634.074013] [<ffffffff81f69038>] start_kernel+0x4a8/0x4c9
[ 634.074013] [<ffffffff81f68120>] ?
early_idt_handler_array+0x120/0x120
[ 634.074013] [<ffffffff81f68339>] x86_64_start_reservations+0x2a/0x2c
[ 634.074013] [<ffffffff81f68485>] x86_64_start_kernel+0x14a/0x16d
It would seem vrf_master_ifindex_rcu() can be called without RCU held in
other contexts as well so to be sure acquire rcu before dereferencing the
vrf ptr.
Also add curly braces around both the "if" and "else" parts as per the
style guide.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
include/net/vrf.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/net/vrf.h b/include/net/vrf.h
index 40e3793c7a05..22dfe2195092 100644
--- a/include/net/vrf.h
+++ b/include/net/vrf.h
@@ -35,7 +35,6 @@ struct net_vrf {
#if IS_ENABLED(CONFIG_NET_VRF)
-/* called with rcu_read_lock() */
static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
{
struct net_vrf_dev *vrf_ptr;
@@ -44,12 +43,14 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
if (!dev)
return 0;
- if (netif_is_vrf(dev))
+ if (netif_is_vrf(dev)) {
ifindex = dev->ifindex;
- else {
+ } else {
+ rcu_read_lock();
vrf_ptr = rcu_dereference(dev->vrf_ptr);
if (vrf_ptr)
ifindex = vrf_ptr->ifindex;
+ rcu_read_unlock();
}
return ifindex;
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v2] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock
2015-08-18 16:17 [PATCH net-next] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock Nikolay Aleksandrov
@ 2015-08-18 18:15 ` Nikolay Aleksandrov
2015-08-18 20:10 ` David Ahern
2015-08-18 18:40 ` [PATCH net-next v3] " Nikolay Aleksandrov
2015-08-18 19:45 ` [PATCH net-next] " David Ahern
2 siblings, 1 reply; 7+ messages in thread
From: Nikolay Aleksandrov @ 2015-08-18 18:15 UTC (permalink / raw)
To: netdev; +Cc: dsa, shm, davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
While running net-next I hit this:
[ 634.073119] ===============================
[ 634.073150] [ INFO: suspicious RCU usage. ]
[ 634.073182] 4.2.0-rc6+ #45 Not tainted
[ 634.073213] -------------------------------
[ 634.073244] include/net/vrf.h:38 suspicious rcu_dereference_check()
usage!
[ 634.073274]
other info that might help us debug this:
[ 634.073307]
rcu_scheduler_active = 1, debug_locks = 1
[ 634.073338] 2 locks held by swapper/0/0:
[ 634.073369] #0: (((&n->timer))){+.-...}, at: [<ffffffff8112bc35>]
call_timer_fn+0x5/0x480
[ 634.073412] #1: (slock-AF_INET){+.-...}, at: [<ffffffff8174f0f5>]
icmp_send+0x155/0x5f0
[ 634.073450]
stack backtrace:
[ 634.073483] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc6+ #45
[ 634.073514] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
VirtualBox 12/01/2006
[ 634.073545] 0000000000000000 0593ba8242d9ace4 ffff88002fc03b48
ffffffff81803f1b
[ 634.073612] 0000000000000000 ffffffff81e12500 ffff88002fc03b78
ffffffff811003c5
[ 634.073642] 0000000000000000 ffff88002ec4e600 ffffffff81f00f80
ffff88002fc03cf0
[ 634.073669] Call Trace:
[ 634.073694] <IRQ> [<ffffffff81803f1b>] dump_stack+0x4c/0x65
[ 634.073728] [<ffffffff811003c5>] lockdep_rcu_suspicious+0xc5/0x100
[ 634.073763] [<ffffffff8174eb56>] icmp_route_lookup+0x176/0x5c0
[ 634.073793] [<ffffffff8174f2fb>] ? icmp_send+0x35b/0x5f0
[ 634.073818] [<ffffffff8174f274>] ? icmp_send+0x2d4/0x5f0
[ 634.073844] [<ffffffff8174f3ce>] icmp_send+0x42e/0x5f0
[ 634.073873] [<ffffffff8170b662>] ipv4_link_failure+0x22/0xa0
[ 634.073899] [<ffffffff8174bdda>] arp_error_report+0x3a/0x80
[ 634.073926] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.073952] [<ffffffff816d396e>] neigh_invalidate+0x8e/0x110
[ 634.073984] [<ffffffff816d62ae>] neigh_timer_handler+0x1ae/0x290
[ 634.074013] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.074013] [<ffffffff8112bce3>] call_timer_fn+0xb3/0x480
[ 634.074013] [<ffffffff8112bc35>] ? call_timer_fn+0x5/0x480
[ 634.074013] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.074013] [<ffffffff8112c2bc>] run_timer_softirq+0x20c/0x430
[ 634.074013] [<ffffffff810af50e>] __do_softirq+0xde/0x630
[ 634.074013] [<ffffffff810afc97>] irq_exit+0x117/0x120
[ 634.074013] [<ffffffff81810976>] smp_apic_timer_interrupt+0x46/0x60
[ 634.074013] [<ffffffff8180e950>] apic_timer_interrupt+0x70/0x80
[ 634.074013] <EOI> [<ffffffff8106b9d6>] ? native_safe_halt+0x6/0x10
[ 634.074013] [<ffffffff81101d8d>] ? trace_hardirqs_on+0xd/0x10
[ 634.074013] [<ffffffff81027d43>] default_idle+0x23/0x200
[ 634.074013] [<ffffffff8102852f>] arch_cpu_idle+0xf/0x20
[ 634.074013] [<ffffffff810f89ba>] default_idle_call+0x2a/0x40
[ 634.074013] [<ffffffff810f8dcc>] cpu_startup_entry+0x39c/0x4c0
[ 634.074013] [<ffffffff817f9cad>] rest_init+0x13d/0x150
[ 634.074013] [<ffffffff81f69038>] start_kernel+0x4a8/0x4c9
[ 634.074013] [<ffffffff81f68120>] ?
early_idt_handler_array+0x120/0x120
[ 634.074013] [<ffffffff81f68339>] x86_64_start_reservations+0x2a/0x2c
[ 634.074013] [<ffffffff81f68485>] x86_64_start_kernel+0x14a/0x16d
It would seem vrf_master_ifindex_rcu() can be called without RCU held in
other contexts as well so introduce a new helper which acquires rcu and
returns the ifindex.
Also add curly braces around both the "if" and "else" parts as per the
style guide.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: use a new helper that acquires and releases rcu instead
include/net/vrf.h | 21 ++++++++++++++++++---
net/ipv4/icmp.c | 4 ++--
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/include/net/vrf.h b/include/net/vrf.h
index 3bb4af462ed6..b039850a94a3 100644
--- a/include/net/vrf.h
+++ b/include/net/vrf.h
@@ -34,7 +34,6 @@ struct net_vrf {
#if IS_ENABLED(CONFIG_NET_VRF)
-/* called with rcu_read_lock() */
static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
{
struct net_vrf_dev *vrf_ptr;
@@ -43,9 +42,9 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
if (!dev)
return 0;
- if (netif_is_vrf(dev))
+ if (netif_is_vrf(dev)) {
ifindex = dev->ifindex;
- else {
+ } else {
vrf_ptr = rcu_dereference(dev->vrf_ptr);
if (vrf_ptr)
ifindex = vrf_ptr->ifindex;
@@ -54,6 +53,17 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
return ifindex;
}
+static inline int vrf_master_ifindex(const struct net_device *dev)
+{
+ int ifindex;
+
+ rcu_read_lock();
+ ifindex = vrf_master_ifindex_rcu(dev);
+ rcu_read_unlock();
+
+ return ifindex;
+}
+
/* called with rcu_read_lock */
static inline int vrf_dev_table_rcu(const struct net_device *dev)
{
@@ -133,6 +143,11 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
return 0;
}
+static inline int vrf_master_ifindex(const struct net_device *dev)
+{
+ return 0;
+}
+
static inline int vrf_dev_table_rcu(const struct net_device *dev)
{
return 0;
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index c6f1ce149ffb..f16488efa1c8 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -426,7 +426,7 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
fl4.flowi4_mark = mark;
fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
fl4.flowi4_proto = IPPROTO_ICMP;
- fl4.flowi4_oif = vrf_master_ifindex_rcu(skb->dev) ? : skb->dev->ifindex;
+ fl4.flowi4_oif = vrf_master_ifindex(skb->dev) ? : skb->dev->ifindex;
security_skb_classify_flow(skb, flowi4_to_flowi(&fl4));
rt = ip_route_output_key(net, &fl4);
if (IS_ERR(rt))
@@ -460,7 +460,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
fl4->flowi4_proto = IPPROTO_ICMP;
fl4->fl4_icmp_type = type;
fl4->fl4_icmp_code = code;
- fl4->flowi4_oif = vrf_master_ifindex_rcu(skb_in->dev) ? : skb_in->dev->ifindex;
+ fl4->flowi4_oif = vrf_master_ifindex(skb_in->dev) ? : skb_in->dev->ifindex;
security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4));
rt = __ip_route_output_key(net, fl4);
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v3] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock
2015-08-18 16:17 [PATCH net-next] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock Nikolay Aleksandrov
2015-08-18 18:15 ` [PATCH net-next v2] " Nikolay Aleksandrov
@ 2015-08-18 18:40 ` Nikolay Aleksandrov
2015-08-19 3:24 ` David Miller
2015-08-18 19:45 ` [PATCH net-next] " David Ahern
2 siblings, 1 reply; 7+ messages in thread
From: Nikolay Aleksandrov @ 2015-08-18 18:40 UTC (permalink / raw)
To: netdev; +Cc: dsa, shm, davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
While running net-next I hit this:
[ 634.073119] ===============================
[ 634.073150] [ INFO: suspicious RCU usage. ]
[ 634.073182] 4.2.0-rc6+ #45 Not tainted
[ 634.073213] -------------------------------
[ 634.073244] include/net/vrf.h:38 suspicious rcu_dereference_check()
usage!
[ 634.073274]
other info that might help us debug this:
[ 634.073307]
rcu_scheduler_active = 1, debug_locks = 1
[ 634.073338] 2 locks held by swapper/0/0:
[ 634.073369] #0: (((&n->timer))){+.-...}, at: [<ffffffff8112bc35>]
call_timer_fn+0x5/0x480
[ 634.073412] #1: (slock-AF_INET){+.-...}, at: [<ffffffff8174f0f5>]
icmp_send+0x155/0x5f0
[ 634.073450]
stack backtrace:
[ 634.073483] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc6+ #45
[ 634.073514] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
VirtualBox 12/01/2006
[ 634.073545] 0000000000000000 0593ba8242d9ace4 ffff88002fc03b48
ffffffff81803f1b
[ 634.073612] 0000000000000000 ffffffff81e12500 ffff88002fc03b78
ffffffff811003c5
[ 634.073642] 0000000000000000 ffff88002ec4e600 ffffffff81f00f80
ffff88002fc03cf0
[ 634.073669] Call Trace:
[ 634.073694] <IRQ> [<ffffffff81803f1b>] dump_stack+0x4c/0x65
[ 634.073728] [<ffffffff811003c5>] lockdep_rcu_suspicious+0xc5/0x100
[ 634.073763] [<ffffffff8174eb56>] icmp_route_lookup+0x176/0x5c0
[ 634.073793] [<ffffffff8174f2fb>] ? icmp_send+0x35b/0x5f0
[ 634.073818] [<ffffffff8174f274>] ? icmp_send+0x2d4/0x5f0
[ 634.073844] [<ffffffff8174f3ce>] icmp_send+0x42e/0x5f0
[ 634.073873] [<ffffffff8170b662>] ipv4_link_failure+0x22/0xa0
[ 634.073899] [<ffffffff8174bdda>] arp_error_report+0x3a/0x80
[ 634.073926] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.073952] [<ffffffff816d396e>] neigh_invalidate+0x8e/0x110
[ 634.073984] [<ffffffff816d62ae>] neigh_timer_handler+0x1ae/0x290
[ 634.074013] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.074013] [<ffffffff8112bce3>] call_timer_fn+0xb3/0x480
[ 634.074013] [<ffffffff8112bc35>] ? call_timer_fn+0x5/0x480
[ 634.074013] [<ffffffff816d6100>] ? neigh_lookup+0x2c0/0x2c0
[ 634.074013] [<ffffffff8112c2bc>] run_timer_softirq+0x20c/0x430
[ 634.074013] [<ffffffff810af50e>] __do_softirq+0xde/0x630
[ 634.074013] [<ffffffff810afc97>] irq_exit+0x117/0x120
[ 634.074013] [<ffffffff81810976>] smp_apic_timer_interrupt+0x46/0x60
[ 634.074013] [<ffffffff8180e950>] apic_timer_interrupt+0x70/0x80
[ 634.074013] <EOI> [<ffffffff8106b9d6>] ? native_safe_halt+0x6/0x10
[ 634.074013] [<ffffffff81101d8d>] ? trace_hardirqs_on+0xd/0x10
[ 634.074013] [<ffffffff81027d43>] default_idle+0x23/0x200
[ 634.074013] [<ffffffff8102852f>] arch_cpu_idle+0xf/0x20
[ 634.074013] [<ffffffff810f89ba>] default_idle_call+0x2a/0x40
[ 634.074013] [<ffffffff810f8dcc>] cpu_startup_entry+0x39c/0x4c0
[ 634.074013] [<ffffffff817f9cad>] rest_init+0x13d/0x150
[ 634.074013] [<ffffffff81f69038>] start_kernel+0x4a8/0x4c9
[ 634.074013] [<ffffffff81f68120>] ?
early_idt_handler_array+0x120/0x120
[ 634.074013] [<ffffffff81f68339>] x86_64_start_reservations+0x2a/0x2c
[ 634.074013] [<ffffffff81f68485>] x86_64_start_kernel+0x14a/0x16d
It would seem vrf_master_ifindex_rcu() can be called without RCU held in
other contexts as well so introduce a new helper which acquires rcu and
returns the ifindex.
Also add curly braces around both the "if" and "else" parts as per the
style guide.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v3: keep the rcu comment above vrf_master_ifindex_rcu()
v2: use a new helper that acquires and releases rcu instead
include/net/vrf.h | 20 ++++++++++++++++++--
net/ipv4/icmp.c | 4 ++--
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/include/net/vrf.h b/include/net/vrf.h
index 3bb4af462ed6..5bfb16237fd7 100644
--- a/include/net/vrf.h
+++ b/include/net/vrf.h
@@ -43,9 +43,9 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
if (!dev)
return 0;
- if (netif_is_vrf(dev))
+ if (netif_is_vrf(dev)) {
ifindex = dev->ifindex;
- else {
+ } else {
vrf_ptr = rcu_dereference(dev->vrf_ptr);
if (vrf_ptr)
ifindex = vrf_ptr->ifindex;
@@ -54,6 +54,17 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
return ifindex;
}
+static inline int vrf_master_ifindex(const struct net_device *dev)
+{
+ int ifindex;
+
+ rcu_read_lock();
+ ifindex = vrf_master_ifindex_rcu(dev);
+ rcu_read_unlock();
+
+ return ifindex;
+}
+
/* called with rcu_read_lock */
static inline int vrf_dev_table_rcu(const struct net_device *dev)
{
@@ -133,6 +144,11 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
return 0;
}
+static inline int vrf_master_ifindex(const struct net_device *dev)
+{
+ return 0;
+}
+
static inline int vrf_dev_table_rcu(const struct net_device *dev)
{
return 0;
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index c6f1ce149ffb..f16488efa1c8 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -426,7 +426,7 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
fl4.flowi4_mark = mark;
fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
fl4.flowi4_proto = IPPROTO_ICMP;
- fl4.flowi4_oif = vrf_master_ifindex_rcu(skb->dev) ? : skb->dev->ifindex;
+ fl4.flowi4_oif = vrf_master_ifindex(skb->dev) ? : skb->dev->ifindex;
security_skb_classify_flow(skb, flowi4_to_flowi(&fl4));
rt = ip_route_output_key(net, &fl4);
if (IS_ERR(rt))
@@ -460,7 +460,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
fl4->flowi4_proto = IPPROTO_ICMP;
fl4->fl4_icmp_type = type;
fl4->fl4_icmp_code = code;
- fl4->flowi4_oif = vrf_master_ifindex_rcu(skb_in->dev) ? : skb_in->dev->ifindex;
+ fl4->flowi4_oif = vrf_master_ifindex(skb_in->dev) ? : skb_in->dev->ifindex;
security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4));
rt = __ip_route_output_key(net, fl4);
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock
2015-08-18 16:17 [PATCH net-next] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock Nikolay Aleksandrov
2015-08-18 18:15 ` [PATCH net-next v2] " Nikolay Aleksandrov
2015-08-18 18:40 ` [PATCH net-next v3] " Nikolay Aleksandrov
@ 2015-08-18 19:45 ` David Ahern
2 siblings, 0 replies; 7+ messages in thread
From: David Ahern @ 2015-08-18 19:45 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev; +Cc: shm, davem, Nikolay Aleksandrov
On 8/18/15 10:17 AM, Nikolay Aleksandrov wrote:
> diff --git a/include/net/vrf.h b/include/net/vrf.h
> index 40e3793c7a05..22dfe2195092 100644
> --- a/include/net/vrf.h
> +++ b/include/net/vrf.h
> @@ -35,7 +35,6 @@ struct net_vrf {
>
>
> #if IS_ENABLED(CONFIG_NET_VRF)
> -/* called with rcu_read_lock() */
> static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
> {
> struct net_vrf_dev *vrf_ptr;
> @@ -44,12 +43,14 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
> if (!dev)
> return 0;
>
> - if (netif_is_vrf(dev))
> + if (netif_is_vrf(dev)) {
> ifindex = dev->ifindex;
> - else {
> + } else {
> + rcu_read_lock();
> vrf_ptr = rcu_dereference(dev->vrf_ptr);
> if (vrf_ptr)
> ifindex = vrf_ptr->ifindex;
> + rcu_read_unlock();
> }
>
> return ifindex;
>
The intent of the _rcu in the name is to mean it is called with
rcu_read_lock held which is the case for __fib_validate_source and
ip_route_input_slow. It looks like the icmp callers (icmp_reply and
icmp_route_lookup) are the exceptions. For those create a
static inline int vrf_master_ifindex(const struct net_device *dev)
{
}
that does the rcu lock/unlock and calls vrf_master_ifindex_rcu in between.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock
2015-08-18 18:15 ` [PATCH net-next v2] " Nikolay Aleksandrov
@ 2015-08-18 20:10 ` David Ahern
2015-08-18 20:24 ` Nikolay Aleksandrov
0 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2015-08-18 20:10 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev; +Cc: shm, davem, Nikolay Aleksandrov
On 8/18/15 12:15 PM, Nikolay Aleksandrov wrote:
> diff --git a/include/net/vrf.h b/include/net/vrf.h
> index 3bb4af462ed6..b039850a94a3 100644
> --- a/include/net/vrf.h
> +++ b/include/net/vrf.h
> @@ -34,7 +34,6 @@ struct net_vrf {
>
>
> #if IS_ENABLED(CONFIG_NET_VRF)
> -/* called with rcu_read_lock() */
> static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
> {
> struct net_vrf_dev *vrf_ptr;
That comment is true for this version.
David
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock
2015-08-18 20:10 ` David Ahern
@ 2015-08-18 20:24 ` Nikolay Aleksandrov
0 siblings, 0 replies; 7+ messages in thread
From: Nikolay Aleksandrov @ 2015-08-18 20:24 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, shm, davem
> On Aug 18, 2015, at 11:10 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
>
> On 8/18/15 12:15 PM, Nikolay Aleksandrov wrote:
>> diff --git a/include/net/vrf.h b/include/net/vrf.h
>> index 3bb4af462ed6..b039850a94a3 100644
>> --- a/include/net/vrf.h
>> +++ b/include/net/vrf.h
>> @@ -34,7 +34,6 @@ struct net_vrf {
>>
>>
>> #if IS_ENABLED(CONFIG_NET_VRF)
>> -/* called with rcu_read_lock() */
>> static inline int vrf_master_ifindex_rcu(const struct net_device *dev)
>> {
>> struct net_vrf_dev *vrf_ptr;
>
> That comment is true for this version.
>
> David
>
It is, but as we established the name implies the same. Anyway I’ll post a v3 and
keep it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v3] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock
2015-08-18 18:40 ` [PATCH net-next v3] " Nikolay Aleksandrov
@ 2015-08-19 3:24 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2015-08-19 3:24 UTC (permalink / raw)
To: razor; +Cc: netdev, dsa, shm, nikolay
From: Nikolay Aleksandrov <razor@blackwall.org>
Date: Tue, 18 Aug 2015 21:40:16 +0300
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>
> While running net-next I hit this:
...
> It would seem vrf_master_ifindex_rcu() can be called without RCU held in
> other contexts as well so introduce a new helper which acquires rcu and
> returns the ifindex.
> Also add curly braces around both the "if" and "else" parts as per the
> style guide.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> v3: keep the rcu comment above vrf_master_ifindex_rcu()
> v2: use a new helper that acquires and releases rcu instead
Applied, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-08-19 3:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-18 16:17 [PATCH net-next] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock Nikolay Aleksandrov
2015-08-18 18:15 ` [PATCH net-next v2] " Nikolay Aleksandrov
2015-08-18 20:10 ` David Ahern
2015-08-18 20:24 ` Nikolay Aleksandrov
2015-08-18 18:40 ` [PATCH net-next v3] " Nikolay Aleksandrov
2015-08-19 3:24 ` David Miller
2015-08-18 19:45 ` [PATCH net-next] " David Ahern
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).