On 6/18/2019 10:11 AM, William Tu wrote:
On Tue, Jun 18, 2019 at 9:15 AM William Tu <u9012...@gmail.com> wrote:
On Mon, Jun 17, 2019 at 10:46 PM Eli Britstein <el...@mellanox.com> wrote:

On 6/18/2019 1:22 AM, Gregory Rose wrote:
On 6/12/2019 2:20 AM, Eli Britstein wrote:
Could you please have a look (and even better try?) still need to
tidy up

https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Felibr-mellanox%2Flinux%2Ftree%2Fovs-ipv6-gre&amp;data=02%7C01%7Celibr%40mellanox.com%7C25425e700b58418b19fa08d6f37253ae%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636964069686971905&amp;sdata=Ki55ZQGwQKNNDkB4PZ1zpk9aqC42TXbt5WT%2F86SDauo%3D&amp;reserved=0



Hi Eli,

I finally managed to getting around and building your kernel.  At
least it does do away with the protocol error message but
I haven't gotten it working yet.  Stay tuned and I'll provide feedback.
Hi Greg,

I think William was right pointing out the comment from Pravin. I agree
with them that if we can avoid changing the kernel, and support it via
lib/dpif-netlink-rtnl.c, it is the way to go, and we should abandon the
kernel changes.

I haven't deep dived into it yet to see how ip6erspan works (at least no
creation error, didn't run traffic though) and ip6gre doesn't.

Hi Eli and Greg,

I've found the root cause using upstream kernel 5.2.
When adding an ip6gre device type, the
ovs_vport_cmd_new
   new_vport
     netdev_create
       ovs_netdev_link

and fails due to
     if (vport->dev->flags & IFF_LOOPBACK ||
         (vport->dev->type != ARPHRD_ETHER &&
          vport->dev->type != ARPHRD_NONE) ||
         ovs_is_internal_dev(vport->dev)) {
         err = -EINVAL;

Because ip6gre is ARPHRD_IP6GRE
Which means ip6gre never works using upstream kernel :(

If we use ip6gretap, then its type is ARPHRD_ETHER,
and the ip6gretap device can be created successfully.

And for ip6erspan, it's never been a issue because it is ARPHRD_ETHER.

And when using compat module, ip6gre works due to a coincident here:
at datapath/linux/compat/ip6_gre.c
static struct rtnl_link_ops ip6gre_link_ops __read_mostly = {
     .kind       = "ip6gre",
     .maxtype    = RPL_IFLA_GRE_MAX,
   <skip>

static struct rtnl_link_ops ip6gre_tap_ops __read_mostly = {
     .kind       = "ip6gre",
     .maxtype    = RPL_IFLA_GRE_MAX,
     .policy     = ip6gre_policy,
     .priv_size  = sizeof(struct ip6_tnl),

We happen to use ip6gre_tap link ops in type "ip6gre"

Regards,
William

Furthermore, I think the following is missing, as there is no ip6gretap,
only ip6gre:

Hi Greg and Eli,

Should we add both ip6gre (L3) and ip6gretap (L2) support?
@Eli, which mode do you want to use?

I apply the patch below, and ip6gretap port can be created successfully
by doing using kernel 5.2 upstream ovs module:
# ovs-vsctl add-port br0 at_gre1 -- \
      set int at_gre1 type=ip6gretap options:remote_ip=fc00:100::1

but not the type=ip6gre

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 2e23a8c14fcf..d666f239aeed 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -104,7 +104,7 @@ vport_type_to_kind(enum ovs_vport_type type,
      case OVS_VPORT_TYPE_IP6ERSPAN:
          return "ip6erspan";
      case OVS_VPORT_TYPE_IP6GRE:
-        return "ip6gre";
+        return "ip6gretap";
      case OVS_VPORT_TYPE_NETDEV:
      case OVS_VPORT_TYPE_INTERNAL:
      case OVS_VPORT_TYPE_LISP:
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index c554666acce0..28897153744b 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -698,7 +698,7 @@ get_vport_type(const struct dpif_netlink_vport *vport)
          return "ip6erspan";

      case OVS_VPORT_TYPE_IP6GRE:
-        return "ip6gre";
+        return "ip6gretap";

      case OVS_VPORT_TYPE_UNSPEC:
      case __OVS_VPORT_TYPE_MAX:
@@ -729,7 +729,7 @@ netdev_to_ovs_vport_type(const char *type)
          return OVS_VPORT_TYPE_ERSPAN;
      } else if (!strcmp(type, "ip6erspan")) {
          return OVS_VPORT_TYPE_IP6ERSPAN;
-    } else if (!strcmp(type, "ip6gre")) {
+    } else if (!strcmp(type, "ip6gretap")) {
          return OVS_VPORT_TYPE_IP6GRE;
      } else if (!strcmp(type, "gre")) {
          return OVS_VPORT_TYPE_GRE;
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index ab591667f447..da95f680d454 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -1216,10 +1216,10 @@ netdev_vport_tunnel_register(void)
            },
            {{NULL, NULL, 0, 0}}
          },
-        { "ip6gre_sys",
+        { "ip6gretap_sys",
            {
                TUNNEL_FUNCTIONS_COMMON,
-              .type = "ip6gre",
+              .type = "ip6gretap",
                .build_header = netdev_gre_build_header,
                .push_header = netdev_gre_push_header,
                .pop_header = netdev_gre_pop_header
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 17353046cc6e..2157d7de11ae 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -172,7 +172,7 @@ tnl_type_to_nw_proto(const char type[])
          return IPPROTO_TCP;
      }
      if (!strcmp(type, "gre") || !strcmp(type, "erspan") ||
-        !strcmp(type, "ip6erspan") || !strcmp(type, "ip6gre")) {
+        !strcmp(type, "ip6erspan") || !strcmp(type, "ip6gretap")) {
          return IPPROTO_GRE;
      }
      if (!strcmp(type, "vxlan")) {

That's pretty much the same patch I've got and am testing right now.  I think it's the right way to go.  Do you want to post
this to netdev or would you prefer me to do it?

Thanks,

- Greg
_______________________________________________
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss

Reply via email to