Re: [PATCH 1/5] DCCP: Shift ccid3_li_hist

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Arnaldo Carvalho de Melo:
|  The idea of having dccp_tfrc_lib.ko is to have things that are
|  not  specific to any CCID implementation, so that if some new CCID
|  uses the TFRC algorithms it would share code with CCID3, etc.
|  
|         So it is not really correct for the code in net/dccp/ccids/lib/
|  to access the half connection internal state, as it really don't know
|  what is the layout of such internal state.
|  
|       In your patches you move ccid3_hc_rx_update_li from
|  net/dccp/ccids/ccid3.c to net/dccp/ccids/lib/loss_interval.c and then
|  renamed it to dccp_li_update_li, but this function still uses
|  ccid3_hc_rx_sk, not good.
|  
|       So I'm looking at the intent of the patches and will post here
|  how I think it should have been done and the sequence of patches
|  preparing for such change, etc.
I understand your train of thought, since the same issue had bugged me;
I think I may have a solution for that in my patch set.

The problem is that
 * TFRC (dccp_tfrc_lib.ko) needs to know the length of the first loss
   interval (which is computed by ccid3_hc_rx_update_li)
 * fully agree that the TFRC module should not (need to) know the internals
   of the calling module - when CCID4 comes, the same problem will come again

The choice I have been using was to use a function pointer for calc_first_li:

/** tfrc_lh_interval_new  -  Insert new record into the Loss Interval database
 * @lh:            Loss Interval database
 * @rh:            Receive history containing a fresh loss event
 * @calc_first_li: Caller-dependent routine to compute length of first interval
 */
int tfrc_lh_interval_new(struct tfrc_loss_hist *lh, struct tfrc_rx_hist *rh,
                         u32 (*calc_first_li)(struct sock *), struct sock *sk)

The price to pay is that the argument for *calc_first_li needs to be carried, too,
but other than that it separates the CCID3 and TFRC interfaces.

This function is not actually used (though visible) in CCID3, the one that is called is

**
 *  tfrc_rx_handle_loss  -  Loss detection and further processing
 *  @h:             The non-empty RX history object
 *  @lh:            Loss Intervals database to update
 *  @skb:           Currently received packet
 *  @ndp:           The NDP count belonging to @skb
 *  @calc_first_li: Caller-dependent computation of first loss interval in @lh
 *  Chooses action according to pending loss, updates LI database when a new
 *  loss was detected, and does required post-processing. Returns 1 when caller
 *  should send feedback, 0 otherwise.
 */
int tfrc_rx_handle_loss(struct tfrc_rx_hist *h,
                        struct tfrc_loss_hist *lh,
                        struct sk_buff *skb, u32 ndp,
                        u32 (*calc_first_li)(struct sock *), struct sock *sk)

This function is a `wrapper', which will call tfrc_lh_interval_new() when there is a
loss event (3 duplicate packets received). This is a single-function interface
to the TFRC module, covering al the loss processing.

So there is some minor ugliness (two functions with a function pointer and argument),
but this affords a separation of interfaces.
-
To unsubscribe from this list: send the line "unsubscribe dccp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel]     [IETF DCCP]     [Linux Networking]     [Git]     [Security]     [Linux Assembly]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux