| Message ID | 20230503011239.2100488-4-ihrachys@redhat.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement MTU Path Discovery for multichassis ports | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 5/2/23 21:12, Ihar Hrachyshka wrote: > This will allow all chassis hosting a port to extract interface MTU from > if-status-mgr. This will be used in a later patch to calculate the > effective path MTU for each port. > > In addition, it's the right thing to do to claim and mark an interface > on all chassis as ovn-installed, even if the chassis is "additional". > > Fixes: fa8c591fa2a7 ("Support LSP:options:requested-chassis as a list") > Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> > --- > controller/binding.c | 46 ++++++++++++++++++++++++++---------------- > controller/binding.h | 4 ++++ > controller/if-status.c | 8 ++++++-- > controller/if-status.h | 5 ++++- > tests/ovn.at | 10 +++++---- > 5 files changed, 49 insertions(+), 24 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 561b857fa..d75bde3eb 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -57,6 +57,10 @@ struct claimed_port { > static struct shash _claimed_ports = SHASH_INITIALIZER(&_claimed_ports); > static struct sset _postponed_ports = SSET_INITIALIZER(&_postponed_ports); > > +static void > +remove_additional_chassis(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec); > + > struct sset * > get_postponed_ports(void) > { > @@ -1028,6 +1032,26 @@ set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb, > } > } > > +void > +set_pb_additional_chassis_in_sbrec(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec, > + bool is_set) > +{ > + if (!is_additional_chassis(pb, chassis_rec)) { > + VLOG_INFO("Claiming lport %s for this additional chassis.", > + pb->logical_port); > + for (size_t i = 0; i < pb->n_mac; i++) { > + VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); > + } > + sbrec_port_binding_update_additional_chassis_addvalue(pb, chassis_rec); > + if (pb->chassis == chassis_rec) { > + sbrec_port_binding_set_chassis(pb, NULL); > + } > + } else if (!is_set) { > + remove_additional_chassis(pb, chassis_rec); > + } > +} > + > bool > local_bindings_pb_chassis_is_set(struct shash *local_bindings, > const char *pb_name, > @@ -1229,7 +1253,7 @@ claim_lport(const struct sbrec_port_binding *pb, > set_pb_chassis_in_sbrec(pb, chassis_rec, true); > } else { > if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, iface_rec, > - sb_readonly); > + sb_readonly, can_bind); > } > register_claim_timestamp(pb->logical_port, now); > sset_find_and_delete(postponed_ports, pb->logical_port); > @@ -1241,27 +1265,15 @@ claim_lport(const struct sbrec_port_binding *pb, > } else { > if (pb->n_up && !pb->up[0]) { > if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, > - iface_rec, sb_readonly); > + iface_rec, sb_readonly, > + can_bind); > } > } > } > } else if (can_bind == CAN_BIND_AS_ADDITIONAL) { > if (!is_additional_chassis(pb, chassis_rec)) { > - if (sb_readonly) { > - return false; > - } > - > - VLOG_INFO("Claiming lport %s for this additional chassis.", > - pb->logical_port); > - for (size_t i = 0; i < pb->n_mac; i++) { > - VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); > - } > - > - sbrec_port_binding_update_additional_chassis_addvalue(pb, > - chassis_rec); > - if (pb->chassis == chassis_rec) { > - sbrec_port_binding_set_chassis(pb, NULL); > - } > + if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, iface_rec, > + sb_readonly, can_bind); > update_tracked = true; > } > } > diff --git a/controller/binding.h b/controller/binding.h > index 6c3a98b02..27a954efe 100644 > --- a/controller/binding.h > +++ b/controller/binding.h > @@ -194,6 +194,10 @@ bool is_additional_chassis(const struct sbrec_port_binding *pb, > void set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb, > const struct sbrec_chassis *chassis_rec, > bool is_set); > +void > +set_pb_additional_chassis_in_sbrec(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec, > + bool is_set); > > /* Corresponds to each Port_Binding.type. */ > enum en_lport_type { > diff --git a/controller/if-status.c b/controller/if-status.c > index f2ea21635..e60156c4a 100644 > --- a/controller/if-status.c > +++ b/controller/if-status.c > @@ -226,7 +226,7 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr, > const struct sbrec_port_binding *pb, > const struct sbrec_chassis *chassis_rec, > const struct ovsrec_interface *iface_rec, > - bool sb_readonly) > + bool sb_readonly, enum can_bind bind_type) > { > const char *iface_id = pb->logical_port; > struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id); > @@ -236,7 +236,11 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr, > } > > if (!sb_readonly) { > - set_pb_chassis_in_sbrec(pb, chassis_rec, true); > + if (bind_type == CAN_BIND_AS_MAIN) { > + set_pb_chassis_in_sbrec(pb, chassis_rec, true); > + } else if (bind_type == CAN_BIND_AS_ADDITIONAL) { > + set_pb_additional_chassis_in_sbrec(pb, chassis_rec, true); > + } > } > > switch (iface->state) { > diff --git a/controller/if-status.h b/controller/if-status.h > index ab1625b18..8186bdf08 100644 > --- a/controller/if-status.h > +++ b/controller/if-status.h > @@ -19,6 +19,7 @@ > #include "openvswitch/shash.h" > > #include "binding.h" > +#include "lport.h" > > struct if_status_mgr; > struct simap; > @@ -30,7 +31,7 @@ void if_status_mgr_claim_iface(struct if_status_mgr *, > const struct sbrec_port_binding *pb, > const struct sbrec_chassis *chassis_rec, > const struct ovsrec_interface *iface_rec, > - bool sb_readonly); > + bool sb_readonly, enum can_bind bind_type); > void if_status_mgr_release_iface(struct if_status_mgr *, const char *iface_id); > void if_status_mgr_delete_iface(struct if_status_mgr *, const char *iface_id); > > @@ -44,6 +45,8 @@ void if_status_mgr_get_memory_usage(struct if_status_mgr *mgr, > struct simap *usage); > bool if_status_mgr_iface_is_present(struct if_status_mgr *mgr, > const char *iface_id); > +uint16_t if_status_mgr_iface_get_mtu(const struct if_status_mgr *mgr, > + const char *iface_id); This function was already declared in patch 2, so it appears to be re-declared here. > bool if_status_handle_claims(struct if_status_mgr *mgr, > struct local_binding_data *binding_data, > const struct sbrec_chassis *chassis_rec, > diff --git a/tests/ovn.at b/tests/ovn.at > index 213ad18fa..616036156 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -14283,10 +14283,12 @@ wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > wait_column "$hv2_uuid" Port_Binding additional_chassis logical_port=lsp0 > wait_column "$hv2_uuid" Port_Binding requested_additional_chassis logical_port=lsp0 > > -# Check ovn-installed updated for main chassis > +# Check ovn-installed updated for both chassis > wait_for_ports_up > -OVS_WAIT_UNTIL([test `as hv1 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = '"true"']) > -OVS_WAIT_UNTIL([test x`as hv2 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = x]) > + > +for hv in hv1 hv2; do > + OVS_WAIT_UNTIL([test `as $hv ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = '"true"']) > +done > > # Check that setting iface:encap-ip populates Port_Binding:additional_encap > wait_row_count Encap 2 chassis_name=hv1 > @@ -14313,7 +14315,7 @@ wait_column "$hv2_uuid" Port_Binding requested_chassis logical_port=lsp0 > wait_column "" Port_Binding additional_chassis logical_port=lsp0 > wait_column "" Port_Binding requested_additional_chassis logical_port=lsp0 > > -# Check ovn-installed updated for main chassis and not for other chassis > +# Check ovn-installed updated for main chassis and removed from additional chassis > wait_for_ports_up > OVS_WAIT_UNTIL([test `as hv2 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = '"true"']) > OVS_WAIT_UNTIL([test x`as hv1 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = x])
diff --git a/controller/binding.c b/controller/binding.c index 561b857fa..d75bde3eb 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -57,6 +57,10 @@ struct claimed_port { static struct shash _claimed_ports = SHASH_INITIALIZER(&_claimed_ports); static struct sset _postponed_ports = SSET_INITIALIZER(&_postponed_ports); +static void +remove_additional_chassis(const struct sbrec_port_binding *pb, + const struct sbrec_chassis *chassis_rec); + struct sset * get_postponed_ports(void) { @@ -1028,6 +1032,26 @@ set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb, } } +void +set_pb_additional_chassis_in_sbrec(const struct sbrec_port_binding *pb, + const struct sbrec_chassis *chassis_rec, + bool is_set) +{ + if (!is_additional_chassis(pb, chassis_rec)) { + VLOG_INFO("Claiming lport %s for this additional chassis.", + pb->logical_port); + for (size_t i = 0; i < pb->n_mac; i++) { + VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); + } + sbrec_port_binding_update_additional_chassis_addvalue(pb, chassis_rec); + if (pb->chassis == chassis_rec) { + sbrec_port_binding_set_chassis(pb, NULL); + } + } else if (!is_set) { + remove_additional_chassis(pb, chassis_rec); + } +} + bool local_bindings_pb_chassis_is_set(struct shash *local_bindings, const char *pb_name, @@ -1229,7 +1253,7 @@ claim_lport(const struct sbrec_port_binding *pb, set_pb_chassis_in_sbrec(pb, chassis_rec, true); } else { if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, iface_rec, - sb_readonly); + sb_readonly, can_bind); } register_claim_timestamp(pb->logical_port, now); sset_find_and_delete(postponed_ports, pb->logical_port); @@ -1241,27 +1265,15 @@ claim_lport(const struct sbrec_port_binding *pb, } else { if (pb->n_up && !pb->up[0]) { if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, - iface_rec, sb_readonly); + iface_rec, sb_readonly, + can_bind); } } } } else if (can_bind == CAN_BIND_AS_ADDITIONAL) { if (!is_additional_chassis(pb, chassis_rec)) { - if (sb_readonly) { - return false; - } - - VLOG_INFO("Claiming lport %s for this additional chassis.", - pb->logical_port); - for (size_t i = 0; i < pb->n_mac; i++) { - VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); - } - - sbrec_port_binding_update_additional_chassis_addvalue(pb, - chassis_rec); - if (pb->chassis == chassis_rec) { - sbrec_port_binding_set_chassis(pb, NULL); - } + if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, iface_rec, + sb_readonly, can_bind); update_tracked = true; } } diff --git a/controller/binding.h b/controller/binding.h index 6c3a98b02..27a954efe 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -194,6 +194,10 @@ bool is_additional_chassis(const struct sbrec_port_binding *pb, void set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb, const struct sbrec_chassis *chassis_rec, bool is_set); +void +set_pb_additional_chassis_in_sbrec(const struct sbrec_port_binding *pb, + const struct sbrec_chassis *chassis_rec, + bool is_set); /* Corresponds to each Port_Binding.type. */ enum en_lport_type { diff --git a/controller/if-status.c b/controller/if-status.c index f2ea21635..e60156c4a 100644 --- a/controller/if-status.c +++ b/controller/if-status.c @@ -226,7 +226,7 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr, const struct sbrec_port_binding *pb, const struct sbrec_chassis *chassis_rec, const struct ovsrec_interface *iface_rec, - bool sb_readonly) + bool sb_readonly, enum can_bind bind_type) { const char *iface_id = pb->logical_port; struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id); @@ -236,7 +236,11 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr, } if (!sb_readonly) { - set_pb_chassis_in_sbrec(pb, chassis_rec, true); + if (bind_type == CAN_BIND_AS_MAIN) { + set_pb_chassis_in_sbrec(pb, chassis_rec, true); + } else if (bind_type == CAN_BIND_AS_ADDITIONAL) { + set_pb_additional_chassis_in_sbrec(pb, chassis_rec, true); + } } switch (iface->state) { diff --git a/controller/if-status.h b/controller/if-status.h index ab1625b18..8186bdf08 100644 --- a/controller/if-status.h +++ b/controller/if-status.h @@ -19,6 +19,7 @@ #include "openvswitch/shash.h" #include "binding.h" +#include "lport.h" struct if_status_mgr; struct simap; @@ -30,7 +31,7 @@ void if_status_mgr_claim_iface(struct if_status_mgr *, const struct sbrec_port_binding *pb, const struct sbrec_chassis *chassis_rec, const struct ovsrec_interface *iface_rec, - bool sb_readonly); + bool sb_readonly, enum can_bind bind_type); void if_status_mgr_release_iface(struct if_status_mgr *, const char *iface_id); void if_status_mgr_delete_iface(struct if_status_mgr *, const char *iface_id); @@ -44,6 +45,8 @@ void if_status_mgr_get_memory_usage(struct if_status_mgr *mgr, struct simap *usage); bool if_status_mgr_iface_is_present(struct if_status_mgr *mgr, const char *iface_id); +uint16_t if_status_mgr_iface_get_mtu(const struct if_status_mgr *mgr, + const char *iface_id); bool if_status_handle_claims(struct if_status_mgr *mgr, struct local_binding_data *binding_data, const struct sbrec_chassis *chassis_rec, diff --git a/tests/ovn.at b/tests/ovn.at index 213ad18fa..616036156 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -14283,10 +14283,12 @@ wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 wait_column "$hv2_uuid" Port_Binding additional_chassis logical_port=lsp0 wait_column "$hv2_uuid" Port_Binding requested_additional_chassis logical_port=lsp0 -# Check ovn-installed updated for main chassis +# Check ovn-installed updated for both chassis wait_for_ports_up -OVS_WAIT_UNTIL([test `as hv1 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = '"true"']) -OVS_WAIT_UNTIL([test x`as hv2 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = x]) + +for hv in hv1 hv2; do + OVS_WAIT_UNTIL([test `as $hv ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = '"true"']) +done # Check that setting iface:encap-ip populates Port_Binding:additional_encap wait_row_count Encap 2 chassis_name=hv1 @@ -14313,7 +14315,7 @@ wait_column "$hv2_uuid" Port_Binding requested_chassis logical_port=lsp0 wait_column "" Port_Binding additional_chassis logical_port=lsp0 wait_column "" Port_Binding requested_additional_chassis logical_port=lsp0 -# Check ovn-installed updated for main chassis and not for other chassis +# Check ovn-installed updated for main chassis and removed from additional chassis wait_for_ports_up OVS_WAIT_UNTIL([test `as hv2 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = '"true"']) OVS_WAIT_UNTIL([test x`as hv1 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = x])
This will allow all chassis hosting a port to extract interface MTU from if-status-mgr. This will be used in a later patch to calculate the effective path MTU for each port. In addition, it's the right thing to do to claim and mark an interface on all chassis as ovn-installed, even if the chassis is "additional". Fixes: fa8c591fa2a7 ("Support LSP:options:requested-chassis as a list") Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> --- controller/binding.c | 46 ++++++++++++++++++++++++++---------------- controller/binding.h | 4 ++++ controller/if-status.c | 8 ++++++-- controller/if-status.h | 5 ++++- tests/ovn.at | 10 +++++---- 5 files changed, 49 insertions(+), 24 deletions(-)