Skip to content

gnrc/6lo: Set more data flag on all but last fragment#9471

Merged
miri64 merged 2 commits intoRIOT-OS:masterfrom
jnohlgard:pr/netif-6lo-more-data
Aug 14, 2018
Merged

gnrc/6lo: Set more data flag on all but last fragment#9471
miri64 merged 2 commits intoRIOT-OS:masterfrom
jnohlgard:pr/netif-6lo-more-data

Conversation

@jnohlgard
Copy link
Copy Markdown
Member

Contribution description

Use the flag introduced in #9469 to signal to the link layer that more data will follow this packet on outgoing fragments, except for the final frag of the datagram.

Issues/PRs references

Depends on #9469
Does not do much difference without also merging #9470

@jnohlgard jnohlgard added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking State: waiting for other PR State: The PR requires another PR to be merged first GNRC CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 30, 2018
@jnohlgard jnohlgard added this to the Release 2018.07 milestone Jun 30, 2018
@jnohlgard jnohlgard requested a review from miri64 June 30, 2018 16:12
@jnohlgard jnohlgard force-pushed the pr/netif-6lo-more-data branch from 45ed8c0 to 3eb6ccc Compare June 30, 2018 16:49
@jnohlgard
Copy link
Copy Markdown
Member Author

With ContikiMAC (unpublished) this improves network latency significantly for fragmented unicast packets.

@PeterKietzmann
Copy link
Copy Markdown
Member

The change seems reasonable although I'd like to get @miri64's feedback here. Furthermore, I wanted to reference this mail as a reminder to ongoing fragmentation efforts.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jul 2, 2018

The reassembly/fragmentation work did not start yet (see lacking replies to the mail you referenced). However, I'm currently refactoring 6Lo with the goal to get Fragmentation and IPHC more separated (see https://github.com/RIOT-OS/RIOT/projects/18 for progress). I don't think however, that that much collides with that effort and should be easily rebasable when it comes to conflicts.

@PeterKietzmann
Copy link
Copy Markdown
Member

I don't think however, that that much collides with that effort and should be easily rebasable when it comes to conflicts.

Fine, let's wait for #9469 and #9470 then. I can do testing afterwards and we go with this PR as well (if everything works as expected).

@jnohlgard jnohlgard force-pushed the pr/netif-6lo-more-data branch from 3eb6ccc to 13ce688 Compare August 13, 2018 07:27
@PeterKietzmann PeterKietzmann removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 13, 2018
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Aug 13, 2018
@miri64
Copy link
Copy Markdown
Member

miri64 commented Aug 13, 2018

Can you please rebase.

@jnohlgard jnohlgard force-pushed the pr/netif-6lo-more-data branch from 13ce688 to 938f74f Compare August 14, 2018 07:58
@jnohlgard
Copy link
Copy Markdown
Member Author

rebased

Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good so far. I still want to test this though.

#endif

/* Check weater to send the first or an Nth fragment */
/* Check whether to send the first or an Nth fragment */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put this spelling fix in a separate commit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@jnohlgard jnohlgard force-pushed the pr/netif-6lo-more-data branch from 938f74f to 3cc0886 Compare August 14, 2018 08:09
@miri64
Copy link
Copy Markdown
Member

miri64 commented Aug 14, 2018

6lo: Speling correction in comment

Spelling 😆

@jnohlgard
Copy link
Copy Markdown
Member Author

6lo: Speling correction in comment

Spelling 😆

intentional ;)

@miri64
Copy link
Copy Markdown
Member

miri64 commented Aug 14, 2018

Okay 😀

Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, fragmentation still works with various datagram sizes, and my sniff confirms that the "Frame pending" flag is now set for all fragments of a datagram, but the last.

@miri64 miri64 merged commit 914f320 into RIOT-OS:master Aug 14, 2018
@jnohlgard jnohlgard deleted the pr/netif-6lo-more-data branch August 20, 2018 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants