diff mbox

[ovs-dev] ovn-northd: Fix ping failure of vlan networks.

Message ID OF2DC22D98.DB2EFFFA-ON48258128.003FA560-48258128.0040036E@zte.com.cn
State Superseded
Headers show

Commit Message

wang qianyu May 22, 2017, 11:39 a.m. UTC
There are two computer node, each have one vm. And the two vms in 
indifferent vlan networks. The ping between the vms is not success.

The reason is that, acl of to-localnet port or from-localnet port is
signed to contrack. So the pair of icmp request and reply have different
zone id in one ovs node. This makes the ct state not correct.

This patch do the modification that localnet port do not use ct.

Signed-off-by: wangqianyu <wang.qianyu@zte.com.cn>
---
 ovn/northd/ovn-northd.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

         /* Ingress and Egress Pre-ACL Table (Priority 110).
          *
          * Not to do conntrack on ND packets. */

Comments

Ben Pfaff May 31, 2017, 11:42 p.m. UTC | #1
On Mon, May 22, 2017 at 07:39:22PM +0800, wang.qianyu@zte.com.cn wrote:
> There are two computer node, each have one vm. And the two vms in 
> indifferent vlan networks. The ping between the vms is not success.
> 
> The reason is that, acl of to-localnet port or from-localnet port is
> signed to contrack. So the pair of icmp request and reply have different
> zone id in one ovs node. This makes the ct state not correct.
> 
> This patch do the modification that localnet port do not use ct.
> 
> Signed-off-by: wangqianyu <wang.qianyu@zte.com.cn>

This patch was word-wrapped, but I was able to deal with that.

I don't exactly understand the problem.  Does conntrack not work at all
with packets that go to/from localnet ports?  Or does it have something
to do with VLAN tags?

Please document the new flows in ovn-northd.8.xml.

Also, checkpatch reported the following:

ERROR: Improper whitespace around control block
#17 FILE: b/ovn/northd/ovn-northd.c:1355:
                if(!strcmp(nbsp->type, "localnet")) {

ERROR: Improper whitespace around control block
#28 FILE: b/ovn/northd/ovn-northd.c:2637:
        if(od->localnet_port) {

WARNING: Line length is >79-characters long
#32 FILE: b/ovn/northd/ovn-northd.c:2641:
            ds_put_format(&match_in, "ip && inport == %s", od->localnet_port->json_key);

WARNING: Line length is >79-characters long
#33 FILE: b/ovn/northd/ovn-northd.c:2642:
            ds_put_format(&match_out, "ip && outport == %s", od->localnet_port->json_key);

Thanks a lot for working on OVN!
wang qianyu June 1, 2017, 1:22 a.m. UTC | #2
Hi Ben, thanks for your review.

Conntrack have no problem with localnet port, but the pipline hase problem 
in the follow circumstance

------   vlan      ----
|ovs1|----------  |ovs2| 
------            -----
  |                 |
 vm1               vm2

net1 10.0.0.0/24 has vm1 with ip 10.0.0.10, net2 10.0.0.0/24 has vm2 with 
ip 20.0.0.10. net1 and net2 link to same route. net1 and net2 have 
localnet ports as inport/outport when packet forwarded between ovs1 and 
ovs2. 

when vm1 ping vm2, by the route forward, the out port of icmp request is 
localnet port of net2 in ovs1. And in reverse, ovs1 will use localnet port 
of net1 as inport of icmp reply from vm2.

The request and reply is not the same localnet port in ovs. Because of 
different localnet port with different zone id, when localnet port use ct, 
the ct state can not change to established.

So the icmp relpy will be dropped because of the error ct state.





Ben Pfaff <blp@ovn.org>
2017/06/01 07:42
 
        收件人:        wang.qianyu@zte.com.cn, 
        抄送:  dev@openvswitch.org, zhou.huijing@zte.com.cn, 
xu.rong@zte.com.cn
        主题:  [spam可疑邮件]Re: [ovs-dev]  [PATCH] ovn-northd: Fix ping 
failure of vlan networks.


On Mon, May 22, 2017 at 07:39:22PM +0800, wang.qianyu@zte.com.cn wrote:
> There are two computer node, each have one vm. And the two vms in 

> indifferent vlan networks. The ping between the vms is not success.

> 

> The reason is that, acl of to-localnet port or from-localnet port is

> signed to contrack. So the pair of icmp request and reply have different

> zone id in one ovs node. This makes the ct state not correct.

> 

> This patch do the modification that localnet port do not use ct.

> 

> Signed-off-by: wangqianyu <wang.qianyu@zte.com.cn>


This patch was word-wrapped, but I was able to deal with that.

I don't exactly understand the problem.  Does conntrack not work at all
with packets that go to/from localnet ports?  Or does it have something
to do with VLAN tags?

Please document the new flows in ovn-northd.8.xml.

Also, checkpatch reported the following:

ERROR: Improper whitespace around control block
#17 FILE: b/ovn/northd/ovn-northd.c:1355:
                if(!strcmp(nbsp->type, "localnet")) {

ERROR: Improper whitespace around control block
#28 FILE: b/ovn/northd/ovn-northd.c:2637:
        if(od->localnet_port) {

WARNING: Line length is >79-characters long
#32 FILE: b/ovn/northd/ovn-northd.c:2641:
            ds_put_format(&match_in, "ip && inport == %s", 
od->localnet_port->json_key);

WARNING: Line length is >79-characters long
#33 FILE: b/ovn/northd/ovn-northd.c:2642:
            ds_put_format(&match_out, "ip && outport == %s", 
od->localnet_port->json_key);

Thanks a lot for working on OVN!
Ben Pfaff June 1, 2017, 2:19 a.m. UTC | #3
Should we fix the problem another way, by introducing a way to make sure
that the same port is used, or at least that the same zone is used?  It
does not sound like a good idea to simply bypass the firewall.

On Thu, Jun 01, 2017 at 09:22:24AM +0800, wang.qianyu@zte.com.cn wrote:
> Hi Ben, thanks for your review.
> 
> Conntrack have no problem with localnet port, but the pipline hase problem 
> in the follow circumstance
> 
> ------   vlan      ----
> |ovs1|----------  |ovs2| 
> ------            -----
>   |                 |
>  vm1               vm2
> 
> net1 10.0.0.0/24 has vm1 with ip 10.0.0.10, net2 10.0.0.0/24 has vm2 with 
> ip 20.0.0.10. net1 and net2 link to same route. net1 and net2 have 
> localnet ports as inport/outport when packet forwarded between ovs1 and 
> ovs2. 
> 
> when vm1 ping vm2, by the route forward, the out port of icmp request is 
> localnet port of net2 in ovs1. And in reverse, ovs1 will use localnet port 
> of net1 as inport of icmp reply from vm2.
> 
> The request and reply is not the same localnet port in ovs. Because of 
> different localnet port with different zone id, when localnet port use ct, 
> the ct state can not change to established.
> 
> So the icmp relpy will be dropped because of the error ct state.
> 
> 
> 
> 
> 
> Ben Pfaff <blp@ovn.org>
> 2017/06/01 07:42
>  
>         收件人:        wang.qianyu@zte.com.cn, 
>         抄送:  dev@openvswitch.org, zhou.huijing@zte.com.cn, 
> xu.rong@zte.com.cn
>         主题:  [spam可疑邮件]Re: [ovs-dev]  [PATCH] ovn-northd: Fix ping 
> failure of vlan networks.
> 
> 
> On Mon, May 22, 2017 at 07:39:22PM +0800, wang.qianyu@zte.com.cn wrote:
> > There are two computer node, each have one vm. And the two vms in 
> > indifferent vlan networks. The ping between the vms is not success.
> > 
> > The reason is that, acl of to-localnet port or from-localnet port is
> > signed to contrack. So the pair of icmp request and reply have different
> > zone id in one ovs node. This makes the ct state not correct.
> > 
> > This patch do the modification that localnet port do not use ct.
> > 
> > Signed-off-by: wangqianyu <wang.qianyu@zte.com.cn>
> 
> This patch was word-wrapped, but I was able to deal with that.
> 
> I don't exactly understand the problem.  Does conntrack not work at all
> with packets that go to/from localnet ports?  Or does it have something
> to do with VLAN tags?
> 
> Please document the new flows in ovn-northd.8.xml.
> 
> Also, checkpatch reported the following:
> 
> ERROR: Improper whitespace around control block
> #17 FILE: b/ovn/northd/ovn-northd.c:1355:
>                 if(!strcmp(nbsp->type, "localnet")) {
> 
> ERROR: Improper whitespace around control block
> #28 FILE: b/ovn/northd/ovn-northd.c:2637:
>         if(od->localnet_port) {
> 
> WARNING: Line length is >79-characters long
> #32 FILE: b/ovn/northd/ovn-northd.c:2641:
>             ds_put_format(&match_in, "ip && inport == %s", 
> od->localnet_port->json_key);
> 
> WARNING: Line length is >79-characters long
> #33 FILE: b/ovn/northd/ovn-northd.c:2642:
>             ds_put_format(&match_out, "ip && outport == %s", 
> od->localnet_port->json_key);
> 
> Thanks a lot for working on OVN!
> 
> 
>
wang qianyu June 1, 2017, 2:57 a.m. UTC | #4
Thanks for you rapidly reply.

We think localnet port never be the real destination port of vm instance. 
Like patch port of route, localnet port just used for interim.

And nouse of ct to localnet will not cause the bypass of firewall. Because 
of the real destination  port of vm1 or vm2 have their own ct.

The introducing of same port or same zone in different networks maybe not 
suitable, this is not consensus with the isolation of networks.
diff mbox

Patch

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 83db753..5d1587e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1,4 +1,4 @@ 
-/*
+/*
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at:
@@ -416,6 +416,7 @@  struct ovn_datapath {
     /* The "derived" OVN port representing the instance of l3dgw_port on
      * the "redirect-chassis". */
     struct ovn_port *l3redirect_port;
+    struct ovn_port *localnet_port;
 };
 
 struct macam_node {
@@ -1351,6 +1352,10 @@  join_logical_ports(struct northd_context *ctx,
                     ovs_list_push_back(nb_only, &op->list);
                 }
 
+                if(!strcmp(nbsp->type, "localnet")) {
+                   od->localnet_port = op;
+                }
+
                 op->lsp_addrs
                     = xmalloc(sizeof *op->lsp_addrs * nbsp->n_addresses);
                 for (size_t j = 0; j < nbsp->n_addresses; j++) {
@@ -2629,6 +2634,21 @@  build_pre_acls(struct ovn_datapath *od, struct hmap 
*lflows)
             ds_destroy(&match_in);
             ds_destroy(&match_out);
         }
+        if(od->localnet_port) {
+            struct ds match_in = DS_EMPTY_INITIALIZER;
+            struct ds match_out = DS_EMPTY_INITIALIZER;
+
+            ds_put_format(&match_in, "ip && inport == %s", 
od->localnet_port->json_key);
+            ds_put_format(&match_out, "ip && outport == %s", 
od->localnet_port->json_key);
+            ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+                          ds_cstr(&match_in), "next;");
+            ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
+                          ds_cstr(&match_out), "next;");
+
+            ds_destroy(&match_in);
+            ds_destroy(&match_out);
+        }
+