Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ridale/lwip #13

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

ridale/lwip #13

wants to merge 6 commits into from

Conversation

ridale
Copy link
Contributor

@ridale ridale commented Apr 8, 2020

This project uses the ethdriver global component and liblwip to provide a UDP echo server.

It was taken, in part from the camkes-apps-ethernet-demo-x86--devel but currently only works with EthdriverARMPlat as I pulled the x86 camkes #defs during debugging.

It would be reasonable to push the eth_interface.c code into a lwipserver similar to the picoserver global component. I could not however figure out a clean interface for binding traffic handlers to the interface between an app component and the any global lwipserver component.

I am happy to be directed on how this should look.

Signed-off-by: Richard Lemon <[email protected]>
Signed-off-by: Richard Lemon <[email protected]>
Signed-off-by: Richard Lemon <[email protected]>
@kent-mcleod
Copy link
Member

Hi @ridale, thanks for raising the PR. Is there an expected way for providing <lwip/lwipopts.h> to the server component?

I could not however figure out a clean interface for binding traffic handlers to the interface between an app component and the any global lwipserver component.

We could leave this as an example of an application that links an lwip instance into itself. Then you could use it in a system where the ethdriver component presents multiple ethernet MAC adresses with each client component having a separate, isolated networking stack?

@ridale
Copy link
Contributor Author

ridale commented Apr 21, 2020

Hi @kent-mcleod sorry about the delay been working on getting the USB stack to work on sabre.
I put an lwipopts.h that is generic and minimal into the lwip src/include directory for my work, in https://github.com/ridale/util_libs/tree/ridale/lwip

happy to leave this as is as an example.


static void malloc_dma_unpin(void *cookie, void *addr, size_t size)
{
}
Copy link
Member

Choose a reason for hiding this comment

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

For empty functions, pleas add a comment in the body that they are empty on purpose. So it's clear the contain was not lonst somewhere during merging or rebasing or. The parameters should also be flagged as unused to silence compiler warnings.

@@ -0,0 +1,43 @@
#include <camkes.h>
Copy link
Member

Choose a reason for hiding this comment

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

Please add a header with copyright infos (author, license).

/* Start the timer for the TCP stack */
timer_periodic(0, NS_IN_MS * LWIP_TICK_MS);
}
/* Callback that gets called when the timer fires. */
Copy link
Member

Choose a reason for hiding this comment

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

please add an empty line before the comment

{
int retval = 0;
return retval;
}
Copy link
Member

Choose a reason for hiding this comment

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

please add a a line break at the end of the file

@ridale
Copy link
Contributor Author

ridale commented Aug 20, 2021

Sorry this is an old pull request that I am getting to work again with the newer version of seL4 and probably doesn't warrant reviewing at the moment. It builds but I have not tested it on hardware and will not be able to for some time.

If you would like you can close this pull request and I will reopen it if I get a chance to look at it again.

I understand from one of your issue reports seL4/util_libs#68 that you would like to pull lwip support, there are the same problems with cached dma in the camkes global component code as well. I think that is one of my other pull requests.

@lsf37 lsf37 marked this pull request as draft August 20, 2021 06:44
@lsf37
Copy link
Member

lsf37 commented Aug 20, 2021

I've set the PR as draft to mark that you're intending further work, so we can leave it open for now.

# the BSD 2-Clause license. Note that NO WARRANTY is provided.
# See "LICENSE_BSD2.txt" for details.
#
# @TAG(DATA61_BSD)
Copy link
Member

@axel-h axel-h Mar 8, 2022

Choose a reason for hiding this comment

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

This is obsolete, please use:

Suggested change
# @TAG(DATA61_BSD)
# SPDX-License-Identifier: BSD-2-Clause

Copy link
Member

Choose a reason for hiding this comment

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

In this case (because it is "ours") the rest of the header should be collapsed as well into just the copyright line and SPDX identifier like in the rest of the sources.

* the BSD 2-Clause license. Note that NO WARRANTY is provided.
* See "LICENSE_BSD2.txt" for details.
*
* @TAG(DATA61_BSD)
Copy link
Member

@axel-h axel-h Mar 8, 2022

Choose a reason for hiding this comment

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

This is obsolete, please use:

Suggested change
* @TAG(DATA61_BSD)
* SPDX-License-Identifier: BSD-2-Clause

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants