diff mbox series

[ovs-dev,v2,3/3] OVN: add protocol unreachable support to OVN router ports

Message ID eba82449c0e074c90f95ecf676696b49dbed11ee.1529316047.git.lorenzo.bianconi@redhat.com
State Accepted
Headers show
Series add TCP/UDP port unreachable support to OVN logical router | expand

Commit Message

Lorenzo Bianconi June 18, 2018, 11:56 a.m. UTC 
Add priority-70 flows to generate ICMP protocol unreachable messages
in reply to packets directed to the router's IP address on IP protocols
other than UDP, TCP, and ICMP

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 ovn/northd/ovn-northd.8.xml |  4 ----
 ovn/northd/ovn-northd.c     | 14 ++++++++++++++
 tests/ovn.at                |  1 +
 3 files changed, 15 insertions(+), 4 deletions(-)

Comments

Daniel Alvarez Sanchez June 29, 2018, 10:15 a.m. UTC | #1 
Hi all,

We are hitting issues with this patch on OpenStack CI in this particular
test [0].
The scenario is one VM trying to ping the router interface; replies are
sent by
the router interface but never reached the instance back as the last NAT
action
doesn't happen. Stopping northd and deleting manually the Logical Flows
inserted
by this patch, fixed the issue.

VM with IP 10.0.0.9 trying to ping 172.24.4.1. Router interface for this
network is
172.24.4.12 and router interface for the LS of the VM is 10.0.0.1.
Deleting the following lflows solved the issue:

_uuid               : 82dd40a7-1ff9-4676-b450-7b5dfdb2fb3a
actions             : "icmp4 {eth.dst <-> eth.src; ip4.dst <-> ip4.src;
ip.ttl = 255; icmp4.type = 3; icmp4.code = 2; next; };"
external_ids        : {source="ovn-northd.c:5185",
stage-name=lr_in_ip_input}
logical_datapath    : 3b6be958-290c-4479-9864-007dd89bb056
match               : "ip4 && ip4.dst == 10.0.0.1 && !ip.later_frag"
pipeline            : ingress
priority            : 70
table_id            : 1
hash                : 0

_uuid               : c276a38d-f49c-4e8b-a005-25e92a17ba03
actions             : "icmp4 {eth.dst <-> eth.src; ip4.dst <-> ip4.src;
ip.ttl = 255; icmp4.type = 3; icmp4.code = 2; next; };"
external_ids        : {source="ovn-northd.c:5185",
stage-name=lr_in_ip_input}
logical_datapath    : 4908c283-fffd-42bc-80ef-86ed63e50b53
match               : "ip4 && ip4.dst == 10.0.0.1 && !ip.later_frag"
pipeline            : ingress
priority            : 70
table_id            : 1
hash                : 0

_uuid               : 07df1365-c4fd-4301-8e0f-96bf07a39f21
actions             : "icmp4 {eth.dst <-> eth.src; ip4.dst <-> ip4.src;
ip.ttl = 255; icmp4.type = 3; icmp4.code = 2; next; };"
external_ids        : {source="ovn-northd.c:5185",
stage-name=lr_in_ip_input}
logical_datapath    : 3b6be958-290c-4479-9864-007dd89bb056
match               : "ip4 && ip4.dst == 172.24.4.12 && !ip.later_frag"
pipeline            : ingress
priority            : 70
table_id            : 1
hash                : 0


[0]
https://github.com/openstack/neutron-tempest-plugin/blob/8bc66e3205b834e17e9a9e6b72b6203a7a02cada/neutron_tempest_plugin/scenario/test_floatingip.py#L180


On Mon, Jun 18, 2018 at 1:58 PM Lorenzo Bianconi <
lorenzo.bianconi@redhat.com> wrote:

> Add priority-70 flows to generate ICMP protocol unreachable messages
> in reply to packets directed to the router's IP address on IP protocols
> other than UDP, TCP, and ICMP
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
>  ovn/northd/ovn-northd.8.xml |  4 ----
>  ovn/northd/ovn-northd.c     | 14 ++++++++++++++
>  tests/ovn.at                |  1 +
>  3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 18a481b3d..cfd35115e 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -1342,10 +1342,6 @@ nd_na {
>          <p>
>            These flows should not match IP fragments with nonzero offset.
>          </p>
> -
> -        <p>
> -          Details TBD.  Not yet implemented.
> -        </p>
>        </li>
>
>        <li>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 27d7aab06..7777b83f5 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -5175,6 +5175,20 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>                          "next; };";
>              ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 80,
>                            ds_cstr(&match), action);
> +
> +            ds_clear(&match);
> +            ds_put_format(&match,
> +                          "ip4 && ip4.dst == %s && !ip.later_frag",
> +                          op->lrp_networks.ipv4_addrs[i].addr_s);
> +            action = "icmp4 {"
> +                        "eth.dst <-> eth.src; "
> +                        "ip4.dst <-> ip4.src; "
> +                        "ip.ttl = 255; "
> +                        "icmp4.type = 3; "
> +                        "icmp4.code = 2; "
> +                        "next; };";
> +            ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 70,
> +                          ds_cstr(&match), action);
>          }
>
>          ds_clear(&match);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4648a303c..6553d17c6 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -10444,6 +10444,7 @@ OVN_POPULATE_ARP
>  ovn-nbctl --wait=hv sync
>
>  test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1)
> $(ip_to_hex 192 168 1 254) 11 0000 7dae fcfc 0303
> +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1)
> $(ip_to_hex 192 168 1 254) 84 0000 7dae fcfd 0302
>  OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
>
>  test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2
> 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 7bae 4486
> --
> 2.17.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Lorenzo Bianconi June 29, 2018, 12:12 p.m. UTC | #2 
>
> Hi all,

Hi Daniel,

>
> We are hitting issues with this patch on OpenStack CI in this particular test [0].
> The scenario is one VM trying to ping the router interface; replies are sent by
> the router interface but never reached the instance back as the last NAT action
> doesn't happen. Stopping northd and deleting manually the Logical Flows inserted
> by this patch, fixed the issue.
>

this should be the same issue reported by Darrell ('ovn: Fix gateway
load balancing')
Regards,

Lorenzo

> VM with IP 10.0.0.9 trying to ping 172.24.4.1. Router interface for this network is
> 172.24.4.12 and router interface for the LS of the VM is 10.0.0.1.
> Deleting the following lflows solved the issue:
>
> _uuid               : 82dd40a7-1ff9-4676-b450-7b5dfdb2fb3a
> actions             : "icmp4 {eth.dst <-> eth.src; ip4.dst <-> ip4.src; ip.ttl = 255; icmp4.type = 3; icmp4.code = 2; next; };"
> external_ids        : {source="ovn-northd.c:5185", stage-name=lr_in_ip_input}
> logical_datapath    : 3b6be958-290c-4479-9864-007dd89bb056
> match               : "ip4 && ip4.dst == 10.0.0.1 && !ip.later_frag"
> pipeline            : ingress
> priority            : 70
> table_id            : 1
> hash                : 0
>
> _uuid               : c276a38d-f49c-4e8b-a005-25e92a17ba03
> actions             : "icmp4 {eth.dst <-> eth.src; ip4.dst <-> ip4.src; ip.ttl = 255; icmp4.type = 3; icmp4.code = 2; next; };"
> external_ids        : {source="ovn-northd.c:5185", stage-name=lr_in_ip_input}
> logical_datapath    : 4908c283-fffd-42bc-80ef-86ed63e50b53
> match               : "ip4 && ip4.dst == 10.0.0.1 && !ip.later_frag"
> pipeline            : ingress
> priority            : 70
> table_id            : 1
> hash                : 0
>
> _uuid               : 07df1365-c4fd-4301-8e0f-96bf07a39f21
> actions             : "icmp4 {eth.dst <-> eth.src; ip4.dst <-> ip4.src; ip.ttl = 255; icmp4.type = 3; icmp4.code = 2; next; };"
> external_ids        : {source="ovn-northd.c:5185", stage-name=lr_in_ip_input}
> logical_datapath    : 3b6be958-290c-4479-9864-007dd89bb056
> match               : "ip4 && ip4.dst == 172.24.4.12 && !ip.later_frag"
> pipeline            : ingress
> priority            : 70
> table_id            : 1
> hash                : 0
>
>
> [0] https://github.com/openstack/neutron-tempest-plugin/blob/8bc66e3205b834e17e9a9e6b72b6203a7a02cada/neutron_tempest_plugin/scenario/test_floatingip.py#L180
>
>
> On Mon, Jun 18, 2018 at 1:58 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
>>
>> Add priority-70 flows to generate ICMP protocol unreachable messages
>> in reply to packets directed to the router's IP address on IP protocols
>> other than UDP, TCP, and ICMP
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>> ---
>>  ovn/northd/ovn-northd.8.xml |  4 ----
>>  ovn/northd/ovn-northd.c     | 14 ++++++++++++++
>>  tests/ovn.at                |  1 +
>>  3 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
>> index 18a481b3d..cfd35115e 100644
>> --- a/ovn/northd/ovn-northd.8.xml
>> +++ b/ovn/northd/ovn-northd.8.xml
>> @@ -1342,10 +1342,6 @@ nd_na {
>>          <p>
>>            These flows should not match IP fragments with nonzero offset.
>>          </p>
>> -
>> -        <p>
>> -          Details TBD.  Not yet implemented.
>> -        </p>
>>        </li>
>>
>>        <li>
>> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
>> index 27d7aab06..7777b83f5 100644
>> --- a/ovn/northd/ovn-northd.c
>> +++ b/ovn/northd/ovn-northd.c
>> @@ -5175,6 +5175,20 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>>                          "next; };";
>>              ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 80,
>>                            ds_cstr(&match), action);
>> +
>> +            ds_clear(&match);
>> +            ds_put_format(&match,
>> +                          "ip4 && ip4.dst == %s && !ip.later_frag",
>> +                          op->lrp_networks.ipv4_addrs[i].addr_s);
>> +            action = "icmp4 {"
>> +                        "eth.dst <-> eth.src; "
>> +                        "ip4.dst <-> ip4.src; "
>> +                        "ip.ttl = 255; "
>> +                        "icmp4.type = 3; "
>> +                        "icmp4.code = 2; "
>> +                        "next; };";
>> +            ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 70,
>> +                          ds_cstr(&match), action);
>>          }
>>
>>          ds_clear(&match);
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index 4648a303c..6553d17c6 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -10444,6 +10444,7 @@ OVN_POPULATE_ARP
>>  ovn-nbctl --wait=hv sync
>>
>>  test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 11 0000 7dae fcfc 0303
>> +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 84 0000 7dae fcfd 0302
>>  OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
>>
>>  test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 7bae 4486
>> --
>> 2.17.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Lucas Alvares Gomes June 29, 2018, 12:14 p.m. UTC | #3 
Hi,

> this should be the same issue reported by Darrell ('ovn: Fix gateway
> load balancing')
> Regards,
>

Yeah that's right, I have applied Darrell's patch [0] and re-run the
same test that myself and Daniel were debugging [1] and I can confirm
that it works now.

Thanks for the pointers!

[0] http://patchwork.ozlabs.org/patch/935938/
[1] https://github.com/openstack/neutron-tempest-plugin/blob/8bc66e3205b834e17e9a9e6b72b6203a7a02cada/neutron_tempest_plugin/scenario/test_floatingip.py#L180-L200

Cheers,
Lucas
Daniel Alvarez Sanchez June 29, 2018, 12:59 p.m. UTC | #4 
Yes, let's hope we can get it in soon... expecting an v3 from Darrell
apparently.
Thanks!

On Fri, Jun 29, 2018 at 2:15 PM Lucas Alvares Gomes <lucasagomes@gmail.com>
wrote:

>  Hi,
>
> > this should be the same issue reported by Darrell ('ovn: Fix gateway
> > load balancing')
> > Regards,
> >
>
> Yeah that's right, I have applied Darrell's patch [0] and re-run the
> same test that myself and Daniel were debugging [1] and I can confirm
> that it works now.
>
> Thanks for the pointers!
>
> [0] http://patchwork.ozlabs.org/patch/935938/
> [1]
> https://github.com/openstack/neutron-tempest-plugin/blob/8bc66e3205b834e17e9a9e6b72b6203a7a02cada/neutron_tempest_plugin/scenario/test_floatingip.py#L180-L200
>
> Cheers,
> Lucas
>
Ben Pfaff July 10, 2018, 8:35 p.m. UTC | #5 
This got fixed, right?

On Fri, Jun 29, 2018 at 02:59:39PM +0200, Daniel Alvarez Sanchez wrote:
> Yes, let's hope we can get it in soon... expecting an v3 from Darrell
> apparently.
> Thanks!
> 
> On Fri, Jun 29, 2018 at 2:15 PM Lucas Alvares Gomes <lucasagomes@gmail.com>
> wrote:
> 
> >  Hi,
> >
> > > this should be the same issue reported by Darrell ('ovn: Fix gateway
> > > load balancing')
> > > Regards,
> > >
> >
> > Yeah that's right, I have applied Darrell's patch [0] and re-run the
> > same test that myself and Daniel were debugging [1] and I can confirm
> > that it works now.
> >
> > Thanks for the pointers!
> >
> > [0] http://patchwork.ozlabs.org/patch/935938/
> > [1]
> > https://github.com/openstack/neutron-tempest-plugin/blob/8bc66e3205b834e17e9a9e6b72b6203a7a02cada/neutron_tempest_plugin/scenario/test_floatingip.py#L180-L200
> >
> > Cheers,
> > Lucas
> >
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Lorenzo Bianconi July 11, 2018, 10:35 a.m. UTC | #6 
> This got fixed, right?
> 

Hi Ben,

it should be fixed with: f1bb2232af23 ('ovn: Fix gateway load balancing')

Regards,
Lorenzo

> On Fri, Jun 29, 2018 at 02:59:39PM +0200, Daniel Alvarez Sanchez wrote:
> > Yes, let's hope we can get it in soon... expecting an v3 from Darrell
> > apparently.
> > Thanks!
> > 
> > On Fri, Jun 29, 2018 at 2:15 PM Lucas Alvares Gomes <lucasagomes@gmail.com>
> > wrote:
> > 
> > >  Hi,
> > >
> > > > this should be the same issue reported by Darrell ('ovn: Fix gateway
> > > > load balancing')
> > > > Regards,
> > > >
> > >
> > > Yeah that's right, I have applied Darrell's patch [0] and re-run the
> > > same test that myself and Daniel were debugging [1] and I can confirm
> > > that it works now.
> > >
> > > Thanks for the pointers!
> > >
> > > [0] http://patchwork.ozlabs.org/patch/935938/
> > > [1]
> > > https://github.com/openstack/neutron-tempest-plugin/blob/8bc66e3205b834e17e9a9e6b72b6203a7a02cada/neutron_tempest_plugin/scenario/test_floatingip.py#L180-L200
> > >
> > > Cheers,
> > > Lucas
> > >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 18a481b3d..cfd35115e 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -1342,10 +1342,6 @@  nd_na {
         <p>
           These flows should not match IP fragments with nonzero offset.
         </p>
-
-        <p>
-          Details TBD.  Not yet implemented.
-        </p>
       </li>
 
       <li>
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 27d7aab06..7777b83f5 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -5175,6 +5175,20 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                         "next; };";
             ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 80,
                           ds_cstr(&match), action);
+
+            ds_clear(&match);
+            ds_put_format(&match,
+                          "ip4 && ip4.dst == %s && !ip.later_frag",
+                          op->lrp_networks.ipv4_addrs[i].addr_s);
+            action = "icmp4 {"
+                        "eth.dst <-> eth.src; "
+                        "ip4.dst <-> ip4.src; "
+                        "ip.ttl = 255; "
+                        "icmp4.type = 3; "
+                        "icmp4.code = 2; "
+                        "next; };";
+            ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 70,
+                          ds_cstr(&match), action);
         }
 
         ds_clear(&match);
diff --git a/tests/ovn.at b/tests/ovn.at
index 4648a303c..6553d17c6 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -10444,6 +10444,7 @@  OVN_POPULATE_ARP
 ovn-nbctl --wait=hv sync
 
 test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 11 0000 7dae fcfc 0303
+test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 84 0000 7dae fcfd 0302
 OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
 
 test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 7bae 4486