Hi Han,

thanks for the note about log message, I’ll fix this in v3 right after the 
question with other_config is resolved.
Frankly speaking I also don’t understand why to sync ovn-set-local-ip option to 
other_config —
I did this only because Numan asked to do :)

Regards,
Vladislav Odintsov

> On 17 Feb 2022, at 09:39, Han Zhou <hz...@ovn.org> wrote:
> 
> On Fri, Feb 11, 2022 at 11:22 AM Numan Siddique <num...@ovn.org 
> <mailto:num...@ovn.org>> wrote:
>> 
>> On Wed, Feb 9, 2022 at 6:33 PM Vladislav Odintsov <odiv...@gmail.com>
> wrote:
>>> 
>>> When transport node has multiple interfaces (vlans) and
>>> ovn-encap-ip on different hosts need to be configured
>>> from different VLANs source IP for encapsulated packet
>>> can be not the same, which is expected by remote system.
>>> 
>>> Explicitely setting local_ip resolves such problem.
>>> 
>>> Signed-off-by: Vladislav Odintsov <odiv...@gmail.com>
>> 
>> Hi Vladislav,
>> 
>> Can you please add the code to copy the new added config from OVS
>> database to the
>> other_config column of Chassis table in Southbound db ?
>> 
>> Please take a look at "struct ovs_chassis_cfg" in controller/chassis.c
>> 
>> Thanks
>> Numan
> 
> Hi Numan,
> 
> May I ask what's the purpose of adding this to other_config in SB? I
> understand that there are fields in other_config that is of importance for
> other chassises, so need to be added to SB, but for this one, how would it
> be used in SB DB?
> 
> Hi Vladislav,
> 
> Regarding this:
>                VLOG_ERR("ovn-encap-ip has been configured as a list. This "
>                         "is unsupported for IPsec.");
> 
> Before your change it applies to IPsec only, but with your change this
> would apply to non-IPsec as well, if ovn-set-local-ip is true. Could you
> update the error log as well? (it is better to be ratelimited, but it is
> not the fault of your patch)
> 
> Thanks,
> Han
> 
>> 
>>> ---
>>> controller/encaps.c             | 37 +++++++++++++++++++++------------
>>> controller/ovn-controller.8.xml |  7 +++++++
>>> tests/ovn-controller.at         |  9 ++++++++
>>> 3 files changed, 40 insertions(+), 13 deletions(-)
>>> 
>>> diff --git a/controller/encaps.c b/controller/encaps.c
>>> index 66e0cd8cd..3b0c92931 100644
>>> --- a/controller/encaps.c
>>> +++ b/controller/encaps.c
>>> @@ -23,6 +23,7 @@
>>> #include "openvswitch/vlog.h"
>>> #include "lib/ovn-sb-idl.h"
>>> #include "ovn-controller.h"
>>> +#include "smap.h"
>>> 
>>> VLOG_DEFINE_THIS_MODULE(encaps);
>>> 
>>> @@ -176,8 +177,31 @@ tunnel_add(struct tunnel_ctx *tc, const struct
> sbrec_sb_global *sbg,
>>>         smap_add(&options, "dst_port", dst_port);
>>>     }
>>> 
>>> +    const struct ovsrec_open_vswitch *cfg =
>>> +        ovsrec_open_vswitch_table_first(ovs_table);
>>> +
>>> +    bool set_local_ip = false;
>>> +    if (cfg) {
>>> +        /* If the tos option is configured, get it */
>>> +        const char *encap_tos = smap_get_def(&cfg->external_ids,
>>> +           "ovn-encap-tos", "none");
>>> +
>>> +        if (encap_tos && strcmp(encap_tos, "none")) {
>>> +            smap_add(&options, "tos", encap_tos);
>>> +        }
>>> +
>>> +        /* If ovn-set-local-ip option is configured, get it */
>>> +        set_local_ip = smap_get_bool(&cfg->external_ids,
> "ovn-set-local-ip",
>>> +                                     false);
>>> +    }
>>> +
>>>     /* Add auth info if ipsec is enabled. */
>>>     if (sbg->ipsec) {
>>> +        set_local_ip = true;
>>> +        smap_add(&options, "remote_name", new_chassis_id);
>>> +    }
>>> +
>>> +    if (set_local_ip) {
>>>         const struct sbrec_chassis *this_chassis = tc->this_chassis;
>>>         const char *local_ip = NULL;
>>> 
>>> @@ -200,19 +224,6 @@ tunnel_add(struct tunnel_ctx *tc, const struct
> sbrec_sb_global *sbg,
>>>         if (local_ip) {
>>>             smap_add(&options, "local_ip", local_ip);
>>>         }
>>> -        smap_add(&options, "remote_name", new_chassis_id);
>>> -    }
>>> -
>>> -    const struct ovsrec_open_vswitch *cfg =
>>> -        ovsrec_open_vswitch_table_first(ovs_table);
>>> -    /* If the tos option is configured, get it */
>>> -    if (cfg) {
>>> -        const char *encap_tos = smap_get_def(&cfg->external_ids,
>>> -           "ovn-encap-tos", "none");
>>> -
>>> -        if (encap_tos && strcmp(encap_tos, "none")) {
>>> -            smap_add(&options, "tos", encap_tos);
>>> -        }
>>>     }
>>> 
>>>     /* If there's an existing chassis record that does not need any
> change,
>>> diff --git a/controller/ovn-controller.8.xml
> b/controller/ovn-controller.8.xml
>>> index e9708fe64..cc9a7d1c2 100644
>>> --- a/controller/ovn-controller.8.xml
>>> +++ b/controller/ovn-controller.8.xml
>>> @@ -304,6 +304,13 @@
>>>         of how many entries there are in the cache.  By default this
> is set to
>>>         30000 (30 seconds).
>>>       </dd>
>>> +      <dt><code>external_ids:ovn-set-local-ip</code></dt>
>>> +      <dd>
>>> +        The boolean flag indicates if <code>ovn-controller</code> when
> create
>>> +        tunnel ports should set <code>local_ip</code> parameter.  Can
> be
>>> +        heplful to pin source outer IP for the tunnel when multiple
> interfaces
>>> +        are used on the host for overlay traffic.
>>> +      </dd>
>>>     </dl>
>>> 
>>>     <p>
>>> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
>>> index 2f39e5f3e..9e6302e5a 100644
>>> --- a/tests/ovn-controller.at
>>> +++ b/tests/ovn-controller.at
>>> @@ -298,6 +298,15 @@ OVS_WAIT_UNTIL([check_tunnel_property type geneve])
>>> ovs-vsctl del-port ovn-fakech-0
>>> OVS_WAIT_UNTIL([check_tunnel_property type geneve])
>>> 
>>> +# set `ovn-set-local-ip` option to true and check if tunnel parameters
>>> +OVS_WAIT_WHILE([check_tunnel_property options:local_ip
> "\"192.168.0.1\""])
>>> +ovs-vsctl set open . external_ids:ovn-set-local-ip=true
>>> +OVS_WAIT_UNTIL([check_tunnel_property options:local_ip
> "\"192.168.0.1\""])
>>> +
>>> +# Change the local_ip on the OVS side and check than OVN fixes it
>>> +ovs-vsctl set interface ovn-fakech-0 options:local_ip="1.1.1.1"
>>> +OVS_WAIT_UNTIL([check_tunnel_property options:local_ip
> "\"192.168.0.1\""])
>>> +
>>> # Gracefully terminate daemons
>>> OVN_CLEANUP_SBOX([hv])
>>> OVN_CLEANUP_VSWITCH([main])
>>> --
>>> 2.30.0
>>> 
>>> _______________________________________________
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>> 
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> d...@openvswitch.org <mailto:d...@openvswitch.org>
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev 
> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to