[ovs-dev] [PATCH v3 5/5] ovn: DNAT and SNAT on a gateway router.
Mickey Spiegel
emspiege at us.ibm.com
Fri Jun 3 20:21:28 UTC 2016
Please see replies inline.
-----Guru Shetty <guru at ovn.org> wrote: -----
>To: Mickey Spiegel/San Jose/IBM at IBMUS
>From: Guru Shetty <guru at ovn.org>
>Date: 06/02/2016 10:41PM
>Cc: ovs dev <dev at openvswitch.org>
>Subject: Re: [ovs-dev] [PATCH v3 5/5] ovn: DNAT and SNAT on a gateway
>router.
>
>
>>
>> Looking at the code, it looks like any IP packet matches the priority-50 flow. Something like:
>> For all IP packets, a priority-50 flow with an action <code>ct_dnat;</code>.
>
>Agreed. Will fix this as part of the next version.
>
>>
>> >index 7852d83..3ce88a7 100644
>> >--- a/ovn/northd/ovn-northd.c
>> >+++ b/ovn/northd/ovn-northd.c
>> >@@ -105,12 +105,15 @@ enum ovn_stage {
>> > /* Logical router ingress stages. */ \
>> > PIPELINE_STAGE(ROUTER, IN, ADMISSION, 0, "lr_in_admission") \
>> > PIPELINE_STAGE(ROUTER, IN, IP_INPUT, 1, "lr_in_ip_input") \
>> >- PIPELINE_STAGE(ROUTER, IN, IP_ROUTING, 2, "lr_in_ip_routing") \
>> >- PIPELINE_STAGE(ROUTER, IN, ARP_RESOLVE, 3, "lr_in_arp_resolve") \
>> >- PIPELINE_STAGE(ROUTER, IN, ARP_REQUEST, 4, "lr_in_arp_request") \
>> >+ PIPELINE_STAGE(ROUTER, IN, SNAT, 2, "lr_in_snat") \
>>
>> I find calling this table SNAT misleading, since it is actually reverting the destination address.
>> How about UNDO_SNAT or UNSNAT?
>I am okay with UNSNAT. What about DNAT as it is currently doing both unDNAT and DNAT? Leave it at DNAT?
Yes, leaving it at DNAT looks good to me.
>> You are allowing the key to be a prefix, with mask other than OVS_BE32_MAX.
>> This implies that a user can specify multiple overlapping prefixes.
>> Conflicts should be resolved by going with the longest matching prefix.
>>
>> We actually need this behavior for OpenStack. If the source IP address is a
>> fixed IP that has a corresponding floating IP, then the source IP address
>> should be mapped to the floating IP address rather than the router gateway
>> address. We need longest match to win to get this behavior.
>>
>> So, the priority should be (mask+1).
>
>Right. I will get this right in the next version.
Thanks. That will let us move forward with the OpenStack networking-ovn plugin.
>>
>> >--- a/ovn/ovn-nb.ovsschema
>> >+++ b/ovn/ovn-nb.ovsschema
>> >@@ -1,7 +1,7 @@
>> > {
>> > "name": "OVN_Northbound",
>> >- "version": "2.1.2",
>> >- "cksum": "429668869 5325",
>> >+ "version": "2.1.3",
>> >+ "cksum": "400575131 5731",
>> > "tables": {
>> > "Logical_Switch": {
>> > "columns": {
>> >@@ -78,6 +78,14 @@
>> > "max": "unlimited"}},
>> > "default_gw": {"type": {"key": "string", "min": 0, "max": 1}},
>> > "enabled": {"type": {"key": "boolean", "min": 0, "max": 1}},
>> >+ "dnat" : {
>> >+ "type": {"key": {"type": "string"},
>> >+ "value": {"type" : "string"},
>> >+ "min": 0, "max": "unlimited"}},
>> >+ "snat" : {
>> >+ "type": {"key": {"type": "string"},
>> >+ "value": {"type" : "string"},
>> >+ "min": 0, "max": "unlimited"}},
>>
>> As mentioned above, in OpenStack, if the source IP address is a fixed IP
>> that has a corresponding floating IP, then the source IP address should be
>> mapped to the floating IP address rather than the router gateway address.
>>
>> Separate dnat and snat lists as defined here can be made to work to support
>> the OpenStack behavior using this patch, by copying floating IPs into both
>> the dnat and snat lists. However this is a little unwieldy.
>
>I see your point. But the behavior you mention is a very OpenStack
>thing. OVN config being independent of OpenStack specific behavior
>looks better to me, Unless we can think up with a general purpose schema.
I definitely agree that the OVN config should be independent of OpenStack.
See below for the proposed changes.
>>
>> If we ever move to distributed handling of floating IPs, which I am still
>> working on, then the duplication will be difficult to handle for east/west
>> flows. It would be easier if one definition could apply to both dnat and snat.
>> An additional value could indicate if the rule applies to only dnat or both.
>> Or, perhaps there could just be a single list of nat rules with an additional
>> value indicating if the rule applies to snat or dnat or both.
>
>This needs some thought. Do you strongly feel that this needs to be
>handled now, or would you mind sending a patch that proposes your
>schema after this schema goes in?
I am always a little nervous about putting things in schema that I know will change later,
especially when it comes to the structure of the config.
I am thinking of ovn-nb.ovsschema changes along the following lines:
"Logical_Router": {
"columns": {
"name": {"type": "string"},
"ports": {"type": {"key": {"type": "uuid",
"refTable": "Logical_Router_Port",
"refType": "strong"},
"min": 0,
"max": "unlimited"}},
"static_routes": {"type": {"key": {"type": "uuid",
"refTable": "Logical_Router_Static_Route",
"refType": "strong"},
"min": 0,
"max": "unlimited"}},
"default_gw": {"type": {"key": "string", "min": 0, "max": 1}},
"enabled": {"type": {"key": "boolean", "min": 0, "max": 1}},
+ "nat": {"type": {"key": {"type": "uuid",
+ "refTable": "NAT8,
+ "refType": "strong"},
+ "min": 0,
+ "max": "unlimited"}},
"options": {
"type": {"key": "string",
"value": "string",
"min": 0,
"max": "unlimited"}},
"external_ids": {
"type": {"key": "string", "value": "string",
"min": 0, "max": "unlimited"}}},
"isRoot": true},
+ &NAT8: {
+ "columns": {
+ "outside_ip": {"type": {"key": "string", "min": 0, "max": 1}},
+ "inside_ip": {"type": {"key": "string", "min": 0, "max": 1}},
+ "direction": {"type": {"key": {"type": "string",
+ "enum": ["set", ["dnat", &snat", "dnat_and_snat"]]}}},
+ "isRoot": false},
DNAT maps from destination outside_ip to inside_ip.
SNAT maps from source inside_ip to outside_ip.
If direction == dnat or direction == dnat_and_snat, then check for inside_ip mask == 32
If direction == snat or direction == dnat_and_snat, then check for outside_ip == 32
>As an addendum, the current schema does dnat:30.0.0.3=192.168.1.2. I
>would like to enhance it to also be able to provide ports. Something
>like dnat:30.0.0.3:4443=192.168.1.2:80
>
>So we will need to include the above with the new table that you are
>thinking too.
You can add outside_protocol, outside_l4_port, inside_protocol, and inside_l4_port.
>> Note also that for distributed handling of floating IPs, we will need MAC
>> addresses to go along with the floating IPs. This makes me think it might
>> be worth having an indirection to a separate nat table.
We will add outside_mac in the patch for distributed handling of floating IPs.
Mickey
More information about the dev
mailing list