All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin Hundebøll" <martin@hundeboll.net>
To: Antonio Quartulli <ordex@autistici.org>
Cc: The list for a Better Approach To Mobile Ad-hoc Networking
	<b.a.t.m.a.n@lists.open-mesh.org>
Subject: Re: [B.A.T.M.A.N.] [PATCHv3 2/3] batman-adv: Receive fragmented packets and merge
Date: Thu, 23 May 2013 07:07:49 +0200	[thread overview]
Message-ID: <519DA425.30708@hundeboll.net> (raw)
In-Reply-To: <20130521191011.GA3435@ritirata.org>

Hi Antonio,

Thanks for your carefull reviews!

On 2013-05-21 21:10, Antonio Quartulli wrote:
> On Tue, May 21, 2013 at 12:48:18PM +0200, Martin Hundebøll wrote:
>> +	/* Forward the fragment, if the merged packet would be too big to
>> +	 * be assembled.
>> +	 */
>> +	total_size = ntohs(packet->total_size) + ETH_HLEN;
>> +	if (total_size > neigh_node->if_incoming->net_dev->mtu) {
>
> I guess here you forgot to remove the ETH_LEN from the total_size computation.

True.

>> +		batadv_inc_counter(bat_priv, BATADV_CNT_FRAG_FWD);
>> +		batadv_add_counter(bat_priv, BATADV_CNT_FRAG_FWD_BYTES,
>> +				   skb->len + ETH_HLEN);
>
> and here should ETH_HLEN be counted in the amount of bytes sent?

I would say so, yes, since batadv_send_skb_packet() will add an ethernet 
header to the packet before transmitting it.

> Moreover we are always avoiding third person in kernel doc ("Returns" should be
> "Return" and so on..) and you should not put () (in the kernel doc).

Will do so and send another revision...

-- 
Kind Regards
Martin Hundebøll
Frederiks Allé 99, 1.th
8000 Aarhus C
Denmark

+45 61 65 54 61
martin@hundeboll.net

  reply	other threads:[~2013-05-23  5:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-21 10:48 [B.A.T.M.A.N.] [PATCHv3 0/3] Fragmentation version 2 Martin Hundebøll
2013-05-21 10:48 ` [B.A.T.M.A.N.] [PATCHv3 1/3] batman-adv: Remove old fragmentation code Martin Hundebøll
2013-05-21 10:48 ` [B.A.T.M.A.N.] [PATCHv3 2/3] batman-adv: Receive fragmented packets and merge Martin Hundebøll
2013-05-21 19:10   ` Antonio Quartulli
2013-05-23  5:07     ` Martin Hundebøll [this message]
2013-05-21 10:48 ` [B.A.T.M.A.N.] [PATCHv3 3/3] batman-adv: Fragment and send skbs larger than mtu Martin Hundebøll
2013-05-21 19:19   ` Antonio Quartulli
2013-05-23  5:38     ` Martin Hundebøll
2013-05-23  7:29       ` Antonio Quartulli
2013-05-23  8:46         ` Antonio Quartulli
2013-05-23  8:51 ` [B.A.T.M.A.N.] [PATCHv3 0/3] Fragmentation version 2 Antonio Quartulli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=519DA425.30708@hundeboll.net \
    --to=martin@hundeboll.net \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    --cc=ordex@autistici.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.