net-2.6.22 fixes

PATCH Applied [email protected] 4 participants 21 messages Apr 18, 2007 – Apr 22, 2007 4 messages not in the archive

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

[email protected]

author

2007-04-18 21:32 UTC #0 lore raw

cover net-2.6.22 fixes

Hi Dave,

Here go 7 patches for IrDA, against your latest net-2.6.22.
They are mainly af_irda.c fixes/cleanup. Also, I just sent another af_irda.c oops fix, against net-2.6 ("IrDA: Correctly handling socket error"). If you want it against net-2.6.22 as well, just let me know.

Cheers,
Samuel.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[email protected]

author

2007-04-18 21:32 UTC #1 lore raw

PATCH 1/7 af_irda: irda_recvmsg_stream cleanup

From: Olaf Kirch <[email protected]>

This patch cleans up some code in irda_recvmsg_stream, replacing some homebrew code with prepare_to_wait/finish_wait, and by making the code honor sock_rcvtimeo.

Signed-off-by Olaf Kirch <[email protected]>

Signed-off-by Samuel Ortiz <[email protected]>

---
 net/irda/af_irda.c |   31 +++++++++++++------------------
 1 files changed, 13 insertions(+), 18 deletions(-)

net/irda/af_irda.c +13 ~~-18~~

Index: net-2.6.22-quilt/net/irda/af_irda.c
===================================================================
@@ -1403,8 +1403,8 @@
 	struct irda_sock *self = irda_sk(sk);
 	int noblock = flags & MSG_DONTWAIT;
 	size_t copied = 0;
-	int target = 1;
-	DECLARE_WAITQUEUE(waitq, current);
+	int target;
+	long timeo;
 
 	IRDA_DEBUG(3, "%s()\n", __FUNCTION__);

@@ -1417,8 +1417,8 @@
 	if (flags & MSG_OOB)
 		return -EOPNOTSUPP;
 
-	if (flags & MSG_WAITALL)
-		target = size;
+	target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
+	timeo = sock_rcvtimeo(sk, noblock);
 
 	msg->msg_namelen = 0;

@@ -1426,19 +1426,14 @@
 		int chunk;
 		struct sk_buff *skb = skb_dequeue(&sk->sk_receive_queue);
 
-		if (skb==NULL) {
+		if (skb == NULL) {
+			DEFINE_WAIT(wait);
 			int ret = 0;
 
 			if (copied >= target)
 				break;
 
-			/* The following code is a cut'n'paste of the
-			 * wait_event_interruptible() macro.
-			 * We don't us the macro because the test condition
-			 * is messy. - Jean II */
-			set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
-			add_wait_queue(sk->sk_sleep, &waitq);
-			set_current_state(TASK_INTERRUPTIBLE);
+			prepare_to_wait_exclusive(sk->sk_sleep, &wait, TASK_INTERRUPTIBLE);
 
 			/*
 			 *	POSIX 1003.1g mandates this order.

@@ -1451,17 +1446,17 @@
 			else if (noblock)
 				ret = -EAGAIN;
 			else if (signal_pending(current))
-				ret = -ERESTARTSYS;
+				ret = sock_intr_errno(timeo);
+			else if (sk->sk_state != TCP_ESTABLISHED)
+				ret = -ENOTCONN;
 			else if (skb_peek(&sk->sk_receive_queue) == NULL)
 				/* Wait process until data arrives */
 				schedule();
 
-			current->state = TASK_RUNNING;
-			remove_wait_queue(sk->sk_sleep, &waitq);
-			clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+			finish_wait(sk->sk_sleep, &wait);
 
-			if(ret)
-				return(ret);
+			if (ret)
+				return ret;
 			if (sk->sk_shutdown & RCV_SHUTDOWN)
 				break;

David Miller

maintainer Applied

2007-04-21 05:05 UTC #2 lore raw

Re: [PATCH 1/7] [IrDA] af_irda: irda_recvmsg_stream cleanup

From: [email protected]
Date: Thu, 19 Apr 2007 00:32:03 +0300

Quoted 8 lines

From: Olaf Kirch <[email protected]>
This patch cleans up some code in irda_recvmsg_stream, replacing some homebrew code with prepare_to_wait/finish_wait, and by making the code honor sock_rcvtimeo.
Signed-off-by Olaf Kirch <[email protected]>
Signed-off-by Samuel Ortiz <[email protected]>

Applied to net-2.6.22, thanks.

[email protected]

author

2007-04-18 21:32 UTC #3 lore raw

PATCH 2/7 af_irda: Silence kernel message in irda_recvmsg_stream

In reply to [email protected]

From: Olaf Kirch <[email protected]>

This patch silences an IRDA_ASSERT in irda_recvmsg_stream, as described in
http://bugzilla.kernel.org/show_bug.cgi?id=7512 irda_disconnect_indication would set sk->sk_err to ECONNRESET, and a subsequent call to recvmsg would print an irritating kernel message and return -1.

When a connected socket is closed by the peer, recvmsg should return 0 rather than an error. This patch fixes this.

Signed-off-by Olaf Kirch <[email protected]>

Signed-off-by Samuel Ortiz <[email protected]>

---
 net/irda/af_irda.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

net/irda/af_irda.c +11 -7

Index: net-2.6.22-quilt/net/irda/af_irda.c
===================================================================
@@ -131,15 +131,13 @@
 	}
 
 	/* Prevent race conditions with irda_release() and irda_shutdown() */
+	bh_lock_sock(sk);
 	if (!sock_flag(sk, SOCK_DEAD) && sk->sk_state != TCP_CLOSE) {
-		lock_sock(sk);
 		sk->sk_state     = TCP_CLOSE;
-		sk->sk_err       = ECONNRESET;
 		sk->sk_shutdown |= SEND_SHUTDOWN;
 
 		sk->sk_state_change(sk);
 		sock_orphan(sk);
-		release_sock(sk);
 
 		/* Close our TSAP.
 		 * If we leave it open, IrLMP put it back into the list of

@@ -159,6 +157,7 @@
 			self->tsap = NULL;
 		}
 	}
+	bh_unlock_sock(sk);
 
 	/* Note : once we are there, there is not much you want to do
 	 * with the socket anymore, apart from closing it.

@@ -1062,7 +1061,8 @@
 
 	if (sk->sk_state != TCP_ESTABLISHED) {
 		sock->state = SS_UNCONNECTED;
-		return sock_error(sk);	/* Always set at this point */
+		err = sock_error(sk);
+		return err? err : -ECONNRESET;
 	}
 
 	sock->state = SS_CONNECTED;

@@ -1356,7 +1356,9 @@
 	IRDA_DEBUG(4, "%s()\n", __FUNCTION__);
 
 	IRDA_ASSERT(self != NULL, return -1;);
-	IRDA_ASSERT(!sock_error(sk), return -1;);
+
+	if ((err = sock_error(sk)) < 0)
+		return err;
 
 	skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
 				flags & MSG_DONTWAIT, &err);

@@ -1403,13 +1405,15 @@
 	struct irda_sock *self = irda_sk(sk);
 	int noblock = flags & MSG_DONTWAIT;
 	size_t copied = 0;
-	int target;
+	int target, err;
 	long timeo;
 
 	IRDA_DEBUG(3, "%s()\n", __FUNCTION__);
 
 	IRDA_ASSERT(self != NULL, return -1;);
-	IRDA_ASSERT(!sock_error(sk), return -1;);
+
+	if ((err = sock_error(sk)) < 0)
+		return err;
 
 	if (sock->flags & __SO_ACCEPTCON)
 		return(-EINVAL);

David Miller

maintainer

2007-04-21 05:08 UTC #4 lore raw

Re: [PATCH 2/7] [IrDA] af_irda: Silence kernel message in irda_recvmsg_stream

From: [email protected]
Date: Thu, 19 Apr 2007 00:32:04 +0300

Quoted 7 lines

From: Olaf Kirch <[email protected]>
This patch silences an IRDA_ASSERT in irda_recvmsg_stream, as described in
http://bugzilla.kernel.org/show_bug.cgi?id=7512 irda_disconnect_indication would set sk->sk_err to ECONNRESET, and a subsequent call to recvmsg would print an irritating kernel message and return -1.

Quoted 5 lines

When a connected socket is closed by the peer, recvmsg should return 0 rather than an error. This patch fixes this.
Signed-off-by Olaf Kirch <[email protected]>
Signed-off-by Samuel Ortiz <[email protected]>

There were some conflicts in this one wrt. the sock_orphan() bug fix we put into mainline recently, but I did my best to resolve it when I applied this to net-2.6.22

You can have a look and verify things when I put up my net-2.6.2 tree later this evening.

Thanks.

A message that is not in the archive (sent off-list or not yet received): 20070420.220900.45154094.davem-fT/PcQaiUtIeIZ0/[email protected]

Samuel Ortiz

author

2007-04-21 12:01 UTC #6 lore raw

Re: [PATCH 2/7] [IrDA] af_irda: Silence kernel message in irda_recvmsg_stream

Quoted 6 lines · David Miller

There were some conflicts in this one wrt. the sock_orphan() bug fix we put into mainline recently, but I did my best to resolve it when I applied this to net-2.6.22
You can have a look and verify things when I put up my net-2.6.2 tree later this evening.

Verified. It's all good, thanks a lot.

Cheers,
Samuel.

A message that is not in the archive (sent off-list or not yet received): [email protected]

David Miller

2007-04-21 17:43 UTC #8 lore raw

Re: [PATCH 2/7] [IrDA] af_irda: Silence kernel message in irda_recvmsg_stream

From: Samuel Ortiz <[email protected]>
Date: Sat, 21 Apr 2007 15:01:52 +0300

Quoted 8 lines · David Miller

Quoted 6 lines
There were some conflicts in this one wrt. the sock_orphan() bug fix we put into mainline recently, but I did my best to resolve it when I applied this to net-2.6.22
You can have a look and verify things when I put up my net-2.6.2 tree later this evening.
Verified. It's all good, thanks a lot.

Thanks for checking.

[email protected]

author

2007-04-18 21:32 UTC #9 lore raw

PATCH 3/7 af_irda: irda_accept cleanup

In reply to [email protected]

This patch removes a cut'n'paste copy of wait_event_interruptible from irda_accept.

Signed-off-by Samuel Ortiz <[email protected]>

Acked-by Olaf Kirch <[email protected]>

---
 net/irda/af_irda.c |   34 ++++++++--------------------------
 1 files changed, 8 insertions(+), 26 deletions(-)

net/irda/af_irda.c +8 ~~-26~~

Index: net-2.6.22-quilt/net/irda/af_irda.c
===================================================================
@@ -873,37 +873,19 @@
 	 * calling us, the data is waiting for us ;-)
 	 * Jean II
 	 */
-	skb = skb_dequeue(&sk->sk_receive_queue);
-	if (skb == NULL) {
-		int ret = 0;
-		DECLARE_WAITQUEUE(waitq, current);
+	while (1) {
+		skb = skb_dequeue(&sk->sk_receive_queue);
+		if (skb)
+			break;
 
 		/* Non blocking operation */
 		if (flags & O_NONBLOCK)
 			return -EWOULDBLOCK;
 
-		/* The following code is a cut'n'paste of the
-		 * wait_event_interruptible() macro.
-		 * We don't us the macro because the condition has
-		 * side effects : we want to make sure that only one
-		 * skb get dequeued - Jean II */
-		add_wait_queue(sk->sk_sleep, &waitq);
-		for (;;) {
-			set_current_state(TASK_INTERRUPTIBLE);
-			skb = skb_dequeue(&sk->sk_receive_queue);
-			if (skb != NULL)
-				break;
-			if (!signal_pending(current)) {
-				schedule();
-				continue;
-			}
-			ret = -ERESTARTSYS;
-			break;
-		}
-		current->state = TASK_RUNNING;
-		remove_wait_queue(sk->sk_sleep, &waitq);
-		if(ret)
-			return -ERESTARTSYS;
+		err = wait_event_interruptible(*(sk->sk_sleep),
+					skb_peek(&sk->sk_receive_queue));
+		if (err)
+			return err;
 	}
 
 	newsk = newsock->sk;

David Miller

maintainer Applied

2007-04-21 05:09 UTC #10 lore raw

Re: [PATCH 3/7] [IrDA] af_irda: irda_accept cleanup

From: [email protected]
Date: Thu, 19 Apr 2007 00:32:05 +0300

Quoted 5 lines

This patch removes a cut'n'paste copy of wait_event_interruptible from irda_accept.
Signed-off-by Samuel Ortiz <[email protected]>
Acked-by Olaf Kirch <[email protected]>

Applied to net-2.6.22, thanks.

[email protected]

author

2007-04-18 21:32 UTC #11 lore raw

PATCH 4/7 af_irda: IRDA_ASSERT cleanups

In reply to [email protected]

In af_irda.c, the multiple IRDA_ASSERT() are either hiding bugs, useless, or returning the wrong value.
Let's clean that up.

Signed-off-by Samuel Ortiz <[email protected]>

---
 net/irda/af_irda.c |   32 +++++---------------------------
 1 files changed, 5 insertions(+), 27 deletions(-)

net/irda/af_irda.c +5 ~~-27~~

Index: net-2.6.22-quilt/net/irda/af_irda.c
===================================================================
@@ -89,7 +89,6 @@
 
 	self = instance;
 	sk = instance;
-	IRDA_ASSERT(sk != NULL, return -1;);
 
 	err = sock_queue_rcv_skb(sk, skb);
 	if (err) {

@@ -306,8 +305,6 @@
 
 	IRDA_DEBUG(2, "%s()\n", __FUNCTION__);
 
-	IRDA_ASSERT(self != NULL, return;);
-
 	skb = alloc_skb(TTP_MAX_HEADER + TTP_SAR_HEADER,
 			GFP_ATOMIC);
 	if (skb == NULL) {

@@ -337,7 +334,7 @@
 
 	self = instance;
 	sk = instance;
-	IRDA_ASSERT(sk != NULL, return;);
+	BUG_ON(sk == NULL);
 
 	switch (flow) {
 	case FLOW_STOP:

@@ -449,7 +446,7 @@
 	IRDA_DEBUG(2, "%s()\n", __FUNCTION__);
 
 	self = (struct irda_sock *) priv;
-	IRDA_ASSERT(self != NULL, return;);
+	BUG_ON(self == NULL);
 
 	/* Nothing for the caller */
 	self->cachelog = NULL;

@@ -546,8 +543,6 @@
 {
 	IRDA_DEBUG(2, "%s(%p, %s)\n", __FUNCTION__, self, name);
 
-	IRDA_ASSERT(self != NULL, return -1;);
-
 	if (self->iriap) {
 		IRDA_WARNING("%s(): busy with a previous query\n",
 			     __FUNCTION__);

@@ -635,8 +630,6 @@
 
 	IRDA_DEBUG(2, "%s(), name=%s\n", __FUNCTION__, name);
 
-	IRDA_ASSERT(self != NULL, return -1;);
-
 	/* Ask lmp for the current discovery log
 	 * Note : we have to use irlmp_get_discoveries(), as opposed
 	 * to play with the cachelog directly, because while we are

@@ -784,8 +777,6 @@
 	struct irda_sock *self = irda_sk(sk);
 	int err;
 
-	IRDA_ASSERT(self != NULL, return -1;);
-
 	IRDA_DEBUG(2, "%s(%p)\n", __FUNCTION__, self);
 
 	if (addr_len != sizeof(struct sockaddr_irda))

@@ -841,8 +832,6 @@
 
 	IRDA_DEBUG(2, "%s()\n", __FUNCTION__);
 
-	IRDA_ASSERT(self != NULL, return -1;);
-
 	err = irda_create(newsock, sk->sk_protocol);
 	if (err)
 		return err;

@@ -889,10 +878,12 @@
 	}
 
 	newsk = newsock->sk;
+	if (newsk == NULL)
+		return -EIO;
+
 	newsk->sk_state = TCP_ESTABLISHED;
 
 	new = irda_sk(newsk);
-	IRDA_ASSERT(new != NULL, return -1;);
 
 	/* Now attach up the new socket */
 	new->tsap = irttp_dup(self->tsap, new);

@@ -1154,8 +1145,6 @@
 {
 	IRDA_DEBUG(2, "%s(%p)\n", __FUNCTION__, self);
 
-	IRDA_ASSERT(self != NULL, return;);
-
 	/* Unregister with IrLMP */
 	irlmp_unregister_client(self->ckey);
 	irlmp_unregister_service(self->skey);

@@ -1274,7 +1263,6 @@
 		return -ENOTCONN;
 
 	self = irda_sk(sk);
-	IRDA_ASSERT(self != NULL, return -1;);
 
 	/* Check if IrTTP is wants us to slow down */

@@ -1337,8 +1325,6 @@
 
 	IRDA_DEBUG(4, "%s()\n", __FUNCTION__);
 
-	IRDA_ASSERT(self != NULL, return -1;);
-
 	if ((err = sock_error(sk)) < 0)
 		return err;

@@ -1392,8 +1378,6 @@
 
 	IRDA_DEBUG(3, "%s()\n", __FUNCTION__);
 
-	IRDA_ASSERT(self != NULL, return -1;);
-
 	if ((err = sock_error(sk)) < 0)
 		return err;

@@ -1527,7 +1511,6 @@
 		return -ENOTCONN;
 
 	self = irda_sk(sk);
-	IRDA_ASSERT(self != NULL, return -1;);
 
 	/*
 	 * Check that we don't send out too big frames. This is an unreliable

@@ -1596,7 +1579,6 @@
 	}
 
 	self = irda_sk(sk);
-	IRDA_ASSERT(self != NULL, return -1;);
 
 	/* Check if an address was specified with sendto. Jean II */
 	if (msg->msg_name) {

@@ -1670,8 +1652,6 @@
 	struct sock *sk = sock->sk;
 	struct irda_sock *self = irda_sk(sk);
 
-	IRDA_ASSERT(self != NULL, return -1;);
-
 	IRDA_DEBUG(1, "%s(%p)\n", __FUNCTION__, self);
 
 	sk->sk_state       = TCP_CLOSE;

@@ -1844,8 +1824,6 @@
 	struct ias_attrib *	ias_attr;	/* Attribute in IAS object */
 	int opt;
 
-	IRDA_ASSERT(self != NULL, return -1;);
-
 	IRDA_DEBUG(2, "%s(%p)\n", __FUNCTION__, self);
 
 	if (level != SOL_IRLMP)

David Miller

maintainer Applied

2007-04-21 05:10 UTC #12 lore raw

Re: [PATCH 4/7] [IrDA] af_irda: IRDA_ASSERT cleanups

From: [email protected]
Date: Thu, 19 Apr 2007 00:32:06 +0300

Quoted 5 lines

In af_irda.c, the multiple IRDA_ASSERT() are either hiding bugs, useless, or returning the wrong value.
Let's clean that up.
Signed-off-by Samuel Ortiz <[email protected]>

Applied to net-2.6.22, thanks.

[email protected]

author

2007-04-18 21:32 UTC #13 lore raw

PATCH 5/7 IrDA monitor mode

In reply to [email protected]

Through a protocol specific ioctl, one can disable IrDA TX in order to monitor an IrDA link.

Signed-off-by Samuel Ortiz <[email protected]>

---
 include/linux/irda.h     |    7 +++++
 include/net/irda/irlap.h |    2 +
 net/irda/af_irda.c       |   58 +++++++++++++++++++++++++++++++++++++++++++++-
 net/irda/irlap_frame.c   |    8 ++++++
 4 files changed, 74 insertions(+), 1 deletions(-)

include/linux/irda.h +7 -0

Index: net-2.6.22-quilt/include/linux/irda.h
===================================================================
@@ -172,6 +172,12 @@
 #define SIOCSDTRRTS    (SIOCDEVPRIVATE + 8)
 #define SIOCGQOS       (SIOCDEVPRIVATE + 9)
 
+/* Protocol private ioctls */
+#define SIOCIRDASETMODE (SIOCPROTOPRIVATE + 0)
+#define SIOCIRDAGETMODE (SIOCPROTOPRIVATE + 1)
+
+#define IRDA_MODE_MONITOR   0x1
+
 /* No reason to include <linux/if.h> just because of this one ;-) */
 #define IRNAMSIZ 16

@@ -209,6 +215,7 @@
 	} ifr_ifru;
 };
 
+#define ifr_name      ifr_ifrn.ifrn_name
 #define ifr_baudrate  ifr_ifru.ifru_qos.baudrate
 #define ifr_receiving ifr_ifru.ifru_receiving 
 #define ifr_dongle    ifr_ifru.ifru_dongle

include/net/irda/irlap.h +2 -0

Index: net-2.6.22-quilt/include/net/irda/irlap.h
===================================================================
@@ -208,6 +208,8 @@
 	int    xbofs_delay;   /* Nr of XBOF's used to MTT */
 	int    bofs_count;    /* Negotiated extra BOFs */
 	int    next_bofs;     /* Negotiated extra BOFs after next frame */
+
+	int    mode;     /* 1 is for monitor mode (TX disabled) */
 };
 
 /*

net/irda/af_irda.c +57 -1

76 lines not on this page · raw message

net/irda/irlap_frame.c +8 -0

Index: net-2.6.22-quilt/net/irda/irlap_frame.c
===================================================================
@@ -101,6 +101,14 @@
 
 	irlap_insert_info(self, skb);
 
+	if (unlikely(self->mode & IRDA_MODE_MONITOR)) {
+		IRDA_DEBUG(3, "%s(): %s is in monitor mode\n", __FUNCTION__,
+			   self->netdev->name);
+		dev_kfree_skb(skb);
+		return;
+	}
+
+
 	dev_queue_xmit(skb);
 }

David Miller

maintainer Applied

2007-04-21 05:11 UTC #14 lore raw

Re: [PATCH 5/7] [IrDA] IrDA monitor mode

From: [email protected]
Date: Thu, 19 Apr 2007 00:32:07 +0300

Quoted 4 lines

Through a protocol specific ioctl, one can disable IrDA TX in order to monitor an IrDA link.
Signed-off-by Samuel Ortiz <[email protected]>

I'd really rather see IRDA use netlink instead of these wonky ioctls, but whatever, if you want to put this dirty into your stack and you're the maintainer I can't really stop you :-)

Applied to net-2.6.22, thanks.

A message that is not in the archive (sent off-list or not yet received): 20070420.221143.132446079.davem-fT/PcQaiUtIeIZ0/[email protected]

Samuel Ortiz

author

2007-04-21 11:42 UTC #16 lore raw

Re: [PATCH 5/7] [IrDA] IrDA monitor mode

Hi Dave,

Quoted 6 lines · David Miller

From: [email protected]
Date: Thu, 19 Apr 2007 00:32:07 +0300
Quoted 3 lines
Through a protocol specific ioctl, one can disable IrDA TX in order to monitor an IrDA link.

Quoted 6 lines

Quoted 1 line
Signed-off-by Samuel Ortiz <[email protected]>
I'd really rather see IRDA use netlink instead of these wonky ioctls, but whatever, if you want to put this dirty into your stack and you're the maintainer I can't really stop you :-)

I actually asked for your advice about it some time ago:
http://marc.info/?l=linux-netdev&m=117590341703544&w=2
But by looking at the flow of emails on netdev sent to you (and that's only netdev...), I can easily see how you could have missed it :-)

Regarding this patch, I came up with this ioctl because I thought implementing an IrDA netlink layer would be an overkill for just one single command.
What I propose is that you revert this patch before we introduce a new ioctl upstream while I start implementing a netlink IrDA layer. Would that be ok with you, or can we let this new ioctl in and have it coexisting with a future netlink layer ?

Cheers,
Samuel.

A message that is not in the archive (sent off-list or not yet received): [email protected]

David Miller

2007-04-21 17:46 UTC #18 lore raw

Re: [PATCH 5/7] [IrDA] IrDA monitor mode

From: Samuel Ortiz <[email protected]>
Date: Sat, 21 Apr 2007 14:42:40 +0300

Quoted 5 lines

I actually asked for your advice about it some time ago:
http://marc.info/?l=linux-netdev&m=117590341703544&w=2
But by looking at the flow of emails on netdev sent to you (and that's only netdev...), I can easily see how you could have missed it :-)

Quoted 7 lines

Regarding this patch, I came up with this ioctl because I thought implementing an IrDA netlink layer would be an overkill for just one single command.
What I propose is that you revert this patch before we introduce a new ioctl upstream while I start implementing a netlink IrDA layer. Would that be ok with you, or can we let this new ioctl in and have it coexisting with a future netlink layer ?

The only concern is that once we add the ioctl, we can't get rid of it, it's there forever. Please consider this very carefully.

You might discover that once you create the IRDA netlink layer, you will find it convenient to make it support all the IRDA stack configuration not just this one thing :-)

Samuel Ortiz

2007-04-22 11:28 UTC #19 lore raw

Re: [irda-users] [PATCH 5/7] [IrDA] IrDA monitor mode

Quoted 16 lines · David Miller

From: Samuel Ortiz <[email protected]>
Date: Sat, 21 Apr 2007 14:42:40 +0300
Quoted 5 lines
I actually asked for your advice about it some time ago:
http://marc.info/?l=linux-netdev&m=117590341703544&w=2
But by looking at the flow of emails on netdev sent to you (and that's only netdev...), I can easily see how you could have missed it :-)
Quoted 7 lines
Regarding this patch, I came up with this ioctl because I thought implementing an IrDA netlink layer would be an overkill for just one single command.
What I propose is that you revert this patch before we introduce a new ioctl upstream while I start implementing a netlink IrDA layer. Would that be ok with you, or can we let this new ioctl in and have it coexisting with a future netlink layer ?

Quoted 7 lines

The only concern is that once we add the ioctl, we can't get rid of it, it's there forever. Please consider this very carefully.
You might discover that once you create the IRDA netlink layer, you will find it convenient to make it support all the IRDA stack configuration not just this one thing :-)

Fair enough, let's revert, and I'll come back to you with an IrDA netlink layer:

---
This reverts commit 8eb943bbb8d2b59a5dccfed691da2e3797ef7e8e.

Let's not introduce yet another ioctl. The IrDA monitoring feature will be implemented through the addition of an IrDA netlink layer.

Signed-off-by Samuel Ortiz <[email protected]>

---
 include/linux/irda.h     |    7 -----
 include/net/irda/irlap.h |    2 -
 net/irda/af_irda.c       |   58 +---------------------------------------------
 net/irda/irlap_frame.c   |    8 ------
 4 files changed, 1 insertions(+), 74 deletions(-)

include/linux/irda.h +0 -7

index 1050f98..09d8f10 100644
@@ -172,12 +172,6 @@ struct irda_ias_set {
 #define SIOCSDTRRTS    (SIOCDEVPRIVATE + 8)
 #define SIOCGQOS       (SIOCDEVPRIVATE + 9)
 
-/* Protocol private ioctls */
-#define SIOCIRDASETMODE (SIOCPROTOPRIVATE + 0)
-#define SIOCIRDAGETMODE (SIOCPROTOPRIVATE + 1)
-
-#define IRDA_MODE_MONITOR   0x1
-
 /* No reason to include <linux/if.h> just because of this one ;-) */
 #define IRNAMSIZ 16

@@ -215,7 +209,6 @@ struct if_irda_req {
 	} ifr_ifru;
 };
 
-#define ifr_name      ifr_ifrn.ifrn_name
 #define ifr_baudrate  ifr_ifru.ifru_qos.baudrate
 #define ifr_receiving ifr_ifru.ifru_receiving 
 #define ifr_dongle    ifr_ifru.ifru_dongle

include/net/irda/irlap.h +0 -2

index 02b08d2..e77eb88 100644
@@ -208,8 +208,6 @@ struct irlap_cb {
 	int    xbofs_delay;   /* Nr of XBOF's used to MTT */
 	int    bofs_count;    /* Negotiated extra BOFs */
 	int    next_bofs;     /* Negotiated extra BOFs after next frame */
-
-	int    mode;     /* 1 is for monitor mode (TX disabled) */
 };
 
 /*

net/irda/af_irda.c +1 ~~-57~~

76 lines not on this page · raw message

net/irda/irlap_frame.c +0 -8

index 8acc356..3c5a68e 100644
@@ -101,14 +101,6 @@ void irlap_queue_xmit(struct irlap_cb *self, struct sk_buff *skb)
 
 	irlap_insert_info(self, skb);
 
-	if (unlikely(self->mode & IRDA_MODE_MONITOR)) {
-		IRDA_DEBUG(3, "%s(): %s is in monitor mode\n", __FUNCTION__,
-			   self->netdev->name);
-		dev_kfree_skb(skb);
-		return;
-	}
-
-
 	dev_queue_xmit(skb);
 }

Signature · 1 line

1.5.1.1

David Miller

maintainer Applied

2007-04-22 19:59 UTC #20 lore raw

Re: [irda-users] [PATCH 5/7] [IrDA] IrDA monitor mode

From: Samuel Ortiz <[email protected]>
Date: Sun, 22 Apr 2007 14:29:07 +0300

Quoted 5 lines

Fair enough, let's revert, and I'll come back to you with an IrDA netlink layer:
---
This reverts commit 8eb943bbb8d2b59a5dccfed691da2e3797ef7e8e.

Applied, thanks.

[email protected]

author

2007-04-18 21:32 UTC #21 lore raw

PATCH 6/7 Adding carriage returns to mcs7780 debug statements

In reply to [email protected]

Signed-off-by Samuel Ortiz <[email protected]>

---
 drivers/net/irda/mcs7780.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

drivers/net/irda/mcs7780.c +13 ~~-13~~

index 54d1d54..0de8672 100644
@@ -200,14 +200,14 @@ static inline int mcs_setup_transceiver_vishay(struct mcs_cb *mcs)
 /* Setup a communication between mcs7780 and agilent chip. */
 static inline int mcs_setup_transceiver_agilent(struct mcs_cb *mcs)
 {
-	IRDA_WARNING("This transceiver type is not supported yet.");
+	IRDA_WARNING("This transceiver type is not supported yet.\n");
 	return 1;
 }
 
 /* Setup a communication between mcs7780 and sharp chip. */
 static inline int mcs_setup_transceiver_sharp(struct mcs_cb *mcs)
 {
-	IRDA_WARNING("This transceiver type is not supported yet.");
+	IRDA_WARNING("This transceiver type is not supported yet.\n");
 	return 1;
 }

@@ -279,7 +279,7 @@ static inline int mcs_setup_transceiver(struct mcs_cb *mcs)
 		break;
 
 	default:
-		IRDA_WARNING("Unknown transceiver type: %d",
+		IRDA_WARNING("Unknown transceiver type: %d\n",
 			     mcs->transceiver_type);
 		ret = 1;
 	}

@@ -318,7 +318,7 @@ static inline int mcs_setup_transceiver(struct mcs_cb *mcs)
 		return ret;
 
 error:
-	IRDA_ERROR("%s", msg);
+	IRDA_ERROR("%s\n", msg);
 	return ret;
 }

@@ -587,7 +587,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	} while(cnt++ < 100 && (rval & MCS_IRINTX));
 
 	if(cnt >= 100) {
-		IRDA_ERROR("unable to change speed");
+		IRDA_ERROR("unable to change speed\n");
 		ret = -EIO;
 		goto error;
 	}

@@ -638,7 +638,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 
 		default:
 			ret = 1;
-			IRDA_WARNING("Unknown transceiver type: %d",
+			IRDA_WARNING("Unknown transceiver type: %d\n",
 				     mcs->transceiver_type);
 		}
 	if (unlikely(ret))

@@ -733,7 +733,7 @@ static int mcs_net_open(struct net_device *netdev)
 	sprintf(hwname, "usb#%d", mcs->usbdev->devnum);
 	mcs->irlap = irlap_open(netdev, &mcs->qos, hwname);
 	if (!mcs->irlap) {
-		IRDA_ERROR("mcs7780: irlap_open failed");
+		IRDA_ERROR("mcs7780: irlap_open failed\n");
 		goto error2;
 	}

@@ -862,7 +862,7 @@ static int mcs_hard_xmit(struct sk_buff *skb, struct net_device *ndev)
 			  mcs->out_buf, wraplen, mcs_send_irq, mcs);
 
 	if ((ret = usb_submit_urb(mcs->tx_urb, GFP_ATOMIC))) {
-		IRDA_ERROR("failed tx_urb: %d", ret);
+		IRDA_ERROR("failed tx_urb: %d\n", ret);
 		switch (ret) {
 		case -ENODEV:
 		case -EPIPE:

@@ -897,7 +897,7 @@ static int mcs_probe(struct usb_interface *intf,
 	if (!ndev)
 		goto error1;
 
-	IRDA_DEBUG(1, "MCS7780 USB-IrDA bridge found at %d.", udev->devnum);
+	IRDA_DEBUG(1, "MCS7780 USB-IrDA bridge found at %d.\n", udev->devnum);
 
 	/* what is it realy for? */
 	SET_MODULE_OWNER(ndev);

@@ -905,7 +905,7 @@ static int mcs_probe(struct usb_interface *intf,
 
 	ret = usb_reset_configuration(udev);
 	if (ret != 0) {
-		IRDA_ERROR("mcs7780: usb reset configuration failed");
+		IRDA_ERROR("mcs7780: usb reset configuration failed\n");
 		goto error2;
 	}

@@ -950,7 +950,7 @@ static int mcs_probe(struct usb_interface *intf,
 	if (ret != 0)
 		goto error2;
 
-	IRDA_DEBUG(1, "IrDA: Registered MosChip MCS7780 device as %s",
+	IRDA_DEBUG(1, "IrDA: Registered MosChip MCS7780 device as %s\n",
 		   ndev->name);
 
 	mcs->transceiver_type = transceiver_type;

@@ -981,7 +981,7 @@ static void mcs_disconnect(struct usb_interface *intf)
 	free_netdev(mcs->netdev);
 
 	usb_set_intfdata(intf, NULL);
-	IRDA_DEBUG(0, "MCS7780 now disconnected.");
+	IRDA_DEBUG(0, "MCS7780 now disconnected.\n");
 }
 
 /* Module insertion */

@@ -992,7 +992,7 @@ static int __init mcs_init(void)
 	/* register this driver with the USB subsystem */
 	result = usb_register(&mcs_driver);
 	if (result)
-		IRDA_ERROR("usb_register failed. Error number %d", result);
+		IRDA_ERROR("usb_register failed. Error number %d\n", result);
 
 	return result;
 }

Signature · 10 lines

1.5.1

--


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

David Miller

maintainer Applied

2007-04-21 05:12 UTC #22 lore raw

Re: [PATCH 6/7] [IrDA] Adding carriage returns to mcs7780 debug statements

From: [email protected]
Date: Thu, 19 Apr 2007 00:32:08 +0300

Quoted 1 line

Signed-off-by Samuel Ortiz <[email protected]>

Applied to net-2.6.22, thanks.

[email protected]

author

2007-04-18 21:32 UTC #23 lore raw

PATCH 7/7 Misc spelling corrections.

In reply to [email protected]

From: Guennadi Liakhovetski <gl-o/[email protected]>

Spelling corrections, from "to" to "too".

Signed-off-by G. Liakhovetski <gl-o/[email protected]>

Signed-off-by Samuel Ortiz <[email protected]>

---
 net/irda/irlap_event.c |    2 +-
 net/irda/irlap_frame.c |   14 +++++++-------
 net/irda/irttp.c       |    4 ++--
 net/irda/parameters.c  |    8 ++++----
 4 files changed, 14 insertions(+), 14 deletions(-)

net/irda/irlap_event.c +1 -1

Index: net-2.6.22-quilt/net/irda/irlap_event.c
===================================================================
@@ -590,7 +590,7 @@
 		if (!self->discovery_log) {
 			IRDA_WARNING("%s: discovery log is gone! "
 				     "maybe the discovery timeout has been set"
-				     " to short?\n", __FUNCTION__);
+				     " too short?\n", __FUNCTION__);
 			break;
 		}
 		hashbin_insert(self->discovery_log,

net/irda/irlap_frame.c +7 -7

Index: net-2.6.22-quilt/net/irda/irlap_frame.c
===================================================================
@@ -421,7 +421,7 @@
 	IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
 
 	if (!pskb_may_pull(skb, sizeof(struct xid_frame))) {
-		IRDA_ERROR("%s: frame to short!\n", __FUNCTION__);
+		IRDA_ERROR("%s: frame too short!\n", __FUNCTION__);
 		return;
 	}

@@ -492,7 +492,7 @@
 	char *text;
 
 	if (!pskb_may_pull(skb, sizeof(struct xid_frame))) {
-		IRDA_ERROR("%s: frame to short!\n", __FUNCTION__);
+		IRDA_ERROR("%s: frame too short!\n", __FUNCTION__);
 		return;
 	}

@@ -536,7 +536,7 @@
 		/* Check if things are sane at this point... */
 		if((discovery_info == NULL) ||
 		   !pskb_may_pull(skb, 3)) {
-			IRDA_ERROR("%s: discovery frame to short!\n",
+			IRDA_ERROR("%s: discovery frame too short!\n",
 				   __FUNCTION__);
 			return;
 		}

@@ -1181,7 +1181,7 @@
 	IRDA_ASSERT(info != NULL, return;);
 
 	if (!pskb_may_pull(skb, 4)) {
-		IRDA_ERROR("%s: frame to short!\n", __FUNCTION__);
+		IRDA_ERROR("%s: frame too short!\n", __FUNCTION__);
 		return;
 	}

@@ -1270,7 +1270,7 @@
 	IRDA_DEBUG(2, "%s()\n", __FUNCTION__);
 
 	if (!pskb_may_pull(skb, sizeof(*frame))) {
-		IRDA_ERROR("%s: frame to short!\n", __FUNCTION__);
+		IRDA_ERROR("%s: frame too short!\n", __FUNCTION__);
 		return;
 	}
 	frame = (struct test_frame *) skb->data;

@@ -1278,7 +1278,7 @@
 	/* Broadcast frames must carry saddr and daddr fields */
 	if (info->caddr == CBROADCAST) {
 		if (skb->len < sizeof(struct test_frame)) {
-			IRDA_DEBUG(0, "%s() test frame to short!\n",
+			IRDA_DEBUG(0, "%s() test frame too short!\n",
 				   __FUNCTION__);
 			return;
 		}

@@ -1344,7 +1344,7 @@
 
 	/* Check if frame is large enough for parsing */
 	if (!pskb_may_pull(skb, 2)) {
-		IRDA_ERROR("%s: frame to short!\n", __FUNCTION__);
+		IRDA_ERROR("%s: frame too short!\n", __FUNCTION__);
 		dev_kfree_skb(skb);
 		return -1;
 	}

net/irda/irttp.c +2 -2

Index: net-2.6.22-quilt/net/irda/irttp.c
===================================================================
@@ -551,7 +551,7 @@
 	}
 
 	if (skb->len > self->max_seg_size) {
-		IRDA_DEBUG(1, "%s(), UData is to large for IrLAP!\n",
+		IRDA_DEBUG(1, "%s(), UData is too large for IrLAP!\n",
 			   __FUNCTION__);
 		goto err;
 	}

@@ -598,7 +598,7 @@
 	 *  inside an IrLAP frame
 	 */
 	if ((self->tx_max_sdu_size == 0) && (skb->len > self->max_seg_size)) {
-		IRDA_ERROR("%s: SAR disabled, and data is to large for IrLAP!\n",
+		IRDA_ERROR("%s: SAR disabled, and data is too large for IrLAP!\n",
 			   __FUNCTION__);
 		ret = -EMSGSIZE;
 		goto err;

net/irda/parameters.c +4 -4

Index: net-2.6.22-quilt/net/irda/parameters.c
===================================================================
@@ -160,7 +160,7 @@
 	}
 	/* Check if buffer is long enough for insertion */
 	if (len < (2+p.pl)) {
-		IRDA_WARNING("%s: buffer to short for insertion!\n",
+		IRDA_WARNING("%s: buffer too short for insertion!\n",
 			     __FUNCTION__);
 		return -1;
 	}

@@ -216,7 +216,7 @@
 
 	/* Check if buffer is long enough for parsing */
 	if (len < (2+p.pl)) {
-		IRDA_WARNING("%s: buffer to short for parsing! "
+		IRDA_WARNING("%s: buffer too short for parsing! "
 			     "Need %d bytes, but len is only %d\n",
 			     __FUNCTION__, p.pl, len);
 		return -1;

@@ -304,7 +304,7 @@
 
 	/* Check if buffer is long enough for parsing */
 	if (len < (2+p.pl)) {
-		IRDA_WARNING("%s: buffer to short for parsing! "
+		IRDA_WARNING("%s: buffer too short for parsing! "
 			     "Need %d bytes, but len is only %d\n",
 			     __FUNCTION__, p.pl, len);
 		return -1;

@@ -343,7 +343,7 @@
 
 	/* Check if buffer is long enough for parsing */
 	if (len < (2+p.pl)) {
-		IRDA_WARNING("%s: buffer to short for parsing! "
+		IRDA_WARNING("%s: buffer too short for parsing! "
 			     "Need %d bytes, but len is only %d\n",
 			     __FUNCTION__, p.pl, len);
 		return -1;

David Miller

maintainer

2007-04-21 05:12 UTC #24 lore raw

Re: [PATCH 7/7] [IrDA] Misc spelling corrections.

From: [email protected]
Date: Thu, 19 Apr 2007 00:32:09 +0300

Quoted 6 lines

From: Guennadi Liakhovetski <[email protected]>
Spelling corrections, from "to" to "too".
Signed-off-by G. Liakhovetski <[email protected]>
Signed-off-by Samuel Ortiz <[email protected]>

Also applied to net-2.6.22, thanks a lot.