[ovs-dev] [PATCH ovn v3 1/7] ovn-sb: Add plugged_by column to Port_Binding.
Han Zhou
hzhou at ovn.org
Thu Aug 26 05:38:35 UTC 2021
On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl <frode.nordahl at canonical.com>
wrote:
>
> To allow for use of optional plugging support we add a new
> plugged_by column with weakRef to the Chassis table. The
> ovn-controller can monitor this column and only process events
> for its chassis UUID.
>
> ovn-northd will fill this column with UUID of Chassis referenced
> in Logical_Switch_Port options:requested-chassis when
> options:plug-type is defined.
Thanks Frode. Please see my comments below.
>
> Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
> ---
> northd/ovn-northd.c | 31 ++++++++++++++++++++++++++++++
> ovn-nb.xml | 38 ++++++++++++++++++++++++++++++++++++
> ovn-sb.ovsschema | 10 +++++++---
> ovn-sb.xml | 15 +++++++++++++++
> tests/ovn-northd.at | 47 +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 138 insertions(+), 3 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 3d8e21a4f..a66f92ddd 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -3222,6 +3222,35 @@ ovn_port_update_sbrec(struct northd_context *ctx,
> * ha_chassis_group cleared in the same transaction. */
> sbrec_port_binding_set_ha_chassis_group(op->sb, NULL);
> }
> +
> + const char *plug_type; /* May be NULL. */
> + const char *requested_chassis; /* May be NULL. */
> + bool reset_plugged_by = false;
> + plug_type = smap_get(&op->nbsp->options, "plug-type");
> + requested_chassis = smap_get(&op->nbsp->options,
> + "requested-chassis");
> + if (plug_type && requested_chassis) {
> + const struct sbrec_chassis *chassis; /* May be NULL. */
> + chassis = chassis_lookup_by_name(sbrec_chassis_by_name,
> + requested_chassis);
> + if (chassis) {
> + sbrec_port_binding_set_plugged_by(op->sb, chassis);
Why do we need a separate column for this while we already have the value
in the SB port-binding options?
> + } else {
> + reset_plugged_by = true;
> + static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(
> + 1, 1);
> + VLOG_WARN_RL(
> + &rl,
> + "Unknown chassis '%s' set as "
> + "options:requested-chassis on LSP '%s'.",
> + requested_chassis, op->nbsp->name);
Would it be normal that the chassis is not registered to SB when the NB
option is set?
> + }
> + } else if (op->sb->plugged_by) {
> + reset_plugged_by = true;
> + }
> + if (reset_plugged_by) {
> + sbrec_port_binding_set_plugged_by(op->sb, NULL);
> + }
> } else {
> const char *chassis = NULL;
> if (op->peer && op->peer->od && op->peer->od->nbr) {
> @@ -15066,6 +15095,8 @@ main(int argc, char *argv[])
> add_column_noalert(ovnsb_idl_loop.idl,
> &sbrec_port_binding_col_nat_addresses);
> ovsdb_idl_add_column(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_chassis);
> + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> + &sbrec_port_binding_col_plugged_by);
> ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> &sbrec_port_binding_col_gateway_chassis);
> ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 8a942b54c..2812a0d11 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1020,6 +1020,44 @@
> DHCP reply.
> </p>
> </column>
> +
> + <group title="VIF Plugging Options">
> + <p>
> + These options apply to logical ports with
> + <ref column="options" key="requested-chassis"/> and
Now that the "requested-chassis" is used for two different scenarios, it
would be better to describe both use cases at one place instead of two, for
the reader can easily miss one of these two sections when reading.
> + <ref column="options" key="plug-type"/> set.
> + </p>
> +
> + <column name="options" key="plug-type">
> + If set, OVN will attempt to to perform plugging of this
VIF. In
> + order to get this port plugged by the OVN controller, OVN
must be
> + built with support for VIF plugging. The default behavior
is for
> + the CMS to do the VIF plugging.
> + Supported values: representor
> + </column>
> +
> + <column name="options" key="plug-pf-mac">
> + MAC address for identifying PF device. When
> + <ref column="options" key="plug-vf-num"/> is also set, this
> + option is used to identify PF to use as base to locate the
correct
> + VF representor port. When
> + <ref column="options" key="plug-vf-num"/> is not set this
> + option is used to locate a PF representor port.
> + </column>
> +
> + <column name="options" key="plug-vf-num">
> + Logical VF number relative to PF device specified in
> + <ref column="options" key="plug-pf-mac"/>.
> + </column>
> +
> + <column name="options" key="plug-mtu-request">
> + Requested MTU for plugged interfaces. When set the OVN
controller
> + will fill the <ref table="Interface" column="mtu_request"/>
column
> + of the Open vSwitch database's
> + <ref table="Interface" db="vswitch"/> table. This in turn
will
> + make OVS vswitchd update the MTU of the linked interface.
> + </column>
For the above 3 options, they are used only for plug-type "representor",
right? So it is better to have a naming convention that for each type, the
options should have a prefix: "plug:<plug type>:<option name for the plug
type>", e.g. "plug:representor:pf-mac".
Thanks,
Han
> + </group>
> </group>
>
> <group title="Virtual port Options">
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index e5ab41db9..4df326ff4 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Southbound",
> - "version": "20.20.0",
> - "cksum": "605270161 26670",
> + "version": "20.21.0",
> + "cksum": "888060012 26935",
> "tables": {
> "SB_Global": {
> "columns": {
> @@ -232,7 +232,11 @@
> "external_ids": {"type": {"key": "string",
> "value": "string",
> "min": 0,
> - "max": "unlimited"}}},
> + "max": "unlimited"}},
> + "plugged_by": {"type": {"key": {"type": "uuid",
> + "refTable": "Chassis",
> + "refType": "weak"},
> + "min": 0, "max": 1}}},
> "indexes": [["datapath", "tunnel_key"], ["logical_port"]],
> "isRoot": true},
> "MAC_Binding": {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 687555c47..d32e9b6f2 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2983,6 +2983,21 @@ tcp.flags = RST;
> </dd>
> </dl>
> </column>
> + <column name="plugged_by">
> + Chassis that should plug this port, this column must be a
> + <ref table="Chassis"/> record. This is populated by
> + <code>ovn-northd</code> when the <ref
> + table="Logical_Switch_Port"
> + column="options"
> + key="requested-chassis"
> + db="OVN_Northbound"/> and <ref
> + table="Logical_Switch_Port"
> + column="options"
> + key="plug-type"
> + db="OVN_Northbound"/> is defined. In order to get this port
plugged by
> + the OVN controller, OVN must be built with support for VIF
plugging.
> + The default behavior is for the CMS to do the VIF plugging.
> + </column>
> </group>
>
> <group title="Patch Options">
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 93bb0e1da..5324f484b 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -5208,3 +5208,50 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep
cr-DR | sort], [0], [dnl
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([check requested-chassis and plug-type fills plugged_by col])
> +AT_KEYWORDS([plugging])
> +ovn_start NORTHD_TYPE
> +
> +# Add chassis ch1.
> +check ovn-sbctl chassis-add ch1 geneve 127.0.0.2
> +check ovn-sbctl chassis-add ch2 geneve 127.0.0.3
> +check ovn-sbctl chassis-add ch3 geneve 127.0.0.4
> +
> +wait_row_count Chassis 3
> +
> +ch1_uuid=`ovn-sbctl --bare --columns _uuid find Chassis name="ch1"`
> +ch2_uuid=`ovn-sbctl --bare --columns _uuid find Chassis name="ch2"`
> +ch3_uuid=`ovn-sbctl --bare --columns _uuid find Chassis name="ch3"`
> +
> +ovn-nbctl ls-add S1
> +ovn-nbctl --wait=sb lsp-add S1 S1-vm1
> +
> +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 \
> + options:requested-chassis=ch1
> +
> +wait_row_count Port_Binding 1 logical_port=S1-vm1 plugged_by!=$ch1_uuid
> +
> +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 \
> + options:plug-type=representor
> +
> +wait_row_count Port_Binding 1 logical_port=S1-vm1 plugged_by=$ch1_uuid
> +
> +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 \
> + options:requested-chassis=ch2 options:plug-type=representor
> +
> +wait_row_count Port_binding 1 logical-port=S1-vm1 plugged_by=$ch2_uuid
> +
> +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 \
> + options:requested-chassis=ch3 options:plug-type=representor
> +
> +wait_row_count Port_binding 1 logical-port=S1-vm1 plugged_by=$ch3_uuid
> +
> +ovn-nbctl --wait=sb remove logical_switch_port S1-vm1 \
> + options plug-type=representor
> +
> +wait_row_count Port_binding 1 logical-port=S1-vm1 plugged_by!=$ch3_uuid
> +
> +AT_CLEANUP
> +])
> --
> 2.32.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list