On Fri, Apr 07, 2017 at 04:12:50PM +0300, Roi Dayan wrote:
> From: Paul Blakey <pa...@mellanox.com>
>
> Add a new configuration option - hw-offload that enables netdev
> flow api. Enabling this option will allow offloading flows
> using netdev implementation instead of the kernel datapath.
> This configuration option defaults to false - disabled.
>
> Signed-off-by: Paul Blakey <pa...@mellanox.com>
> Reviewed-by: Roi Dayan <r...@mellanox.com>
> Reviewed-by: Simon Horman <simon.hor...@netronome.com>
> ---
> lib/netdev.c | 30 ++++++++++++++++++++++++++++++
> lib/netdev.h | 2 ++
> vswitchd/bridge.c | 1 +
> vswitchd/vswitch.xml | 11 +++++++++++
> 4 files changed, 44 insertions(+)
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index f7b80b2..977844a 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -2092,7 +2092,37 @@ netdev_init_flow_api(struct netdev *netdev)
> {
> const struct netdev_class *class = netdev->netdev_class;
>
> + if (!netdev_flow_api_enabled) {
> + return EOPNOTSUPP;
> + }
> +
> return (class->init_flow_api
> ? class->init_flow_api(netdev)
> : EOPNOTSUPP);
> }
> +
> +bool netdev_flow_api_enabled = false;
> +
> +#ifdef __linux__
> +void
> +netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
> +{
> + if (smap_get_bool(ovs_other_config, "hw-offload", false)) {
> + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> +
> + if (ovsthread_once_start(&once)) {
> + netdev_flow_api_enabled = true;
> +
> + VLOG_INFO("netdev: Flow API Enabled");
> +
> + ovsthread_once_done(&once);
> + }
> + }
> +}
> +#else
> +void
> +netdev_set_flow_api_enabled(const struct smap *ovs_other_config OVS_UNUSED)
> +{
> + netdev_flow_api_enabled = false;
> +}
> +#endif
> diff --git a/lib/netdev.h b/lib/netdev.h
> index 6d2db7d..e2d1799 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -178,6 +178,8 @@ int netdev_flow_get(struct netdev *, struct match *,
> struct nlattr **actions,
> int netdev_flow_del(struct netdev *, const ovs_u128 *,
> struct dpif_flow_stats *);
> int netdev_init_flow_api(struct netdev *);
> +extern bool netdev_flow_api_enabled;
> +void netdev_set_flow_api_enabled(const struct smap *ovs_other_config);
>
> /* native tunnel APIs */
> /* Structure to pass parameters required to build a tunnel header. */
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 867a26d..0f65858 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -2953,6 +2953,7 @@ bridge_run(void)
> cfg = ovsrec_open_vswitch_first(idl);
>
> if (cfg) {
> + netdev_set_flow_api_enabled(&cfg->other_config);
> dpdk_init(&cfg->other_config);
> }
>
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 14297bf..de86049 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -170,6 +170,17 @@
> <p>
> The default is 10000.
> </p>
> + </column>
> +
> + <column name="other_config" key="hw-offload"
> + type='{"type": "boolean"}'>
> + <p>
> + Set this value to <code>true</code> to enable netdev flow offload.
> + </p>
> + <p>
> + The default value is <code>false</code>. Changing this value
> requires
> + restarting the daemon
> + </p>
Since this is a compile time feature, I think somehow we need to
document that it's only available on Linux. Something like:
Currently Open vSwitch supports hardware offloading on
Linux systems. On other systems, this value is ignored.
This looks better to me:
---8<---
diff --git a/lib/netdev.c b/lib/netdev.c
index 64f5e85..4b5f1990 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2330,13 +2330,13 @@ netdev_ports_flow_init(void)
}
void
-netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
+netdev_set_flow_api_enabled(bool enabled)
{
if (!netdev_flow_api_supported) {
return;
}
- if (smap_get_bool(ovs_other_config, "hw-offload", false)) {
+ if (enabled) {
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
if (ovsthread_once_start(&once)) {
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 0f65858..18bbeb6 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2953,7 +2953,8 @@ bridge_run(void)
cfg = ovsrec_open_vswitch_first(idl);
if (cfg) {
- netdev_set_flow_api_enabled(&cfg->other_config);
+ netdev_set_flow_api_enabled(smap_get_bool(&cfg->other_config,
+ "hw-offload", false));
dpdk_init(&cfg->other_config);
}
---8<---
Also that it's exporting a variable, so maybe this would be better:
(compile tested on Linux only)
---8<---
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index acce90e..0adc9c7 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1088,7 +1088,7 @@ dpif_netlink_flow_flush(struct dpif *dpif_)
flow.cmd = OVS_FLOW_CMD_DEL;
flow.dp_ifindex = dpif->dp_ifindex;
- if (netdev_flow_api_enabled) {
+ if (netdev_is_flow_api_enabled()) {
netdev_ports_flow_flush(DPIF_HMAP_KEY(dpif_));
}
@@ -1428,7 +1428,7 @@ dpif_netlink_get_dump_type(char *str) {
if (!str || !strcmp(str, "ovs") || !strcmp(str, "dpctl")) {
type |= DUMP_OVS_FLOWS;
}
- if ((netdev_flow_api_enabled && !str)
+ if ((netdev_is_flow_api_enabled() && !str)
|| (str && (!strcmp(str, "offloaded") || !strcmp(str, "dpctl")))) {
type |= DUMP_OFFLOADED_FLOWS;
}
@@ -2207,7 +2207,7 @@ dpif_netlink_operate(struct dpif *dpif_, struct dpif_op
**ops, size_t n_ops)
int i = 0;
int err = 0;
- if (netdev_flow_api_enabled) {
+ if (netdev_is_flow_api_enabled()) {
while (n_ops > 0) {
count = 0;
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index a48c6d9..41d959d 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2052,7 +2052,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
int error;
int ifindex;
- if (netdev_flow_api_enabled) {
+ if (netdev_is_flow_api_enabled()) {
if (kbits_rate) {
VLOG_WARN_RL(&rl, "%s: policing with offload isn't supported",
netdev_name);
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index d310771..5f5e571 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -796,7 +796,7 @@ netdev_vport_get_ifindex__(const struct netdev *netdev_)
static int
netdev_vport_get_ifindex(const struct netdev *netdev_)
{
- if (netdev_flow_api_enabled)
+ if (netdev_is_flow_api_enabled())
return netdev_vport_get_ifindex__(netdev_);
else
return -EOPNOTSUPP;
diff --git a/lib/netdev.c b/lib/netdev.c
index f020d46..64f5e85 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -95,6 +95,15 @@ struct netdev_registered_class {
struct ovs_refcount refcnt;
};
+
+#ifdef __linux__
+static bool netdev_flow_api_supported = true;
+#else
+static bool netdev_flow_api_supported = false;
+#endif
+
+static bool netdev_flow_api_enabled = false;
+
/* This is set pretty low because we probably won't learn anything from the
* additional log messages. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
@@ -2090,12 +2099,18 @@ netdev_flow_del(struct netdev *netdev, const ovs_u128
*ufid,
: EOPNOTSUPP);
}
+bool
+netdev_is_flow_api_enabled(void)
+{
+ return netdev_flow_api_enabled;
+}
+
int
netdev_init_flow_api(struct netdev *netdev)
{
const struct netdev_class *class = netdev->netdev_class;
- if (!netdev_flow_api_enabled) {
+ if (!netdev_is_flow_api_enabled()) {
return EOPNOTSUPP;
}
@@ -2302,9 +2317,6 @@ netdev_ports_flow_get(const void *obj, struct match
*match,
return ENOENT;
}
-bool netdev_flow_api_enabled = false;
-
-#ifdef __linux__
static void
netdev_ports_flow_init(void)
{
@@ -2320,6 +2332,10 @@ netdev_ports_flow_init(void)
void
netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
{
+ if (!netdev_flow_api_supported) {
+ return;
+ }
+
if (smap_get_bool(ovs_other_config, "hw-offload", false)) {
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
@@ -2337,10 +2353,3 @@ netdev_set_flow_api_enabled(const struct smap
*ovs_other_config)
}
}
}
-#else
-void
-netdev_set_flow_api_enabled(const struct smap *ovs_other_config OVS_UNUSED)
-{
- netdev_flow_api_enabled = false;
-}
-#endif
diff --git a/lib/netdev.h b/lib/netdev.h
index 00cbbdf..5b0a475 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -178,7 +178,7 @@ int netdev_flow_get(struct netdev *, struct match *, struct
nlattr **actions,
int netdev_flow_del(struct netdev *, const ovs_u128 *,
struct dpif_flow_stats *);
int netdev_init_flow_api(struct netdev *);
-extern bool netdev_flow_api_enabled;
+bool netdev_is_flow_api_enabled(void);
void netdev_set_flow_api_enabled(const struct smap *ovs_other_config);
struct dpif_port;
---8<---
Thanks,
--
Flavio
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
