Commit 7dce0496 authored by Jean Tourrilhes's avatar Jean Tourrilhes Committed by Linus Torvalds

[PATCH] : Discovery locking fixes

	o [CRITICA] Fix remaining locking problem with discovery log
	o [CRITICA] Don't call expiry callback under spinlock
	o [FEATURE] Simplify/cleanup/optimise discovery/expiry code
parent 8d82694d
......@@ -52,8 +52,8 @@ struct irda_sock {
__u8 max_header_size;
struct qos_info qos_tx;
__u16 mask; /* Hint bits mask */
__u16 hints; /* Hint bits */
__u16_host_order mask; /* Hint bits mask */
__u16_host_order hints; /* Hint bits */
void *ckey; /* IrLMP client handle */
void *skey; /* IrLMP service handle */
......@@ -63,7 +63,7 @@ struct irda_sock {
struct ias_value *ias_result; /* Result of remote IAS query */
hashbin_t *cachelog; /* Result of discovery query */
struct discovery_t *cachediscovery; /* Result of selective discovery query */
__u32 cachedaddr; /* Result of selective discovery query */
int nslots; /* Number of slots to use for discovery */
......
......@@ -46,12 +46,20 @@
* little endian format. A little endian machine stores MSB of word in
* byte[1] and LSB in byte[0]. A big endian machine stores MSB in byte[0]
* and LSB in byte[1].
*
* This structure is used in the code for things that are endian neutral
* but that fit in a word so that we can manipulate them efficiently.
* By endian neutral, I mean things that are really an array of bytes,
* and always used as such, for example the hint bits. Jean II
*/
typedef union {
__u16 word;
__u8 byte[2];
} __u16_host_order;
/* Same purpose, different application */
#define u16ho(array) (* ((__u16 *) array))
/* Types of discovery */
typedef enum {
DISCOVERY_LOG, /* What's in our discovery log */
......@@ -62,30 +70,31 @@ typedef enum {
#define NICKNAME_MAX_LEN 21
/* Basic discovery information about a peer */
typedef struct irda_device_info discinfo_t; /* linux/irda.h */
/*
* The DISCOVERY structure is used for both discovery requests and responses
*/
typedef struct discovery_t {
irda_queue_t q; /* Must be first! */
__u32 saddr; /* Which link the device was discovered */
__u32 daddr; /* Remote device address */
LAP_REASON condition; /* More info about the discovery */
irda_queue_t q; /* Must be first! */
__u16_host_order hints; /* Discovery hint bits */
__u8 charset; /* Encoding of nickname */
char nickname[22]; /* The name of the device (21 bytes + \0) */
int name_len; /* Lenght of nickname */
discinfo_t data; /* Basic discovery information */
int name_len; /* Lenght of nickname */
int gen_addr_bit; /* Need to generate a new device address? */
int nslots; /* Number of slots to use when discovering */
unsigned long timestamp; /* Time discovered */
unsigned long first_timestamp; /* First time discovered */
LAP_REASON condition; /* More info about the discovery */
int gen_addr_bit; /* Need to generate a new device
* address? */
int nslots; /* Number of slots to use when
* discovering */
unsigned long timestamp; /* Last time discovered */
unsigned long firststamp; /* First time discovered */
} discovery_t;
void irlmp_add_discovery(hashbin_t *cachelog, discovery_t *discovery);
void irlmp_add_discovery_log(hashbin_t *cachelog, hashbin_t *log);
void irlmp_expire_discoveries(hashbin_t *log, __u32 saddr, int force);
struct irda_device_info *irlmp_copy_discoveries(hashbin_t *log, int *pn, __u16 mask);
struct irda_device_info *irlmp_copy_discoveries(hashbin_t *log, int *pn,
__u16 mask, int old_entries);
#endif
......@@ -34,7 +34,7 @@
#include <net/irda/irlan_event.h>
void irlan_client_start_kick_timer(struct irlan_cb *self, int timeout);
void irlan_client_discovery_indication(discovery_t *, DISCOVERY_MODE, void *);
void irlan_client_discovery_indication(discinfo_t *, DISCOVERY_MODE, void *);
void irlan_client_wakeup(struct irlan_cb *self, __u32 saddr, __u32 daddr);
void irlan_client_open_ctrl_tsap( struct irlan_cb *self);
......
......@@ -58,7 +58,7 @@
#define LM_IDLE_TIMEOUT 2*HZ /* 2 seconds for now */
typedef enum {
S_PNP,
S_PNP = 0,
S_PDA,
S_COMPUTER,
S_PRINTER,
......@@ -72,22 +72,24 @@ typedef enum {
S_END,
} SERVICE;
typedef void (*DISCOVERY_CALLBACK1) (discovery_t *, DISCOVERY_MODE, void *);
typedef void (*DISCOVERY_CALLBACK2) (hashbin_t *, void *);
/* For selective discovery */
typedef void (*DISCOVERY_CALLBACK1) (discinfo_t *, DISCOVERY_MODE, void *);
/* For expiry (the same) */
typedef void (*DISCOVERY_CALLBACK2) (discinfo_t *, DISCOVERY_MODE, void *);
typedef struct {
irda_queue_t queue; /* Must be first */
__u16 hints; /* Hint bits */
__u16_host_order hints; /* Hint bits */
} irlmp_service_t;
typedef struct {
irda_queue_t queue; /* Must be first */
__u16 hint_mask;
__u16_host_order hint_mask;
DISCOVERY_CALLBACK1 disco_callback; /* Selective discovery */
DISCOVERY_CALLBACK1 expir_callback; /* Selective expiration */
DISCOVERY_CALLBACK2 expir_callback; /* Selective expiration */
void *priv; /* Used to identify client */
} irlmp_client_t;
......@@ -199,11 +201,11 @@ __u16 irlmp_service_to_hint(int service);
void *irlmp_register_service(__u16 hints);
int irlmp_unregister_service(void *handle);
void *irlmp_register_client(__u16 hint_mask, DISCOVERY_CALLBACK1 disco_clb,
DISCOVERY_CALLBACK1 expir_clb, void *priv);
DISCOVERY_CALLBACK2 expir_clb, void *priv);
int irlmp_unregister_client(void *handle);
int irlmp_update_client(void *handle, __u16 hint_mask,
DISCOVERY_CALLBACK1 disco_clb,
DISCOVERY_CALLBACK1 expir_clb, void *priv);
DISCOVERY_CALLBACK2 expir_clb, void *priv);
void irlmp_register_link(struct irlap_cb *, __u32 saddr, notify_t *);
void irlmp_unregister_link(__u32 saddr);
......@@ -222,11 +224,11 @@ int irlmp_disconnect_request(struct lsap_cb *, struct sk_buff *userdata);
void irlmp_discovery_confirm(hashbin_t *discovery_log, DISCOVERY_MODE mode);
void irlmp_discovery_request(int nslots);
struct irda_device_info *irlmp_get_discoveries(int *pn, __u16 mask, int nslots);
discinfo_t *irlmp_get_discoveries(int *pn, __u16 mask, int nslots);
void irlmp_do_expiry(void);
void irlmp_do_discovery(int nslots);
discovery_t *irlmp_get_discovery_response(void);
void irlmp_discovery_expiry(discovery_t *expiry);
void irlmp_discovery_expiry(discinfo_t *expiry, int number);
int irlmp_data_request(struct lsap_cb *, struct sk_buff *);
void irlmp_data_indication(struct lsap_cb *, struct sk_buff *);
......
......@@ -401,11 +401,10 @@ static void irda_getvalue_confirm(int result, __u16 obj_id,
*
* Got a selective discovery indication from IrLMP.
*
* IrLMP is telling us that this node is matching our hint bit
* filter. Check if it's a newly discovered node (or if node changed its
* hint bits), and then wake up any process waiting for answer...
* IrLMP is telling us that this node is new and matching our hint bit
* filter. Wake up any process waiting for answer...
*/
static void irda_selective_discovery_indication(discovery_t *discovery,
static void irda_selective_discovery_indication(discinfo_t *discovery,
DISCOVERY_MODE mode,
void *priv)
{
......@@ -419,18 +418,8 @@ static void irda_selective_discovery_indication(discovery_t *discovery,
return;
}
/* Check if node is discovered is a new one or an old one.
* We check when how long ago this node was discovered, with a
* coarse timeout (we may miss some discovery events or be delayed).
* Note : by doing this test here, we avoid waking up a process ;-)
*/
if((jiffies - discovery->first_timestamp) >
(sysctl_discovery_timeout * HZ)) {
return; /* Too old, not interesting -> goodbye */
}
/* Pass parameter to the caller */
self->cachediscovery = discovery;
self->cachedaddr = discovery->daddr;
/* Wake up process if its waiting for device to be discovered */
wake_up_interruptible(&self->query_wait);
......@@ -455,7 +444,7 @@ static void irda_discovery_timeout(u_long priv)
/* Nothing for the caller */
self->cachelog = NULL;
self->cachediscovery = NULL;
self->cachedaddr = 0;
self->errno = -ETIME;
/* Wake up process if its still waiting... */
......@@ -627,7 +616,7 @@ static int irda_find_lsap_sel(struct irda_sock *self, char *name)
*/
static int irda_discover_daddr_and_lsap_sel(struct irda_sock *self, char *name)
{
struct irda_device_info *discoveries; /* Copy of the discovery log */
discinfo_t *discoveries; /* Copy of the discovery log */
int number; /* Number of nodes in the log */
int i;
int err = -ENETUNREACH;
......@@ -642,7 +631,8 @@ static int irda_discover_daddr_and_lsap_sel(struct irda_sock *self, char *name)
* Note : we have to use irlmp_get_discoveries(), as opposed
* to play with the cachelog directly, because while we are
* making our ias query, le log might change... */
discoveries = irlmp_get_discoveries(&number, self->mask, self->nslots);
discoveries = irlmp_get_discoveries(&number, self->mask.word,
self->nslots);
/* Check if the we got some results */
if (discoveries == NULL)
return -ENETUNREACH; /* No nodes discovered */
......@@ -1137,7 +1127,7 @@ static int irda_create(struct socket *sock, int protocol)
/* Register as a client with IrLMP */
self->ckey = irlmp_register_client(0, NULL, NULL, NULL);
self->mask = 0xffff;
self->mask.word = 0xffff;
self->rx_flow = self->tx_flow = FLOW_START;
self->nslots = DISCOVERY_DEFAULT_SLOTS;
self->daddr = DEV_ADDR_ANY; /* Until we get connected */
......@@ -1997,6 +1987,7 @@ static int irda_setsockopt(struct socket *sock, int level, int optname,
if (optlen < sizeof(int))
return -EINVAL;
/* The input is really a (__u8 hints[2]), easier as an int */
if (get_user(opt, (int *)optval))
return -EFAULT;
......@@ -2015,16 +2006,17 @@ static int irda_setsockopt(struct socket *sock, int level, int optname,
if (optlen < sizeof(int))
return -EINVAL;
/* The input is really a (__u8 hints[2]), easier as an int */
if (get_user(opt, (int *)optval))
return -EFAULT;
/* Set the new hint mask */
self->mask = (__u16) opt;
self->mask.word = (__u16) opt;
/* Mask out extension bits */
self->mask &= 0x7f7f;
self->mask.word &= 0x7f7f;
/* Check if no bits */
if(!self->mask)
self->mask = 0xFFFF;
if(!self->mask.word)
self->mask.word = 0xFFFF;
break;
default:
......@@ -2115,7 +2107,7 @@ static int irda_getsockopt(struct socket *sock, int level, int optname,
switch (optname) {
case IRLMP_ENUMDEVICES:
/* Ask lmp for the current discovery log */
discoveries = irlmp_get_discoveries(&list.len, self->mask,
discoveries = irlmp_get_discoveries(&list.len, self->mask.word,
self->nslots);
/* Check if the we got some results */
if (discoveries == NULL)
......@@ -2347,7 +2339,7 @@ static int irda_getsockopt(struct socket *sock, int level, int optname,
return -EFAULT;
/* Tell IrLMP we want to be notified */
irlmp_update_client(self->ckey, self->mask,
irlmp_update_client(self->ckey, self->mask.word,
irda_selective_discovery_indication,
NULL, (void *) self);
......@@ -2355,7 +2347,7 @@ static int irda_getsockopt(struct socket *sock, int level, int optname,
irlmp_discovery_request(self->nslots);
/* Wait until a node is discovered */
if (!self->cachediscovery) {
if (!self->cachedaddr) {
int ret = 0;
IRDA_DEBUG(1, "%s(), nothing discovered yet, going to sleep...\n", __FUNCTION__);
......@@ -2370,7 +2362,7 @@ static int irda_getsockopt(struct socket *sock, int level, int optname,
/* Wait for IR-LMP to call us back */
__wait_event_interruptible(self->query_wait,
(self->cachediscovery!=NULL || self->errno==-ETIME),
(self->cachedaddr != 0 || self->errno == -ETIME),
ret);
/* If watchdog is still activated, kill it! */
......@@ -2387,19 +2379,25 @@ static int irda_getsockopt(struct socket *sock, int level, int optname,
__FUNCTION__);
/* Tell IrLMP that we have been notified */
irlmp_update_client(self->ckey, self->mask, NULL, NULL, NULL);
irlmp_update_client(self->ckey, self->mask.word,
NULL, NULL, NULL);
/* Check if the we got some results */
if (!self->cachediscovery)
if (!self->cachedaddr)
return -EAGAIN; /* Didn't find any devices */
daddr = self->cachedaddr;
/* Cleanup */
self->cachediscovery = NULL;
self->cachedaddr = 0;
/* Note : We don't return anything to the user.
* We could return the device that triggered the wake up,
* but it's probably better to force the user to query
* the whole discovery log and let him pick one device...
/* We return the daddr of the device that trigger the
* wakeup. As irlmp pass us only the new devices, we
* are sure that it's not an old device.
* If the user want more details, he should query
* the whole discovery log and pick one device...
*/
if (put_user(daddr, (int *)optval))
return -EFAULT;
break;
default:
return -ENOPROTOOPT;
......
This diff is collapsed.
......@@ -46,7 +46,7 @@
#include <net/irda/ircomm_tty_attach.h>
static void ircomm_tty_ias_register(struct ircomm_tty_cb *self);
static void ircomm_tty_discovery_indication(discovery_t *discovery,
static void ircomm_tty_discovery_indication(discinfo_t *discovery,
DISCOVERY_MODE mode,
void *priv);
static void ircomm_tty_getvalue_confirm(int result, __u16 obj_id,
......@@ -305,7 +305,7 @@ int ircomm_tty_send_initial_parameters(struct ircomm_tty_cb *self)
* device it is, and which services it has.
*
*/
static void ircomm_tty_discovery_indication(discovery_t *discovery,
static void ircomm_tty_discovery_indication(discinfo_t *discovery,
DISCOVERY_MODE mode,
void *priv)
{
......
......@@ -145,7 +145,7 @@ void irlan_client_wakeup(struct irlan_cb *self, __u32 saddr, __u32 daddr)
* Remote device with IrLAN server support discovered
*
*/
void irlan_client_discovery_indication(discovery_t *discovery,
void irlan_client_discovery_indication(discinfo_t *discovery,
DISCOVERY_MODE mode,
void *priv)
{
......
......@@ -419,7 +419,7 @@ static int irlap_state_ndm(struct irlap_cb *self, IRLAP_EVENT event,
info->s);
if (self->slot == info->s) {
discovery_rsp = irlmp_get_discovery_response();
discovery_rsp->daddr = info->daddr;
discovery_rsp->data.daddr = info->daddr;
irlap_send_discovery_xid_frame(self, info->S,
self->slot,
......@@ -576,7 +576,7 @@ static int irlap_state_query(struct irlap_cb *self, IRLAP_EVENT event,
ASSERT(info->discovery != NULL, return -1;);
IRDA_DEBUG(4, "%s(), daddr=%08x\n", __FUNCTION__,
info->discovery->daddr);
info->discovery->data.daddr);
if (!self->discovery_log) {
WARNING("%s: discovery log is gone! "
......@@ -586,7 +586,7 @@ static int irlap_state_query(struct irlap_cb *self, IRLAP_EVENT event,
}
hashbin_insert(self->discovery_log,
(irda_queue_t *) info->discovery,
info->discovery->daddr, NULL);
info->discovery->data.daddr, NULL);
/* Keep state */
/* irlap_next_state(self, LAP_QUERY); */
......@@ -704,7 +704,7 @@ static int irlap_state_reply(struct irlap_cb *self, IRLAP_EVENT event,
irlap_discovery_indication(self, info->discovery);
} else if ((info->s >= self->slot) && (!self->frame_sent)) {
discovery_rsp = irlmp_get_discovery_response();
discovery_rsp->daddr = info->daddr;
discovery_rsp->data.daddr = info->daddr;
irlap_send_discovery_xid_frame(self, info->S,
self->slot, FALSE,
......
......@@ -335,7 +335,7 @@ void irlap_send_discovery_xid_frame(struct irlap_cb *self, int S, __u8 s,
if (command)
frame->daddr = cpu_to_le32(bcast);
else
frame->daddr = cpu_to_le32(discovery->daddr);
frame->daddr = cpu_to_le32(discovery->data.daddr);
switch (S) {
case 1:
......@@ -366,20 +366,20 @@ void irlap_send_discovery_xid_frame(struct irlap_cb *self, int S, __u8 s,
if (!command || (frame->slotnr == 0xff)) {
int len;
if (discovery->hints.byte[0] & HINT_EXTENSION) {
if (discovery->data.hints[0] & HINT_EXTENSION) {
info = skb_put(skb, 2);
info[0] = discovery->hints.byte[0];
info[1] = discovery->hints.byte[1];
info[0] = discovery->data.hints[0];
info[1] = discovery->data.hints[1];
} else {
info = skb_put(skb, 1);
info[0] = discovery->hints.byte[0];
info[0] = discovery->data.hints[0];
}
info = skb_put(skb, 1);
info[0] = discovery->charset;
info[0] = discovery->data.charset;
len = IRDA_MIN(discovery->name_len, skb_tailroom(skb));
info = skb_put(skb, len);
memcpy(info, discovery->nickname, len);
memcpy(info, discovery->data.info, len);
}
irlap_queue_xmit(self, skb);
}
......@@ -422,24 +422,25 @@ static void irlap_recv_discovery_xid_rsp(struct irlap_cb *self,
}
memset(discovery, 0, sizeof(discovery_t));
discovery->daddr = info->daddr;
discovery->saddr = self->saddr;
discovery->data.daddr = info->daddr;
discovery->data.saddr = self->saddr;
discovery->timestamp = jiffies;
IRDA_DEBUG(4, "%s(), daddr=%08x\n", __FUNCTION__, discovery->daddr);
IRDA_DEBUG(4, "%s(), daddr=%08x\n", __FUNCTION__,
discovery->data.daddr);
discovery_info = skb_pull(skb, sizeof(struct xid_frame));
/* Get info returned from peer */
discovery->hints.byte[0] = discovery_info[0];
discovery->data.hints[0] = discovery_info[0];
if (discovery_info[0] & HINT_EXTENSION) {
IRDA_DEBUG(4, "EXTENSION\n");
discovery->hints.byte[1] = discovery_info[1];
discovery->charset = discovery_info[2];
discovery->data.hints[1] = discovery_info[1];
discovery->data.charset = discovery_info[2];
text = (char *) &discovery_info[3];
} else {
discovery->hints.byte[1] = 0;
discovery->charset = discovery_info[1];
discovery->data.hints[1] = 0;
discovery->data.charset = discovery_info[1];
text = (char *) &discovery_info[2];
}
/*
......@@ -447,8 +448,8 @@ static void irlap_recv_discovery_xid_rsp(struct irlap_cb *self,
* FCS bytes resides.
*/
skb->data[skb->len] = '\0';
strncpy(discovery->nickname, text, NICKNAME_MAX_LEN);
discovery->name_len = strlen(discovery->nickname);
strncpy(discovery->data.info, text, NICKNAME_MAX_LEN);
discovery->name_len = strlen(discovery->data.info);
info->discovery = discovery;
......@@ -523,18 +524,18 @@ static void irlap_recv_discovery_xid_cmd(struct irlap_cb *self,
return;
}
discovery->daddr = info->daddr;
discovery->saddr = self->saddr;
discovery->data.daddr = info->daddr;
discovery->data.saddr = self->saddr;
discovery->timestamp = jiffies;
discovery->hints.byte[0] = discovery_info[0];
discovery->data.hints[0] = discovery_info[0];
if (discovery_info[0] & HINT_EXTENSION) {
discovery->hints.byte[1] = discovery_info[1];
discovery->charset = discovery_info[2];
discovery->data.hints[1] = discovery_info[1];
discovery->data.charset = discovery_info[2];
text = (char *) &discovery_info[3];
} else {
discovery->hints.byte[1] = 0;
discovery->charset = discovery_info[1];
discovery->data.hints[1] = 0;
discovery->data.charset = discovery_info[1];
text = (char *) &discovery_info[2];
}
/*
......@@ -542,8 +543,8 @@ static void irlap_recv_discovery_xid_cmd(struct irlap_cb *self,
* FCS bytes resides.
*/
skb->data[skb->len] = '\0';
strncpy(discovery->nickname, text, NICKNAME_MAX_LEN);
discovery->name_len = strlen(discovery->nickname);
strncpy(discovery->data.info, text, NICKNAME_MAX_LEN);
discovery->name_len = strlen(discovery->data.info);
info->discovery = discovery;
} else
......
This diff is collapsed.
......@@ -1616,8 +1616,8 @@ irnet_discovervalue_confirm(int result,
*
* Got a discovery indication from IrLMP, post an event
*
* Note : IrLMP take care of matching the hint mask for us, we only
* check if it is a "new" node...
* Note : IrLMP take care of matching the hint mask for us, and also
* check if it is a "new" node for us...
*
* As IrLMP filter on the IrLAN hint bit, we get both IrLAN and IrNET
* nodes, so it's only at connection time that we will know if the
......@@ -1633,7 +1633,7 @@ irnet_discovervalue_confirm(int result,
* is to messy, so we leave that to user space...
*/
static void
irnet_discovery_indication(discovery_t * discovery,
irnet_discovery_indication(discinfo_t * discovery,
DISCOVERY_MODE mode,
void * priv)
{
......@@ -1643,21 +1643,12 @@ irnet_discovery_indication(discovery_t * discovery,
DASSERT(priv == &irnet_server, , IRDA_OCB_ERROR,
"Invalid instance (0x%X) !!!\n", (unsigned int) priv);
/* Check if node is discovered is a new one or an old one.
* We check when how long ago this node was discovered, with a
* coarse timeout (we may miss some discovery events or be delayed).
*/
if((jiffies - discovery->first_timestamp) >= (sysctl_discovery_timeout * HZ))
{
return; /* Too old, not interesting -> goodbye */
}
DEBUG(IRDA_OCB_INFO, "Discovered new IrNET/IrLAN node %s...\n",
discovery->nickname);
discovery->info);
/* Notify the control channel */
irnet_post_event(NULL, IRNET_DISCOVER,
discovery->saddr, discovery->daddr, discovery->nickname);
discovery->saddr, discovery->daddr, discovery->info);
DEXIT(IRDA_OCB_TRACE, "\n");
}
......@@ -1672,7 +1663,7 @@ irnet_discovery_indication(discovery_t * discovery,
* check if it is a "new" node...
*/
static void
irnet_expiry_indication(discovery_t * expiry,
irnet_expiry_indication(discinfo_t * expiry,
DISCOVERY_MODE mode,
void * priv)
{
......@@ -1683,11 +1674,11 @@ irnet_expiry_indication(discovery_t * expiry,
"Invalid instance (0x%X) !!!\n", (unsigned int) priv);
DEBUG(IRDA_OCB_INFO, "IrNET/IrLAN node %s expired...\n",
expiry->nickname);
expiry->info);
/* Notify the control channel */
irnet_post_event(NULL, IRNET_EXPIRE,
expiry->saddr, expiry->daddr, expiry->nickname);
expiry->saddr, expiry->daddr, expiry->info);
DEXIT(IRDA_OCB_TRACE, "\n");
}
......
......@@ -150,11 +150,11 @@ static void
void *);
#ifdef DISCOVERY_EVENTS
static void
irnet_discovery_indication(discovery_t *,
irnet_discovery_indication(discinfo_t *,
DISCOVERY_MODE,
void *);
static void
irnet_expiry_indication(discovery_t *,
irnet_expiry_indication(discinfo_t *,
DISCOVERY_MODE,
void *);
#endif
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment