eap Mailing List: Re: [Issue 203] Comments on EAP-Peer state machine

[Nick Petroni (npetroni]

> 1. portEnabled
>
> portEnabled seems very .1x/PPP specific.  EAP is being used in cases
> such as IKE and PANA where this concept may a bit more abstract.
> Perhaps we can change the name of the variable and/or expand its
> definition to include other cases.
>
> Suggestion:
> Perhaps "indicates that a lower layer communcation channel has been
> established".
[npetroni] this is fine for me. the name is not really of importance to me
so I would just as soon leave it the same. If others think a name change
is necessary it's fine. I like the definition change.

> Suggestion:
> Remove variable.  We could explore this in an apppendix or a separate
> document.
[npetroni] previously disussed. I agree it should be removed.


> 3. Interface to method
>
> The interface to the method seems too complicated.
>
> First I don't think that intCheck should be a different process.
> Integrity checking is done as part of the method processing.  Also
> methods aren't required to ingore packets, some methods with ignore some
> problems and return errors on others.  I think this behavior should be
> incorporated into m.process.
[npetroni] intCheck is needed because SOMETIMES a method will decide not
to use the packet at all. in this case, intCheck is the way the method
tells the SM it will not be using that packet. Methods with other ways of
dealing with this will just not set intCheck to be true.

> On a separate but related note it seems that the method state and
> decision is very complex. I don't really see the need for the
> independent methodState and decision variables.  M.process() should
> return one of serverl possible results: IGNORE, CONTINUE, COND_SUCCESS,
> SUCCESS, FAILURE.  MethodState should be able to take on CONTINUE,
> COND_SUCCESS, SUCCESS, FAILURE.  I don't think a separate decision
> variable is necessary.
[npetroni] This is meant to show that a method really has its own state
machine and that these states are separate from the final decision. I
think it provides nice way of separating the two, but I am willing to
discuss this alternative notation.

>
> 4. Idle timer
>
> It seems that there is a problem with the idle timer.  It would be
> possible for the peer to never time out if it keeps on receiving packets
> that it discards.  This is not so good.
[npetroni] hmm, that seems like a trivial DoS, no? Send enough bad packets
to the Peer and it goes away for a while?

> 5.  rxSuccess and rxFailure
>
> Shouldn't rxSuccess and rxFailure check to see if (reqId == lastReqID)?
[npetroni] probably so. I think this should be added.