[ovs-dev] [PATCH 01/19] tunneling: Remove call to eth_type_trans() on receive.

Jesse Gross jesse at nicira.com
Thu Dec 9 19:26:12 UTC 2010


On Thu, Dec 9, 2010 at 11:25 AM, Jesse Gross <jesse at nicira.com> wrote:
> On Thu, Dec 9, 2010 at 9:32 AM, Ben Pfaff <blp at nicira.com> wrote:
>> On Wed, Dec 08, 2010 at 10:13:59PM -0800, Jesse Gross wrote:
>>> On receive we call eth_type_trans() to set skb->protocol.  However,
>>> that function also sets skb->pkt_type, which requires several
>>> comparisons to MAC addresses.  Nothing in OVS cares about pkt_type,
>>> so this is wasteful.  If we actually do egress to the IP stack
>>> through an internal device then we'll call eth_type_trans() to get
>>> everything correctly setup.  It's possible for device drivers to
>>> see an incorrect pkt_type or not correctly parse legacy IPX (which
>>> eth_type_trans() also handles) but it's highly unlikely that they
>>> will care.
>>
>> The old version of this function did a skb_reset_mac_header(),
>> indirectly in eth_type_trans().  The new version doesn't.  That implies
>> that the MAC header is already correct, but if that is so then
>>        struct ethhdr *eh = (struct ethhdr *)skb->data;
>> could be written as
>>        struct ethhdr *eh = eth_hdr(skb);
>> So I don't know what to think here, since it wasn't obvious to me from
>> the callers that the MAC header had been set properly.
>>
>> Otherwise look fine to me, thanks.
>>
>
> The MAC header pointer isn't correct here (it points at the outer
> header, which is not very useful).  The inner Ethernet header starts
> at skb->data, which is why that line is written the way it is.
>
> Nothing uses the outer Ethernet header but we do use the outer IP
> header for ECN decapsulation, which is why I didn't reset them here
> (we were resetting the network header before, which is a bug fixed in
> the next commit).  Everything gets correctly set in flow_extract() for
> the packet as processed by OVS.

I also added this comment to the top of tnl_rcv:

	/* Packets received by this function are in the following state:
	 * - skb->data points to the inner Ethernet header.
	 * - The inner Ethernet header is in the linear data area.
	 * - skb->csum does not include the inner Ethernet header.
	 * - The layer pointers point at the outer headers.
	 */




More information about the dev mailing list