On Sun, May 28, 2017 at 02:59:49PM +0300, Roi Dayan wrote:
> From: Paul Blakey <pa...@mellanox.com>
>
> Add a new configuration tc-policy option that controls tc
> flower flag. Possible options are none, skip_sw, skip_hw.
> The default is none which is to insert the rule both to sw and hw.
> This option is only relevant if hw-offload is enabled.
>
> Signed-off-by: Paul Blakey <pa...@mellanox.com>
> Reviewed-by: Roi Dayan <r...@mellanox.com>
> Reviewed-by: Simon Horman <simon.hor...@netronome.com>
Thanks, the comment below not withstanding this looks good to me
and I would be happy to apply it if someone provided a review.
> ---
> lib/netdev.c | 6 ++++++
> lib/tc.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> lib/tc.h | 3 +++
> vswitchd/vswitch.xml | 17 +++++++++++++++++
> 4 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 2807a9d..19d809b 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -54,6 +54,9 @@
> #include "openvswitch/vlog.h"
> #include "flow.h"
> #include "util.h"
> +#ifdef __linux__
> +#include "tc.h"
> +#endif
>
> VLOG_DEFINE_THIS_MODULE(netdev);
>
> @@ -2117,6 +2120,9 @@ netdev_set_flow_api_enabled(const struct smap
> *ovs_other_config)
>
> VLOG_INFO("netdev: Flow API Enabled");
>
> + tc_set_policy(smap_get_def(ovs_other_config, "tc-policy",
> + TC_POLICY_DEFAULT));
> +
> ovsthread_once_done(&once);
> }
> }
In v7 Flavio Leitner made the following comment regarding this.
"Well, this enforces that the user sets tc-policy before enabling HW
offloading, otherwise it won't get set, right?"
My comment, along the same lines, is that it looks like it would be better
if tc_set_policy() was called independent of netdev_set_flow_api_enabled()
to allow them to be configured independently.
I realise that at this point the patch-set has some limitations regarding
restarting ovs when offloading of flows is enabled and disabled. And
probably the code above plays well with that restriction in place. But I
think it would be good to look into ways to lift that restriction and thus
it would seems worthwhile revisiting where tc_set_policy() is called.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
