On Thursday 03 January 2008 00:50, David Sowder (Zothar) wrote: > To be clear, a TOO OLD/TOO NEW peer is currently connected in the sense > that the node is exchanging traffic with it. The node just doesn't > route any traffic through the peer and the traffic with the peer is > limited to keeping the connectivity, N2NMs and UoM exchanges (I don't > believe I'm forgetting anything, but it wouldn't be the first time).
Yes.
>
> Robert Hailey wrote:
> > On Jan 2, 2008, at 3:44 PM, Matthew Toseland wrote:
> >
> >
> >> On Wednesday 02 January 2008 17:22, Robert Hailey wrote:
> >>
> >>> On Dec 29, 2007, at 5:39 AM, Florent Daignière wrote:
> >>>
> >>>
> >>>>>> synchronized void updateShouldDisconnectNow() {
> >>>>>> //FIXME: We should not update VERIFIED unless we
> >>>>>> HANDSHAKE WITH
> >>>>>> THE NODE
> >>>>>> - if (isConnected()) {
> >>>>>> + if (isConnected() || verifiedIncompatibleOlderVersion ||
> >>>>>> verifiedIncompatibleNewerVersion) {
> >>>>>> verifiedIncompatibleOlderVersion =
> >>>>>> forwardInvalidVersion();
> >>>>>> verifiedIncompatibleNewerVersion =
> >>>>>> reverseInvalidVersion();
> >>>>>> }
> >>>>>>
> >>>>> I suggest you call isRoutable() instead
> >>>>>
> >>>>> NextGen$
> >>>>>
> >>>> Hmm, on a latter thought, I don't understand why it helps... nor why
> >>>> we
> >>>> are calling it from PacketSender
> >>>>
> >>>> What about getting rid of both the "if" branch and the call in
> >>>> PacketSender ?
> >>>>
> >>> In PacketSender this function is called ~precisely~ at the version
> >>> transition time so that all peers will be re-evaluated concerning
> >>> compatible versioning; this seems like an odd place to stick this
> >>> code, but also updateShouldDisconnectNow() does not explicitly
> >>> disconnect the peer node, it only updates the 'incompatible' flags.
> >>>
> >> We don't want to disconnect. We want to stay connected and to not
> >> route
> >> requests to the node or accept them from it. We will still accept
> >> e.g. Update
> >> Over Mandatory traffic.
> >>
> >
> > Fair enough, but this is only confused semantics. I said disconnect as
> > it is in the function name, perhaps a better name would be
> > updateVersionRoutablity().
> >
> >
> >>> Concerning the if branch I added, the intended behaviour change is
> >>> that this function should only (1) allow the disconnection of a
> >>> connected peer by setting the incompatible flag,
> >>>
> >> No, we don't want to disconnect. We just want to make it non-
> >> routable, by
> >> updating verified*.
> >>
> >
> > I used the wrong term. It will only update the verified* booleans (and
> > thus make routable or not) if connected. But see below...
> >
> >
> >>> Now, if the nodes can handshake, this code has probably has little
> >>> effect, as it simply further restricts one of only two places that
> >>> the
> >>> incompatible flags are set (the other being on a handshake). What it
> >>> will do is if the nodes can't talk to each other (like in my re-keyed
> >>> node case), is not show 'TOO_OLD'/'TOO_NEW', but instead
> >>> 'DISCONNECTED'.
> >>>
> >> So you are setting verified* to false, because it's not verified?
> >>
> >
> > Not setting it to false, but... not allowing it to be set to true if
> > that node cannot reach us (e.g. disconnected).
> >
> >
> >>> And I suppose in the same error case... if the node is
> >>> restarted, it would never be able to verify that the peer is too old/
> >>> new and fetch the ark (keeping the version number up-to-date).
> >>>
> >> Again, we should always fetch the ARK.
> >>
> >
> > The original implementation appears to use verifiedIncompatible* to
> > optimize against a time-based trigger (Version-Incompatibility) from a
> > bunch of ark requests being started.
> >
> > On Jan 2, 2008, at 12:30 PM, David Sowder wrote:
> >
> >> I'm not sure of the thinking of others regarding changes to the
> >> verifiedIncompatible variables since, but my thinking in my initial
> >> implementation was that they would only be set after a handshake, thus
> >> we already had a connected to them and ARKs weren't needed. That
> >> won't
> >> change until the peer re-handshakes after restarting with a different
> >> version of the node's software.
> >>
> >
> > As best I can see, the verified* booleans have changed meanings since
> > update-over-mandatory. If it is still useful to know if the peer node
> > is presently-verified-{older/newer}, then we should split the booleans
> > (isOlder,isVerifiedOlder); otherwise just fetch an ark even if it is
> > an older (as Matthew suggested). In either case the misleading
> > comments and variable names should be removed and/or changed.
> >
> > --
> > Robert Hailey
> >
> > _______________________________________________
> > Devl mailing list
> > Devl@freenetproject.org
> > http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
> >
>
> _______________________________________________
> Devl mailing list
> Devl@freenetproject.org
> http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
>
>
pgpdSe626ti5w.pgp
Description: PGP signature
_______________________________________________ Devl mailing list Devl@freenetproject.org http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
