[ovs-dev] [PATCH ovs V2 03/21] other-config: Add hw-offload switch to control netdev flow offloading

[Joe Stringer]

On 22 January 2017 at 08:13, Paul Blakey <paulb at mellanox.com> wrote:
>
>
> On 05/01/2017 03:26, Joe Stringer wrote:
>>
>> On 25 December 2016 at 03:39, Paul Blakey <paulb at mellanox.com> wrote:
>>>
>>> Add a new configuration option - hw-offload that enables netdev
>>> flow api. Enabling this option will allow offloading flows
>>> using netdev implementation instead of the kernel datapath.
>>> This configuration option defaults to false - disabled.
>>>
>>> Signed-off-by: Paul Blakey <paulb at mellanox.com>
>>> Reviewed-by: Roi Dayan <roid at mellanox.com>
>>> ---
>>>  lib/netdev.c         | 18 ++++++++++++++++++
>>>  lib/netdev.h         |  2 ++
>>>  vswitchd/bridge.c    |  2 ++
>>>  vswitchd/vswitch.xml | 11 +++++++++++
>>>  4 files changed, 33 insertions(+)
>>>
>>> diff --git a/lib/netdev.c b/lib/netdev.c
>>> index 3ac3c48..b289166 100644
>>> --- a/lib/netdev.c
>>> +++ b/lib/netdev.c
>>> @@ -2071,7 +2071,25 @@ netdev_init_flow_api(struct netdev *netdev)
>>>  {
>>>      const struct netdev_class *class = netdev->netdev_class;
>>>
>>> +    if (!netdev_flow_api_enabled) {
>>> +        return EOPNOTSUPP;
>>> +    }
>>> +
>>>      return (class->init_flow_api
>>>              ? class->init_flow_api(netdev)
>>>              : EOPNOTSUPP);
>>>  }
>>> +
>>> +bool netdev_flow_api_enabled = false;
>>> +
>>> +void
>>> +netdev_set_flow_api_enabled(bool enabled)
>>> +{
>>> +    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>>> +
>>> +    if (ovsthread_once_start(&once)) {
>>> +        netdev_flow_api_enabled = enabled;
>>> +        VLOG_INFO("netdev: Flow API %s", enabled ? "Enabled" :
>>> "Disabled");
>>> +        ovsthread_once_done(&once);
>>> +    }
>>> +}
>>
>>
>> Requiring restart to apply this option seems a bit arbitrary, why not
>> allow it to be changed at runtime?
>>
>
> Because it isn't something that is supposed to change often,
> It might cause problems if its enabled later on with different (netdev)
> implementations. We can try and test changing it to runtime at a later time.

Forcing the user to restart OVS to make a database setting take effect
is an unusual requirement. Even uncommon configurations that require
datapath-level changes (eg, reconfiguring queues) is applied in OVS
without requiring the user to intervene and restart. I understand that
even DPDK init doesn't require restart any more.

Here's another question: Let's say that hardware offloads is enabled
and traffic runs. User turns it off, restarts OVS. What happens to the
hardware flows? Presumably OVS doesn't manage them any more because
hw-offloads is off, but they would continue to forward traffic. If OVS
then gets different OpenFlow rules then the forwarding could be wrong.