[ovs-dev] [PATCH] datapath: Ensure that skb->protocol is set in internal_dev_xmit().

Jesse Gross jesse at nicira.com
Fri Jan 6 21:20:58 UTC 2012


On Mon, Jan 2, 2012 at 9:52 AM, Ben Pfaff <blp at nicira.com> wrote:
> On Fri, Dec 30, 2011 at 12:52:14AM -0500, Jesse Gross wrote:
>> It seems like a bigger problem that this field isn't initialized than
>> just for OVS.  Should we be proposing to fix AF_PACKET upstream?
>
> I don't know.  I don't know what the use case is for setting the
> protocol explicitly.  Maybe it is useful for non-Ethernet media?
> Maybe it is intended only for use with non-"raw" sockets?

I think it's probably primarily used by non-raw sockets since it's
required in that case.  However, it does appear that the intended use
case for it is to initialize skb->protocol.  I found this patch from
Patrick McHardy to initialize it for dhcp and he reported that
otherwise packets don't hit filters that look at protocol, as
expected:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=474000

> I don't know what such a fix would look like.  AF_PACKET is supposed
> to work with various media, but there's no header_op for parsing a
> packet's protocol, so we'd have to figure out what the right thing to
> do is there.  Would we need a new header_op?

My guess is that it was implemented this way to be protocol
independent but it doesn't seem like the right way to go about it,
since you know what device you're sending on and the device should
know how to extract the protocol from it.  I think a new header_op
along the lines of dev_parse_header() makes the most sense.

> In any case, we'll need a backportable fix.

I agree and I think your fix is the right one for backporting, I'd
just like to know what the right way to do it is first.



More information about the dev mailing list