On Thu, Feb 17, 2022 at 8:58 AM Vladislav Odintsov <[email protected]> wrote:
>
> 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 <[email protected]> wrote:
> >
> > On Fri, Feb 11, 2022 at 11:22 AM Numan Siddique <[email protected]
> > <mailto:[email protected]>> wrote:
> >>
> >> On Wed, Feb 9, 2022 at 6:33 PM Vladislav Odintsov <[email protected]>
> > 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]>
> >>
> >> 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?
My understanding is that we just clone all the local ovs configs into
chassis's other_config and my suggestion was to
make sure we are consistent with the present behaviour. Have we
missed copying some of the ovs configs ?
I'm actually fine either way. If you think it's better not to copy to
the sb db, then I am fine with it.
Numan
> > 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
> >>> [email protected]
> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>>
> >> _______________________________________________
> >> dev mailing list
> >> [email protected]
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > _______________________________________________
> > dev mailing list
> > [email protected] <mailto:[email protected]>
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
> _______________________________________________
> 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
