1. 04 Sep, 2008 40 commits
    • Gerrit Renker's avatar
      dccp ccid-2: Stop polling · 83337dae
      Gerrit Renker authored
      This updates CCID2 to use the CCID dequeuing mechanism, converting from
      previous constant-polling to a now event-driven mechanism.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      83337dae
    • Gerrit Renker's avatar
      dccp: Refine the wait-for-ccid mechanism · 146993cf
      Gerrit Renker authored
      This extends the existing wait-for-ccid routine so that it may be used with
      different types of CCID. It further addresses the problems listed below.
      
      The code looks if the write queue is non-empty and grants the TX CCID up to
      `timeout' jiffies to drain the queue. It will instead purge that queue if
       * the delay suggested by the CCID exceeds the time budget;
       * a socket error occurred while waiting for the CCID;
       * there is a signal pending (eg. annoyed user pressed Control-C);
       * the CCID does not support delays (we don't know how long it will take).
      
      
                       D e t a i l s  [can be removed]
                       -------------------------------
      DCCP's sending mechanism functions a bit like non-blocking I/O: dccp_sendmsg()
      will enqueue up to net.dccp.default.tx_qlen packets (default=5), without waiting
      for them to be released to the network.
      
      Rate-based CCIDs, such as CCID3/4, can impose sending delays of up to maximally
      64 seconds (t_mbi in RFC 3448). Hence the write queue may still contain packets
      when the application closes. Since the write queue is congestion-controlled by
      the CCID, draining the queue is also under control of the CCID.
      
      There are several problems that needed to be addressed:
       1) The queue-drain mechanism only works with rate-based CCIDs. If CCID2 for
          example has a full TX queue and becomes network-limited just as the
          application wants to close, then waiting for CCID2 to become unblocked could
          lead to an indefinite  delay (i.e., application "hangs").
       2) Since each TX CCID in turn uses a feedback mechanism, there may be changes
          in its sending policy while the queue is being drained. This can lead to
          further delays during which the application will not be able to terminate.
       3) The minimum wait time for CCID3/4 can be expected to be the queue length
          times the current inter-packet delay. For example if tx_qlen=100 and a delay
          of 15 ms is used for each packet, then the application would have to wait
          for a minimum of 1.5 seconds before being allowed to exit.
       4) There is no way for the user/application to control this behaviour. It would
          be good to use the timeout argument of dccp_close() as an upper bound. Then
          the maximum time that an application is willing to wait for its CCIDs to can
          be set via the SO_LINGER option.
      
      These problems are addressed by giving the CCID a grace period of up to the
      `timeout' value.
      
      The wait-for-ccid function is, as before, used when the application 
       (a) has read all the data in its receive buffer and
       (b) if SO_LINGER was set with a non-zero linger time, or
       (c) the socket is either in the OPEN (active close) or in the PASSIVE_CLOSEREQ
           state (client application closes after receiving CloseReq).
      
      In addition, there is a catch-all case by calling __skb_queue_purge() after 
      waiting for the CCID. This is necessary since the write queue may still have
      data when
       (a) the host has been passively-closed,
       (b) abnormal termination (unread data, zero linger time),
       (c) wait-for-ccid could not finish within the given time limit.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      146993cf
    • Gerrit Renker's avatar
      dccp: Extend CCID packet dequeueing interface · e7937772
      Gerrit Renker authored
      This extends the packet dequeuing interface of dccp_write_xmit() to allow
       1. CCIDs to take care of timing when the next packet may be sent;
       2. delayed sending (as before, with an inter-packet gap up to 65.535 seconds).
      
      The main purpose is to take CCID2 out of its polling mode (when it is network-
      limited, it tries every millisecond to send, without interruption).
      The interface can also be used to support other CCIDs.
      
      The mode of operation for (2) is as follows:
       * new packet is enqueued via dccp_sendmsg() => dccp_write_xmit(),
       * ccid_hc_tx_send_packet() detects that it may not send (e.g. window full), 
       * it signals this condition via `CCID_PACKET_WILL_DEQUEUE_LATER',
       * dccp_write_xmit() returns without further action;
       * after some time the wait-condition for CCID becomes true,
       * that CCID schedules the tasklet,
       * tasklet function calls ccid_hc_tx_send_packet() via dccp_write_xmit(),
       * since the wait-condition is now true, ccid_hc_tx_packet() returns "send now",
       * packet is sent, and possibly more (since dccp_write_xmit() loops).
      
      Code reuse: the taskled function calls dccp_write_xmit(), the timer function
                  reduces to a wrapper around the same code.
      
      If the tasklet finds that the socket is locked, it re-schedules the tasklet
      function (not the tasklet) after one jiffy.
      
      Changed DCCP_BUG to dccp_pr_debug when transmit_skb returns an error (e.g. when a
      local qdisc is used, NET_XMIT_DROP=1 can be returned for many packets).
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      e7937772
    • Gerrit Renker's avatar
      dccp: Return-value convention of hc_tx_send_packet() · f4a66ca4
      Gerrit Renker authored
      This patch reorganises the return value convention of the CCID TX sending
      function, to permit more flexible schemes, as required by subsequent patches.
      
      Currently the convention is 
       * values < 0     mean error,
       * a value == 0   means "send now", and
       * a value x > 0  means "send in x milliseconds".
      
      The patch provides symbolic constants and a function to interpret return values.
      In addition, it caps the maximum positive return value to 0xFFFF milliseconds,
      corresponding to 65.535 seconds. 
      
      This is possible since in CCID-3 the maximum inter-packet gap is t_mbi = 64 sec.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      f4a66ca4
    • Gerrit Renker's avatar
      dccp ccid-2: Separate option parsing from CCID processing · c8bf462b
      Gerrit Renker authored
      This patch replaces an almost identical replication of code: large parts
      of dccp_parse_options() re-appeared as ccid2_ackvector() in ccid2.c.
      
      Apart from the duplication, this caused two more problems:
       1. CCIDs should not need to be concerned with parsing header options;
       2. one can not assume that Ack Vectors appear as a contiguous area within an
          skb, it is legal to insert other options and/or padding in between. The
          current code would throw an error and stop reading in such a case.
      
      The patch provides a new data structure and associated list housekeeping.
      
      Only small changes were necessary to integrate with CCID-2: data structure
      initialisation, adapt list traversal routine, and add call to the provided
      cleanup routine.
      
      The latter also lead to fixing the following BUG: CCID-2 so far ignored
      Ack Vectors on all packets other than Ack/DataAck, which is incorrect,
      since Ack Vectors can be present on any packet that has an Ack field.
      
      Details:
      --------
       * received Ack Vectors are parsed by dccp_parse_options() alone, which passes
         the result on to the CCID-specific routine ccid_hc_tx_parse_options();
       * CCIDs interested in using/decoding Ack Vector information will add code
         to fetch parsed Ack Vectors via this interface;
       * a data structure, `struct dccp_ackvec_parsed' is provided as interface;
       * this structure arranges Ack Vectors of the same skb into a FIFO order;
       * a doubly-linked list is used to keep the required FIFO code small.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      c8bf462b
    • Gerrit Renker's avatar
      dccp ccid-2: Remove old infrastructure · 5a577b48
      Gerrit Renker authored
      This removes
       * functions for which updates have been provided in the preceding patches and
       * the @av_vec_len field - it is no longer necessary since the buffer length is
         now always computed dynamically;
       * conditional debugging code (CONFIG_IP_DCCP_ACKVEC).
      
      The reason for removing the conditional debugging code is that Ack Vectors are 
      an almost inevitable necessity - RFC 4341 says that for CCID-2, Ack Vectors must
      be used. Furthermore, the code would be only interesting for coding - after some 
      extensive testing with this patch set, having the debug code around is no longer
      of real help.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      5a577b48
    • Gerrit Renker's avatar
      dccp ccid-2: Schedule Sync as out-of-band mechanism · c2f42077
      Gerrit Renker authored
      The problem with Ack Vectors is that 
      
        i) their length is variable and can in principle grow quite large,
       ii) it is hard to predict exactly how large they will be.
      
      Due to the second point it seems not a good idea to reduce the MPS; in
      particular when on average there is enough room for the Ack Vector and an
      increase in length is momentarily due to some burst loss, after which the
      Ack Vector returns to its normal/average length.
      
      The solution taken by this patch is to subtract a minimum-expected Ack Vector
      length from the MPS (previous patch), and to defer any larger Ack Vectors onto
      a separate Sync - but only if indeed there is no space left on the skb.
      
      This patch provides the infrastructure to schedule Sync-packets for transporting
      (urgent) out-of-band data. Its signalling is quicker than scheduling an Ack, since
      it does not need to wait for new application data.
      
      It can thus serve other parts of the DCCP code as well.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      c2f42077
    • Gerrit Renker's avatar
      dccp ccid-2: Consolidate Ack-Vector processing within main DCCP module · 283fb4a5
      Gerrit Renker authored
      This aggregates Ack Vector processing (handling input and clearing old state)
      into one function, for the following reasons and benefits:
       * all Ack Vector-specific processing is now in one place;
       * duplicated code is removed;
       * ensuring sanity: from an Ack Vector point of view, it is better to clear the
                          old state first before entering new state;
       * Ack Event handling happens mostly within the CCIDs, not the main DCCP module.
      
      Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>  
      283fb4a5
    • Gerrit Renker's avatar
      dccp ccid-2: Update code for the Ack Vector input/registration routine · e28fe59f
      Gerrit Renker authored
      This patch uupdates the code which registers new packets as received, using the
      new circular buffer interface. It contributes a new algorithm which 
      	* supports both tail/head pointers and buffer wrap-around and
      	* deals with overflow (head/tail move in lock-step).
      
      The updated code is also partioned differently, into
      	1. dealing with the empty buffer,
      	2. adding new packets into non-empty buffer,
      	3. reserving space when encountering a `hole' in the sequence space,
      	4. updating old state and deciding when old state is irrelevant.
      
      Protection against large burst losses: With regard to (3), it is too costly to
      reserve space when there are large bursts of losses. When bursts get too large,
      the code does no longer reserve space and just fills in cells normally. This
      measure reduces space consumption by a factor of 63.
      
      The code reuses in part the previous implementation by Arnaldo de Melo.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      e28fe59f
    • Gerrit Renker's avatar
      dccp ccid-2: Algorithm to update buffer state · 68b1de15
      Gerrit Renker authored
      This provides a routine to consistently update the buffer state when the
      peer acknowledges receipt of Ack Vectors; updating state in the list of Ack
      Vectors as well as in the circular buffer.
      
      While based on RFC 4340, several additional (and necessary) precautions were
      added to protect the consistency of the buffer state. These additions are
      essential, since analysis and experience showed that the basic algorithm was
      insufficient for this task (which lead to problems that were hard to debug).
      
      The algorithm now
       * deals with HC-sender acknowledging to HC-receiver and vice versa,
       * keeps track of the last unacknowledged but received seqno in tail_ackno,
       * has special cases to reset the overflow condition when appropriate,
       * is protected against receiving older information (would mess up buffer state).
      
      Note: The older code performed an unnecessary step, where the sender cleared
      Ack Vector state by parsing the Ack Vector received by the HC-receiver. Doing
      this was entirely redundant, since
       * the receiver always puts the full acknowledgment window (groups 2,3 in 11.4.2)
         into the Ack Vectors it sends; hence the HC-receiver is only interested in the
         highest state that the HC-sender received;
       * this means that the acknowledgment number on the (Data)Ack from the HC-sender
         is sufficient; and work done in parsing earlier state is not necessary, since
         the later state subsumes the  earlier one (see also RFC 4340, A.4).
      This older interface (dccp_ackvec_parse()) is therefore removed.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      68b1de15
    • Gerrit Renker's avatar
      dccp ccid-2: Implementation of circular Ack Vector buffer with overflow handling · d7dc7e5f
      Gerrit Renker authored
      This completes the implementation of a circular buffer for Ack Vectors, by 
      extending the current (linear array-based) implementation.  The changes are:
      
       (a) An `overflow' flag to deal with the case of overflow. As before, dynamic
           growth of the buffer will not be supported; but code will be added to deal
           robustly with overflowing Ack Vector buffers.
      
       (b) A `tail_seqno' field. When naively implementing the algorithm of Appendix A
           in RFC 4340, problems arise whenever subsequent Ack Vector records overlap,
           which can bring the entire run length calculation completely out of synch.
           (This is documented on http://www.erg.abdn.ac.uk/users/gerrit/dccp/notes/\
                                                   ack_vectors/tracking_tail_ackno/ .)
       (c) The buffer lengthi is now computed dynamically (i.e. current fill level),
           as the span between head to tail.
      
      As a result, dccp_ackvec_pending() is now simpler - the #ifdef is no longer 
      necessary since buf_empty is always true when IP_DCCP_ACKVEC is not configured.
      
      Note on overflow handling: 
      -------------------------
       The Ack Vector code previously simply started to drop packets when the
       Ack Vector buffer overflowed. This means that the userspace application
       will not be able to receive, only because of an Ack Vector storage problem.
       
       Furthermore, overflow may be transient, so that applications may later
       recover from the overflow. Recovering from dropped packets is more difficult
       (e.g. video key frames).
       
       Hence the patch uses a different policy: when the buffer overflows, the oldest
       entries are subsequently overwritten. This has a higher chance of recovery.
       Details are on http://www.erg.abdn.ac.uk/users/gerrit/dccp/notes/ack_vectors/Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      d7dc7e5f
    • Gerrit Renker's avatar
      dccp ccid-2: Separate internals of Ack Vectors from option-parsing code · 4829007c
      Gerrit Renker authored
      This patch
       * separates Ack Vector housekeeping code from option-insertion code;
       * shifts option-specific code from ackvec.c into options.c;
       * introduces a dedicated routine to take care of the Ack Vector records;
       * simplifies the dccp_ackvec_insert_avr() routine: the BUG_ON was redundant, 
         since the list is automatically arranged in descending order of ack_seqno.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      4829007c
    • Gerrit Renker's avatar
      dccp ccid-2: Ack Vector interface clean-up · ff49e270
      Gerrit Renker authored
      This patch brings the Ack Vector interface up to date. Its main purpose is
      to lay the basis for the subsequent patches of this set, which will use the
      new data structure fields and routines.
      
      There are no real algorithmic changes, rather an adaptation:
      
       (1) Replaced the static Ack Vector size (2) with a #define so that it can
           be adapted (with low loss / Ack Ratio, a value of 1 works, so 2 seems
           to be sufficient for the moment) and added a solution so that computing
           the ECN nonce will continue to work - even with larger Ack Vectors.
      
       (2) Replaced the #defines for Ack Vector states with a complete enum.
      
       (3) Replaced #defines to compute Ack Vector length and state with general
           purpose routines (inlines), and updated code to use these.
      
       (4) Added a `tail' field (conversion to circular buffer in subsequent patch).
      
       (5) Updated the (outdated) documentation for Ack Vector struct.
      
       (6) All sequence number containers now trimmed to 48 bits.
      
       (7) Removal of unused bits:
           * removed dccpav_ack_nonce from struct dccp_ackvec, since this is already
             redundantly stored in the `dccpavr_ack_nonce' (of Ack Vector record);
           * removed Elapsed Time for Ack Vectors (it was nowhere used);
           * replaced semantics of dccpavr_sent_len with dccpavr_ack_runlen, since
             the code needs to be able to remember the old run length; 
           * reduced the de-/allocation routines (redundant / duplicate tests).
      
      
      Justification for removing Elapsed Time information [can be removed]:
      ---------------------------------------------------------------------
       1. The Elapsed Time information for Ack Vectors was nowhere used in the code.
       2. DCCP does not implement rate-based pacing of acknowledgments. The only
          recommendation for always including Elapsed Time is in section 11.3 of
          RFC 4340: "Receivers that rate-pace acknowledgements SHOULD [...]
          include Elapsed Time options". But such is not the case here.
       3. It does not really improve estimation accuracy. The Elapsed Time field only
          records the time between the arrival of the last acknowledgeable packet and
          the time the Ack Vector is sent out. Since Linux does not (yet) implement
          delayed Acks, the time difference will typically be small, since often the
          arrival of a data packet triggers sending feedback at the HC-receiver.
      
      
      Justification for changes in de-/allocation routines [can be removed]:
      ----------------------------------------------------------------------
        * INIT_LIST_HEAD in dccp_ackvec_record_new was redundant, since the list
          pointers were later overwritten when the node was added via list_add();
        * dccp_ackvec_record_new() was called in a single place only;
        * calls to list_del_init() before calling dccp_ackvec_record_delete() were
          redundant, since subsequently the entire element was k-freed;
        * since all calls to dccp_ackvec_record_delete() were preceded to a call to
          list_del_init(), the WARN_ON test would never evaluate to true;
        * since all calls to dccp_ackvec_record_delete() were made from within
          list_for_each_entry_safe(), the test for avr == NULL was redundant;
        * list_empty() in ackvec_free was redundant, since the same condition is
          embedded in the loop condition of the subsequent list_for_each_entry_safe().
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      ff49e270
    • Gerrit Renker's avatar
      dccp: Reduce noise in output and convert to ktime_t · b8c6bcee
      Gerrit Renker authored
      This fixes the problem that dccp_probe output can grow quite large without
      apparent benefit (many identical data points), creating huge files (up to
      over one Gigabyte for a few minutes' test run) which are very hard to 
      post-process (in one instance it got so bad that gnuplot ate up all memory
      plus swap).
      
      The cause for the problem is that the kprobe is inserted into dccp_sendmsg(),
      which can be called in a polling-mode (whenever the TX queue is full due to
      congestion-control issues, EAGAIN is returned). This creates many very 
      similar data points, i.e. the increase of processing time does not increase
      the quality/information of the probe output.
      
      The fix is to attach the probe to a different function -- write_xmit was
      chosen since it gets called continually (both via userspace and timer);
      an input-path function would stop sampling as soon as the other end stops
      sending feedback.
      
      For comparison the output file sizes for the same 20 second test
      run over a lossy link:
                 * before / without patch:  118   Megabytes
                 * after  / with patch:       1.2 Megabytes
      and there was much less noise in the output.     
      
      To allow backward compatibility with scripts that people use, the now-unused
      `size' field in the output has been replaced with the CCID identifier. This
      also serves for future compatibility - support for CCID2 is work in progress
      (depends on the still unfinished SRTT/RTTVAR updates).
      
      While at it, the update to ktime_t was also performed.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      b8c6bcee
    • Gerrit Renker's avatar
      dccp: Merge now-reduced connect_init() function · a9c1656a
      Gerrit Renker authored
      After moving the assignment of GAR/ISS from dccp_connect_init() to
      dccp_transmit_skb(), the former function becomes very small, so that
      a merger with dccp_connect() suggests itself.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      a9c1656a
    • Gerrit Renker's avatar
      dccp: Fix the adjustments to AWL and SWL · bfbddd08
      Gerrit Renker authored
      This fixes a problem and a potential loophole with regard to seqno/ackno
      validity: the problem is that the initial adjustments to AWL/SWL were
      only performed at the begin of the connection, during the handshake.
      
      Since the Sequence Window feature is always greater than Wmin=32 (7.5.2), 
      it is however necessary to perform these adjustments at least for the first
      W/W' (variables as per 7.5.1) packets in the lifetime of a connection.
      
      This requirement is complicated by the fact that W/W' can change at any time
      during the lifetime of a connection.
      
      Therefore the consequence is to perform this safety check each time SWL/AWL
      are updated.
      
      A second problem solved by this patch is that the remote/local Sequence Window
      feature values (which set the bounds for AWL/SWL/SWH) are undefined until the
      feature negotiation has completed.
      
      During the initial handshake we have more stringent sequence number protection,
      the changes added by this patch effect that {A,S}W{L,H} are within the correct
      bounds at the instant that feature negotiation completes (since the SeqWin
      feature activation handlers call dccp_update_gsr/gss()). 
      
      A detailed rationale is below -- can be removed from the commit message.
      
      
      1. Server sequence number checks during initial handshake
      ---------------------------------------------------------
      The server can not use the fields of the listening socket for seqno/ackno checks
      and thus needs to store all relevant information on a per-connection basis on
      the dccp_request socket. This is a size-constrained structure and has currently
      only ISS (dreq_iss) and ISR (dreq_isr) defined.
      Adding further fields (SW{L,H}, AW{L,H}) would increase the size of the struct
      and it is questionable whether this will have any practical gain. The currently
      implemented solution is as follows.
       * receiving first Request: dccp_v{4,6}_conn_request sets 
                                  ISR := P.seqno, ISS := dccp_v{4,6}_init_sequence()
      
       * sending first Response:  dccp_v{4,6}_send_response via dccp_make_response()	
                                  sets P.seqno := ISS, sets P.ackno := ISR
      
       * receiving retransmitted Request: dccp_check_req() overrides ISR := P.seqno
      
       * answering retransmitted Request: dccp_make_response() sets ISS += 1,
                                          otherwise as per first Response
      
       * completing the handshake: succeeds in dccp_check_req() for the first Ack
                                   where P.ackno == ISS (P.seqno is not tested)
      
       * creating child socket: ISS, ISR are copied from the request_sock
      
      This solution will succeed whenever the server can receive the Request and the
      subsequent Ack in succession, without retransmissions. If there is packet loss,
      the client needs to retransmit until this condition succeeds; it will otherwise
      eventually give up. Adding further fields to the request_sock could increase
      the robustness a bit, in that it would make possible to let a reordered Ack
      (from a retransmitted Response) pass. The argument against such a solution is
      that if the packet loss is not persistent and an Ack gets through, why not
      wait for the one answering the original response: if the loss is persistent, it
      is probably better to not start the connection in the first place.
      
      Long story short: the present design (by Arnaldo) is simple and will likely work
      just as well as a more complicated solution. As a consequence, {A,S}W{L,H} are
      not needed until the moment the request_sock is cloned into the accept queue.
      
      At that stage feature negotiation has completed, so that the values for the local
      and remote Sequence Window feature (7.5.2) are known, i.e. we are now in a better
      position to compute {A,S}W{L,H}.
      
      
      2. Client sequence number checks during initial handshake
      ---------------------------------------------------------
      Until entering PARTOPEN the client does not need the adjustments, since it 
      constrains the Ack window to the packet it sent.
      
       * sending first Request: dccp_v{4,6}_connect() choose ISS, 
                                dccp_connect() then sets GAR := ISS (as per 8.5),
      			  dccp_transmit_skb() (with the previous bug fix) sets
      			         GSS := ISS, AWL := ISS, AWH := GSS
       * n-th retransmitted Request (with previous patch):
      	                  dccp_retransmit_skb() via timer calls
      			  dccp_transmit_skb(), which sets GSS := ISS+n
                                and then AWL := ISS, AWH := ISS+n
      	                  
       * receiving any Response: dccp_rcv_request_sent_state_process() 
      	                   -- accepts packet if AWL <= P.ackno <= AWH;
      			   -- sets GSR = ISR = P.seqno
      
       * sending the Ack completing the handshake: dccp_send_ack() calls 
                                 dccp_transmit_skb(), which sets GSS += 1
      			   and AWL := ISS, AWH := GSS
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      bfbddd08
    • Gerrit Renker's avatar
      dccp: Schedule an Ack when receiving timestamps · 2975abd2
      Gerrit Renker authored
      This schedules an Ack when receiving a timestamp, exploiting the
      existing inet_csk_schedule_ack() function, saving one case in the
      `dccp_ack_pending()' function.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      2975abd2
    • Gerrit Renker's avatar
      dccp ccid-3: Remove dead states · d0995e6a
      Gerrit Renker authored
      This patch is thanks to an investigation by Leandro Sales de Melo and his
      colleagues. They worked out two state diagrams which highlight the fact that
      the xxx_TERM states in CCID-3/4 are in fact not necessary.
      
      And this can be confirmed by in turn looking at the code: the xxx_TERM states
      are only ever set in ccid3_hc_{rx,tx}_exit(). These two functions are part
      of the following call chain:
      
       * ccid_hc_{tx,rx}_exit() are called from ccid_delete() only;
       * ccid_delete() invokes ccid_hc_{tx,rx}_exit() in the way of a destructor:
         after calling ccid_hc_{tx,rx}_exit(), the CCID is released from memory;
       * ccid_delete() is in turn called only by ccid_hc_{tx,rx}_delete();
       * ccid_hc_{tx,rx}_delete() is called only if 
         - feature negotiation failed   (dccp_feat_activate_values()),
         - when changing the RX/TX CCID (to eject the current CCID),
         - when destroying the socket   (in dccp_destroy_sock()).
      
      In other words, when CCID-3 sets the state to xxx_TERM, it is at a time where
      no more processing should be going on, hence it is not necessary to introduce
      a dedicated exit state - this is implicit when unloading the CCID.
      
      The patch removes this state, one switch-statement collapses as a result.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      d0995e6a
    • Gerrit Renker's avatar
      dccp ccid-3: Remove duplicate documentation · 5fe94963
      Gerrit Renker authored
      This removes RX-socket documentation which is either duplicate or non-existent.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      5fe94963
    • Gerrit Renker's avatar
      dccp: Unused argument in CCID tx function · c506d91d
      Gerrit Renker authored
      This removes the argument `more' from ccid_hc_tx_packet_sent, since it was
      nowhere used in the entire code.
      
      (Anecdotally, this argument was not even used in the original KAME code where
       the function originally came from; compare the variable moreToSend in the
       freebsd61-dccp-kame-28.08.2006.patch now maintained by Emmanuel Lochin.)
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      c506d91d
    • Gerrit Renker's avatar
      dccp: Replace magic CCID-specific numbers by symbolic constants · f10ecaee
      Gerrit Renker authored
      The constants DCCPO_{MIN,MAX}_CCID_SPECIFIC are nowhere used in the code, but
      instead for the CCID-specific options numbers are used.
      
      This patch unifies the use of CCID-specific option numbers, by adding symbolic
      names reflecting the definitions in RFC 4340, 10.3.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      f10ecaee
    • Gerrit Renker's avatar
      dccp ccid-3: Remove redundant 'options_received' struct · ce177ae2
      Gerrit Renker authored
      The `options_received' struct is redundant, since it re-duplicates the existing
      `p' and `x_recv' fields. This patch removes the sub-struct and migrates the
      format conversion operations (cf. below) to ccid3_hc_tx_parse_options().
      
                           Why the fields are redundant
                           ----------------------------
      The Loss Event Rate p and the Receive Rate x_recv are initially 0 when first 
      loading CCID-3, as ccid_new() zeroes out the entire ccid3_hc_tx_sock. 
      
      When Loss Event Rate or Receive Rate options are received, they are stored by
      ccid3_hc_tx_parse_options() into the fields `ccid3or_loss_event_rate' and
      `ccid3or_receive_rate' of the sub-struct `options_received' in ccid3_hc_tx_sock.
      
      After parsing (considering only the established state - dccp_rcv_established()),
      the packet is passed on to ccid_hc_tx_packet_recv(). This calls the CCID-3
      specific routine ccid3_hc_tx_packet_recv(), which performs the following copy
      operations between fields of ccid3_hc_tx_sock:
      
       * hctx->options_received.ccid3or_receive_rate is copied into hctx->x_recv,
         after scaling it for fixpoint arithmetic, by 2^64;
       * hctx->options_received.ccid3or_loss_event_rate is copied into hctx->p,
         considering the above special cases; in addition, a value of 0 here needs to
         be mapped into p=0 (when no Loss Event Rate option has been received yet).
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      ce177ae2
    • Gerrit Renker's avatar
      dccp tfrc/ccid-3: Computing Loss Rate from Loss Event Rate · 535c55df
      Gerrit Renker authored
      This adds a function to take care of the following cases occurring in the
      computation of the Loss Rate p:
      
       * 1/(2^32-1) is mapped into 0% as per RFC 4342, 8.5;
       * 1/0        is mapped into the maximum of 100%;
       * we want to avoid that p = 1/x is rounded down to 0 when x is very large,
         since this means accidentally re-entering slow-start (indicated by p==0).
      
      In the last case, the minimum-resolution value of p is returned.
      
      Furthermore, a bug in ccid3_hc_rx_getsockopt is fixed (1/0 was mapped into ~0U),
      which now allows to consistently print the scaled p-values as
      
              printf("Loss Event Rate = %u.%04u %%\n", rx_info.tfrcrx_p / 10000, 
                                                       rx_info.tfrcrx_p % 10000);
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      535c55df
    • Gerrit Renker's avatar
      dccp: Add packet type information to CCID-specific option parsing · 3306c781
      Gerrit Renker authored
      This patch ...
       1. adds packet type information to ccid_hc_{rx,tx}_parse_options(). This is 
          necessary, since table 3 in RFC 4340, 5.8 leaves it to the CCIDs to state
          which options may (not) appear on what packet type.
       
       2. adds such a check for CCID-3's {Loss Event, Receive} Rate as specified in
          RFC 4340 8.3 ("Receive Rate options MUST NOT be sent on DCCP-Data packets")
          and 8.5 ("Loss Event Rate options MUST NOT be sent on DCCP-Data packets").
      
       3. removes an unused argument `idx' from ccid_hc_{rx,tx}_parse_options(). This
          is also no longer necessary, since the CCID-specific option-parsing routines
          are passed every single parameter of the type-length-value option encoding.
      
      Also added documentation and made argument naming scheme consistent.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      3306c781
    • Gerrit Renker's avatar
      dccp ccid-3: Simplify and consolidate tx_parse_options · 47a61e7b
      Gerrit Renker authored
      This simplifies and consolidates the TX option-parsing code:
      
       1. The Loss Intervals option is not currently used, so dead code related to
          this option is removed. I am aware of no plans to support the option, but
          if someone wants to implement it (e.g. for inter-op tests), it is better
          to start afresh than having to also update currently unused code.
      
       2. The Loss Event and Receive Rate options have a lot of code in common (both
          are 32 bit, both have same length etc.), so this is consolidated.
      
       3. The test against GSR is not necessary, because
          - on first loading CCID3, ccid_new() zeroes out all fields in the socket; 
          - ccid3_hc_tx_packet_recv() treats 0 and ~0U equivalently, due to
      
      	pinv = opt_recv->ccid3or_loss_event_rate;
      	if (pinv == ~0U || pinv == 0)
      		hctx->p = 0;
      
          - as a result, the sequence number field is removed from opt_recv.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      47a61e7b
    • Gerrit Renker's avatar
      dccp ccid-3: Remove ugly RTT-sampling history lookup · 63b3a73b
      Gerrit Renker authored
      This removes the RTT-sampling function tfrc_tx_hist_rtt(), since
      
       1. it suffered from complex passing of return values (the return value both
          indicated successful lookup while the value doubled as RTT sample);
      
       2. when for some odd reason the sample value equalled 0, this triggered a bug
          warning about "bogus Ack", due to the ambiguity of the return value;
      
       3. on a passive host which has not sent anything the TX history is empty and
          thus will lead to unwanted "bogus Ack" warnings such as
          ccid3_hc_tx_packet_recv: server(e7b7d518): DATAACK with bogus ACK-28197148
          ccid3_hc_tx_packet_recv: server(e7b7d518): DATAACK with bogus ACK-26641606.
      
      The fix is to replace the implicit encoding by performing the steps manually.					       
      
      Furthermore, the "bogus Ack" warning has been removed, since it can actually be
      triggered due to several reasons (network reordering, old packet, (3) above),
      hence it is not very useful.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      63b3a73b
    • Gerrit Renker's avatar
      dccp ccid-3: Bug fix for the inter-packet scheduling algorithm · de6f2b59
      Gerrit Renker authored
      This fixes a subtle bug in the calculation of the inter-packet gap and shows
      that t_delta, as it is currently used, is not needed. And hence replaced.
      
      The algorithm from RFC 3448, 4.6 below continually computes a send time t_nom,
      which is initialised with the current time t_now; t_gran = 1E6 / HZ specifies
      the scheduling granularity, s the packet size, and X the sending rate:
      
        t_distance = t_nom - t_now;		// in microseconds
        t_delta    = min(t_ipi, t_gran) / 2;	// `delta' parameter in microseconds
      
        if (t_distance >= t_delta) {
      	reschedule after (t_distance / 1000) milliseconds;
        } else {
        	t_ipi  = s / X;			// inter-packet interval in usec
      	t_nom += t_ipi;			// compute the next send time
      	send packet now;
        }
      
      
      1) Description of the bug
      -------------------------
      Rescheduling requires a conversion into milliseconds, due to this call chain:
      
       * ccid3_hc_tx_send_packet() returns a timeout in milliseconds,
       * this value is converted by msecs_to_jiffies() in dccp_write_xmit(),
       * and finally used as jiffy-expires-value for sk_reset_timer().
      
      The highest jiffy resolution with HZ=1000 is 1 millisecond, so using a higher
      granularity does not make much sense here.
      
      As a consequence, values of t_distance < 1000 are truncated to 0. This issue 
      has so far been resolved by using instead
      
        if (t_distance >= t_delta + 1000)
      	reschedule after (t_distance / 1000) milliseconds;
      
      The bug is in artificially inflating t_delta to t_delta' = t_delta + 1000. This
      is unnecessarily large, a more adequate value is t_delta' = max(t_delta, 1000).
      
      
      2) Consequences of using the corrected t_delta'
      -----------------------------------------------
      Since t_delta <= t_gran/2 = 10^6/(2*HZ), we have t_delta <= 1000 as long as
      HZ >= 500. This means that t_delta' = max(1000, t_delta) is constant at 1000.
      
      On the other hand, when using a coarse HZ value of HZ < 500, we have three
      sub-cases that can all be reduced to using another constant of t_gran/2.
      
       (a) The first case arises when t_ipi > t_gran. Here t_delta' is the constant
           t_delta' = max(1000, t_gran/2) = t_gran/2.
      
       (b) If t_ipi <= 2000 < t_gran = 10^6/HZ usec, then t_delta = t_ipi/2 <= 1000,
           so that t_delta' = max(1000, t_delta) = 1000 < t_gran/2. 
      
       (c) If 2000 < t_ipi <= t_gran, we have t_delta' = max(t_delta, 1000) = t_ipi/2.
      
      In the second and third cases we have delay values less than t_gran/2, which is
      in the order of less than or equal to half a jiffy. 
      
      How these are treated depends on how fractions of a jiffy are handled: they
      are either always rounded down to 0, or always rounded up to 1 jiffy (assuming
      non-zero values). In both cases the error is on average in the order of 50%.
      
      Thus we are not increasing the error when in the second/third case we replace
      a value less than t_gran/2 with 0, by setting t_delta' to the constant t_gran/2.
      
      
      3) Summary
      ----------
      Fixing (1) and considering (2), the patch replaces t_delta with a constant,
      whose value depends on CONFIG_HZ, changing the above algorithm to:
       
        if (t_distance >= t_delta')
      	reschedule after (t_distance / 1000) milliseconds;
      
      where t_delta' = 10^6/(2*HZ) if HZ < 500, and t_delta' = 1000 otherwise.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      de6f2b59
    • Gerrit Renker's avatar
      dccp ccid-3: No more CCID control blocks in LISTEN state · b2e317f4
      Gerrit Renker authored
      The CCIDs are activated as last of the features, at the end of the handshake,
      were the LISTEN state of the master socket is inherited into the server
      state of the child socket. Thus, the only states visible to CCIDs now are
      OPEN/PARTOPEN, and the closing states.
      
      This allows to remove tests which were previously necessary to protect
      against referencing a socket in the listening state (in CCID3), but which
      now have become redundant.
      
      As a further byproduct of enabling the CCIDs only after the connection has been
      fully established, several typecast-initialisations of ccid3_hc_{rx,tx}_sock
      can now be eliminated:
       * the CCID is loaded, so it is not necessary to test if it is NULL,
       * if it is possible to load a CCID and leave the private area NULL, then this
          is a bug, which should crash loudly - and earlier,
       * the test for state==OPEN || state==PARTOPEN now reduces only to the closing
         phase (e.g. when the node has received an unexpected Reset).		  
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      b2e317f4
    • Gerrit Renker's avatar
      dccp ccid-3: Remove ccid3hc{tx,rx}_ prefixes · 842d1ef1
      Gerrit Renker authored
      This patch does the same for CCID-3 as the previous patch for CCID-2:
      
              s#ccid3hctx_##g;
              s#ccid3hcrx_##g;
      
      plus manual editing to retain consistency.
      
      Please note: expanded the fields of the `struct tfrc_tx_info' in the hc_tx_sock,
      since using short #define identifiers is not a good idea. The only place where
      this embedded struct was used is ccid3_hc_tx_getsockopt().
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      842d1ef1
    • Gerrit Renker's avatar
      dccp ccid-2: Remove ccid2hc{tx,rx}_ prefixes · 1fb87509
      Gerrit Renker authored
      This patch fixes two problems caused by the ubiquitous long "hctx->ccid2htx_"
      and "hcrx->ccid2hcrx_" prefixes:
       * code becomes hard to read;
       * multiple-line statements are almost inevitable even for simple expressions;
      The prefixes are not really necessary (compare with "struct tcp_sock").
      
      There had been previous discussion of this on dccp@vger, but so far this was
      not followed up (most people agreed that the prefixes are too long). 
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Signed-off-by: default avatarLeandro Melo de Sales <leandroal@gmail.com>
      1fb87509
    • Gerrit Renker's avatar
      dccp: Special case of the MPS for client-PARTOPEN with DataAcks · 88ddac51
      Gerrit Renker authored
      To increase robustness, it is necessary to resend Confirm feature-negotiation
      options, even though the RFC does not mandate it. But feature negotiation
      options can take (much) more room than the options on common DataAck packets.
      
      Instead of reducing the MPS always for a case which only applies to the three
      messages send during initial handshake, this patch devises a special case:
      
         if the payload length of the DataAck in PARTOPEN is too large, an Ack is sent
         to carry the options, and the feature-negotiation list is then flushed.
      
         This means that the server gets two Acks for one Response. If both Acks get
         lost, it is probably better to restart the connection anyway and devising yet
         another special-case does not seem worth the extra complexity.
      
      The patch (over-)estimates the expected overhead to be 32*4 bytes -- commonly
      seen values were 20-90 bytes for initial feature-negotiation options. 
      
      It uses sizeof(u32) to mean "aligned units of 4 bytes". For consistency,
      another use of sizeof is modified.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      88ddac51
    • Gerrit Renker's avatar
      dccp: Leave headroom for options when calculating the MPS · 55ebe3ab
      Gerrit Renker authored
      The Maximum Packet Size (MPS) is of interest for applications which want
      to transfer data, so it is only relevant to the data transfer phase of a
      connection (unless one wants to send data on the DCCP-Request, but that is
      not considered here).
      
      The strategy chosen to deal with this requirement is to leave room for only 
      such options that may appear on data packets.
      
      A special consideration applies to Ack Vectors: this is purely guesswork,
      since these can have any length between 3 and 1020 bytes. The strategy
      chosen here is to subtract a configurable minimum, the value of 16 bytes
      (2 bytes for type/length plus 14 Ack Vector cells) has been found by 
      experimentatation. If people experience this as too much or too little,
      this could later be turned into a Kconfig option.	
      
      There are currently no CCID-specific header options which may appear on data
      packets, hence it is not necessary to define a corresponding CCID field.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      55ebe3ab
    • Gerrit Renker's avatar
      dccp ccid-2: Use feature-negotiation to report Ack Ratio changes · 2faae558
      Gerrit Renker authored
      This uses the new feature-negotiation framework to signal Ack Ratio changes,
      as required by RFC 4341, sec. 6.1.2.
      
      This raises some problems for CCID-2 since it can at the moment not cope
      gracefully with Ack Ratio of e.g. 2. A FIXME has thus been added which
      reverts to the existing policy of bypassing the Ack Ratio sysctl.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      2faae558
    • Gerrit Renker's avatar
      dccp: Support for exchanging of NN options in established state · 4861a354
      Gerrit Renker authored
      This patch provides support for the reception of NN options in (PART)OPEN state. 
      
      It is a combination of change_recv() and confirm_recv(), specifically geared
      towards receiving the `fast-path' NN options.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      4861a354
    • Gerrit Renker's avatar
      dccp: Support for the exchange of NN options in established state · 624a965a
      Gerrit Renker authored
      In contrast to static feature negotiation at the begin of a connection, which
      establishes the capabilities of both endpoints, this patch introduces support
      for dynamic exchange of feature negotiation options.
      
      Such a dynamic exchange is necessary in at least two cases:
       * CCID-2's Ack Ratio (RFC 4341, 6.1.2) which changes during the connection;
       * Sequence Window values that, as per RFC 4340, 7.5.2, should be sent "as
         as the connection progresses".
      
      Both are NN (non-negotiable) features. Hence dynamic feature "negotiation" is
      distinguished from static/pre-connection negotiation by the following:
       * no new capabilities are negotiated (those that matter for the connection
         are negotiated prior to setting up the connection, comparable to SIP);
       * features must be understood by each endpoint: as per RFC 4340, 6.4, 
         Sequence Window is "Req'd" and Ack Ratio must be understood when CCID-2
         is used as per the note underneath Table 4.
      
      These characteristics are reflected in the implementation:
       * only NN options can be exchanged after connection setup;
       * NN options are activated directly after validating them. The rationale is
         that a peer must accept every valid NN value (RFC 4340, 6.3.2), hence it
         will either accept the value and send a "Confirm R", or it will send an
         empty Confirm (which will reset the connection according to FN rules). 
       * An Ack is scheduled directly after activation to accelerate communicating
         the update to the peer.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      624a965a
    • Gerrit Renker's avatar
      dccp: Debugging functions for feature negotiation · 76f738a7
      Gerrit Renker authored
      Since all feature-negotiation processing now takes place in feat.c, functions
      for producing verbose debugging output are concentrated there.
      
      New functions to print out values, entry records, and options are provided,
      and also a macro is defined to not always have the function name in the
      output line.
      
      Thanks a lot to Wei Yongjun and Giuseppe Galeota for help with errors in an
      earlier revision of this patch.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      76f738a7
    • Gerrit Renker's avatar
      dccp: Initialisation and type-checking of feature sysctls · 0a482267
      Gerrit Renker authored
      This patch takes care of initialising and type-checking sysctls related to
      feature negotiation. Type checking is important since some of the sysctls
      now directly act on the feature-negotiation process.
      
      The sysctls are initialised with the known default values for each feature.
      For the type-checking the value constraints from RFC 4340 are used:
      
       * Sequence Window uses the specified Wmin=32, the maximum is ulong (4 bytes),
         tested and confirmed that it works up to 4294967295 - for Gbps speed;
       * Ack Ratio is between 0 .. 0xffff (2-byte unsigned integer);
       * CCIDs are between 0 .. 255;
       * request_retries, retries1, retries2 also between 0..255 for good measure;
       * tx_qlen is checked to be non-negative;
       * sync_ratelimit remains as before.
      
      Further changes:
      ----------------
      Performed s@sysctl_dccp_feat@sysctl_dccp@g since the sysctls are now in feat.c.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      0a482267
    • Gerrit Renker's avatar
      dccp: Implement both feature-local and feature-remote Sequence Window feature · 51c7d4fa
      Gerrit Renker authored
      This adds full support for local/remote Sequence Window feature, from which the 
        * sequence-number-validity (W) and 
        * acknowledgment-number-validity (W') windows 
      derive as specified in RFC 4340, 7.5.3. 
      
      Specifically, the following changes are introduced:
        * integrated new socket fields into dccp_sk;
        * updated the update_gsr/gss routines with regard to these fields;
        * updated handler code: the Sequence Window feature is located at the TX side,
          so the local feature is meant if the handler-rx flag is false;
        * the initialisation of `rcv_wnd' in reqsk is removed, since
          - rcv_wnd is not used by the code anywhere;
          - sequence number checks are not done in the LISTEN state (cf. 7.5.3);
          - dccp_check_req checks the Ack number validity more rigorously;
        * the `struct dccp_minisock' became empty and is now removed.
      
      Until the handshake completes with activating negotiated values, the local/remote
      Sequence-Window values are undefined and thus can not reliably be estimated.
      This issue is addressed in a separate patch.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      51c7d4fa
    • Gerrit Renker's avatar
      dccp: Auto-load (when supported) CCID plugins for negotiation · 09856c10
      Gerrit Renker authored
      This adds auto-loading of CCIDs (when module loading is enabled) 
      for the purpose of feature negotiation. 
      
      The problem with loading the CCIDs at the end of feature negotiation is
      that this would happen in software interrupt context. Besides, if the host
      advertises CCIDs during negotiation, it should have them ready to use, in
      case an agreeing peer wants to use it for the connection.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Signed-off-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      09856c10
    • Gerrit Renker's avatar
      dccp: Initialisation framework for feature negotiation · 5d3dac26
      Gerrit Renker authored
      This initialises feature negotiation from two tables, which are initialised
      from sysctls. 
      
      As a novel feature, specifics of the implementation (e.g. currently short
      seqnos and ECN are not supported) are advertised for robustness.
      Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
      Acked-by: default avatarIan McDonald <ian.mcdonald@jandi.co.nz>
      5d3dac26