[ovs-dev] [PATCH] ofproto-dpif : propagate may_enable flag as link aliveness
Ben Pfaff
blp at ovn.org
Fri Mar 17 18:20:38 UTC 2017
On Fri, Mar 03, 2017 at 01:00:05PM +0000, László Sürü wrote:
> hereby I'm sending the implementation of link liveness propagation functionality
> including the additional functionalities requested last year (last activity at Wed Mar 16 23:13:24 UTC 2016).
>
> The idea is to use OFPPS_LIVE bit to propagate link aliveness state towards the controller also when sending port status.
> The ofport->may_enable flag could be used for this purpose, thus any change in LIVE bit is propagated towards conrtoller in OFPT_PORT_STATUS message.
> OFPPS_LIVE bit is set only when links is not down not administratively, neither operationally as recommended in OF papers.
> I added 9 new unit tests to verify link state changes when monitored with cfm, bfd or lacp for OF 1.3, OF 1.4 and OF 1.5.
> I updated related unit tests according to the changes of ofproto-dpif.
Thanks a lot for the revision!
I am a little concerned about ofport_modified(). It seems like this
does not honor the status of protocols like BFD or CFM. I think this
means that, if a port changes for any reason, liveness will initially
ignore the status of these protocols, until the next time their status
changes. That is probably not a good situation. Do you have any
comments on that?
I'm passing along an incremental I recommend folding into the next
revision of the patch, to make it better fit the usual OVS coding style.
(I do hope that the next revision will take less than a year!)
Thanks,
Ben.
--8<--------------------------cut here-------------------------->8--
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b3f3a56921d3..450123a043a2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3453,7 +3453,6 @@ port_run(struct ofport_dpif *ofport)
if (ofport->may_enable != enable) {
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
- enum ofputil_port_state of_state;
ofproto->backer->need_revalidate = REV_PORT_TOGGLED;
@@ -3461,13 +3460,16 @@ port_run(struct ofport_dpif *ofport)
rstp_port_set_mac_operational(ofport->rstp_port, enable);
}
- /* liveness is propagated only,
- * when link is nor administratively neither operatively down */
+ /* Propagate liveness, unless the link is administratively or
+ * operationally down. */
if (!(ofport->up.pp.config & OFPUTIL_PC_PORT_DOWN) &&
!(ofport->up.pp.state & OFPUTIL_PS_LINK_DOWN)) {
- of_state = ofport->up.pp.state;
- (enable) ? (of_state |= OFPUTIL_PS_LIVE)
- : (of_state &= ~OFPUTIL_PS_LIVE);
+ enum ofputil_port_state of_state = ofport->up.pp.state;
+ if (enable) {
+ of_state |= OFPUTIL_PS_LIVE;
+ } else {
+ of_state &= ~OFPUTIL_PS_LIVE;
+ }
ofproto_port_set_state(&ofport->up, of_state);
}
}
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 65cb558355bc..3bb88dcb270a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2464,7 +2464,7 @@ ofport_modified(struct ofport *port, struct ofputil_phy_port *pp)
| (pp->config & OFPUTIL_PC_PORT_DOWN));
port->pp.state = ((port->pp.state & ~OFPUTIL_PS_LINK_DOWN)
| (pp->state & OFPUTIL_PS_LINK_DOWN));
- /* set LIVE bit aligned with PORT_DOWN and LINK_DOWN bits */
+ /* Initialize LIVE bit aligned with PORT_DOWN and LINK_DOWN bits. */
if ((port->pp.config & OFPUTIL_PC_PORT_DOWN) ||
(port->pp.state & OFPUTIL_PS_LINK_DOWN)) {
port->pp.state &= ~OFPUTIL_PS_LIVE;
More information about the dev
mailing list