On Sat, Sep 1, 2018 at 1:05 AM, Ben Pfaff <[email protected]> wrote:
> On Thu, Jul 12, 2018 at 12:59:34PM +0530, Sriharsha Basavapatna via dev wrote:
>> This is the fourth patch in the patch-set to support dynamic rebalancing
>> of offloaded flows.
>>
>> A new OVS configuration parameter "offload-rebalance", is added to ovsdb.
>> The default value of this is "disable". To enable this feature, set the
>> value of this parameter to "pps-rate", which provides packets-per-second
>> rate based policy to dynamically offload and un-offload flows.
>>
>> Note: This option can be enabled only when 'hw-offload' policy is enabled.
>> It also requires 'tc-policy' to be set to 'skip_sw'; otherwise, flow
>> offload errors (specifically ENOSPC error this feature depends on) reported
>> by an offloaded device are supressed by TC-Flower kernel module.
>>
>> Signed-off-by: Sriharsha Basavapatna <[email protected]>
>> Co-authored-by: Venkat Duvvuru <[email protected]>
>> Signed-off-by: Venkat Duvvuru <[email protected]>
>> Reviewed-by: Sathya Perla <[email protected]>
>
> Thanks.

Thanks for your comments.
>
> I would add an update to NEWS to mention the new rebalancing feature.
>
> I would fold this patch in with the previous one.

done.
>
> Please declare variables at point of first use, when possible, and avoid
> adding unecessary parentheses to conditions.

done.
>
> I think the log message "netdev: Using policy 'disable'" is going to
> confuse people.  If it is necessary to log the policy, the log message
> should explain better than that.

removed feature log messages.
>
> I don't think the enum is going to help anything.  I would use a bool.

done.
>
> I would drop OFFLOAD_REBALANCE_POLICY_DEFAULT.

done.
>
> Please look at the ovs-vswitchd.conf.db(5) manpage.  The formatting is
> not going to look good.  I recommend using just a bool, honestly,
> instead of this enum.

done.
>
> Thanks,
>
> Ben.

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

Reply via email to