On 6/1/2018 8:30 AM, Eric Garver wrote:
On Fri, Jun 01, 2018 at 07:40:49AM -0700, Gregory Rose wrote:
On 6/1/2018 6:15 AM, Eric Garver wrote:
I'm a bit late, but I have comments below.
I'm also a bit out of touch, so I may be missing some context - if so, I
apologize.
On Thu, May 31, 2018 at 03:50:31PM -0700, Greg Rose wrote:
When verifying the built-in gre kernel module check for ERSPAN support.
Reported-by: Guru Shetty <[email protected]>
Signed-off-by: Greg Rose <[email protected]>
---
lib/dpif-netlink-rtnl.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index bec3fce..197cfb6 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -45,8 +45,8 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
#ifndef IFLA_GRE_MAX
#define IFLA_GRE_MAX 0
#endif
-#if IFLA_GRE_MAX < 18
-#define IFLA_GRE_COLLECT_METADATA 18
+#if IFLA_GRE_MAX < 24
+#define IFLA_GRE_ERSPAN_HWID 24
#endif
#ifndef IFLA_GENEVE_MAX
@@ -74,7 +74,7 @@ static const struct nl_policy vxlan_policy[] = {
[IFLA_VXLAN_GPE] = { .type = NL_A_FLAG, .optional = true },
};
static const struct nl_policy gre_policy[] = {
- [IFLA_GRE_COLLECT_METADATA] = { .type = NL_A_FLAG },
I think we still need to verify _COLLECT_METADATA was passed back.
+ [IFLA_GRE_ERSPAN_HWID] = { .type = NL_A_U16 },
};
static const struct nl_policy geneve_policy[] = {
[IFLA_GENEVE_COLLECT_METADATA] = { .type = NL_A_FLAG },
@@ -207,7 +207,7 @@ dpif_netlink_rtnl_gre_verify(const struct
netdev_tunnel_config OVS_UNUSED *tnl,
err = rtnl_policy_parse(kind, reply, gre_policy, gre,
ARRAY_SIZE(gre_policy));
if (!err) {
- if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
+ if (!nl_attr_get_u16(gre[IFLA_GRE_ERSPAN_HWID])) {
We need to verify COLLECT_METADATA is actually in use here. We should
only be checking for ERSPAN if ERSPAN was requested by the user.
I'm not super familiar with the code but the intent is to prevent the
built-in ip_gre kernel module
from being used if it doesn't support ERSPAN. If the built-in gre and
dpif_netlink_rtnl_probe_oot_tunnels() is responsible for deciding if
in-tree (linux) or out-of-tree (ovs) modules should be used.
There are two cases:
1) If the out-of-tree tunnel modules are loaded, the out-of-tree
compat interface will be used
2) otherwise in-tree tunnel modules are used (rtnetlink, i.e. code
in this file).
- if device create fails, then it falls back to the compat
interface
Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the
compat interface is needed
for kernels up to 4.15 so that we can support ERSPAN. If the built-in
gre/ip_gre kernel modules
don't have the ERSPAN support in them then we have to use the compat
interface.
ip_gre kernel modules
are loaded and the kernel doesn't have ERSPAN (4.16 and above) then OVS
users won't
be able to use ERSPAN features.
IIRC, the out-of-tree version of ip_gre will not be used on newer
kernels anyway. See references to USE_UPSTREAM_TUNNEL.
The target for USE_UPSTREAM_TUNNEL is moved to 4.16 now. That's when
ERSPAN becomes
fully supported. Going forward the ERSPAN feature is the determinant
for whether gr/ip_gre
compat mode is used or not.
Thanks,
- Greg
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev