Re: [PATCH 17/25] Share ccid3_tx_state_name function via tfrc_ccids
2007/11/10, Gerrit Renker [EMAIL PROTECTED]:
| [CCID-3/4] Share ccid3_tx_state_name function via tfrc_ccids
There is a build problem in declaring the following routines as shared:
#if defined(CONFIG_IP_DCCP_CCID3_DEBUG) || defined(CONFIG_IP_DCCP_CCID4_DEBUG)
static const char *tfrc_tx_state_name(enum tfrc_hc_tx_states state)
{
/* ... */
}
#endif
#if defined(CONFIG_IP_DCCP_CCID3_DEBUG) || defined(CONFIG_IP_DCCP_CCID4_DEBUG)
static const char *tfrc_rx_state_name(enum tfrc_hc_rx_states state)
{
/* ... */
#endif
When enabling CCID3_DEBUG, but not CCID4_DEBUG, the following build warnings
result:
/usr/src/davem-2.6/net/dccp/ccids/lib/tfrc_ccids.h:163: warning:
'tfrc_tx_state_name' defined but not used
/usr/src/davem-2.6/net/dccp/ccids/lib/tfrc_ccids.h:180: warning:
'tfrc_rx_state_name' defined but not used
The reason is that the header file is #included in ccid4.c and this function
is only called from
within ccid4_pr_debug(). But when CCID4_DEBUG is disabled, this macro expands
to an emptry string,
so the compiler thinks that tfrc_{rx,tx}_state_name() is not used -- which is
actually the case.
So not very happy with the above solution. When getting serious about
abstracting and sharing
common code, the best think is probably to use a single debugging function,
tfrc_pr_debug.
This would be enabled when at least one of CCID3_DEBUG or CCID4_DEBUG is
enabled, and one could
add some magic that the module parameters influence the boolean variable
tfrc_debug.
The same could be done for the RTO timer - since the identical menu appears
in both places.
Maybe it is better for the moment to `un-share' the code.
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yes Gerrit, this is one of the point that I will talk to you about. I
also agree with you, the best for the moment is to un-share this
function.
Leandro.
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/25] Enforces a minimum interval of 10 milliseconds as per CCID-4 draft
2007/11/9, Tommi Saviranta [EMAIL PROTECTED]: On Thu, Nov 08, 2007 at 11:18:30 +, Gerrit Renker wrote: Maybe the code would be clearer to read this way: hctx-ccid4hctx_t_ipi = scaled_div32(((u64)hctx-ccid4hctx_s) 6, hctx-ccid4hctx_x)); if (hctx-ccid4hctx_t_ipi MIN_SEND_RATE) hctx-ccid4hctx_t_ipi = MIN_SEND_RATE; Yes, it would. I was going to ask whether one of them would be considered more effective, but I then realised this is probably one of those cases where you should write readable code and let the compiler to worry about the rest. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Yes, I also agree with this. Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/25] CCID-4 Kconfig and Makefile
2007/11/8, Gerrit Renker [EMAIL PROTECTED]: | +config IP_DCCP_CCID4_DEBUG | + bool CCID4 debugging messages | + depends on IP_DCCP_CCID4 snip | +# The TFRC Library: currently has CCID 3 and CCID 4 as customer | config IP_DCCP_TFRC_LIB | - depends on IP_DCCP_CCID3 | + depends on IP_DCCP_CCID3 || IP_DCCP_CCID4 | def_tristate IP_DCCP_CCID3 | | config IP_DCCP_TFRC_DEBUG Suggestion: if CCID3 is not compiled as module, there will currently be no TFRC debugging output for dccp_tfrc_lib when only CCID4 is enabled. Something like config IP_DCCP_TFRC_DEBUG bool default y if IP_DCCP_CCID3_DEBUG || IP_DCCP_CCID4_DEBUG would enable this. On the other hand, the number of `debugs' keeps continually growing. Maybe it is possible to just use tfrc_debug for everything that speaks TFRC, otherwise we currently have * tfrc_debug to enable debug messages in dccp_tfrc_lib * ccid3_debug dccp_ccid3 * ccid4_debug dccp_ccid4 * dccp_debug to see the rest - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Yes, it is better to have tfrc_debug for everything related to TFRC. I will also fix the IP_DCCP_TFRC_DEBUG kconfig option like you suggested. Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/25] CCID-4 Kconfig and Makefile
2007/11/13, Gerrit Renker [EMAIL PROTECTED]: Quoting Leandro: | 2007/11/8, Gerrit Renker [EMAIL PROTECTED]: | | +config IP_DCCP_CCID4_DEBUG | | + bool CCID4 debugging messages | | + depends on IP_DCCP_CCID4 | snip | | +# The TFRC Library: currently has CCID 3 and CCID 4 as customer | | config IP_DCCP_TFRC_LIB | | - depends on IP_DCCP_CCID3 | | + depends on IP_DCCP_CCID3 || IP_DCCP_CCID4 | | def_tristate IP_DCCP_CCID3 | | | | config IP_DCCP_TFRC_DEBUG | Suggestion: if CCID3 is not compiled as module, there will currently be | no TFRC debugging output for dccp_tfrc_lib when only CCID4 is enabled. | Something like | | config IP_DCCP_TFRC_DEBUG | bool | default y if IP_DCCP_CCID3_DEBUG || IP_DCCP_CCID4_DEBUG | | would enable this. | | On the other hand, the number of `debugs' keeps continually growing. | Maybe it is possible to just use tfrc_debug for everything that speaks | TFRC, otherwise we currently have | * tfrc_debug to enable debug messages in dccp_tfrc_lib | * ccid3_debug dccp_ccid3 | * ccid4_debug dccp_ccid4 | * dccp_debug to see the rest | - | | Yes, it is better to have tfrc_debug for everything related to TFRC. I | will also fix the IP_DCCP_TFRC_DEBUG kconfig option like you | suggested. | Please do as you see fit, but don't worry too much at this stage - this can all be resolved later (i.e. further abstraction can be done later). - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Then I will let this as it is and after we can working on this. Ok? - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/25] Share TFRC sender states via tfrc_ccids
2007/11/8, Gerrit Renker [EMAIL PROTECTED]:
| @@ -76,16 +76,16 @@ static const char *ccid4_tx_state_name(e
| #endif
|
| static void ccid4_hc_tx_set_state(struct sock *sk,
| - enum ccid4_hc_tx_states state)
| + enum tfrc_hc_tx_states state)
| {
| struct ccid4_hc_tx_sock *hctx = ccid4_hc_tx_sk(sk);
| - enum ccid4_hc_tx_states oldstate = hctx-ccid4hctx_state;
| + enum tfrc_hc_tx_states oldstate = hctx-tfrchctx_state;
|
| ccid4_pr_debug(%s(%p) %-8.8s - %s\n,
| dccp_role(sk), sk, ccid4_tx_state_name(oldstate),
| ccid4_tx_state_name(state));
| WARN_ON(state == oldstate);
| - hctx-ccid4hctx_state = state;
| + hctx-tfrchctx_state = state;
| }
|
I think this function is a candidate for sharing: no need to keep
this function separate for CCID3/4 when both use the same enum of
states; i.e. the function could be written tfrc_hc_tx_set_state().
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yes, I already have a patch to share this function.
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce]: Test Tree and CCID4 Tree updated
2007/11/10, Gerrit Renker [EMAIL PROTECTED]:
This is to announce that the test tree on
git://eden-feed.erg.abdn.ac.uk/dccp_exp
has now been updated with regard to removal of spinlocks (changelog as per
previous mail).
The CCID4 tree has also been updated to match the changes, changes are below.
CCID 4 Update
I have patched your second patch [2/25] with regard to the code at the
bottom of the message and traced those two hunks throughout the patch set.
The set compiles cleanly (but see other email regarding state names).
I have further `repaired' all the whitespace errors: almost half of the
patches had blanks instead of tabs for indentation. Please check next
time - it is easy to avoid but tedious to get out once it is in.
Can you please check the CCID4 tree out
git pull git://eden-feed.erg.abdn.ac.uk/dccp_exp ccid4
and see if you are ok with the state of your patches. Ian has a nice script
to convert a tree into a set of patches, which can be used for this purpose:
http://wlug.org.nz/KernelDevelopmentWithGit
Note - I have only done compilation tests, it would be good if you run your
tests.
Will also do some more CCID4-specific testing when I get a little more time.
Gerrit
--- Difference to Patch 2/25 before/after -
--- a/net/dccp/ccids/ccid4.c
+++ b/net/dccp/ccids/ccid4.c
@@ -781,8 +781,6 @@ static void ccid4_hc_rx_packet_recv(stru
u32 sample, payload_size = skb-len - dccp_hdr(skb)-dccph_doff * 4;
u8 is_data_packet = dccp_data_packet(skb);
- spin_lock(hcrx-ccid4hcrx_hist.lock);
-
if (unlikely(hcrx-ccid4hcrx_state == TFRC_RSTATE_NO_DATA)) {
if (is_data_packet) {
do_feedback = FBACK_INITIAL;
@@ -846,8 +844,6 @@ update_records:
tfrc_rx_hist_update(hcrx-ccid4hcrx_hist, skb, ndp);
done_receiving:
- spin_unlock(hcrx-ccid4hcrx_hist.lock);
-
if (do_feedback)
ccid4_hc_rx_send_feedback(sk, skb, do_feedback);
}
OK. Thank you for the update. I already change my .vimrc to
automatically set noexpandtab.
Leandro.
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How much is available for reading on the socket?
2007/11/9, Arnaldo Carvalho de Melo [EMAIL PROTECTED]: Em Fri, Nov 09, 2007 at 01:51:27PM +, Gerrit Renker escreveu: | using ioctl (socket, FIONREAD, readsize) I can get how much is | available to read from the socket using TCP. Is there a equivalent | option for DCCP? | | Looks like dccp_ioctl() happily returns -ENOIOCTLCMD (for now). If you | have IO and memory to waste, you can always recv(..., MSG_PEEK) to work | around this. | Arnaldo wrote a patch which realises SIOCINQ (it is in 2.6.24-rc1/2). And, on Linux: include/linux/sockios.h /* Linux-specific socket ioctls */ #define SIOCINQ FIONREAD - Arnaldo And I'm using this on the gstreamer plugin and it is working ok. Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/25] Towards CCID3/CCID4 code integration
Hi Gerrit,
2007/11/3, Gerrit Renker [EMAIL PROTECTED]:
I've had a look at the recent set of patches, some more detailed comments
below.
Basically, I think it is too early to think of merging: it will constrain your
work, at the moment there is really not much being shared, and to me it is not
clear what shape the CCID4 code will eventually take.
As it is done currently, I can't see much benefit: there are only three
functions in tfrc_ccids.c:
* rfc3390_initial_rate
* tfrc_hc_tx_idle_rtt
* tfrc_hc_tx_update_win_count
The first and last of the above functions are declared inline. Inlines inside
a
source file are not often considered a good idea; according to current wisdom
one
should leave it to gcc to decide whether to make a function inline or not.
The inline declarations were done just to maintain the current code,
which was current using inline. But, in fact I can remove the inline
declaration.
If you do want to declare these two inline, you can put them into the header
file --
which then leaves you with only a single function in tfrc_ccids.c.
The idea of my patches is to propose an initial code sharing between
ccid3/ccid4 (towards ccid3/4 integration). After these patches, we
can work to share more code between ccid3/ccid4 to tfrc_ccids.c, like
you suggested using generic code.
If there is really not more that you want to share between the CCIDs, then
there is not much benefit in allocating an extra file; on the contrary, it
just
increases the confusion (what is this file doing?). Besides, there is
already
such a file for this purpose -- check net/dccp/ccids/lib/tfrc_module.c
But it exists. One example is the function which handles
DROPPED_PACKET_OPTION, that you mentioned. I just didn't try to share
more function because I intended to make all of you check the code and
discuss the directions and see if it is worthy to continue the job or
not.
With regard to header files, there is a similar observation: due to putting
everything into tfrc_ccids.h, ccid3.h now becomes a one-liner:
#include lib/tfrc_ccids.h,
so that you might as well remove ccid3.h entirely; ccid4.h has three more
lines,
when these are instead put at the top of ccid4.c, then ccid4.h is also no
longer
necessary.
I maintain the ccid{3/4}.h in case we need to use them eventually in the future.
There is a disparity: almost no code is shared, while CCID3 and CCID4 use an
exactly identical socket structure. When two different pieces of code share
the same
underlying data structure, then it is likely possible to share more code
currently done.
Yes, this is the idea. The patches just starts up the job! :)
Hence I am asking you what your plans are: keep the strategy of only sharing
functions
that previously appeared in both ccid3.c and ccid4.c -- in this case the
number of new
files can be reduced; or extend the work to also share functions which have
the same
functionality in both files.
The idea is to extend the work to also share functions which have the
same functionality, but for archive this, it is necessary to analyze
more carefully the code, as you mentioned below.
Individual points:
1 - Certify whether the function is common (checking just the functions
in ccid-4 copied from ccid-3);
2 - Certify whether both functions (in ccid-3 and ccid-4) still common;
(Me or Tommi can already changed it for ccid-4)
Looking at the code, this seems to be just one category, of those functions
which are
100% identical in both files.
3 - Certify whether the both functions doesn't call a ccid specific
function;
i.e: ccid3_hc_tx_packet_sent could be shared, but it calls
ccid3_hc_tx_update_s, which is a ccid-3 specific function.
Then ccid3_hc_tx_packet_sent is not a candidate to be shared
between ccid-{3/4}. i.e: ccid3_hc_rx_packet_recv i.e:
ccid3_hc_rx_insert_options
It is possible to still share code here; by using a generic function which
takes
a function pointer to the CCID-specific function as argument. Realising this
will
reduce the amount of duplicated functionality, but requires careful analysis.
May
be able to help here, depending on how things progress.
Ok.
4 - Certify whether the function has a low probability to be changed
for ccid-4 implementation in the future;
* although ccid3_hc_tx_send_packet could be common, ccid-4 will
probably have a particular way to decide when send a packet;
* ccid4_first_li will certainly change because the short loss interval
length is calculated as N/K, instead of using just N
(ccid3_first_li);
Ok, these functions really do different things and so keeping them separate
seems the
right thing to do.
Ok.
* ccid3_hc_tx_parse_options could be common, but until the IETF
folks do not decide if DROPPED_PACKET_OPTION will be into the ccid-3,
I decided to NOT share it.
Can't see the
DCCP and GStreamer: File descriptor sharing
Hi all, I want to share a same socket-fd between two process using dccp. The gstreamer plugin architecture uses a concept of sink and src (each one named element), where sink send and src receive data. In this case, one of then connects and inform the file descriptor to the other. Then I thought that if I just share the socket_fd the things will work, but it didn't. I need something like this: On dccp_server_sink_element: 1 - sock_fd = socket(...); // sock_id == 10 (for instance), sink tell dccp_server_src_element this value 2 - bind(sock_fd); 3 - listen(sock_fd); On dccp_server_src_element: 1 - sock_fd = 10 // for the first version I will just pass this value via set property, but after I will use unix_socket (as suggested by Arnaldo) to communicate both elements 2 - wrote = send(sock_fd, ...); In the client side (similar to server side: On dccp_client_sink_element: 1 - sock_fd = socket(...); // sock_id == 5 (for instance) 2 - connect(sock_fd); 3 - send(sock_fd); On dccp_client_src_element: 1 - read(sock_fd) // passed by dccp_client_sink_element. Is there any additional manipulation to enable the sock_fd sharing. The error show is Invalid File Descriptor when I try to use it in the srcs elements. Thanks, Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 25/25] Final adjustments to ccid3.c, ccid4.c and lib/tfrc_ccids.c
2007/11/1, Tommi Saviranta [EMAIL PROTECTED]:
On Wed, Oct 31, 2007 at 21:32:10 -0300, Leandro wrote:
[CCID-3/4] Final adjustments to ccid3.c, ccid4.c and lib/tfrc_ccids.c
According to the chosen strategy explained in [PATCH 15/25], some
defines become unnecessary. This patch removes them and adjusts
ccid3.c, ccid4.c and lib/ccid34_lib.c to use tfrc_ccids definitions
directly.
Signed-off-by: Leandro Melo de Sales [EMAIL PROTECTED]
Tabs have transformed into spaces again, probably thanks to copy-paste.
I will queue this for the next patchset. Thanks
-static inline void ccid4_update_send_interval(struct ccid4_hc_tx_sock
*hctx)
+static inline void ccid4_update_send_interval(struct tfrc_hc_tx_sock *hctx)
{
/* Calculate new t_ipi = s / X_inst (X_inst is in 64 *
bytes/second).
* TFRC-SP enforces a minimum interval of 10 milliseconds. */
-hctx-ccid4hctx_t_ipi =
- max_t(u32, scaled_div32(((u64)hctx-ccid4hctx_s) 6,
- hctx-ccid4hctx_x), MIN_SEND_RATE);
+hctx-tfrchctx_t_ipi =
+ max_t(u32, scaled_div32(((u64)hctx-tfrchctx_s) 6,
+ hctx-tfrchctx_x), MIN_SEND_RATE);
-static inline void ccid4_hc_tx_x_header_penalty(struct ccid4_hc_tx_sock
*hctx)
+static inline void ccid4_hc_tx_x_header_penalty(struct tfrc_hc_tx_sock
*hctx)
{
-hctx-ccid4hctx_x *= hctx-ccid4hctx_s;
-do_div(hctx-ccid4hctx_x, (hctx-ccid4hctx_s + CCID4HCTX_H));
+hctx-tfrchctx_x *= hctx-tfrchctx_s;
+do_div(hctx-tfrchctx_x, (hctx-tfrchctx_s + CCID4HCTX_H));
}
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/25] Share ccid3_hc_rx_sock struct and ccid3_hc_rx_sk function via tfrc_ccids
2007/11/1, Gerrit Renker [EMAIL PROTECTED]: I think we need to get back to deciding on a naming scheme for the structs. It is not your fault, but with the current scheme the names become extremely cryptic: | + * @tfrchcrx_last_counter - Tracks window counter (RFC 4342, 8.1) | + * @tfrchcrx_state - Receiver state, one of %tfrc_hc_rx_states | + * @tfrchcrx_bytes_recv - Total sum of DCCP payload bytes | + * @tfrchcrx_x_recv - Receiver estimate of send rate (RFC 3448, sec. 4.3) | + * @tfrchcrx_rtt - Receiver estimate of RTT | + * @tfrchcrx_last_feedback - Time at which last feedback was sent | + * @tfrchcrx_hist - Packet history, exported by TFRC module | + * @tfrchcrx_li_hist - Loss Interval database, exported by TFRC module | + * @tfrchcrx_s - Received packet size in bytes | + * @tfrchcrx_pinv - Inverse of Loss Event Rate (RFC 4342, sec. 8.5) | + */ Similar thing with the #defines - my eyes fail to parse this: | #define ccid3_hc_tx_sk tfrc_hc_tx_sk | | +#define ccid3_hc_rx_sock tfrc_hc_rx_sock | +#define ccid3hcrx_s tfrchcrx_s | +#define ccid3hcrx_x_recv tfrchcrx_x_recv | +#define ccid3hcrx_pinv tfrchcrx_pinv | +#define ccid3hcrx_last_feedback tfrchcrx_last_feedback | +#define ccid3hcrx_bytes_recv tfrchcrx_bytes_recv | +#define ccid3hcrx_last_counter tfrchcrx_last_counter | +#define ccid3hcrx_rtt tfrchcrx_rtt | +#define ccid3hcrx_s tfrchcrx_s | +#define ccid3hcrx_hist tfrchcrx_hist | +#define ccid3hcrx_li_hist tfrchcrx_li_hist My suggestion is to use something like * `ttx_' for TFRC TX related structs/operations/fields * `trx_' for RX * Arnaldo's scheme for the CCIDs -- c3rx_ / c4rx_ for RX structs/fields -- c3tx_ / c4tx_ for TX structs/fields Of course that costs work. But now is probably the best time to change it - so better get it cleaned up before passing it on to Arnaldo. I totally agree with you. I will update the patches. Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/25] Initial lib for sharing common code between ccid-3/ccid-4
Just forward a private e-mail with Tommi about his comments. 2007/11/1, Tommi Saviranta [EMAIL PROTECTED]: On Wed, Oct 31, 2007 at 21:30:24 -0300, Leandro wrote: [CCID-3/4 lib] Initial lib for sharing common code between ccid-3/ccid-4 The lib is called tfrc_ccids.o Index: leandro.new/net/dccp/ccids/lib/Makefile === --- leandro.new.orig/net/dccp/ccids/lib/Makefile +++ leandro.new/net/dccp/ccids/lib/Makefile @@ -1,4 +1,5 @@ obj-$(CONFIG_IP_DCCP_TFRC_LIB) += dccp_tfrc_lib.o dccp_tfrc_lib-y := tfrc_module.otfrc_equation.o \ - packet_history.o loss_interval.o + packet_history.o loss_interval.o \ +tfrc_ccids.o Whitespace police hits! I don't remember Documentation/CodingStyle mentioning anything about whitespaces in Makefiles. Nevertheless, packet_history.o is indented here with spaces, tfrc_ccids.o with tabulators, I think we should stick to just one kind of whitespace. - packet_history.o loss_interval.o + packet_history.o loss_interval.o \ + tfrc_ccids.o From Documentation/CodingStyle: Tabs are 8 characters, and thus indentations are also 8 characters. There are heretic movements that try to make indentations 4 (or even 2!) characters deep, and that is akin to trying to define the value of PI to be 3. (...) In short, 8-char indents make things easier to read, and have the added benefit of warning you when you're nesting your functions too deep. Heed that warning. So, the kind that we are looking for it to use tabs (8 chars). Since tfrc_ccids.o I used tabs, I think that it is ok. Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/25] Towards CCID3/CCID4 code integration
Hi Gerrit,
based on conversations with you, Arnaldo and Ian, please consider
the following patches related to ccid-4 initial implementation and
code share with ccid-3. Also, please reset the ccid4 branch to have
the latest dccp branch code before apply these patches series. It
would be interesting if you merge the patches related to code share
between ccid-3 and ccid-4 into the dccp branch to eliminate some of
our job and reduce (or even avoid) code repetition, in case that
ccid-3 is changed.
Some considerations about the criterious that I used to share a
function/struct/consts between ccid-3 and ccid-4.
1 - Certify whether the function is common (checking just the
functions in ccid-4 copied from ccid-3);
2 - Certify whether both functions (in ccid-3 and ccid-4) still
common; (Me or Tommi can already changed it for ccid-4)
3 - Certify whether the both functions doesn't call a ccid specific function;
i.e: ccid3_hc_tx_packet_sent could be shared, but it calls
ccid3_hc_tx_update_s, which is a ccid-3 specific function. Then
ccid3_hc_tx_packet_sent is not a candidate to be shared between
ccid-{3/4}.
i.e: ccid3_hc_rx_packet_recv
i.e: ccid3_hc_rx_insert_options
4 - Certify whether the function has a low probability to be changed
for ccid-4 implementation in the future;
i.e 1: Albeit ccid3_hc_tx_send_packet could be common, ccid-4 will
probably have a particular way to decide when send a packet
i.e 2: ccid4_first_li will certainly change because the short loss
interval length is calculated as N/K, instead of using just N
(ccid3_first_li).
i.e 3: ccid3_hc_tx_parse_options could be common, but until the
IETF folks do not decide if DROPPED_PACKET_OPTION will be into the
ccid-3, I decided to NOT share it.
5 - Common sense: functions such as ccid3_hc_tx_init and
ccid3_hc_tx_exit are very particular, probably we will want to have
specific procedures to go into ccid-3, different to ccid-4 and
vice-versa.
Patch #1: [CCID-4] Changes to Kconfig and Makefile to add ccid-4 code;
Patch #2: [CCID-4] New ccid-4 based on ccid-3 source code;
Patch #3: [CCID-4] Adjustments of the ccid-4 copyright and kernel
module authors;
Patch #4: [CCID-4] Set packet size to 1460 as per ccid-4 draft;
Patch #5: [CCID-4] Enforces a minimum interval of 10 milliseconds as
per CCID-4 draft;
Patch #6: [CCID-4] Reduce allowed sending rate by a factor that
accounts for packet header size;
Patch #7: [CCID-3/4] Initial lib for sharing common code between ccid-3/ccid-4;
Patch #8: [CCID-3/4] Share common header files and defines between
ccid-3/ccid-4;
Patch #9: [CCID-4] Adapt CCID-4 according to the latest changes to CCID-3;
Patch #10: [CCID-3/4] Share ccid-3 options enum and struct via ccid34_lib;
Patch #11: [CCID-3/4] Share TFRC sender states via ccid34_lib;
Patch #12: [CCID-3/4] Share TFRC receiver states via ccid34_lib;
Patch #13: [CCID-3/4] Share TFRC feedback types struct via ccid34_lib;
Patch #14: [CCID-4] Basic implementation for ccid-4 dropped packet
option as per ccid-4 draft;
Patch #15: [CCID-3/4] Share ccid3_hc_tx_sock struct and ccid3_hc_tx_sk
function via ccid34_lib;
Patch #16: [CCID-3/4] Share ccid3_hc_rx_sock and ccid3_hc_rx_sk via ccid34_lib;
Patch #17: [CCID-3/4] Share ccid3_tx_state_name function via ccid34_lib;
Patch #18: [CCID-3/4] Share rfc3390_initial_rate function via ccid34_lib;
Patch #19: [CCID-3/4] Share ccid3_hc_tx_idle_rtt function via ccd34_lib;
Patch #20: [CCID-3/4] Share ccid3_hc_tx_update_win_count function via
ccid34_lib;
Patch #21: [CCID-3/4] Share ccid3_rx_state_name function via ccid34_lib;
Patch #22: [DCCP] Minor adjustments to probe.c to use ccid34_lib;
Patch #23: [CCID-4] Include ccid-4 to be visible for the DCCP feature
negotiation mechanism;
Patch #24: [DCCP] Final adjustments to probe.c to use ccid34_lib;
Patch #25: [CCID-3/4] Final adjustments to ccid3.c, ccid4.c and
lib/ccid34_lib.c.
Leandro.
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gstreamer e DCCP: issues related to ccid-3
2007/10/25, Ian McDonald [EMAIL PROTECTED]:
On 10/25/07, Gerrit Renker [EMAIL PROTECTED] wrote:
/* something like (but do have a look at the paraslash sources)
do {
wrote = send (socket, (const char *) buf + bytes_written,
count - bytes_written, MSG_NOSIGNAL);
} while (wrote 0 errno == EAGAIN);
*/
You can also look at iperf code, ttcp code etc.
--
Web1: http://wand.net.nz/~iam4/
Web2: http://www.jandi.co.nz
Blog: http://iansblog.jandi.co.nz
Did you implemented a similar solution in the code that you sent to
me? I didn't have enough time to check your code because I was busy on
the ccid-4 patches.
Leandro.
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Gstreamer e DCCP: issues related to ccid-3
Can you please revise to me the problem that you and the VLC guy had
while was implement DCCP support on VLC?
I'm implementing the dccp plugin for gstreamer and everything goes
very well, I can transmit an mp3 sound using ccid-2, but when I try to
use ccid-3, I can just send very little via send function and after
the function returns 11 (EAGAIN). Is there any different parameter
that I should set? I think that the problem that you had in VLC should
be the same for me and in this case you help me. I have been working
with Arnaldo on this stuffs but I decided to make it public sharing
this problem to all of you and get some help.
Basically the functions that I'm using to send are the following:
PS: the second function, gst_dccp_send_buffer, is a call back function
called by gstreamer when it has data to be sent. Thank you.
/* write buffer to given socket incrementally.
* Returns number of bytes written.
*/
gint
gst_dccp_socket_write (int socket, const void *buf, size_t count)
{
size_t bytes_written = 0;
ssize_t wrote;
while (bytes_written count) {
wrote = send (socket, (const char *) buf + bytes_written,
count - bytes_written, MSG_NOSIGNAL);
if (wrote = 0) {
return bytes_written;
}
bytes_written += wrote;
}
if (bytes_written 0)
GST_WARNING (error while writing);
else
GST_LOG (wrote % G_GSIZE_FORMAT bytes succesfully, bytes_written);
return bytes_written;
}
/* write a GDP header to the socket. Return false if fails. */
GstFlowReturn
gst_dccp_send_buffer (GstElement * this, GstBuffer * buffer, int socket)
{
size_t wrote;
gint size = 0;
guint8 *data;
size = GST_BUFFER_SIZE (buffer);
data = GST_BUFFER_DATA (buffer);
GST_LOG_OBJECT (this, writing %d bytes for GDP buffer header\n, size);
printf(writing %d bytes for GDP buffer header\n\n, size);
wrote = gst_dccp_socket_write (socket, data, size);
//g_free (data);
if (wrote != size) {
GST_ELEMENT_ERROR (this, CORE, TOO_LAZY, (NULL),
(Error while sending data));
printf(Error while sending data\n);
return GST_FLOW_ERROR;
}
return GST_FLOW_OK;
}
-
To unsubscribe from this list: send the line unsubscribe dccp in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][DCCP 1/1] Implement SIOCINQ/FIONREAD
Em 23/10/07, Ian McDonald[EMAIL PROTECTED] escreveu: On 10/24/07, Arnaldo Carvalho de Melo [EMAIL PROTECTED] wrote: Just like UDP. Signed-off-by: Arnaldo Carvalho de Melo [EMAIL PROTECTED] Signed-off-by: Leandro Melo de Sales [EMAIL PROTECTED] Signed-off-by: Ian McDonald [EMAIL PROTECTED] Signed-off-by: Leandro Melo de Sales [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC]: field name identifier conventions
2007/10/20, Arnaldo Carvalho de Melo [EMAIL PROTECTED]: Em Sat, Oct 20, 2007 at 07:03:49PM -0200, Arnaldo Carvalho de Melo escreveu: Em Sat, Oct 20, 2007 at 04:54:35PM +0100, Gerrit Renker escreveu: I have a question/suggestion for DCCP/CCID field names, which have a tendency to grow into really_quite_long_strings. The convention for field members seems to be x-x_fieldname Examples are in particular * hctx-ccid2hctx_fieldname * hcrx-ccid3hcrx_fieldname * avr-dccpavr_fieldname * av-dccpav_fieldname The problem is that this naming convention has no apparent benefits: * which struct is used is evident from the context and need not be encoded * someone reading the code is only interested in the fieldnames * with the line length limit of 80 characters this convention almost inevitably leads to multi-line expression for even the simplest kinds of comparisons and expressions. Hence my suggestion is to reduce the replicated x field prefix, so that field members become shorter, as will be expressions, and the code would be easier to read. What is the opinion of other developers / maintainer regarding this? Well, while I agree that the names being overly long its a nuisance at the same time being able to grep for some specific field is really nice and was the intention. Long ago DaveM accepted patches for struct sock, but then there its just -sk_FIELD_NAME, not 10 characters like in the ccids case. struct inode fields predate that even, but then its just -i_FIELD_NAME. I think that providing a unique namespace for struct member names is ok if it doesn't get that long (mea culpa in the aforementioned cases 8)). Forgot to leave a suggestion for the mentioned case: -c2tx_ perhaps :) - Arnaldo P.S.: What we really needed was more intelligent grep, ctags replacements for quickly locating such information in C source files, perhaps one that could understand types and then could allow developers to ask questions like show me all the places where the field foo of type bar appears, but till then the old practice, albeit not uniformly used of using a namespace alias in the form of a prefix seems to suit us :-) - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html I think the arnaldo comments sounds appropriate. Particular for me, the current nomenclature helped me in the fork from ccid3 to ccid4. While reviewing the code (or search for something specific, like arnaldo commented out) just having hctx-last_counter instead of hctx-ccid4hcrx_last_counter in some cases will make us have to backward to a certain parts of the code in order to understand if the hctx was returned by a call to ccid3_hc_rx_sk() function, to the ccid4_hc_rx_sk() function or by the new function that I will suggest to you soon named ccid34_hc_rx_sk(), which will be a shared function used by both, ccid3 and ccid4. Well, I don't know if my comments make sense or not, mainly because I'm new in the dccp hacking, but this is my opinion. What about follow the arnaldo suggestion, have something like hctx-c4_last_counter rather than hctx-ccid4hcrx_last_counter. In this case, if we need to make a search for foo of type bar, we can search for the string tx-c4, or rx-c4 (just a very specific example): # grep -r tx-c4 * Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: CONFLICT using git-pull
FYI, I started to use git-gui and it seems to be very helpful, but I'm still using quilt/gquilt to finish the patches series for ccid3/ccid4 integration. 2007/9/20, Gerrit Renker [EMAIL PROTECTED]: | You said that will define a specific frequency | (for instance, every week) that you will do this. Did you already | define it? The repository is synced normally every working day from Mon .. Fri, based on the davem-2.6 tree - not the one used for the merge windows. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
2007/10/16, Mojtaba Hosseini [EMAIL PROTECTED]: Hello, I've recently started using DCCP as an alternative to our UDP-based implementation for point to point videoconferencing application. I had to struggle a bit to have it working. To save the next newbie the same trouble, I've written a small How to use DCCP tutorial in wiki but what is the best way of sharing this with everyone? Include it as an attachment here? I look forward to your suggestions. Mojtaba Hosseini - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Hi, as I suggested through #dccp-linux at freenode, I think you can add this in the dccp linux wiki at http://linux-net.osdl.org/index.php/DCCP. Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
DCCP feature negotiation via setsockopt/getsockopt
Well, I'm very closer to release the first version of dccp plugin for gstreamer and I want to ask you something related to feature negotiation, which gerrit recently submitted patches related to this last week... In the gstreamer plugin I want to provide a plugin property named ccid, where the programmer will set or get the current ccid. Is dccp implementation for linux already prepared to support this mechanism through feature negotiation (or something related via setsockopt/getsockopt)? Leandro. - To unsubscribe from this list: send the line unsubscribe dccp in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
