diff --git a/src/add-ons/kernel/network/stack/datalink.cpp b/src/add-ons/kernel/network/stack/datalink.cpp index 71f39489b3..01e62cb1f3 100644 --- a/src/add-ons/kernel/network/stack/datalink.cpp +++ b/src/add-ons/kernel/network/stack/datalink.cpp @@ -642,18 +642,8 @@ interface_protocol_send_data(net_datalink_protocol* _protocol, interface_protocol* protocol = (interface_protocol*)_protocol; Interface* interface = (Interface*)protocol->interface; - // TODO: Need to think about this locking. We can't obtain the - // RX Lock here (nor would it make sense) as the ARP - // module calls send_data() with it's lock held (similiar - // to the domain lock, which would violate the locking - // protocol). - - DeviceMonitorList::Iterator iterator = - interface->DeviceInterface()->monitor_funcs.GetIterator(); - while (iterator.HasNext()) { - net_device_monitor* monitor = iterator.Next(); - monitor->receive(monitor, buffer); - } + if (atomic_get(&interface->DeviceInterface()->monitor_count) > 0) + device_interface_monitor_receive(interface->DeviceInterface(), buffer); return protocol->device_module->send_data(protocol->device, buffer); } diff --git a/src/add-ons/kernel/network/stack/device_interfaces.cpp b/src/add-ons/kernel/network/stack/device_interfaces.cpp index 0b9866da8d..a0b300d776 100644 --- a/src/add-ons/kernel/network/stack/device_interfaces.cpp +++ b/src/add-ons/kernel/network/stack/device_interfaces.cpp @@ -63,11 +63,8 @@ device_reader_thread(void* _interface) if (status == B_OK) { // feed device monitors - DeviceMonitorList::Iterator iterator = - interface->monitor_funcs.GetIterator(); - while (net_device_monitor* monitor = iterator.Next()) { - monitor->receive(monitor, buffer); - } + if (atomic_get(&interface->monitor_count) > 0) + device_interface_monitor_receive(interface, buffer); buffer->interface_address = NULL; buffer->type = interface->deframe_func(interface->device, buffer); @@ -165,7 +162,8 @@ allocate_device_interface(net_device* device, net_device_module_info* module) if (interface == NULL) return NULL; - recursive_lock_init(&interface->receive_lock, "interface receive lock"); + recursive_lock_init(&interface->receive_lock, "device interface receive"); + mutex_init(&interface->monitor_lock, "device interface monitors"); char name[128]; snprintf(name, sizeof(name), "%s receive queue", device->name); @@ -199,6 +197,7 @@ error2: uninit_fifo(&interface->receive_queue); error1: recursive_lock_destroy(&interface->receive_lock); + mutex_destroy(&interface->monitor_lock); delete interface; return NULL; @@ -208,7 +207,7 @@ error1: static void notify_device_monitors(net_device_interface* interface, int32 event) { - RecursiveLocker _(interface->receive_lock); + MutexLocker locker(interface->monitor_lock); DeviceMonitorList::Iterator iterator = interface->monitor_funcs.GetIterator(); @@ -239,17 +238,19 @@ dump_device_interface(int argc, char** argv) kprintf("ref_count: %" B_PRId32 "\n", interface->ref_count); kprintf("deframe_func: %p\n", interface->deframe_func); kprintf("deframe_ref_count: %" B_PRId32 "\n", interface->ref_count); - kprintf("monitor_funcs:\n"); - kprintf("receive_funcs:\n"); kprintf("consumer_thread: %ld\n", interface->consumer_thread); - kprintf("receive_lock: %p\n", &interface->receive_lock); - kprintf("receive_queue: %p\n", &interface->receive_queue); + kprintf("monitor_count: %" B_PRId32 "\n", interface->monitor_count); + kprintf("monitor_lock: %p\n", &interface->monitor_lock); + kprintf("monitor_funcs:\n"); DeviceMonitorList::Iterator monitorIterator = interface->monitor_funcs.GetIterator(); while (monitorIterator.HasNext()) kprintf(" %p\n", monitorIterator.Next()); + kprintf("receive_lock: %p\n", &interface->receive_lock); + kprintf("receive_queue: %p\n", &interface->receive_queue); + kprintf("receive_funcs:\n"); DeviceHandlerList::Iterator handlerIterator = interface->receive_funcs.GetIterator(); while (handlerIterator.HasNext()) @@ -374,6 +375,7 @@ put_device_interface(struct net_device_interface* interface) device->module->uninit_device(device); put_module(moduleName); + mutex_destroy(&interface->monitor_lock); recursive_lock_destroy(&interface->receive_lock); delete interface; } @@ -449,6 +451,25 @@ get_device_interface(const char* name, bool create) } +/*! Feeds the device monitors of the \a interface with the specified \a buffer. + You might want to check interface::monitor_count before calling this + function for optimization. +*/ +void +device_interface_monitor_receive(net_device_interface* interface, + net_buffer* buffer) +{ + MutexLocker locker(interface->monitor_lock); + + DeviceMonitorList::Iterator iterator + = interface->monitor_funcs.GetIterator(); + while (iterator.HasNext()) { + net_device_monitor* monitor = iterator.Next(); + monitor->receive(monitor, buffer); + } +} + + status_t up_device_interface(net_device_interface* interface) { @@ -666,8 +687,10 @@ register_device_monitor(net_device* device, net_device_monitor* monitor) if (interface == NULL) return B_DEVICE_NOT_FOUND; - RecursiveLocker _(interface->receive_lock); + MutexLocker monitorLocker(interface->monitor_lock); interface->monitor_funcs.Add(monitor); + atomic_add(&interface->monitor_count, 1); + return B_OK; } @@ -683,14 +706,16 @@ unregister_device_monitor(net_device* device, net_device_monitor* monitor) if (interface == NULL) return B_DEVICE_NOT_FOUND; - RecursiveLocker _(interface->receive_lock); + MutexLocker monitorLocker(interface->monitor_lock); // search for the monitor - DeviceMonitorList::Iterator iterator = interface->monitor_funcs.GetIterator(); + DeviceMonitorList::Iterator iterator + = interface->monitor_funcs.GetIterator(); while (iterator.HasNext()) { if (iterator.Next() == monitor) { iterator.Remove(); + atomic_add(&interface->monitor_count, -1); return B_OK; } } @@ -736,6 +761,7 @@ device_removed(net_device* device) // By now all of the monitors must have removed themselves. If they // didn't, they'll probably wait forever to be callback'ed again. + mutex_lock(&interface->monitor_lock); interface->monitor_funcs.RemoveAll(); // All of the readers should be gone as well since we are out of diff --git a/src/add-ons/kernel/network/stack/device_interfaces.h b/src/add-ons/kernel/network/stack/device_interfaces.h index 46b9890423..6dcc4c002d 100644 --- a/src/add-ons/kernel/network/stack/device_interfaces.h +++ b/src/add-ons/kernel/network/stack/device_interfaces.h @@ -36,9 +36,11 @@ struct net_device_interface : DoublyLinkedListLinkImpl { net_deframe_func deframe_func; int32 deframe_ref_count; + int32 monitor_count; + mutex monitor_lock; DeviceMonitorList monitor_funcs; - DeviceHandlerList receive_funcs; + DeviceHandlerList receive_funcs; recursive_lock receive_lock; thread_id consumer_thread; @@ -58,12 +60,15 @@ void put_device_interface(struct net_device_interface* interface); struct net_device_interface* get_device_interface(uint32 index); struct net_device_interface* get_device_interface(const char* name, bool create = true); +void device_interface_monitor_receive(net_device_interface* interface, + net_buffer* buffer); status_t up_device_interface(net_device_interface* interface); void down_device_interface(net_device_interface* interface); // devices status_t unregister_device_deframer(net_device* device); -status_t register_device_deframer(net_device* device, net_deframe_func deframeFunc); +status_t register_device_deframer(net_device* device, + net_deframe_func deframeFunc); status_t register_domain_device_handler(struct net_device* device, int32 type, struct net_domain* domain); status_t register_device_handler(struct net_device* device, int32 type, diff --git a/src/add-ons/kernel/network/stack/link.cpp b/src/add-ons/kernel/network/stack/link.cpp index f93a7f1b07..01f3fd7706 100644 --- a/src/add-ons/kernel/network/stack/link.cpp +++ b/src/add-ons/kernel/network/stack/link.cpp @@ -46,7 +46,7 @@ public: virtual ~LinkProtocol(); status_t StartMonitoring(const char* deviceName); - status_t StopMonitoring(); + status_t StopMonitoring(const char* deviceName); protected: status_t SocketStatus(bool peek) const; @@ -92,7 +92,7 @@ LinkProtocol::StartMonitoring(const char* deviceName) { MutexLocker _(fLock); - if (fMonitoredDevice) + if (fMonitoredDevice != NULL) return B_BUSY; net_device_interface* interface = get_device_interface(deviceName); @@ -111,11 +111,14 @@ LinkProtocol::StartMonitoring(const char* deviceName) status_t -LinkProtocol::StopMonitoring() +LinkProtocol::StopMonitoring(const char* deviceName) { MutexLocker _(fLock); - // TODO compare our device with the supplied device name? + if (fMonitoredDevice == NULL + || strcmp(fMonitoredDevice->device->name, deviceName) != 0) + return B_BAD_VALUE; + return _Unregister(); } @@ -351,14 +354,20 @@ link_control(net_protocol* _protocol, int level, int option, void* value, return B_NOT_ALLOWED; struct ifreq request; - if (user_memcpy(&request, value, IF_NAMESIZE) < B_OK) + if (user_memcpy(&request, value, IF_NAMESIZE) != B_OK) return B_BAD_ADDRESS; return protocol->StartMonitoring(request.ifr_name); } case SIOCCPACKETCAP: - return protocol->StopMonitoring(); + { + struct ifreq request; + if (user_memcpy(&request, value, IF_NAMESIZE) != B_OK) + return B_BAD_ADDRESS; + + return protocol->StopMonitoring(request.ifr_name); + } } return gNetDatalinkModule.control(sDomain, option, value, _length);