Re: [PATCH 13/25] Share TFRC feedback types struct via ccid34_lib

Re: [PATCH 13/25] Share TFRC feedback types struct via ccid34_lib

[Gerrit Renker]

I have a suggestion regarding types, names, and things shared between
CCID3 and CCID4 via dccp_tfrc_lib:

I think it would be best to name all such shared structures / names,
when they are to appear in the dccp_tfrc_lib, as `tfrc_xxx' instead of
`ccid34_xxx'

For example,

enum tfrc_fback_type {
// ...
};
 
instead of:

|  enum ccid34_fback_type {
|  + FBACK_NONE = 0,
|  + FBACK_INITIAL,
|  + FBACK_PERIODIC,
|  + FBACK_PARAM_CHANGE
|  +};

There are no mechanisms other than TFRC (current work builds around TFRC, rather than entirely new schemes)
so I think that this naming scheme is safe; and it would be consistent throughout the library.

I'd hope that Arnaldo and Ian add their take if they disagree or have other suggestions.

Re: [PATCH 13/25] Share TFRC feedback types struct via ccid34_lib

[Arnaldo Carvalho de Melo]

Em Wed, Oct 31, 2007 at 09:59:14AM +0000, Gerrit Renker escreveu:
> I have a suggestion regarding types, names, and things shared between
> CCID3 and CCID4 via dccp_tfrc_lib:
> 
> I think it would be best to name all such shared structures / names,
> when they are to appear in the dccp_tfrc_lib, as `tfrc_xxx' instead of
> `ccid34_xxx'
> 
> For example,
> 
> enum tfrc_fback_type {
> // ...
> };
>  
> instead of:
> 
> |  enum ccid34_fback_type {
> |  + FBACK_NONE = 0,
> |  + FBACK_INITIAL,
> |  + FBACK_PERIODIC,
> |  + FBACK_PARAM_CHANGE
> |  +};
> 
> There are no mechanisms other than TFRC (current work builds around TFRC, rather than entirely new schemes)
> so I think that this naming scheme is safe; and it would be consistent throughout the library.
> 
> I'd hope that Arnaldo and Ian add their take if they disagree or have other suggestions.

I agree that for things which concept comes from TFRC and are used in
one of the TFRC based DCCP CCIDs the best possible namespace is tfrc_.
If not we'll have to rename everything again when CCID5 comes if it is
also based on TFRC 8-)

And after all, even before ccid4 appeared on the radar I created
dccp_tfrc_lib, include/linux/tfrc.h, etc exactly for sharing code with
potentially new CCIDs that were based on TFRC.

- Arnaldo

Re: [PATCH 13/25] Share TFRC feedback types struct via ccid34_lib

[Gerrit Renker]

Leandro,

following Arnaldo's reply, can you please you the suggested naming scheme (it is also
already used in packet_history.c and loss_interval.c): I will be looking into merging
the patches with the CCID3 set of patches, to reduce work and duplication.

The whole lot of CCID3 patches will then be re-submitted. Arnaldo, I would like to make
your job easier by sending fewer patches (the CCID3 batch is 40 patches, plus Leandro's):
would you be ok with a smaller number, e.g. 5..7?

There will also be a short RFC before this (regarding locking), and I will make sure that
the entire batch becomes fully bisectable.

Gerrit

Quoting Arnaldo Carvalho de Melo:
|  > enum tfrc_fback_type {
|  > // ...
|  > };
|  >  
|  > instead of:
|  > 
|  > |  enum ccid34_fback_type {
|  > |  + FBACK_NONE = 0,
|  > |  + FBACK_INITIAL,
|  > |  + FBACK_PERIODIC,
|  > |  + FBACK_PARAM_CHANGE
|  > |  +};
|  > 
|  > There are no mechanisms other than TFRC (current work builds around TFRC, rather than entirely new schemes)
|  > so I think that this naming scheme is safe; and it would be consistent throughout the library.
|  > 
|  > I'd hope that Arnaldo and Ian add their take if they disagree or have other suggestions.
|  
|  I agree that for things which concept comes from TFRC and are used in
|  one of the TFRC based DCCP CCIDs the best possible namespace is tfrc_.
|  If not we'll have to rename everything again when CCID5 comes if it is
|  also based on TFRC 8-)
|  
|  And after all, even before ccid4 appeared on the radar I created
|  dccp_tfrc_lib, include/linux/tfrc.h, etc exactly for sharing code with
|  potentially new CCIDs that were based on TFRC.
|  

Re: [PATCH 13/25] Share TFRC feedback types struct via ccid34_lib

[Łeandro Sales]

2007/10/31, Gerrit Renker <gerrit@erg.abdn.ac.uk>:
> Leandro,
>
> following Arnaldo's reply, can you please you the suggested naming scheme (it is also
> already used in packet_history.c and loss_interval.c): I will be looking into merging
> the patches with the CCID3 set of patches, to reduce work and duplication.
>
> The whole lot of CCID3 patches will then be re-submitted. Arnaldo, I would like to make
> your job easier by sending fewer patches (the CCID3 batch is 40 patches, plus Leandro's):
> would you be ok with a smaller number, e.g. 5..7?
>
> There will also be a short RFC before this (regarding locking), and I will make sure that
> the entire batch becomes fully bisectable.
>
> Gerrit
>
> Quoting Arnaldo Carvalho de Melo:
> |  > enum tfrc_fback_type {
> |  > // ...
> |  > };
> |  >
> |  > instead of:
> |  >
> |  > | enum ccid34_fback_type {
> |  > | + FBACK_NONE = 0,
> |  > | + FBACK_INITIAL,
> |  > | + FBACK_PERIODIC,
> |  > | + FBACK_PARAM_CHANGE
> |  > | +};
> |  >
> |  > There are no mechanisms other than TFRC (current work builds around TFRC, rather than entirely new schemes)
> |  > so I think that this naming scheme is safe; and it would be consistent throughout the library.
> |  >
> |  > I'd hope that Arnaldo and Ian add their take if they disagree or have other suggestions.
> |
> |  I agree that for things which concept comes from TFRC and are used in
> |  one of the TFRC based DCCP CCIDs the best possible namespace is tfrc_.
> |  If not we'll have to rename everything again when CCID5 comes if it is
> |  also based on TFRC 8-)
> |
> |  And after all, even before ccid4 appeared on the radar I created
> |  dccp_tfrc_lib, include/linux/tfrc.h, etc exactly for sharing code with
> |  potentially new CCIDs that were based on TFRC.
> |
>

Hello Gerrit,
    Yes, for sure! I also agree with you and Arnaldo on this and I'm
currently change to the namespace suggested by you and accepted by
Arnaldo.

Leandro

[PATCH 13/25] Share TFRC feedback types struct via tfrc_ccids

[Leandro]

[CCID-3/4] Share TFRC feedback types struct via tfrc_ccids

Signed-off-by: Leandro Melo de Sales <leandro@embedded.ufcg.edu.br>

Index: leandro.new/net/dccp/ccids/ccid3.c
=================================--- leandro.new.orig/net/dccp/ccids/ccid3.c
+++ leandro.new/net/dccp/ccids/ccid3.c
@@ -660,7 +660,7 @@ static inline void ccid3_hc_rx_update_s(
 }
 
 static void ccid3_hc_rx_send_feedback(struct sock *sk, struct sk_buff *skb,
-				      enum ccid3_fback_type fbtype)
+				      enum tfrc_fback_type fbtype)
 {
 	struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk);
 	struct dccp_sock *dp = dccp_sk(sk);
@@ -777,7 +777,7 @@ static u32 ccid3_first_li(struct sock *s
 static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb)
 {
 	struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk);
-	enum   ccid3_fback_type  do_feedback = FBACK_NONE;
+	enum   tfrc_fback_type  do_feedback = FBACK_NONE;
 	u64 ndp = dccp_sk(sk)->dccps_options_received.dccpor_ndp;
 	u32 sample, payload_size = skb->len - dccp_hdr(skb)->dccph_doff * 4;
 	u8  is_data_packet = dccp_data_packet(skb);
Index: leandro.new/net/dccp/ccids/ccid3.h
=================================--- leandro.new.orig/net/dccp/ccids/ccid3.h
+++ leandro.new/net/dccp/ccids/ccid3.h
@@ -88,14 +88,6 @@ static inline struct ccid3_hc_tx_sock *c
     return hctx;
 }
 
-/* CCID3 feedback types */
-enum ccid3_fback_type {
-	FBACK_NONE = 0,
-	FBACK_INITIAL,
-	FBACK_PERIODIC,
-	FBACK_PARAM_CHANGE
-};
-
 /** struct ccid3_hc_rx_sock - CCID3 receiver half-connection socket
  *
  *  @ccid3hcrx_last_counter  -  Tracks window counter (RFC 4342, 8.1)
Index: leandro.new/net/dccp/ccids/ccid4.c
=================================--- leandro.new.orig/net/dccp/ccids/ccid4.c
+++ leandro.new/net/dccp/ccids/ccid4.c
@@ -686,7 +686,7 @@ static inline void ccid4_hc_rx_update_s(
 }
 
 static void ccid4_hc_rx_send_feedback(struct sock *sk, struct sk_buff *skb,
-				      enum ccid4_fback_type fbtype)
+				      enum tfrc_fback_type fbtype)
 {
 	struct ccid4_hc_rx_sock *hcrx = ccid4_hc_rx_sk(sk);
 	struct dccp_sock *dp = dccp_sk(sk);
@@ -803,7 +803,7 @@ static u32 ccid4_first_li(struct sock *s
 static void ccid4_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb)
 {
 	struct ccid4_hc_rx_sock *hcrx = ccid4_hc_rx_sk(sk);
-	enum   ccid4_fback_type  do_feedback = FBACK_NONE;
+	enum   tfrc_fback_type  do_feedback = FBACK_NONE;
 	u64 ndp = dccp_sk(sk)->dccps_options_received.dccpor_ndp;
 	u32 sample, payload_size = skb->len - dccp_hdr(skb)->dccph_doff * 4;
 	u8  is_data_packet = dccp_data_packet(skb);
Index: leandro.new/net/dccp/ccids/ccid4.h
=================================--- leandro.new.orig/net/dccp/ccids/ccid4.h
+++ leandro.new/net/dccp/ccids/ccid4.h
@@ -111,14 +111,6 @@ static inline struct ccid4_hc_tx_sock *c
     return hctx;
 }
 
-/* CCID4 feedback types */
-enum ccid4_fback_type {
-	FBACK_NONE = 0,
-	FBACK_INITIAL,
-	FBACK_PERIODIC,
-	FBACK_PARAM_CHANGE
-};
-
 /** struct ccid4_hc_rx_sock - CCID4 receiver half-connection socket
  *
  *  @ccid4hcrx_last_counter  -  Tracks window counter (RFC 4342, 8.1)
Index: leandro.new/net/dccp/ccids/lib/tfrc_ccids.h
=================================--- leandro.new.orig/net/dccp/ccids/lib/tfrc_ccids.h
+++ leandro.new/net/dccp/ccids/lib/tfrc_ccids.h
@@ -61,3 +61,11 @@ enum tfrc_hc_rx_states {
 	TFRC_RSTATE_TERM    = 127,
 };
 
+/* CCID3/4 feedback types */
+enum tfrc_fback_type {
+	FBACK_NONE = 0,
+	FBACK_INITIAL,
+	FBACK_PERIODIC,
+	FBACK_PARAM_CHANGE
+};
+

Re: [PATCH 13/25] Share TFRC feedback types struct via tfrc_ccids

[Tommi Saviranta]

On Wed, Oct 31, 2007 at 21:31:25 -0300, Leandro wrote:
> [CCID-3/4] Share TFRC feedback types struct via tfrc_ccids
> 
> Signed-off-by: Leandro Melo de Sales <leandro@embedded.ufcg.edu.br>

Acked-by: Tommi Saviranta <wnd@iki.fi>