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

Joe Stringer joe at ovn.org
Tue Jan 24 19:12:08 UTC 2017


On 24 January 2017 at 00:45, Roi Dayan <roid at mellanox.com> wrote:
>
>
> On 1/23/2017 8:41 PM, Joe Stringer wrote:
>>
>> 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.
>
>
> Hi Joe,
>
> We understand that forcing the user to restart OVS is not something common
> we want to do.
> We can make this option changeable at runtime and flush all the rules when
> hw-offload changes from true to false.
> If something will block us we'll let you know and if a future requirement
> will need this then it can be changed later.
>
> To answer your next question then this is also what happens now when the
> user restarts OVS. All rules are flushed so there shoudn't be HW forwarding
> rules.

How are they flushed?

I don't think ovs-vswitchd flushes flows when it stops.

Typically if you restart OVS, then on startup the flow table will be
empty so before you get a chance to install your OpenFlow flows again,
OVS will be running, dumping datapath flows, and deleting them because
they don't match the current OpenFlow table. However, now that you've
disabled hw flows, I suspect that OVS won't try to manage them. So the
hw flows will continue to forward according to policy prior to
ovs-vswitchd shutdown.

There is also a way to start OVS where it will wait for you to finish
initializing the OpenFlow tables before it starts revalidating the
flows (ovs_vsctl set open_vswitch .
other_config:flow-restore-wait="true") - see utilities/ovs-ctl.in for
how this may be used.

These are all only considerations where the datapath has its own life
beyond the OVS binary. They don't apply when running with userspace
datapath, clearly, because the datapath and ovs-vswitchd are the same
in that case.


More information about the dev mailing list