Hi Mohammad,

thanks for the review!

I’ve added the requested testcase and submitted the v2:
https://patchwork.ozlabs.org/project/ovn/patch/20220209233314.51948-1-odiv...@gmail.com/

Regards,
Vladislav Odintsov

> On 6 Feb 2022, at 12:59, Mohammad Heib <mh...@redhat.com> wrote:
> 
> Hi Vladislav,
> 
> your change looks good to me.
> just if you can add a small test under ./tests to validate your change and
> make sure that future changes will not break your change (see 
> ./tests/ovn-ipsec.at  for a good example).
> 
> Thanks,
> 
> On 1/27/22 18:11, Vladislav Odintsov 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>
>> ---
>>  controller/encaps.c             | 37 +++++++++++++++++++++------------
>>  controller/ovn-controller.8.xml |  7 +++++++
>>  2 files changed, 31 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>
> _______________________________________________
> 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

Reply via email to