Hi Mohammad, thanks for the review!
I’ve added the requested testcase and submitted the v2: https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ Regards, Vladislav Odintsov > On 6 Feb 2022, at 12:59, Mohammad Heib <[email protected]> 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<[email protected]> >> --- >> 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 > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
