Skip to contentSkip to conversation

vlan: add rtnl_dereference() annotations

PATCH Applied Dan Carpenter 4 participants 4 messages Dec 14, 2011

Subsystems (3)
Networking Network drivers Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 vlan: add rtnl_dereference() annotations

The original code generates a Sparse warning:
net/8021q/vlan_core.c:336:9:
error: incompatible types in comparison expression (different address spaces)

It's ok to dereference __rcu pointers here because we are holding the RTNL lock. I've added some calls to rtnl_dereference() to silence the warning.

Signed-off-by Dan Carpenter <[email protected]>

---
I haven't tested this, and I'm not super familiar with this code. Please review it carefully.

net/8021q/vlan_core.c +9 -5
index 1414c93..4d39d80 100644
@@ -326,14 +326,16 @@ int vlan_vids_add_by_dev(struct net_device *dev,
 			 const struct net_device *by_dev)
 {
 	struct vlan_vid_info *vid_info;
+	struct vlan_info *vlan_info;
 	int err;
 
 	ASSERT_RTNL();
 
-	if (!by_dev->vlan_info)
+	vlan_info = rtnl_dereference(by_dev->vlan_info);
+	if (!vlan_info)
 		return 0;
 
-	list_for_each_entry(vid_info, &by_dev->vlan_info->vid_list, list) {
+	list_for_each_entry(vid_info, &vlan_info->vid_list, list) {
 		err = vlan_vid_add(dev, vid_info->vid);
 		if (err)
 			goto unwind;

@@ -342,7 +344,7 @@ int vlan_vids_add_by_dev(struct net_device *dev,
 
 unwind:
 	list_for_each_entry_continue_reverse(vid_info,
-					     &by_dev->vlan_info->vid_list,
+					     &vlan_info->vid_list,
 					     list) {
 		vlan_vid_del(dev, vid_info->vid);
 	}

@@ -355,13 +357,15 @@ void vlan_vids_del_by_dev(struct net_device *dev,
 			  const struct net_device *by_dev)
 {
 	struct vlan_vid_info *vid_info;
+	struct vlan_info *vlan_info;
 
 	ASSERT_RTNL();
 
-	if (!by_dev->vlan_info)
+	vlan_info = rtnl_dereference(by_dev->vlan_info);
+	if (!vlan_info)
 		return;
 
-	list_for_each_entry(vid_info, &by_dev->vlan_info->vid_list, list)
+	list_for_each_entry(vid_info, &vlan_info->vid_list, list)
 		vlan_vid_del(dev, vid_info->vid);
 }
 EXPORT_SYMBOL(vlan_vids_del_by_dev);
Acked-by on v1 1/1
#1 lore raw
Quoted 4 lines · Dan Carpenter

The original code generates a Sparse warning:
net/8021q/vlan_core.c:336:9:
error: incompatible types in comparison expression (different address spaces)

Quoted 9 lines

It's ok to dereference __rcu pointers here because we are holding the RTNL lock. I've added some calls to rtnl_dereference() to silence the warning.

Signed-off-by Dan Carpenter <[email protected]>

---
I haven't tested this, and I'm not super familiar with this code. Please review it carefully.

Acked-by Eric Dumazet <[email protected]>
Acked-by on v1 1/1
#2 lore raw
In reply to Dan Carpenter
Quoted 53 lines · Wed, Dec 14, 2011 at 07:29:43AM CET, [email protected] wrote:

The original code generates a Sparse warning:
net/8021q/vlan_core.c:336:9:
error: incompatible types in comparison expression (different address spaces)

It's ok to dereference __rcu pointers here because we are holding the RTNL lock. I've added some calls to rtnl_dereference() to silence the warning.

Signed-off-by Dan Carpenter <[email protected]>

---
I haven't tested this, and I'm not super familiar with this code. Please review it carefully.

net/8021q/vlan_core.c +4 -2
index 1414c93..4d39d80 100644
@@ -326,14 +326,16 @@ int vlan_vids_add_by_dev(struct net_device *dev,
 			 const struct net_device *by_dev)
 {
 	struct vlan_vid_info *vid_info;
+	struct vlan_info *vlan_info;
 	int err;
 
 	ASSERT_RTNL();
 
-	if (!by_dev->vlan_info)
+	vlan_info = rtnl_dereference(by_dev->vlan_info);
+	if (!vlan_info)
 		return 0;
 
-	list_for_each_entry(vid_info, &by_dev->vlan_info->vid_list, list) {
+	list_for_each_entry(vid_info, &vlan_info->vid_list, list) {
 		err = vlan_vid_add(dev, vid_info->vid);
 		if (err)
 			goto unwind;

@@ -342,7 +344,7 @@ int vlan_vids_add_by_dev(struct net_device *dev,

unwind:
list_for_each_entry_continue_reverse(vid_info,

net/8021q/vlan_core.c +1 -1
-					     &by_dev->vlan_info->vid_list,
+					     &vlan_info->vid_list,

					     list) {
		vlan_vid_del(dev, vid_info->vid);
	}
net/8021q/vlan_core.c +1 -0
@@ -355,13 +357,15 @@ void vlan_vids_del_by_dev(struct net_device *dev,
 			  const struct net_device *by_dev)
 {
 	struct vlan_vid_info *vid_info;
+	struct vlan_info *vlan_info;
 
 	ASSERT_RTNL();

net/8021q/vlan_core.c void vlan_vids_del_by_dev(struct net_device *dev, · line 371

Quoted 10 lines
net/8021q/vlan_core.c +3 -2
-	if (!by_dev->vlan_info)
+	vlan_info = rtnl_dereference(by_dev->vlan_info);
+	if (!vlan_info)
 		return;
 
-	list_for_each_entry(vid_info, &by_dev->vlan_info->vid_list, list)
+	list_for_each_entry(vid_info, &vlan_info->vid_list, list)
 		vlan_vid_del(dev, vid_info->vid);
 }
 EXPORT_SYMBOL(vlan_vids_del_by_dev);
Acked-by Jiri Pirko <[email protected]>

Thanks Dan!

maintainer Applied
#3 lore raw
In reply to Dan Carpenter

From: Dan Carpenter <[email protected]>
Date: Wed, 14 Dec 2011 09:29:43 +0300

Quoted 9 lines

The original code generates a Sparse warning:
net/8021q/vlan_core.c:336:9:
error: incompatible types in comparison expression (different address spaces)

It's ok to dereference __rcu pointers here because we are holding the RTNL lock. I've added some calls to rtnl_dereference() to silence the warning.

Signed-off-by Dan Carpenter <[email protected]>

Applied.