Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[Andi Kleen]

Joris van Rantwijk <joris@deadlock.et.tudelft.nl> writes:
> 
> So... Shouldn't dev_queue_xmit_nit() also process ptype_base then ?

Interesting bug.

It probably should, but unfortunately then it would loop back to all normal
protocols (IP, IPv6, ARP etc.) too, which would not be good.

It may be best to change af_packet to always use ptype_all and match
the protocols itself. Alternatively there would need to be a special
case.

-Andi

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[Joris van Rantwijk]

On 1 Nov 2001, Andi Kleen wrote:
> Joris van Rantwijk <joris@deadlock.et.tudelft.nl> writes:
> > So... Shouldn't dev_queue_xmit_nit() also process ptype_base then ?

> It probably should, but unfortunately then it would loop back to all normal
> protocols (IP, IPv6, ARP etc.) too, which would not be good.

Ah, right. I suspected there was a good reason not to do it, or it
would have been done ages ago.

But it's still a bit weird isn't it ?
You sure won't find this in man packet(7).

Thanks for explaining,
  Joris van Rantwijk.

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[Andi Kleen]

On Thu, Nov 01, 2001 at 04:18:27PM +0100, Joris van Rantwijk wrote:
> Ah, right. I suspected there was a good reason not to do it, or it
> would have been done ages ago.
> But it's still a bit weird isn't it ?
> You sure won't find this in man packet(7).
>
I would more consider it a bug. I didn't know about it while writing 
packet(7)

Here is a patch.

-Andi

--- linux-2.4.13-work/net/packet/af_packet.c-PACKET	Tue Aug  7 17:30:50 2001
+++ linux-2.4.13-work/net/packet/af_packet.c	Thu Nov  1 17:38:12 2001
@@ -250,6 +250,9 @@
 	if (skb->pkt_type == PACKET_LOOPBACK)
 		goto out;
 
+	if (sk->num != htons(ETH_P_ALL) && skb->protocol != sk->num) 
+		goto out; 
+
 	if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
 		goto oom;
 
@@ -413,6 +416,10 @@
 		goto drop;
 
 	sk = (struct sock *) pt->data;
+
+	if (sk->num != htons(ETH_P_ALL) && skb->protocol != sk->num) 
+		goto drop; 
+
 	po = sk->protinfo.af_packet;
 
 	skb->dev = dev;
@@ -824,7 +831,8 @@
 	}
 
 	sk->num = protocol;
-	sk->protinfo.af_packet->prot_hook.type = protocol;
+	/* XXX Always bind to ETH_P_ALL to catch outgoing packets. */ 
+	sk->protinfo.af_packet->prot_hook.type = htons(ETH_P_ALL);
 	sk->protinfo.af_packet->prot_hook.dev = dev;
 
 	sk->protinfo.af_packet->ifindex = dev ? dev->ifindex : 0;
@@ -973,7 +981,7 @@
 	sk->protinfo.af_packet->prot_hook.data = (void *)sk;
 
 	if (protocol) {
-		sk->protinfo.af_packet->prot_hook.type = protocol;
+		sk->protinfo.af_packet->prot_hook.type = htons(ETH_P_ALL);
 		dev_add_pack(&sk->protinfo.af_packet->prot_hook);
 		sock_hold(sk);
 		sk->protinfo.af_packet->running = 1;

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[kuznet]

Hello!

> > So... Shouldn't dev_queue_xmit_nit() also process ptype_base then ?
> 
> Interesting bug.

RTFM of the maillists, Andi.


Someone promised me to fix this in right way, but disappeared.

Generally packet sockets MUST NOT tap on output packets. No differences
of socket of another protocols. UDP does not tap output right?
What the hell packet socket should do this?
Snapping on output is feature which must be regulated by a separate option.
And to be honest I see no tragedy, if this option will not exist for sockets
bound to specific protocols.

Alexey

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[Andi Kleen]

On Thu, Nov 01, 2001 at 08:33:15PM +0300, A.N.Kuznetsov wrote:
> Generally packet sockets MUST NOT tap on output packets. No differences

First if you really meant this dev_xmit_nit() (which you added) could be 
removed. But I see no reason for this MUST NOT; IMHO it is a valid use
case to tap outgoing packets.

> of socket of another protocols. UDP does not tap output right?
> What the hell packet socket should do this?

Packet sockets are a little bit more 'raw' than UDP sockets; and for
sniffing it makes sense and people expect it.

It's also kind of promised by having a PACKET_OUTGOING type.

Now of course if you would be serious with this dev_queue_xmit would 
need to be removed, making it impossible to debug/sniff local protocols
without an external sniffer. That would be of course very broken. 
So it has to be kept. But then allowing it for ETH_P_ALL only is really
ugly imho; if the feature exists it should be implemented for the full
packet functionality which includes binding to protocols.


> Snapping on output is feature which must be regulated by a separate option.

 When dev_xmit_nit is already there it is easy enough to do it, so no reason
 to add such complications.

> And to be honest I see no tragedy, if this option will not exist for sockets
> bound to specific protocols.

 I think the patch should be added.

 -Andi

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[kuznet]

Hello!

> First if you really meant this dev_xmit_nit() (which you added) could be 
> removed.

Sorry? It is used by packet sniffers.


> ugly imho; if the feature exists it should be implemented for the full
> packet functionality which includes binding to protocols.

This is a silly abuse. Sniffers do not bind to protocols, should not
do this and have no reasons to do this.


>  I think the patch should be added.

That which adds all the packet sockets to ptype_all? Do you jest? :-)

Alexey

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[Andi Kleen]

On Thu, Nov 01, 2001 at 09:09:07PM +0300, A.N.Kuznetsov wrote:
> > ugly imho; if the feature exists it should be implemented for the full
> > packet functionality which includes binding to protocols.
> 
> This is a silly abuse. Sniffers do not bind to protocols, should not
> do this and have no reasons to do this.

When you e.g. have a TCP sniffer it makes sense to only bind it to ETH_P_IP.

If the sll_protocol field is not fully supported it should be removed.

> 
> 
> >  I think the patch should be added.
> 
> That which adds all the packet sockets to ptype_all? Do you jest? :-)

Do you worry about the handling of hundreds of packet sockets? 

Using the ptype hash before was nice, but does not look like it is absolutely
required. The overhead this way is not much bigger for a reasonable number
of packet sockets (and for a large number the current ptype hash is likely 
inadequate anyways) 


-Andi

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[kuznet]

Hello!

> When you e.g. have a TCP sniffer it makes sense to only bind it to ETH_P_IP.

For what purpose? To add a small underdeveloped copy of BPF?

If it was an optimization I would understand this of course.
But you propose deoptimization. :-)


> Do you worry about the handling of hundreds of packet sockets? 

I worry about _one_ packet socket, which implements a protocol
in user space. And only about this. It is what packet sockets
are used for.

And I do want to see any refs to it in irrelevant place, which output path is.

To summarize: I wanted to see a patch allowing to detect that
nobody listens on outpu (or even splitting input and output ptype_all.)
So that it becomes possible to use ETH_P_ALL to listen
for all frames, but not to abuse output path.

Opposite is just non-sense with no applications.

Alexey

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[Andi Kleen]

On Thu, Nov 01, 2001 at 09:56:34PM +0300, A.N.Kuznetsov wrote:
> Hello!
> 
> > When you e.g. have a TCP sniffer it makes sense to only bind it to ETH_P_IP.
> 
> For what purpose? To add a small underdeveloped copy of BPF?

Just to have an symmetric API. Everything else is too ugly to explain 
in manpages ;)

> To summarize: I wanted to see a patch allowing to detect that
> nobody listens on outpu (or even splitting input and output ptype_all.)

That would require changing/breaking PF_PACKET, no? 

-Andi

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[kuznet]

Hello!

> Just to have an symmetric API. Everything else is too ugly to explain 
> in manpages ;)

Explaining is easy. Blah-blah-blah... Sockets bound to ETH_P_ALL
are able to get copy of output packets which is useful f.e.
for packet sniffers (ref to [libpacp],[tcpdump]). In later kernels
this can be disabled with option PACKET_NOOUTPUT. When this option
is not supported user of packet socket bound to ETH_P_ALL has to filter
output packets at user level checking for pkt_type == PACKET_OUTPUT
or using an equivalent BPF applet.


> That would require changing/breaking PF_PACKET, no? 

No. Ideally the option could be PACKET_GRAB_OUTPUT and be disabled
by default (for symmetry :-)). But as soon as it was forgotten,
it has to be enabled by default.

Alexey

Bind to protocol with AF_PACKET doesn't work for outgoing packets

[Joris van Rantwijk]

Hello.

I'm trying to see outgoing network packets through the AF_PACKET
interface. This works as long as I bind the packet socket with
sll_protocol==htons(ETH_P_ALL).  I would expect that I can filter
on IP packets by binding to sll_protocol==htons(ETH_P_IP), but when
I try it I suddenly see only the incoming packets and no outgoing at all.

I suspect this is because dev_queue_xmit_nit() only walks the ptype_all
chain (with the ETH_P_ALL taps) and doesn't process the ptype_base[]
lists. net_rx_action() processes ptype_all as well as ptype_base, so
it works fine for incoming packets.

So... Shouldn't dev_queue_xmit_nit() also process ptype_base then ?
Or is this just complete cluelessness on my part ?
(I'm rather new to this so I don't know how it's supposed to work)

I tried this with linux-2.4.12, but it seems relevant to 2.2.x
and 2.0.x as well.

Thanks,
  Joris van Rantwijk
joris@deadlock.et.tudelft.nl - http://deadlock.et.tudelft.nl/~joris/

Re: Bind to protocol with AF_PACKET doesn't work for outgoing packets

[Edgar Toernig]

Joris van Rantwijk wrote:
> 
> I'm trying to see outgoing network packets through the AF_PACKET
> interface. This works as long as I bind the packet socket with
> sll_protocol==htons(ETH_P_ALL).  I would expect that I can filter
> on IP packets by binding to sll_protocol==htons(ETH_P_IP), but when
> I try it I suddenly see only the incoming packets and no outgoing at all.

Deja vu? :-)  See this message:

---------------------
> Subject: Re: PF_PACKET, ETH_P_IP does not catch outgoing packets.
> From: kuznet@ms2.inr.ac.ru
> Date: Thu, 23 Dec 1999 20:41:11 +0300 (MSK)
>
> Hello!
>
> > do not receive outgoing packets.  Just changing the
> > protocol to ETH_P_ALL (or a later bind with that proto)
> > will get all packets.  Is this intentional?  (I don't think
> > so *g*) 
>
> Yes, sort of. It is planned flaw in design. 8) 
>
>
> > Any idea for a quick fix?
>
> No, it is not very easy. If it were easy, it would be made. 8)
> 
> The problem is that bound to protocol sockets are not
> checked at output at all, only ETH_P_ALL ones are checked.
> We could check all, but it affects performance, because
> true protocols (looking exactly as packet socket) really
> need not it. The direction of compromise is not evident.
>
> Someone promised to think on this and repair at the end of 2.1,
> I even reserved sockopt PACKET_RECV_OUTPUT to switch it on/off,
> but, alas, I did not receive any patches.
>
> Alexey
-----------------------

Two years nobody cared.  Seems the BPF is good enough...

Ciao, ET.