On 15/02/2017 13:16, Chandran, Sugesh wrote:
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.
I'm not sure. for example tc use negative approach (skip_sw/skip_hw) and
if you leave it empty it's "both".
Other interface could use a positive approach like offload_hw,
offload_sw, offload_both..
Though it very well could be it would just be hw-offload-policy later..
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
