From 20b534cd5f128d0561161aa0243bd1102378ab65 Mon Sep 17 00:00:00 2001 From: Hugo Santos Date: Fri, 6 Apr 2007 09:48:02 +0000 Subject: [PATCH] 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 --- .../network/devices/ethernet/ethernet.cpp | 4 ++ src/add-ons/kernel/network/stack/datalink.cpp | 45 +++++++++---------- src/add-ons/kernel/network/stack/domains.cpp | 27 ++++++----- .../kernel/network/stack/interfaces.cpp | 1 - 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/add-ons/kernel/network/devices/ethernet/ethernet.cpp b/src/add-ons/kernel/network/devices/ethernet/ethernet.cpp index 90cc08eed7..6889739442 100644 --- a/src/add-ons/kernel/network/devices/ethernet/ethernet.cpp +++ b/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) diff --git a/src/add-ons/kernel/network/stack/datalink.cpp b/src/add-ons/kernel/network/stack/datalink.cpp index 0d0f0259cd..b9e3c364c3 100644 --- a/src/add-ons/kernel/network/stack/datalink.cpp +++ b/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; } } diff --git a/src/add-ons/kernel/network/stack/domains.cpp b/src/add-ons/kernel/network/stack/domains.cpp index 726dd950ac..4c16f96a7d 100644 --- a/src/add-ons/kernel/network/stack/domains.cpp +++ b/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; } diff --git a/src/add-ons/kernel/network/stack/interfaces.cpp b/src/add-ons/kernel/network/stack/interfaces.cpp index 06df962839..da2770402b 100644 --- a/src/add-ons/kernel/network/stack/interfaces.cpp +++ b/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; }