a bit more work towards proper locking including a fix to a refcount bug

- fixed a issue in add_interface_to_domain where the device interface's refcount was always incremented since that function was getting the device interface handle and not returning it unconditionlly
 - if the ethernet device goes down, and the fd is close()ed, return B_FILE_ERROR instead of calling into the driver again


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@20601 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/network/devices/ethernet/ethernet.cpp

@@ -221,6 +221,7 @@ ethernet_down(net_device *_device)
 		sCheckList.Remove(device);
 
 	close(device->fd);
+	device->fd = -1;
 }
 
 
@@ -290,6 +291,9 @@ ethernet_receive_data(net_device *_device, net_buffer **_buffer)
 {
 	ethernet_device *device = (ethernet_device *)_device;
 
+	if (device->fd == -1)
+		return B_FILE_ERROR;
+
 	// TODO: better header space
 	net_buffer *buffer = gBufferModule->create(256);
 	if (buffer == NULL)

src/add-ons/kernel/network/stack/datalink.cpp

@@ -44,14 +44,12 @@ device_reader_thread(void *_interface)
 	net_device_interface *interface = (net_device_interface *)_interface;
 	net_device *device = interface->device;
 	status_t status = B_OK;
-	int32 tries = 0;
 
 	while ((device->flags & IFF_UP) != 0) {
 		net_buffer *buffer;
 		status = device->module->receive_data(device, &buffer);
 		if (status == B_OK) {
 			//dprintf("received buffer of %ld bytes length\n", buffer->size);
-			tries = 0;
 
 			// feed device monitors
 			// TODO: locking!
@@ -88,18 +86,24 @@ device_reader_thread(void *_interface)
 			}
 
 			gNetBufferModule.free(buffer);
-		}
-
-		if (status < B_OK) {
-			// this is a near real-time thread - don't render the system unusable
-			// in case of a device going down
+		} else {
+			// In case of error, give the other threads some
+			// time to run since this is a near real time thread.
+			//
+			// TODO: can this value be lower? 1000 works fine in
+			//       my system. 10ms seems a bit too much and adds
+			//       as latency.
 			snooze(10000);
-
-			if (++tries > 20) {
-				// TODO: bring down the interface!
-				break;
-			}
 		}
+
+		// if the interface went down IFF_UP was removed
+		// and the receive_data() above should have been
+		// interrupted. One check should be enough, specially
+		// considering the snooze above.
+		//
+		// TODO: make sure that when receive_data() returns
+		//       after closing the new device->flags are
+		//       already visible in all processors.
 	}
 
 	return status;
@@ -254,19 +258,13 @@ datalink_control(net_domain *_domain, int32 option, void *value,
 			if (user_memcpy(&request, value, sizeof(struct ifreq)) < B_OK)
 				return B_BAD_ADDRESS;
 
-			benaphore_lock(&domain->lock);
-			status_t status;
+			BenaphoreLocker _(domain->lock);
 
 			net_interface *interface = find_interface(domain,
 				request.ifr_name);
-			if (interface != NULL)
-				status = remove_interface_from_domain(interface);
-			else
-				status = ENODEV;
-
-			benaphore_unlock(&domain->lock);
-
-			return status;
+			if (interface == NULL)
+				return ENODEV;
+			return remove_interface_from_domain(interface);
 		}
 
 		case SIOCGIFCOUNT:
@@ -321,7 +319,7 @@ datalink_control(net_domain *_domain, int32 option, void *value,
 			if (user_memcpy(&request, value, sizeof(struct ifreq)) < B_OK)
 				return B_BAD_ADDRESS;
 
-			benaphore_lock(&domain->lock);
+			BenaphoreLocker _(domain->lock);
 			status_t status = B_OK;
 
 			net_interface *interface = find_interface(domain,
@@ -355,7 +353,6 @@ datalink_control(net_domain *_domain, int32 option, void *value,
 			} else
 				status = B_BAD_VALUE;
 
-			benaphore_unlock(&domain->lock);
 			return status;
 		}
 	}

src/add-ons/kernel/network/stack/domains.cpp

@@ -154,27 +154,30 @@ add_interface_to_domain(net_domain *_domain,
 
 	const char *deviceName = request.ifr_parameter.device[0]
 		? request.ifr_parameter.device : request.ifr_name;
+	const char *baseName = request.ifr_parameter.base_name[0]
+		? request.ifr_parameter.base_name : request.ifr_name;
+
 	net_device_interface *deviceInterface = get_device_interface(deviceName);
 	if (deviceInterface == NULL)
 		return ENODEV;
 
 	BenaphoreLocker locker(domain->lock);
 
+	net_interface_private *interface = NULL;
+	status_t status;
+
 	if (find_interface(domain, request.ifr_name) != NULL)
-		return B_NAME_IN_USE;
+		status = B_NAME_IN_USE;
+	else
+		status = create_interface(domain, request.ifr_name,
+			baseName, deviceInterface, &interface);
 
-	net_interface_private *interface;
-	status_t status = create_interface(domain,
-		request.ifr_name, request.ifr_parameter.base_name[0]
-			? request.ifr_parameter.base_name : request.ifr_name,
-		deviceInterface, &interface);
-	if (status < B_OK) {
-		put_device_interface(deviceInterface);
-		return status;
-	}
+	put_device_interface(deviceInterface);
 
-	list_add_item(&domain->interfaces, interface);
-	return B_OK;
+	if (status == B_OK)
+		list_add_item(&domain->interfaces, interface);
+
+	return status;
 }
 
 

src/add-ons/kernel/network/stack/interfaces.cpp

@@ -652,7 +652,6 @@ device_removed(net_device *device)
 
 	// TODO: make sure all readers are gone
 	//       make sure all watchers are gone
-	//       remove device interface
 
 	return B_OK;
 }