[ovs-dev] [PATCH ovs V3 05/25] other-config: Add tc-policy switch to control tc flower flag
Chandran, Sugesh
sugesh.chandran at intel.com
Wed Feb 15 11:16:22 UTC 2017
Regards
_Sugesh
> -----Original Message-----
> From: Roi Dayan [mailto:roid at mellanox.com]
> Sent: Wednesday, February 15, 2017 7:42 AM
> To: Chandran, Sugesh <sugesh.chandran at intel.com>; dev at openvswitch.org
> Cc: roid at mellanox.com; Paul Blakey <paulb at mellanox.com>; Or Gerlitz
> <ogerlitz at mellanox.com>; Hadar Hen Zion <hadarh at mellanox.com>; Shahar
> Klein <shahark at mellanox.com>; Mark Bloch <markb at mellanox.com>; Rony
> Efraim <ronye at mellanox.com>; Fastabend, John R
> <john.r.fastabend at intel.com>; Joe Stringer <joe at ovn.org>; Andy
> Gospodarek <andy at greyhouse.net>; Lance Richardson
> <lrichard at redhat.com>; Marcelo Ricardo Leitner <mleitner at redhat.com>;
> Simon Horman <simon.horman at netronome.com>; Jiri Pirko
> <jiri at mellanox.com>
> Subject: Re: [PATCH ovs V3 05/25] other-config: Add tc-policy switch to
> control tc flower flag
>
>
>
> On 14/02/2017 01:53, Chandran, Sugesh wrote:
> >
> >
> > Regards
> > _Sugesh
> >
> >
> >> -----Original Message-----
> >> From: Roi Dayan [mailto:roid at mellanox.com]
> >> Sent: Wednesday, February 8, 2017 3:29 PM
> >> To: dev at openvswitch.org
> >> Cc: Paul Blakey <paulb at mellanox.com>; Or Gerlitz
> >> <ogerlitz at mellanox.com>; Hadar Hen Zion <hadarh at mellanox.com>;
> Shahar
> >> Klein <shahark at mellanox.com>; Mark Bloch <markb at mellanox.com>;
> Rony
> >> Efraim <ronye at mellanox.com>; Fastabend, John R
> >> <john.r.fastabend at intel.com>; Joe Stringer <joe at ovn.org>; Andy
> >> Gospodarek <andy at greyhouse.net>; Lance Richardson
> >> <lrichard at redhat.com>; Marcelo Ricardo Leitner
> <mleitner at redhat.com>;
> >> Simon Horman <simon.horman at netronome.com>; Jiri Pirko
> >> <jiri at mellanox.com>; Chandran, Sugesh <sugesh.chandran at intel.com>
> >> Subject: [PATCH ovs V3 05/25] other-config: Add tc-policy switch to
> >> control tc flower flag
> >>
> >
> >> *ovs_other_config)
> >>
> >> VLOG_INFO("netdev: Flow API Enabled");
> >>
> >> + tc_set_policy(smap_get_def(ovs_other_config, "tc-policy",
> >> + TC_POLICY_DEFAULT));
> >> +
> > [Sugesh] I feel we better call this policy as hw-ofld-policy than tc-policy. As
> far as I know tc is one of the way to program the hardware. In the dpdk dpif
> there must be be another flow programming interface that can be used to
> program the offload. It would be nice if we can use the same configuration
> option for all the deployments.
>
> currently the options used in tc-policy are somewhat tc specific.
> the options could be a lot different for other interfaces.
> maybe leave this like this for now?
> if another interface will have a policy then we can revisit this if the same
> options apply or if we can merge those options or maybe we'll see we'll need
> an entirely different [interface]-policy option?
[Sugesh] Ok, I am fine with leaving it . But out of curiosity, just wondering what would be other possible policy options you are expecting for other type of interfaces?? I assume the policy is more or less the way hardware get programmed. It has nothing to do with what approach used to program it, which can be tc or any other APIs.
>
> >> ovsthread_once_done(&once);
> >> }
> >> }
> >> diff --git a/lib/tc.c b/lib/tc.c
> >> index 431242b..b2e3d21 100644
> >> --- a/lib/tc.c
> >> +++ b/lib/tc.c
> >> @@ -38,6 +38,14 @@ VLOG_DEFINE_THIS_MODULE(tc);
> >>
> >> static struct vlog_rate_limit parse_err = VLOG_RATE_LIMIT_INIT(5,
> >> 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;
> >> +
> >> /* Returns tc handle 'major':'minor'. */ unsigned int
> >> tc_make_handle(unsigned int major, unsigned int minor) @@ -719,6
> >> +727,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) {
> > [Sugesh] tc_get_hw_ofld_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) { @@ -989,7 +1009,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); @@ -1033,3
> >> +1053,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 5ca6c55..6f6cc09 100644
> >> --- a/lib/tc.h
> >> +++ b/lib/tc.h
> >> @@ -115,5 +115,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
> >> 942e68f..8b9bdcb 100644
> >> --- a/vswitchd/vswitch.xml
> >> +++ b/vswitchd/vswitch.xml
> >> @@ -183,6 +183,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>false</code>.
> >> + </p>
> >> + </column>
> >> +
> >> <column name="other_config" key="dpdk-init"
> >> type='{"type": "boolean"}'>
> >> <p>
> >> --
> >> 2.7.4
> >
More information about the dev
mailing list