[ovs-dev] [PATCH V11 08/33] other-config: Add tc-policy switch to control tc flower flag

[Flavio Leitner]

On Tue, Jun 13, 2017 at 06:03:30PM +0300, Roi Dayan wrote:
> From: Paul Blakey <paulb at 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 <paulb at mellanox.com>
> Reviewed-by: Roi Dayan <roid at mellanox.com>
> Reviewed-by: Simon Horman <simon.horman at netronome.com>

I'd suggest to consider improving the kernel to provide a define for
policy NONE because then I think we don't need tc_offload_policy enum.

It also has that known limitation of forcing the user to set the
policy before enabling HW offloading.

Both are not a blocker to me.

Acked-by: Flavio Leitner <fbl at sysclose.org>


> ---
>  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 5008a43..3204689 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);
>  
> @@ -2136,6 +2139,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);
>          }
>      }
> diff --git a/lib/tc.c b/lib/tc.c
> index d3eaf98..9ca7b76 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -39,6 +39,14 @@ VLOG_DEFINE_THIS_MODULE(tc);
>  
>  static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5);
>  
> +enum tc_offload_policy {
> +    TC_POLICY_NONE,
> +    TC_POLICY_SKIP_SW,
> +    TC_POLICY_SKIP_HW
> +};
> +
> +static enum tc_offload_policy tc_policy = TC_POLICY_NONE;
> +
>  struct tcmsg *
>  tc_make_request(int ifindex, int type, unsigned int flags,
>                  struct ofpbuf *request)
> @@ -739,6 +747,18 @@ tc_get_flower(int ifindex, int prio, int handle, struct tc_flower *flower)
>      return error;
>  }
>  
> +static int
> +tc_get_tc_cls_policy(enum tc_offload_policy policy)
> +{
> +    if (policy == TC_POLICY_SKIP_HW) {
> +        return TCA_CLS_FLAGS_SKIP_HW;
> +    } else if (policy == TC_POLICY_SKIP_SW) {
> +        return TCA_CLS_FLAGS_SKIP_SW;
> +    }
> +
> +    return 0;
> +}
> +
>  static void
>  nl_msg_put_act_push_vlan(struct ofpbuf *request, uint16_t vid, uint8_t prio)
>  {
> @@ -1044,7 +1064,7 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct tc_flower *flower)
>          }
>      }
>  
> -    nl_msg_put_u32(request, TCA_FLOWER_FLAGS, 0);
> +    nl_msg_put_u32(request, TCA_FLOWER_FLAGS, tc_get_tc_cls_policy(tc_policy));
>  
>      if (flower->tunnel.tunnel) {
>          nl_msg_put_flower_tunnel(request, flower);
> @@ -1089,3 +1109,24 @@ tc_replace_flower(int ifindex, uint16_t prio, uint32_t handle,
>  
>      return error;
>  }
> +
> +void
> +tc_set_policy(const char *policy)
> +{
> +    if (!policy) {
> +        return;
> +    }
> +
> +    if (!strcmp(policy, "skip_sw")) {
> +        tc_policy = TC_POLICY_SKIP_SW;
> +    } else if (!strcmp(policy, "skip_hw")) {
> +        tc_policy = TC_POLICY_SKIP_HW;
> +    } else if (!strcmp(policy, "none")) {
> +        tc_policy = TC_POLICY_NONE;
> +    } else {
> +        VLOG_WARN("tc: Invalid policy '%s'", policy);
> +        return;
> +    }
> +
> +    VLOG_INFO("tc: Using policy '%s'", policy);
> +}
> diff --git a/lib/tc.h b/lib/tc.h
> index 78470a7..a472b99 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -36,6 +36,8 @@
>  
>  #define TC_INGRESS_PARENT TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS)
>  
> +#define TC_POLICY_DEFAULT "none"
> +
>  /* Returns tc handle 'major':'minor'. */
>  static inline unsigned int
>  tc_make_handle(unsigned int major, unsigned int minor)
> @@ -152,5 +154,6 @@ int tc_flush(int ifindex);
>  int tc_dump_flower_start(int ifindex, struct nl_dump *dump);
>  int parse_netlink_to_tc_flower(struct ofpbuf *reply,
>                                 struct tc_flower *flower);
> +void tc_set_policy(const char *policy);
>  
>  #endif /* tc.h */
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 7e5062f..96bf84c 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -195,6 +195,23 @@
>          </p>
>        </column>
>  
> +      <column name="other_config" key="tc-policy"
> +              type='{"type": "string"}'>
> +        <p>
> +            Specified the policy used with HW offloading.
> +            Options:
> +                <code>none</code>    - Add software rule and offload rule to HW.
> +                <code>skip_sw</code> - Offload rule to HW only.
> +                <code>skip_hw</code> - Add software rule without offloading rule to HW.
> +        </p>
> +        <p>
> +            This is only relevant if HW offloading is enabled (hw-offload).
> +        </p>
> +        <p>
> +          The default value is <code>none</code>.
> +        </p>
> +      </column>
> +
>        <column name="other_config" key="dpdk-init"
>                type='{"type": "boolean"}'>
>          <p>
> -- 
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio