Skip to contentSkip to conversation

pull request net: batman-adv 2014-08-04

PATCH In review Antonio Quartulli 3 participants 5 messages Aug 4, 2014 – Aug 5, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

Hello David,

this is a pull request intended for net.

It just contains a patch by Sven Eckelmann that fixes the reception of out-of-order fragments. As explained in the commit message, the issue was due to a wrong assumption about hlist_for_each_entry() in batadv_frag_insert_packet().

Please pull or let me know of any problem.
*If possible, this patch should also be enqueued for stable*

Thanks a lot,
Antonio

The following changes since commit 4f933f414bf629852f361edf0fc5e765e3e78388:

r8152: add missing Makefile rule (2014-08-02 20:53:55 -0700)

are available in the git repository at:

git://git.open-mesh.org/linux-merge.git tags/batman-adv-fix-for-davem

for you to fetch changes up to 9d2155fb59e21f4a38838035d2a26a8a02fec16c:

batman-adv: Fix out-of-order fragmentation support (2014-08-04 16:05:43 +0200)

----------------------------------------------------------------
Fix out-of-order fragments reception.

----------------------------------------------------------------
Sven Eckelmann (1):
batman-adv: Fix out-of-order fragmentation support

 net/batman-adv/fragmentation.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
#1 lore raw

PATCH 1/1 batman-adv: Fix out-of-order fragmentation support

From: Sven Eckelmann <[email protected]>

batadv_frag_insert_packet was unable to handle out-of-order packets because it dropped them directly. This is caused by the way the fragmentation lists is checked for the correct place to insert a fragmentation entry.

The fragmentation code keeps the fragments in lists. The fragmentation entries are kept in descending order of sequence number. The list is traversed and each entry is compared with the new fragment. If the current entry has a smaller sequence number than the new fragment then the new one has to be inserted before the current entry. This ensures that the list is still in descending order.

An out-of-order packet with a smaller sequence number than all entries in the list still has to be added to the end of the list. The used hlist has no information about the last entry in the list inside hlist_head and thus the last entry has to be calculated differently. Currently the code assumes that the iterator variable of hlist_for_each_entry can be used for this purpose after the hlist_for_each_entry finished. This is obviously wrong because the iterator variable is always NULL when the list was completely traversed.

Instead the information about the last entry has to be stored in a different variable.

This problem was introduced in 9b3eab61754d74a93c9840c296013fe3b4a1b606 ("batman-adv: Receive fragmented packets and merge").

---
 net/batman-adv/fragmentation.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
net/batman-adv/fragmentation.c +7 -3
index f14e54a..022d18a 100644
@@ -128,6 +128,7 @@ static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node,
 {
 	struct batadv_frag_table_entry *chain;
 	struct batadv_frag_list_entry *frag_entry_new = NULL, *frag_entry_curr;
+	struct batadv_frag_list_entry *frag_entry_last = NULL;
 	struct batadv_frag_packet *frag_packet;
 	uint8_t bucket;
 	uint16_t seqno, hdr_size = sizeof(struct batadv_frag_packet);
@@ -180,11 +181,14 @@ static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node,
 			ret = true;
 			goto out;
 		}
+
+		/* store current entry because it could be the last in list */
+		frag_entry_last = frag_entry_curr;
 	}
 
-	/* Reached the end of the list, so insert after 'frag_entry_curr'. */
-	if (likely(frag_entry_curr)) {
-		hlist_add_after(&frag_entry_curr->list, &frag_entry_new->list);
+	/* Reached the end of the list, so insert after 'frag_entry_last'. */
+	if (likely(frag_entry_last)) {
+		hlist_add_after(&frag_entry_last->list, &frag_entry_new->list);
 		chain->size += skb->len - hdr_size;
 		chain->timestamp = jiffies;
 		ret = true;
Signature · 1 line
1.8.5.5
maintainer
#2 lore raw

Re: [PATCH] batman-adv: Fix out-of-order fragmentation support

Quoted 2 lines · On Monday 04 August 2014 17:47:01 Antonio Quartulli wrote:

This problem was introduced in 9b3eab61754d74a93c9840c296013fe3b4a1b606 ("batman-adv: Receive fragmented packets and merge").

This is not a commit id from the kernel tree but the one from your out-of-tree repository at http://git.open-mesh.org/batman-adv.git . 610bfc6bc99bc83680d190ebc69359a05fc7f605 seems to be the one of Martin's commit when it was applied in the kernel repository.

Kind regards,
Sven

#3 lore raw

Re: [PATCH] batman-adv: Fix out-of-order fragmentation support

Quoted 8 lines · Sven Eckelmann
Quoted 2 lines · On Monday 04 August 2014 17:47:01 Antonio Quartulli wrote:

This problem was introduced in 9b3eab61754d74a93c9840c296013fe3b4a1b606 ("batman-adv: Receive fragmented packets and merge").

This is not a commit id from the kernel tree but the one from your out-of-tree repository at http://git.open-mesh.org/batman-adv.git . 610bfc6bc99bc83680d190ebc69359a05fc7f605 seems to be the one of Martin's commit when it was applied in the kernel repository.

Thanks Sven!

David, I "fixed" the commit message and pushed the tag "batman-adv-fix-for-davem" again. If you pull now you will get the correct text.

new head is:
batman-adv-for-davem = d9124268d84a836f14a6ead54ff9d8eee4c43be5)

Cheers,

Signature · 1 line
Antonio Quartulli
maintainer
#4 lore raw
In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>
Date: Mon, 4 Aug 2014 17:47:00 +0200

Quoted 9 lines

this is a pull request intended for net.

It just contains a patch by Sven Eckelmann that fixes the reception of out-of-order fragments. As explained in the commit message, the issue was due to a wrong assumption about hlist_for_each_entry() in batadv_frag_insert_packet().

Please pull or let me know of any problem.
*If possible, this patch should also be enqueued for stable*

As you can see this didn't make it in time for the 3.16 release, I've pulled and queued this patch up for -stable.