[haiku-commits] r37845 - haiku/trunk/src/add-ons/kernel/network/stack
- From: axeld@xxxxxxxxxxxxxxxx
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Mon, 2 Aug 2010 17:23:19 +0200 (CEST)
Author: axeld
Date: 2010-08-02 17:23:19 +0200 (Mon, 02 Aug 2010)
New Revision: 37845
Changeset: http://dev.haiku-os.org/changeset/37845
Modified:
haiku/trunk/src/add-ons/kernel/network/stack/datalink.cpp
haiku/trunk/src/add-ons/kernel/network/stack/device_interfaces.cpp
haiku/trunk/src/add-ons/kernel/network/stack/device_interfaces.h
haiku/trunk/src/add-ons/kernel/network/stack/link.cpp
Log:
* Added a dedicated lock for the device monitors. This fixes a locking issue in
interface_protocol_send_data() which accessed the monitors unlocked.
* Changed SIOCCPACKETCAP to check if the device name matches the one used with
SIOCSPACKETCAP.
Modified: haiku/trunk/src/add-ons/kernel/network/stack/datalink.cpp
===================================================================
--- haiku/trunk/src/add-ons/kernel/network/stack/datalink.cpp 2010-08-02
14:45:44 UTC (rev 37844)
+++ haiku/trunk/src/add-ons/kernel/network/stack/datalink.cpp 2010-08-02
15:23:19 UTC (rev 37845)
@@ -642,19 +642,9 @@
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).
+ if (atomic_get(&interface->DeviceInterface()->monitor_count) > 0)
+ device_interface_monitor_receive(interface->DeviceInterface(),
buffer);
- DeviceMonitorList::Iterator iterator =
- interface->DeviceInterface()->monitor_funcs.GetIterator();
- while (iterator.HasNext()) {
- net_device_monitor* monitor = iterator.Next();
- monitor->receive(monitor, buffer);
- }
-
return protocol->device_module->send_data(protocol->device, buffer);
}
Modified: haiku/trunk/src/add-ons/kernel/network/stack/device_interfaces.cpp
===================================================================
--- haiku/trunk/src/add-ons/kernel/network/stack/device_interfaces.cpp
2010-08-02 14:45:44 UTC (rev 37844)
+++ haiku/trunk/src/add-ons/kernel/network/stack/device_interfaces.cpp
2010-08-02 15:23:19 UTC (rev 37845)
@@ -63,11 +63,8 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
+/*! 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 @@
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 @@
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 @@
// 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
Modified: haiku/trunk/src/add-ons/kernel/network/stack/device_interfaces.h
===================================================================
--- haiku/trunk/src/add-ons/kernel/network/stack/device_interfaces.h
2010-08-02 14:45:44 UTC (rev 37844)
+++ haiku/trunk/src/add-ons/kernel/network/stack/device_interfaces.h
2010-08-02 15:23:19 UTC (rev 37845)
@@ -36,9 +36,11 @@
net_deframe_func deframe_func;
int32 deframe_ref_count;
+ int32 monitor_count;
+ mutex monitor_lock;
DeviceMonitorList monitor_funcs;
+
DeviceHandlerList receive_funcs;
-
recursive_lock receive_lock;
thread_id consumer_thread;
@@ -58,12 +60,15 @@
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,
Modified: haiku/trunk/src/add-ons/kernel/network/stack/link.cpp
===================================================================
--- haiku/trunk/src/add-ons/kernel/network/stack/link.cpp 2010-08-02
14:45:44 UTC (rev 37844)
+++ haiku/trunk/src/add-ons/kernel/network/stack/link.cpp 2010-08-02
15:23:19 UTC (rev 37845)
@@ -46,7 +46,7 @@
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 @@
{
MutexLocker _(fLock);
- if (fMonitoredDevice)
+ if (fMonitoredDevice != NULL)
return B_BUSY;
net_device_interface* interface = get_device_interface(deviceName);
@@ -111,11 +111,14 @@
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 @@
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);
Other related posts:
