| Message ID | 20190306143304.16155-1-mmichels@redhat.com |
|---|---|
| State | Accepted |
| Commit | addc7953c9a1d486667e3b71c6648c0de77550b4 |
| Headers | show |
| Series | [ovs-dev] OVN: Add port addresses to IPAM after all ports are joined. | expand |
This will require backporting to 2.11 On 3/6/19 9:33 AM, Mark Michelson wrote: > Joining ports involves setting the peer field on ovn_ports. If a switch > port is visited, and it is connected to a router port, then the switch > port's peer is set to the router port and the router port's peer is set > to the switch port. > > A router port's addresses are added to IPAM if it is peered with a > switch that has dynamic addressing enabled. > > When visiting ports, if a router port is visited before its connected > switch port, then the router port's peer is not set yet. Therefore the > router's port addresses cannot be added to IPAM. The result is that > duplicate addresses can be assigned by a logical switch. > > The fix for this is to wait until all ports have been joined and then > add port addresses to IPAM. This way, we guarantee that all peer > assignments have been set, and no duplicate IP addresses may be assigned > by a switch. > > Reported-by: James Page <james.page@canonical.com> > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- > ovn/northd/ovn-northd.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 3569ea2be..a9de366fb 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -1829,7 +1829,12 @@ join_logical_ports(struct northd_context *ctx, > } > } > } > + } > > + /* Wait until all ports have been connected to add to IPAM since > + * it relies on proper peers to be set > + */ > + HMAP_FOR_EACH (op, key_node, ports) { > ipam_add_port_addresses(op->od, op); > } > } >
Thanks, applied and backported. On Wed, Mar 06, 2019 at 09:34:14AM -0500, Mark Michelson wrote: > This will require backporting to 2.11 > > On 3/6/19 9:33 AM, Mark Michelson wrote: > > Joining ports involves setting the peer field on ovn_ports. If a switch > > port is visited, and it is connected to a router port, then the switch > > port's peer is set to the router port and the router port's peer is set > > to the switch port. > > > > A router port's addresses are added to IPAM if it is peered with a > > switch that has dynamic addressing enabled. > > > > When visiting ports, if a router port is visited before its connected > > switch port, then the router port's peer is not set yet. Therefore the > > router's port addresses cannot be added to IPAM. The result is that > > duplicate addresses can be assigned by a logical switch. > > > > The fix for this is to wait until all ports have been joined and then > > add port addresses to IPAM. This way, we guarantee that all peer > > assignments have been set, and no duplicate IP addresses may be assigned > > by a switch. > > > > Reported-by: James Page <james.page@canonical.com> > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > > --- > > ovn/northd/ovn-northd.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > > index 3569ea2be..a9de366fb 100644 > > --- a/ovn/northd/ovn-northd.c > > +++ b/ovn/northd/ovn-northd.c > > @@ -1829,7 +1829,12 @@ join_logical_ports(struct northd_context *ctx, > > } > > } > > } > > + } > > + /* Wait until all ports have been connected to add to IPAM since > > + * it relies on proper peers to be set > > + */ > > + HMAP_FOR_EACH (op, key_node, ports) { > > ipam_add_port_addresses(op->od, op); > > } > > } > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 3569ea2be..a9de366fb 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -1829,7 +1829,12 @@ join_logical_ports(struct northd_context *ctx, } } } + } + /* Wait until all ports have been connected to add to IPAM since + * it relies on proper peers to be set + */ + HMAP_FOR_EACH (op, key_node, ports) { ipam_add_port_addresses(op->od, op); } }
Joining ports involves setting the peer field on ovn_ports. If a switch port is visited, and it is connected to a router port, then the switch port's peer is set to the router port and the router port's peer is set to the switch port. A router port's addresses are added to IPAM if it is peered with a switch that has dynamic addressing enabled. When visiting ports, if a router port is visited before its connected switch port, then the router port's peer is not set yet. Therefore the router's port addresses cannot be added to IPAM. The result is that duplicate addresses can be assigned by a logical switch. The fix for this is to wait until all ports have been joined and then add port addresses to IPAM. This way, we guarantee that all peer assignments have been set, and no duplicate IP addresses may be assigned by a switch. Reported-by: James Page <james.page@canonical.com> Signed-off-by: Mark Michelson <mmichels@redhat.com> --- ovn/northd/ovn-northd.c | 5 +++++ 1 file changed, 5 insertions(+)