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 <g...@ovn.org>
Signed-off-by: Greg Rose <gvrose8...@gmail.com>
---
  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 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.

- Greg


              err = EINVAL;
          }
      }
@@ -328,7 +328,7 @@ dpif_netlink_rtnl_create(const struct netdev_tunnel_config 
*tnl_cfg,
      case OVS_VPORT_TYPE_ERSPAN:
      case OVS_VPORT_TYPE_IP6ERSPAN:
      case OVS_VPORT_TYPE_IP6GRE:
-        nl_msg_put_flag(&request, IFLA_GRE_COLLECT_METADATA);
I think we still need to use COLLECT_METADATA. Don't we depend on using
lwt?

+        nl_msg_put_u16(&request, IFLA_GRE_ERSPAN_HWID, 0xdead);
          break;
      case OVS_VPORT_TYPE_GENEVE:
          nl_msg_put_flag(&request, IFLA_GENEVE_COLLECT_METADATA);

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to