> > Any response to these queries? I’m uncomfortable TBH with this feature
> going in to 2.18 unless these are addressed/resolved. From an Intel
> perspective we’re more than happy to test and review but there are
> genuine issues to be discussed I feel below before merge.
> 
> I tend to agree here.  The patch set doesn't seem to be ready.
> It has a few major issues like broken upgrades/live migration
> and doesn't seem to be tested enough to enable features by
> default.
> 
> I'd suggest to not rush it, but take a closer look at what is
> going on, write good unit tests for the "untested" software
> fallback, which is a major concern.  Since lots of refactoring
> and changes are happening at the same time, it's better to
> give it time to soak before enabling by default.  It's fine
> to enable if it gives better performance for everyone, but
> we clearly don't have enough time to assess that.
> 
> I'm generally also not very comfortable with changing historical
> names like 'p' and 'b' in functions just for the sake of renaming.
> And I agree with Amber that some cover letter would be nice to
> have.  We should also have performance test results somewhere
> to be sure that we're not introducing regressions.  Performance
> tests for the software fallback are also necessary.
> 
> To be clear, I didn't really review the patch set, but a quick
> glance over patches and discussions doesn't give me any
> confidence at this point.
> 
> Best regards, Ilya Maximets.
> 

+1


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to