allow write() on SOCK_PACKET sockets

Hello.

Unless I am really missing something, there seem to be no reason why the SOCK_PACKET code does not allow to use write() or send() when the socket was bound, and insists on using sendto(). SOCK_RAW code, in comparison, allows write() after bind().
Attached patch allows write() for the SOCK_PACKET sockets when they are bound.
Can this be applied, or am I missing the point?

@@ -322,16 +322,23 @@
 			return(-EINVAL);
 		if (msg->msg_namelen==sizeof(struct sockaddr_pkt))
 			proto=saddr->spkt_protocol;
+		/*
+		 *	Find the device first to size check it 
+		 */
+
+		saddr->spkt_device[13] = 0;
+		dev = dev_get_by_name(saddr->spkt_device);
 	}
 	else
-		return(-ENOTCONN);	/* SOCK_PACKET must be sent giving an address */
-
-	/*
-	 *	Find the device first to size check it 
-	 */
+	{
+		struct packet_opt *po = pkt_sk(sk);
+		if (!po->running || !po->prot_hook.dev)
+			return -ENOTCONN;	/* SOCK_PACKET must be sent giving an address */
+		proto = po->prot_hook.type;
+		dev = po->prot_hook.dev;
+		dev_hold(dev);
+	}
 
-	saddr->spkt_device[13] = 0;
-	dev = dev_get_by_name(saddr->spkt_device);
 	err = -ENODEV;
 	if (dev == NULL)
 		goto out_unlock;

It is counter-intuitive to allow write after bind(). AFAIK RAW only allows write after connect(), not bind().

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Hi Herbert.

I claim that SOCK_RAW allows write() after bind() because a few days ago I changed dosemu code to use SOCK_RAW instead of SOCK_PACKET and write() instead of sendto(). See here:
http://cvs.sourceforge.net/viewcvs.py/dosemu/dosemu/src/dosext/net/net/libpacket.c?r1=1.4&r2=1.5&diff_format=u
http://cvs.sourceforge.net/viewcvs.py/dosemu/dosemu/src/dosext/net/net/pktnew.c?r1=1.7&r2=1.8&diff_format=u
Of course the fact that I did that for dosemu, doesn't mean that I was doing the right thing (so if you know it is wrong - I'll redo it) but at least it was tested and works.
And since that works for SOCK_RAW, I don't see any reasons for it to not work for SOCK_PACKET. And btw, I can use read() quite happily even with SOCK_PACKET, so why not write()...
My patch is simply an adoption of the code SOCK_RAW has. See af_packet.c:packet_sendmsg(), you'll see under "if (saddr == NULL)" just the same code as I was doing for SOCK_PACKET.

Well I just checked net/ipv4/raw.c and it's pretty clear that it does

		err = -EDESTADDRREQ;
		if (sk->sk_state != TCP_ESTABLISHED)
			goto out;

So you need to connect before you can write. I'm intrigued that you can write before connecting on a raw socket. Could you please write up a minimal program that I can play with?

Well read() is different. It returns all packets received on the socket, regardless of where they came from. So for a connectionless socket it makes perfect sense to read() before you've called connect().

OTOH, write() and send() needs to know where the message is going to. That's exactly what connect() provides. So it makes no sense to write()/send() before you've called connect().

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Hi.

Herbert Xu wrote:

I think you are looking at a wrong place. You are looking into IP raw sockets code. Packet sockets are really the different layer. Please have a look into net/packet/af_packet.c instead.

Packet sockets, actually, do not even have connect:
.connect = sock_no_connect

Not an IP raw socket, but the raw packet socket. So yes, I can. And that looks very natural to me, not a hack or something.

I can but I am a bit surprised that dosemu is not a sufficient test-case for *you* :) But I don't seem to be able to send any mail to you:

<[email protected]>: host arnor.apana.org.au[203.14.152.115] said:
550 mail from 217.67.122.194 rejected: administrative prohibition (in reply to RCPT TO command)

Yes, not a good argument on my side, sorry.

That's exactly where the packet sockets are different. Here's the whole point. Have a look into a "struct sockaddr_pkt":

             struct sockaddr_pkt
             {
                 unsigned short  spkt_family;
                 unsigned char   spkt_device[14];
                 unsigned short  spkt_protocol;
             };

Not too much about a destination here. For the packet sockets you only need to know via which eth device you want to send it, and nothing more. And this is what I specify to bind() anyway, so I dont want to duplicate that info all the time. You say it is counter-intuitive. I'll agree with this only if you point me another convinient way to bind to the particular eth device and send/receive via that device without always specifying its name/number. That's what SOCK_RAW allows, but not SOCK_PACKET.

My patch is probably dead anyway though. SOCK_PACKET is mentioned to be deprecated in man, so perhaps noone will apply any patches on it... Just wanted to point out that there is a bug/inconsistency in it.

Yes. Sorry for the confusion.

Should work now.

I see your point. But I don't really like the current code that uses the address from bind for sending. Even though it works here because the packet socket is symmetric wrt sending/receiving, it is counter-intuitive for the socket API in general.

Indeed it is.

Thanks anyway.

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt