[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <1172573845.13609.12.camel@localhost.localdomain>
Date: Tue, 27 Feb 2007 11:57:25 +0100
From: Sergio Paracuellos <sparacuellos@...k-linux.com>
To: Kristian Evensen <kristrev@....uio.no>
Cc: netdev@...r.kernel.org, linux-net@...r.kernel.org
Subject: Re: Adding data to SKB - odd checksum errors
Hi,
This is because if you change the skb you must calcutale checksum for
tcp and ip headers changes again. For example:
struct iphdr *new_iph = NULL;
struct tcphdr *new_th = NULL;
/* copy skb to another skb with skb_copy */
new_skb = skb_copy_expand(skb, skb_headroom(skb), MY_SIZE, GFP_ATOMIC);
/* more bytes for buffer */
skb_put(new_skb, MY_SIZE);
/* now get pointer to the the new ip and tcp headers (should be the same
in the other skb */
new_th = new_skb->h.th;
new_iph = new_skb->nh.iph;
/* suppose I change the len */
new_ip_tot_len = htons(ntohs(new_iph->tot_len) + MY_SIZE);
new_iph->check = new_check(new_iph->tot_len ^ 0xFFFF, new_ip_tot_len,
iph->check)
where new_check can be something similar to:
static inline u_int16_t
new_check(u_int32_t oldvalinv, u_int32_t newval, u_int16_t oldcheck)
{
u_int32_t diffs[] = { oldvalinv, newval };
return csum_fold(csum_partial((char *)diffs, sizeof(diffs),
oldcheck^0xFFFF));
}
Am I in a mistake? I suppose this can be done in this way, but I am not
sure.
Good luck!
Cheers,
Sergio
El dom, 25-02-2007 a las 15:34 +0200, Kristian Evensen escribió:
> Hello,
>
> I am working on an algorithm to add data from the previous skb (on the
> queue) to the front of the current skb. This should be beneficial for a
> certain kind of TCP-traffic, and I am curious as to wether it will work
> or not.
>
> Currently I have implemented a small algorithm to copy the data that
> works most of the time. It is called from write_xmit (right before the
> while-loop) and performs a number of checks (does the skb fit in the
> window, does it have enought space - mostly the same as the
> retrans_try_collapse-function) before it copies the data. I first
> "allocate" new data at the back of the skb using skb_put, and if that is
> succsessfull I copy the new data (using first memmove to move the old
> data and then memcpy), update the seq-number of this skb and calculate a
> new checksum. The algorithm will (currently) not work with non-linear
> skbs. I have pasted the code at the bottom of this mail.
>
> The weird thing about this algorithm is that something will suddenly
> make it go wrong/it makes something else go wrong, and the packets that
> I send have the wrong TCP-checksum. I have looked around the code for
> any counters I might have missed or similar, but I cant find any. If I
> have understod skb's correctly, I shouldn't have to update any counters
> except skb->len (which put does) since I dont expand the SKB I only use
> the space already reserved for it.
>
> Does anyone have any ideas to what might be wrong or can spot any
> errors/misunderstandings in my code?
>
> Thanks,
> Kristian
>
> The code:
>
> This goes into write_xmit before the loop:
> if(sysctl_tcp_thin_aggressive_bundling && tcp_stream_is_thin(tp)){
> if(skb->prev != (struct sk_buff*) &(sk)->sk_write_queue
> && !(TCP_SKB_CB(skb)->flags & TCPCB_FLAG_SYN)
> && (skb_shinfo(skb)->nr_frags == 0 &&
> skb_shinfo(skb->prev)->nr_frags == 0)){
> tcp_trans_try_collapse2(sk, skb, tcp_current_mss(sk, 0))
> }
> }
>
> This is the code that copies the data:
> static int tcp_trans_try_collapse2(struct sock *sk, struct sk_buff *skb,
> int mss_now)
> {
> struct tcp_sock *tp = tcp_sk(sk);
>
> /* Make sure that this isnt refereced by somebody else
> */
> if(!skb_cloned(skb)){
> struct sk_buff *prev_skb = skb_copy(skb->prev, GFP_ATOMIC);
> int skb_size = skb->len, prev_skb_size = prev_skb->len;
> u16 flags = TCP_SKB_CB(prev_skb)->flags;
>
> /* Since this technique currently does not support SACK, I
> * return -1 if the previous has been SACK'd. */
> if(TCP_SKB_CB(prev_skb)->sacked & TCPCB_SACKED_ACKED){
> return -1;
> }
>
> /* Current skb is out of window. */
> if (after(TCP_SKB_CB(skb)->end_seq, tp->snd_una+tp->snd_wnd)){
> return -1;
> }
>
> /* Punt if not enough space exists in the first SKB for
> * the data in the second, or the total combined payload
> * would exceed the MSS.
> */
> if ((prev_skb_size > skb_tailroom(skb)) ||
> ((skb_size + prev_skb_size) > mss_now)){
> return -1;
> }
>
> /*To avoid duplicate copies.*/
> if(TCP_SKB_CB(skb)->seq <= TCP_SKB_CB(prev_skb)->seq)
> return -1;
>
> /*First, check I have enough room*/
> if(skb_tailroom(skb) < prev_skb->len)
> return -1;
>
> copy = skb_put(skb, prev_skb->len);
>
> if(copy){
> memmove(skb->data + prev_skb->len, skb->data, skb->len -
> prev_skb->len);
> memcpy(skb->data, prev_skb->data, prev_skb->len);
> TCP_SKB_CB(skb)->seq = TCP_SKB_CB(prev_skb)->seq;
> skb->csum = csum_partial(skb->data, skb->len, 0);
> }
>
> __kfree_skb(prev_skb);
> }
>
> return 1;
> }
> -
> To unsubscribe from this list: send the line "unsubscribe linux-net" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
“No quiero mandar en naide ni que me manden a mi. Me gusta vivir
errante, hoy aquí y mañana allí, y mi vida sigue adelante.”
Camarón
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists