[ovs-dev] [PATCH 06/14] tunneling: Fix header parsing for ECN decapsulation.
Ben Pfaff
blp at nicira.com
Fri Dec 3 02:19:09 UTC 2010
On Thu, Dec 2, 2010 at 5:08 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Thu, Dec 2, 2010 at 3:45 PM, Ben Pfaff <blp at nicira.com> wrote:
>> On Thu, Dec 02, 2010 at 12:36:55PM -0800, Jesse Gross wrote:
>>> When copying ECN bits from the tunnel header to the inner packet,
>>> the network header was not being properly located when there was
>>> a vlan tag. This simplies and fixes the header parsing to prevent
>>> this problem. It also removes a call to eth_type_trans() which
>>> does several comparisions of MAC addresses which are not needed here
>>> because we do not need to set skb->pkt_type.
>>
>> The eth_type_trans() elimination looks good to me, as long as we don't
>> need the special case for IPX (I sure hope we don't).
>
> Yes, I didn't really want to put in the IPX hack. I don't think it
> actually matter though because nothing inside of OVS cares about the
> difference between ETH_P_802_2 and ETH_P_802_3. If we egress through
> an internal device to the IP stack we actually will call
> eth_type_trans(). A driver could see the difference if the packet
> goes out through a netdev but I highly doubt any of them care.
>
>>
>> In ecn_decapsulate(), we access the IP header (ip_hdr(skb)->tos) before
>> we check that we can get at it with pskb_may_pull(). That doesn't look
>> right to me, especially given that we are careful to check that we can
>> pull an extra 4 bytes when there's an 802.1Q header.
>
> Thanks, you're right. However, I also realized that I forgot what I
> was trying to parse and completely screwed up this patch (for one
> thing it can never do anything useful because it's actually looking at
> the inner IP header). I'm going to drop it for now and send out a
> revised version later.
OK.
Or you could just drop the ecn_decapsulate() half and check in the
eth_type_trans() half. I don't think that these two parts are logically
or technically connected.
More information about the dev
mailing list