Re: [ovs-dev] [PATCH ovn v3] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case
On 24/03/2021 11:12, Numan Siddique wrote: > On Tue, Mar 16, 2021 at 2:32 AM Mark Michelson wrote: >> >> LGMT >> >> Acked-by: Mark Michelson > > Thank you Mark G (and Mark M for the reviews). > > I applied this patch to master. > > Numan > Thank everyone. >> >> On 2/16/21 6:55 AM, Mark Gray wrote: >>> If a chassis has multiple interfaces, 'ovn-encap-ip' can be used >>> to specify the IP address of the interface that is used for tunnel >>> traffic. OVN uses that IP address to configure the 'remote_ip' of >>> a tunnel port. OVS tunnel ports also accept 'options:local_ip', which, >>> according to the OVS documentation specifies "the tunnel destination >>> IP that received packets must match. Default is to match all addresses". >>> OVN does not set 'local_ip'. >>> >>> 'ovs-monitor-ipsec' is an OVS daemon that is used to configure and IPsec >>> IKE daemon on the host. In order to correctly specify an IPsec >>> connection, it requires the source and destination IP address of >>> that connection. In the OVN case, as 'local_ip' is not specified, it >>> is unable to infer the IP address of both sides of a tunnel and, therefore, >>> cannot setup an IPsec connection. >>> >>> This patch configures 'local_ip' on tunnel ports when IPsec has >>> been enabled. This allows for OVS/OVN IPsec to work when 'ovn-encap-ip' >>> is not specified as the chassis default gateway interface. >>> >>> This patch also adds some unit tests. The OVS daemon 'ovs-monitor-ipsec' >>> requires a number of options to be configured on OVS tunnel ports in order >>> to function correctly. These unit tests ensure that these options are >>> configured correctly when IPsec has been enabled through the northbound >>> database. >>> >>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1924041 >>> Signed-off-by: Mark Gray >>> --- >>> >>> v2: Updated topic filter to "PATCH ovn" >>> v3: Rebased due to 0-day bot warning >>> >>> controller/chassis.c | 5 +++ >>> controller/encaps.c | 26 ++- >>> tests/automake.mk| 3 +- >>> tests/ovn-ipsec.at | 104 +++ >>> tests/testsuite.at | 1 + >>> 5 files changed, 137 insertions(+), 2 deletions(-) >>> create mode 100644 tests/ovn-ipsec.at >>> >>> diff --git a/controller/chassis.c b/controller/chassis.c >>> index 310132d09d2e..9b0a36cf076f 100644 >>> --- a/controller/chassis.c >>> +++ b/controller/chassis.c >>> @@ -279,6 +279,11 @@ chassis_parse_ovs_config(const struct >>> ovsrec_open_vswitch_table *ovs_table, >>> return false; >>> } >>> >>> +/* 'ovn-encap-ip' can accept a comma-delimited list of IP addresses >>> instead >>> + * of a single IP address. Although this is undocumented, it can be >>> used >>> + * to enable certain hardware-offloaded use cases in which a host has >>> + * multiple NICs and is assigning SR-IOV VFs to a guest (as logical >>> ports). >>> + */ >>> if (!chassis_parse_ovs_encap_ip(encap_ips, &ovs_cfg->encap_ip_set)) { >>> sset_destroy(&ovs_cfg->encap_type_set); >>> return false; >>> diff --git a/controller/encaps.c b/controller/encaps.c >>> index 7eac4bb064ac..fc93bf1eeb87 100644 >>> --- a/controller/encaps.c >>> +++ b/controller/encaps.c >>> @@ -59,6 +59,7 @@ struct tunnel_ctx { >>> >>> struct ovsdb_idl_txn *ovs_txn; >>> const struct ovsrec_bridge *br_int; >>> +const struct sbrec_chassis *this_chassis; >>> }; >>> >>> struct chassis_node { >>> @@ -176,6 +177,28 @@ tunnel_add(struct tunnel_ctx *tc, const struct >>> sbrec_sb_global *sbg, >>> >>> /* Add auth info if ipsec is enabled. */ >>> if (sbg->ipsec) { >>> +const struct sbrec_chassis *this_chassis = tc->this_chassis; >>> +const char *local_ip = NULL; >>> + >>> +/* Determine 'ovn-encap-ip' of the local chassis as this will be >>> the >>> + * tunnel port's 'local_ip'. We do not support the case in which >>> + * 'ovn-encap-ip' holds multiple comma-delimited IP addresses. >>> + */ >>> +for (int i = 0; i < this_chassis->n_encaps; i++) { >>> +if (local_ip && strcmp(local_ip, this_chassis->encaps[i]->ip)) >>> { >>> +VLOG_ERR("ovn-encap-ip has been configured as a list. This >>> " >>> + "is unsupported for IPsec."); >>> +/* No need to loop further as we know this condition has >>> been >>> + * hit */ >>> +break; >>> +} else { >>> +local_ip = this_chassis->encaps[i]->ip; >>> +} >>> +} >>> + >>> +if (local_ip) { >>> +smap_add(&options, "local_ip", local_ip); >>> +} >>> smap_add(&options, "remote_name", new_chassis_id); >>> } >>> >>> @@ -310,7 +333,8 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, >>> struct tunnel_ctx tc = { >>> .chassis = SHASH_INITIALIZER(&tc.chassis), >>> .port_names = SSET_INIT
Re: [ovs-dev] [PATCH ovn v3] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case
On Tue, Mar 16, 2021 at 2:32 AM Mark Michelson wrote: > > LGMT > > Acked-by: Mark Michelson Thank you Mark G (and Mark M for the reviews). I applied this patch to master. Numan > > On 2/16/21 6:55 AM, Mark Gray wrote: > > If a chassis has multiple interfaces, 'ovn-encap-ip' can be used > > to specify the IP address of the interface that is used for tunnel > > traffic. OVN uses that IP address to configure the 'remote_ip' of > > a tunnel port. OVS tunnel ports also accept 'options:local_ip', which, > > according to the OVS documentation specifies "the tunnel destination > > IP that received packets must match. Default is to match all addresses". > > OVN does not set 'local_ip'. > > > > 'ovs-monitor-ipsec' is an OVS daemon that is used to configure and IPsec > > IKE daemon on the host. In order to correctly specify an IPsec > > connection, it requires the source and destination IP address of > > that connection. In the OVN case, as 'local_ip' is not specified, it > > is unable to infer the IP address of both sides of a tunnel and, therefore, > > cannot setup an IPsec connection. > > > > This patch configures 'local_ip' on tunnel ports when IPsec has > > been enabled. This allows for OVS/OVN IPsec to work when 'ovn-encap-ip' > > is not specified as the chassis default gateway interface. > > > > This patch also adds some unit tests. The OVS daemon 'ovs-monitor-ipsec' > > requires a number of options to be configured on OVS tunnel ports in order > > to function correctly. These unit tests ensure that these options are > > configured correctly when IPsec has been enabled through the northbound > > database. > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1924041 > > Signed-off-by: Mark Gray > > --- > > > > v2: Updated topic filter to "PATCH ovn" > > v3: Rebased due to 0-day bot warning > > > > controller/chassis.c | 5 +++ > > controller/encaps.c | 26 ++- > > tests/automake.mk| 3 +- > > tests/ovn-ipsec.at | 104 +++ > > tests/testsuite.at | 1 + > > 5 files changed, 137 insertions(+), 2 deletions(-) > > create mode 100644 tests/ovn-ipsec.at > > > > diff --git a/controller/chassis.c b/controller/chassis.c > > index 310132d09d2e..9b0a36cf076f 100644 > > --- a/controller/chassis.c > > +++ b/controller/chassis.c > > @@ -279,6 +279,11 @@ chassis_parse_ovs_config(const struct > > ovsrec_open_vswitch_table *ovs_table, > > return false; > > } > > > > +/* 'ovn-encap-ip' can accept a comma-delimited list of IP addresses > > instead > > + * of a single IP address. Although this is undocumented, it can be > > used > > + * to enable certain hardware-offloaded use cases in which a host has > > + * multiple NICs and is assigning SR-IOV VFs to a guest (as logical > > ports). > > + */ > > if (!chassis_parse_ovs_encap_ip(encap_ips, &ovs_cfg->encap_ip_set)) { > > sset_destroy(&ovs_cfg->encap_type_set); > > return false; > > diff --git a/controller/encaps.c b/controller/encaps.c > > index 7eac4bb064ac..fc93bf1eeb87 100644 > > --- a/controller/encaps.c > > +++ b/controller/encaps.c > > @@ -59,6 +59,7 @@ struct tunnel_ctx { > > > > struct ovsdb_idl_txn *ovs_txn; > > const struct ovsrec_bridge *br_int; > > +const struct sbrec_chassis *this_chassis; > > }; > > > > struct chassis_node { > > @@ -176,6 +177,28 @@ tunnel_add(struct tunnel_ctx *tc, const struct > > sbrec_sb_global *sbg, > > > > /* Add auth info if ipsec is enabled. */ > > if (sbg->ipsec) { > > +const struct sbrec_chassis *this_chassis = tc->this_chassis; > > +const char *local_ip = NULL; > > + > > +/* Determine 'ovn-encap-ip' of the local chassis as this will be > > the > > + * tunnel port's 'local_ip'. We do not support the case in which > > + * 'ovn-encap-ip' holds multiple comma-delimited IP addresses. > > + */ > > +for (int i = 0; i < this_chassis->n_encaps; i++) { > > +if (local_ip && strcmp(local_ip, this_chassis->encaps[i]->ip)) > > { > > +VLOG_ERR("ovn-encap-ip has been configured as a list. This > > " > > + "is unsupported for IPsec."); > > +/* No need to loop further as we know this condition has > > been > > + * hit */ > > +break; > > +} else { > > +local_ip = this_chassis->encaps[i]->ip; > > +} > > +} > > + > > +if (local_ip) { > > +smap_add(&options, "local_ip", local_ip); > > +} > > smap_add(&options, "remote_name", new_chassis_id); > > } > > > > @@ -310,7 +333,8 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, > > struct tunnel_ctx tc = { > > .chassis = SHASH_INITIALIZER(&tc.chassis), > > .port_names = SSET_INITIALIZER(&tc.port_names), > > -.br_int = br_int > > +.br_int = br_i
Re: [ovs-dev] [PATCH ovn v3] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case
LGMT Acked-by: Mark Michelson On 2/16/21 6:55 AM, Mark Gray wrote: If a chassis has multiple interfaces, 'ovn-encap-ip' can be used to specify the IP address of the interface that is used for tunnel traffic. OVN uses that IP address to configure the 'remote_ip' of a tunnel port. OVS tunnel ports also accept 'options:local_ip', which, according to the OVS documentation specifies "the tunnel destination IP that received packets must match. Default is to match all addresses". OVN does not set 'local_ip'. 'ovs-monitor-ipsec' is an OVS daemon that is used to configure and IPsec IKE daemon on the host. In order to correctly specify an IPsec connection, it requires the source and destination IP address of that connection. In the OVN case, as 'local_ip' is not specified, it is unable to infer the IP address of both sides of a tunnel and, therefore, cannot setup an IPsec connection. This patch configures 'local_ip' on tunnel ports when IPsec has been enabled. This allows for OVS/OVN IPsec to work when 'ovn-encap-ip' is not specified as the chassis default gateway interface. This patch also adds some unit tests. The OVS daemon 'ovs-monitor-ipsec' requires a number of options to be configured on OVS tunnel ports in order to function correctly. These unit tests ensure that these options are configured correctly when IPsec has been enabled through the northbound database. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1924041 Signed-off-by: Mark Gray --- v2: Updated topic filter to "PATCH ovn" v3: Rebased due to 0-day bot warning controller/chassis.c | 5 +++ controller/encaps.c | 26 ++- tests/automake.mk| 3 +- tests/ovn-ipsec.at | 104 +++ tests/testsuite.at | 1 + 5 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 tests/ovn-ipsec.at diff --git a/controller/chassis.c b/controller/chassis.c index 310132d09d2e..9b0a36cf076f 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -279,6 +279,11 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table, return false; } +/* 'ovn-encap-ip' can accept a comma-delimited list of IP addresses instead + * of a single IP address. Although this is undocumented, it can be used + * to enable certain hardware-offloaded use cases in which a host has + * multiple NICs and is assigning SR-IOV VFs to a guest (as logical ports). + */ if (!chassis_parse_ovs_encap_ip(encap_ips, &ovs_cfg->encap_ip_set)) { sset_destroy(&ovs_cfg->encap_type_set); return false; diff --git a/controller/encaps.c b/controller/encaps.c index 7eac4bb064ac..fc93bf1eeb87 100644 --- a/controller/encaps.c +++ b/controller/encaps.c @@ -59,6 +59,7 @@ struct tunnel_ctx { struct ovsdb_idl_txn *ovs_txn; const struct ovsrec_bridge *br_int; +const struct sbrec_chassis *this_chassis; }; struct chassis_node { @@ -176,6 +177,28 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg, /* Add auth info if ipsec is enabled. */ if (sbg->ipsec) { +const struct sbrec_chassis *this_chassis = tc->this_chassis; +const char *local_ip = NULL; + +/* Determine 'ovn-encap-ip' of the local chassis as this will be the + * tunnel port's 'local_ip'. We do not support the case in which + * 'ovn-encap-ip' holds multiple comma-delimited IP addresses. + */ +for (int i = 0; i < this_chassis->n_encaps; i++) { +if (local_ip && strcmp(local_ip, this_chassis->encaps[i]->ip)) { +VLOG_ERR("ovn-encap-ip has been configured as a list. This " + "is unsupported for IPsec."); +/* No need to loop further as we know this condition has been + * hit */ +break; +} else { +local_ip = this_chassis->encaps[i]->ip; +} +} + +if (local_ip) { +smap_add(&options, "local_ip", local_ip); +} smap_add(&options, "remote_name", new_chassis_id); } @@ -310,7 +333,8 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, struct tunnel_ctx tc = { .chassis = SHASH_INITIALIZER(&tc.chassis), .port_names = SSET_INITIALIZER(&tc.port_names), -.br_int = br_int +.br_int = br_int, +.this_chassis = this_chassis }; tc.ovs_txn = ovs_idl_txn; diff --git a/tests/automake.mk b/tests/automake.mk index df6d0a2a9074..bef40bde07bb 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -34,7 +34,8 @@ TESTSUITE_AT = \ tests/ovn-performance.at \ tests/ovn-ofctrl-seqno.at \ tests/ovn-ipam.at \ - tests/ovn-lflow-cache.at + tests/ovn-lflow-cache.at \ + tests/ovn-ipsec.at SYSTEM_KMOD_TESTSUITE_AT = \ tests/system-common-macros.at \ diff --git a/tests/ovn-ipsec.at b/tests/ovn-ipsec.
[ovs-dev] [PATCH ovn v3] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case
If a chassis has multiple interfaces, 'ovn-encap-ip' can be used to specify the IP address of the interface that is used for tunnel traffic. OVN uses that IP address to configure the 'remote_ip' of a tunnel port. OVS tunnel ports also accept 'options:local_ip', which, according to the OVS documentation specifies "the tunnel destination IP that received packets must match. Default is to match all addresses". OVN does not set 'local_ip'. 'ovs-monitor-ipsec' is an OVS daemon that is used to configure and IPsec IKE daemon on the host. In order to correctly specify an IPsec connection, it requires the source and destination IP address of that connection. In the OVN case, as 'local_ip' is not specified, it is unable to infer the IP address of both sides of a tunnel and, therefore, cannot setup an IPsec connection. This patch configures 'local_ip' on tunnel ports when IPsec has been enabled. This allows for OVS/OVN IPsec to work when 'ovn-encap-ip' is not specified as the chassis default gateway interface. This patch also adds some unit tests. The OVS daemon 'ovs-monitor-ipsec' requires a number of options to be configured on OVS tunnel ports in order to function correctly. These unit tests ensure that these options are configured correctly when IPsec has been enabled through the northbound database. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1924041 Signed-off-by: Mark Gray --- v2: Updated topic filter to "PATCH ovn" v3: Rebased due to 0-day bot warning controller/chassis.c | 5 +++ controller/encaps.c | 26 ++- tests/automake.mk| 3 +- tests/ovn-ipsec.at | 104 +++ tests/testsuite.at | 1 + 5 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 tests/ovn-ipsec.at diff --git a/controller/chassis.c b/controller/chassis.c index 310132d09d2e..9b0a36cf076f 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -279,6 +279,11 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table, return false; } +/* 'ovn-encap-ip' can accept a comma-delimited list of IP addresses instead + * of a single IP address. Although this is undocumented, it can be used + * to enable certain hardware-offloaded use cases in which a host has + * multiple NICs and is assigning SR-IOV VFs to a guest (as logical ports). + */ if (!chassis_parse_ovs_encap_ip(encap_ips, &ovs_cfg->encap_ip_set)) { sset_destroy(&ovs_cfg->encap_type_set); return false; diff --git a/controller/encaps.c b/controller/encaps.c index 7eac4bb064ac..fc93bf1eeb87 100644 --- a/controller/encaps.c +++ b/controller/encaps.c @@ -59,6 +59,7 @@ struct tunnel_ctx { struct ovsdb_idl_txn *ovs_txn; const struct ovsrec_bridge *br_int; +const struct sbrec_chassis *this_chassis; }; struct chassis_node { @@ -176,6 +177,28 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg, /* Add auth info if ipsec is enabled. */ if (sbg->ipsec) { +const struct sbrec_chassis *this_chassis = tc->this_chassis; +const char *local_ip = NULL; + +/* Determine 'ovn-encap-ip' of the local chassis as this will be the + * tunnel port's 'local_ip'. We do not support the case in which + * 'ovn-encap-ip' holds multiple comma-delimited IP addresses. + */ +for (int i = 0; i < this_chassis->n_encaps; i++) { +if (local_ip && strcmp(local_ip, this_chassis->encaps[i]->ip)) { +VLOG_ERR("ovn-encap-ip has been configured as a list. This " + "is unsupported for IPsec."); +/* No need to loop further as we know this condition has been + * hit */ +break; +} else { +local_ip = this_chassis->encaps[i]->ip; +} +} + +if (local_ip) { +smap_add(&options, "local_ip", local_ip); +} smap_add(&options, "remote_name", new_chassis_id); } @@ -310,7 +333,8 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, struct tunnel_ctx tc = { .chassis = SHASH_INITIALIZER(&tc.chassis), .port_names = SSET_INITIALIZER(&tc.port_names), -.br_int = br_int +.br_int = br_int, +.this_chassis = this_chassis }; tc.ovs_txn = ovs_idl_txn; diff --git a/tests/automake.mk b/tests/automake.mk index df6d0a2a9074..bef40bde07bb 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -34,7 +34,8 @@ TESTSUITE_AT = \ tests/ovn-performance.at \ tests/ovn-ofctrl-seqno.at \ tests/ovn-ipam.at \ - tests/ovn-lflow-cache.at + tests/ovn-lflow-cache.at \ + tests/ovn-ipsec.at SYSTEM_KMOD_TESTSUITE_AT = \ tests/system-common-macros.at \ diff --git a/tests/ovn-ipsec.at b/tests/ovn-ipsec.at new file mode 100644 index ..887281d5be0e --- /dev/null +++ b/tests/ovn-ipsec.at @@ -0,0 +1,104 @