Regards _Sugesh
> -----Original Message----- > From: Roi Dayan [mailto:r...@mellanox.com] > Sent: Wednesday, February 15, 2017 7:42 AM > To: Chandran, Sugesh <sugesh.chand...@intel.com>; d...@openvswitch.org > Cc: r...@mellanox.com; Paul Blakey <pa...@mellanox.com>; Or Gerlitz > <ogerl...@mellanox.com>; Hadar Hen Zion <had...@mellanox.com>; Shahar > Klein <shah...@mellanox.com>; Mark Bloch <ma...@mellanox.com>; Rony > Efraim <ro...@mellanox.com>; Fastabend, John R > <john.r.fastab...@intel.com>; Joe Stringer <j...@ovn.org>; Andy > Gospodarek <a...@greyhouse.net>; Lance Richardson > <lrich...@redhat.com>; Marcelo Ricardo Leitner <mleit...@redhat.com>; > Simon Horman <simon.hor...@netronome.com>; Jiri Pirko > <j...@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:r...@mellanox.com] > >> Sent: Wednesday, February 8, 2017 3:29 PM > >> To: d...@openvswitch.org > >> Cc: Paul Blakey <pa...@mellanox.com>; Or Gerlitz > >> <ogerl...@mellanox.com>; Hadar Hen Zion <had...@mellanox.com>; > Shahar > >> Klein <shah...@mellanox.com>; Mark Bloch <ma...@mellanox.com>; > Rony > >> Efraim <ro...@mellanox.com>; Fastabend, John R > >> <john.r.fastab...@intel.com>; Joe Stringer <j...@ovn.org>; Andy > >> Gospodarek <a...@greyhouse.net>; Lance Richardson > >> <lrich...@redhat.com>; Marcelo Ricardo Leitner > <mleit...@redhat.com>; > >> Simon Horman <simon.hor...@netronome.com>; Jiri Pirko > >> <j...@mellanox.com>; Chandran, Sugesh <sugesh.chand...@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 > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
