Hi Richard,
On Sat, Aug 05, 2017 at 10:13:07AM +0200, Richard Cochran wrote:
>
> This testing of two Booleans (status_changed, link_status) smells bad.
> Please refactor this into something that makes sense.
At present, the link could be up or down. On an other hand, when the link stay
up or down, the ts_iface could be changed. So the states could be
LINK_UP -> LINK_DOWN(LINK_STATE_CHANGED): EV_FAULT_DETECTED
LINK_DOWN -> LINK_UP(LINK_STATE_CHANGED): EV_FAULT_CLEARED
LINK_UP -> LINK_UP, TS_IFACE_CHANGED: EV_FAULT_DETECTED
LINK_DOWN -> LINK_DOWN, TS_IFACE_CHANGED: EV_FAULT_DETECTED
For example, when the first time we receive a linkup + ts_iface change
message, we need to return EV_FAULT_DETECTED. But as we know, we may
recive multi linkup message. For these kind of message, we need to return
EV_NONE.
So I want to make the port link_status to enum like
enum link_state {
LINK_DOWN = (1<<0),
LINK_UP = (1<<1),
LINK_STATE_CHANGED = (1<<3),
TS_LABEL_CHANGED = (1<<4),
};
Then after rtnl_link_status(fd, p->name, port_link_status, p);
if (p->link_status == (LINK_UP | LINK_STATE_CHANGED))
return EV_FAULT_CLEARED;
else if ((p->link_status == (LINK_DOWN | LINK_STATE_CHANGED)) ||
(p->link_status & TS_LABEL_CHANGED))
return EV_FAULT_DETECTED;
else
return EV_NONE;
What do you think?
Thanks
Hangbin
>
> > + port_dispatch(p, EV_FAULT_CLEARED, 0);
> > + } else {
> > + port_dispatch(p, EV_FAULT_DETECTED, 0);
> > + /*
> > + * A port going down can affect the BMCA result.
> > + * Force a state decision event.
> > + */
> > + clock_set_sde(p->clock, 1);
> > + }
> > + }
> > }
> >
> > enum fsm_event port_event(struct port *p, int fd_index)
> > @@ -2301,7 +2313,7 @@ enum fsm_event port_event(struct port *p, int
> > fd_index)
> > case FD_RTNL:
> > pr_debug("port %hu: received link status notification",
> > portnum(p));
> > rtnl_link_status(fd, port_link_status, p);
> > - return port_link_status_get(p) ? EV_FAULT_CLEARED :
> > EV_FAULT_DETECTED;
> > + return EV_NONE;
>
> Maybe we can let rtnl_link_status() return the EV_ value from
> port_link_status(), in order to keep a functional pattern and avoid
> the hidden port_dispatch().
>
> > }
> >
> > msg = msg_allocate();
> > --
> > 2.5.5
>
> Thanks,
> Richard
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
