Memory manager for unified EMAC #5750

mikaleppanen · 2017-12-21T10:35:48Z

Description

READY FOR REVIEW

EMAC memory manager changes.

Status

READY FOR REVIEW

Migrations

YES

#5558

Related PRs

List related PRs against other branches:

branch	PR
feature-emac	#5558

Todos

Tests
Documentation

Deploy notes

Steps to test or reproduce

mikaleppanen · 2017-12-21T10:37:21Z

@kjbracey-arm here's the memory manager and K64F/STM driver changes.

kjbracey · 2017-12-21T10:49:16Z

To maybe slightly reduce confusion, this PR should be against feature-emac rather than master. Not sure if you can change that after creation?

mikaleppanen · 2017-12-21T11:00:33Z

Changed the base..... since this branch is based on current master it is ahead of the feature-emac.

kjbracey · 2017-12-21T10:51:26Z

features/FEATURE_LWIP/lwip-interface/LWIPMemoryManager.h

+     * @param mem      Memory buffer
+     * @return         Total length in bytes
+     */
+    virtual uint32_t get_total_len(emac_mem_buf_t *buf);


Let's have some const-correctness - this should be a const buf pointer, and the method should be const.

Should all the methods be const apart from free and alloc - if they only touch the buffer, they can't be changing the memory manager's overall state right? I suppose anything that modifies a buffer could be changing housekeeping info. So maybe const methods if they take const buf pointers.

kjbracey · 2017-12-21T10:57:23Z

features/FEATURE_LWIP/lwip-interface/LWIPMemoryManager.h

+     * @param mem      Memory buffer
+     * @return         The next memory buffer, or NULL if last
+     */
+    virtual emac_mem_buf_t *next(emac_mem_buf_t *buf);


Again, const, and same for the others. Should this be get_next to match the below?

kjbracey · 2017-12-21T11:00:08Z

features/FEATURE_LWIP/lwip-interface/LWIPMemoryManager.cpp

+        pbuf->len -= offset;
+    }
+
+    return (emac_mem_buf_t*)pbuf;


Could you make all the casting here C++ static_cast? Would help enforce the const-correctness I'm calling for elsewhere.

kjbracey · 2017-12-21T11:03:29Z

features/netsocket/EMACMemoryManager.h

+ * Chaining of the buffers is made using singly linked list.
+ *
+ * Memory buffers can be allocated either from heap or from memory pools. Heap buffers are always contiguous.
+ * Memory pool buffers may be either contiguous or chained depending on allocation size.


I think we need to distinguish what you call "buffer" here from the "emac_mem_buf_t". I would suggest you use "buffer" and "buffer chain", and say whether calls work on a buffer or an entire chain. Then the EMAC API passes a chain up or down.

mikaleppanen · 2017-12-21T12:28:34Z

Updated commits based on comments.

0xc0170 · 2017-12-21T15:19:42Z

@mikaleppanen destination branch updated, not certain why commits are still shown here that are already there

mikaleppanen · 2017-12-22T06:56:06Z

Thanks, I changed base branch to master and back to feature-emac and the github updated the commit list.

0xc0170 · 2017-12-22T09:50:29Z

Please review CI (travis and jenkins) both report failures (travis docs and jenkins compiler error)

mikaleppanen · 2018-01-05T10:37:37Z

Updated lwip memory manager to support pool allocation alignment. Updated K64F and STM ethernet drivers.

0xc0170 · 2018-01-05T14:59:32Z

@mikaleppanen Can you rebase this PR, I rebased the branch to resolve some issues that were affecting this PR

mikaleppanen · 2018-01-05T15:17:55Z

@0xc0170 I rebased the PR

0xc0170 · 2018-01-05T15:40:54Z

Travis - docs failure (doxygen needs fixes), and jenkins CI - can't find a header file

0xc0170 · 2018-01-08T08:43:50Z

Please review the jenkins failure (one device fails to connect, the other 2 are regarding missing declarations)

mikaleppanen · 2018-01-09T08:52:50Z

Updated commits. Looks like only cellular MTS_DRAGONFLY_F411RE is failing. @kjbracey-arm is it expected that with the current changes the cellular fails, and it will be updated later?

mikaleppanen · 2018-01-09T11:12:48Z

Updated also cellular for MTS_DRAGONFLY_F411RE . Now that is also building. Rest of the build errors are from drivers that are not ported yet to new emac interface.

mikaleppanen · 2018-01-11T14:12:41Z

Added a commit that adds set preferred alignment, copy to buffer and copy from buffer methods to memory manager.

cmonr · 2018-01-11T16:54:56Z

@mikaleppanen Let us know when this is ready for review by modifying the initial comment. Thanks!

kjbracey · 2018-01-12T07:52:09Z

Mika and I suggest we fold these changes into #5558 for combined review. (The first 4 commits visible here are from #5558). Any objections?

mikaleppanen · 2018-01-30T08:58:16Z

@0xc0170 @kjbracey-arm @SeppoTakalo This pull request is ready for review.

mikaleppanen · 2018-02-09T07:52:27Z

.mbedignore with named files seems not to work gnuarmeclipse exporter, so changed the ublox wifi sdk directory .mbedignore to just contain asterisk. Now it should generate the eclipse project correctly.

0xc0170 · 2018-02-09T11:31:35Z

/morph build

mbed-ci · 2018-02-09T12:00:53Z

Build : SUCCESS

Build number : 1099
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5750/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2018-02-09T13:05:36Z

Exporter Build : SUCCESS

Build number : 777
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5750/

mbed-ci · 2018-02-09T13:49:36Z

Test : SUCCESS

Build number : 907
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5750/907

0xc0170 · 2018-02-09T14:02:31Z

/morph uvisor-test

0xc0170 · 2018-02-09T14:03:57Z

We might need to rebase (uvisor json CI file was recently updated to resolve the errors we are seeing here). Please review after another CI run (I've retriggered it just now)

cmonr · 2018-02-09T16:15:49Z

@0xc0170 Correct, rebase is needed for uVisor CI update.

instead of old memory interface.

mikaleppanen · 2018-02-12T08:41:18Z

Rebased the branch.

0xc0170 · 2018-02-12T09:50:17Z

/morph build

mbed-ci · 2018-02-12T11:27:06Z

Build : SUCCESS

Build number : 1114
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5750/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2018-02-12T14:37:34Z

Exporter Build : SUCCESS

Build number : 792
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5750/

mbed-ci · 2018-02-13T01:55:19Z

Test : SUCCESS

Build number : 929
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5750/929

mikaleppanen changed the base branch from master to feature-emac December 21, 2017 10:56

kjbracey reviewed Dec 21, 2017

View reviewed changes

0xc0170 added the needs: review label Dec 21, 2017

mikaleppanen changed the base branch from feature-emac to master December 22, 2017 06:54

mikaleppanen changed the base branch from master to feature-emac December 22, 2017 06:54

0xc0170 added needs: work and removed needs: review labels Dec 22, 2017

kjbracey mentioned this pull request Jan 15, 2018

Networking update: general refactoring, unifying EMAC #5558

Merged

kjbracey mentioned this pull request Jan 26, 2018

Nanostack EMAC implementation #5944

Merged

0xc0170 added the needs: preceding PR label Jan 30, 2018

cmonr added needs: review and removed needs: work labels Feb 1, 2018

cmonr added needs: work and removed needs: review labels Feb 8, 2018

kjbracey force-pushed the feature-emac branch from ee8191f to 5636fd9 Compare February 12, 2018 08:12

Mika Leppänen and others added 7 commits February 12, 2018 10:12

Created memory manager class to netsocket and updated lwip to use it

5e8d670

instead of old memory interface.

Updated K64F ethernet driver to use memory manager

20d9bff

Updated STM ethernet driver to use emac and memory manager

e09991f

Move STM EMAC driver out out lwIP

7219ca1

Added preferred alignment to emac and copy to/from to memory manager

9d3da68

K64F EMAC: Fix TX error path leaks

854ebd4

Enabled greentea and mbed-os-example-tls/client/sockets test for STM

b352e3b

0xc0170 added needs: CI and removed needs: work labels Feb 12, 2018

cmonr added ready for merge and removed needs: CI labels Feb 13, 2018

0xc0170 merged commit ed4bf78 into ARMmbed:feature-emac Feb 13, 2018

0xc0170 removed the ready for merge label Feb 13, 2018

kjbracey mentioned this pull request May 9, 2018

Merge feature-emac branch into master #6847

Merged

Memory manager for unified EMAC #5750

Memory manager for unified EMAC #5750

Uh oh!

Conversation

mikaleppanen commented Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Status

Migrations

Related PRs

Todos

Deploy notes

Steps to test or reproduce

Uh oh!

mikaleppanen commented Dec 21, 2017

Uh oh!

kjbracey commented Dec 21, 2017

Uh oh!

mikaleppanen commented Dec 21, 2017

Uh oh!

kjbracey Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikaleppanen commented Dec 21, 2017

Uh oh!

0xc0170 commented Dec 21, 2017

Uh oh!

mikaleppanen commented Dec 22, 2017

Uh oh!

0xc0170 commented Dec 22, 2017

Uh oh!

mikaleppanen commented Jan 5, 2018

Uh oh!

0xc0170 commented Jan 5, 2018

Uh oh!

mikaleppanen commented Jan 5, 2018

Uh oh!

0xc0170 commented Jan 5, 2018

Uh oh!

0xc0170 commented Jan 8, 2018

Uh oh!

mikaleppanen commented Jan 9, 2018

Uh oh!

mikaleppanen commented Jan 9, 2018

Uh oh!

mikaleppanen commented Jan 11, 2018

Uh oh!

cmonr commented Jan 11, 2018

Uh oh!

kjbracey commented Jan 12, 2018

Uh oh!

mikaleppanen commented Jan 30, 2018

Uh oh!

mikaleppanen commented Feb 9, 2018

Uh oh!

0xc0170 commented Feb 9, 2018

Uh oh!

mbed-ci commented Feb 9, 2018

Triggering tests

Uh oh!

mbed-ci commented Feb 9, 2018

Uh oh!

mbed-ci commented Feb 9, 2018

Uh oh!

0xc0170 commented Feb 9, 2018

Uh oh!

0xc0170 commented Feb 9, 2018

Uh oh!

cmonr commented Feb 9, 2018

Uh oh!

mikaleppanen commented Feb 12, 2018

Uh oh!

0xc0170 commented Feb 12, 2018

mikaleppanen commented Dec 21, 2017 •

edited

Loading

kjbracey Dec 21, 2017 •

edited

Loading