[ovs-dev] FW: MPLS and VLAN QinQ patch

ravi kerur rkerur at gmail.com
Tue Jun 12 19:52:42 UTC 2012


I am currently debugging this and had planned to send an email asking
for suggestions when code review was nearing completion. This bug has
been elusive for a long time. The packet format themselves looked fine
as I have found a way to test them and have successfully tested them.

The problem is that ovs-vswitchd crashes and happens only in the
automation i.e "make check", or "make dist-check", since user-space
code has been successfully tested manually and I couldn't find a way
to run gdb during automation it's been actively debugged on and off.

I did try "make check-valgrind" as suggested by you but it didn't
reveal anything.

Addtionally,

1. thought automation didn't support eth_types 0x8847 and 0x8848 and
looked for asserts and didn't find it, so this was ruled out.

2. thought ofpbuf handling was not correct (highly unlikely since
user-space code was tested), so completely commented "push_mpls"
function in lib/packets.c and still got ovs-vswitchd crash.

3. currently assuming netlink attribute handling for mpls is wrong
(unlikely same argument as above).

Right now I am just doing code read to see if I can spot the bug, if
you have any suggestion please let me know.

Please note I would have brought this problem once you were done with
the code-review.

Thanks,
Ravi

On Tue, Jun 12, 2012 at 11:13 AM, Ben Pfaff <blp at nicira.com> wrote:
> Oh, I saw one testsuite failure also:
> 448: ofproto-dpif - controller                       FAILED
> (ofproto-dpif.at:323)
> Does this pass for you.
>
> On Tue, Jun 12, 2012 at 11:11 AM, Ben Pfaff <blp at nicira.com> wrote:
>> Ravi, I'm more or less happy with the userspace code here.  I mostly
>> have style kinds of concerns.  (I haven't fully scrutinized every line
>> though.)
>>
>> I did notice that compose_dec_mpls_ttl() decrements "ttl" twice (it
>> has --ttl in two places).
>>
>> I believe that Pravin is going to review the kernel portions of this
>> code now.  Once he's happy, I'll suggest touch-ups for userspace, or
>> just do them myself.
>>
>> On Mon, Jun 11, 2012 at 03:00:45PM -0700, ravi kerur wrote:
>>> Attached latest mpls and qinq patch. It is based off latest git master.
>>>
>>> Incorporates code review comments, takes care of performance issue on offload.
>>>
>>> Thanks,
>>> Ravi
>>>
>>> On Fri, Jun 1, 2012 at 2:32 PM, ravi kerur <rkerur at gmail.com> wrote:
>>> > This is based off
>>> >
>>> > commit 73c0ce349ba8d13a63a249a56aad0bec6e6caf26
>>> > Author: Joe Stringer <joe at wand.net.nz>
>>> > Date:   Tue May 29 00:38:21 2012 +1200
>>> >
>>> > it can wait until your return. I will probably do some performance
>>> > testing in the mean time for both mpls + qinq tcp offload.
>>> >
>>> > Thanks,
>>> > Ravi



More information about the dev mailing list