Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix using uninitialized fitness.

[Ilya Maximets]

On 24.02.2018 01:13, Ben Pfaff wrote:
> On Wed, Feb 21, 2018 at 03:09:00PM +0300, Ilya Maximets wrote:
>> 'upcall_xlate()' makes a decision to compose slow path actions
>> by checking the 'upcall->fitness', which is not initialized in
>> case of calling from the 'upcall_cb()'.
>>
>> 'upcall_cb()' receives the real flow, so the fitness should be
>> initialized as perfect.
>>
>> Fixes following tests on travis:
>>
>> ofproto-dpif.at: ofproto-dpif megaflow - disabled - pmd
>> ofproto-dpif.at: ofproto-dpif - conntrack - output action
>>
>> CC: Ben Pfaff 
>> Fixes: 687bafbb8a79 ("ofproto-dpif-upcall: Slow path flows that
>>   datapath can't fully match.")
>> Signed-off-by: Ilya Maximets 
> 
> Thanks a lot for the fix.
> 
> "struct upcall" is a large data structure (almost 2 kB on x86-32), so
> this one-line fix is going to be very expensive.  To avoid a performance
> regression, I suggest using an assignment statement rather than an
> initializer, which necessarily initializes the entire structure.

Good point. Thanks.

> 
> Would you mind respinning it that way?

Sure, I'll send v2 with this change soon.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix using uninitialized fitness.

[Ben Pfaff]

On Wed, Feb 21, 2018 at 03:09:00PM +0300, Ilya Maximets wrote:
> 'upcall_xlate()' makes a decision to compose slow path actions
> by checking the 'upcall->fitness', which is not initialized in
> case of calling from the 'upcall_cb()'.
> 
> 'upcall_cb()' receives the real flow, so the fitness should be
> initialized as perfect.
> 
> Fixes following tests on travis:
> 
> ofproto-dpif.at: ofproto-dpif megaflow - disabled - pmd
> ofproto-dpif.at: ofproto-dpif - conntrack - output action
> 
> CC: Ben Pfaff 
> Fixes: 687bafbb8a79 ("ofproto-dpif-upcall: Slow path flows that
>   datapath can't fully match.")
> Signed-off-by: Ilya Maximets 

Thanks a lot for the fix.

"struct upcall" is a large data structure (almost 2 kB on x86-32), so
this one-line fix is going to be very expensive.  To avoid a performance
regression, I suggest using an assignment statement rather than an
initializer, which necessarily initializes the entire structure.

Would you mind respinning it that way?

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev