[ovs-dev] [PATCH v8 1/1] Avoid dp_hash recirculation for balance-tcp bond selection mode

Vishal Deep Ajmera vishal.deep.ajmera at ericsson.com
Wed Dec 18 11:34:32 UTC 2019


> 
> I took a look.  The underlying issue is that code here mixes integers,
> ofp_port_t, and odp_port_t.  OVS uses "sparse" annotations to keep these
> from being confused, since they are different in important ways.  I
> spent some time working through the types here and appended a patch that
> fixes them up.  I also made a bunch of style updates throughout the
> code, which might obscure the relevant changes a bit; my apologies.
> 
> It seems odd that dpif_bond_*() succeed if the dpif doesn't support
> them.  Shouldn't they return an error by default, instead of 0?
> 

I will fix this in next revision. May be not to call these APIs when 
datapath do not support them.

> Is there a reason to allow users to turn this off?  What is the downside
> of enabling it?  Why is it disabled by default?  In general, OVS should
> optimize itself to the extent it can rather than relying on a
> knowledgeable user to tweak it.  If it's necessary to make it
> configurable, then the documentation (in vswitch.xml) should explain why
> one would want to turn it on or off and what the default is.
> 
> This introduces a new capabilities flag, which should be documented with
> the other capabilities in vswitch.xml.
> 

Do we read these capabilities in the ofproto layer somewhere ? Or is it for
display and documentation purpose? Trying to see if I can use same name
for 'key' in here.

> I'm appending a suggested diff to fold into your patch.
> 
> I did not do a full technical review for correctness.  I'll do that on
> the next revision.

Thanks Ben for fixing the issues.  I will include your patch in next
revision.

Warm Regards,
Vishal Ajmera


More information about the dev mailing list