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

Network TCP API is adhoc and makes arbitrary assumptions #2256

Closed
zephyrbot opened this issue Aug 25, 2016 · 11 comments
Closed

Network TCP API is adhoc and makes arbitrary assumptions #2256

zephyrbot opened this issue Aug 25, 2016 · 11 comments
Assignees
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@zephyrbot
Copy link
Collaborator

zephyrbot commented Aug 25, 2016

Reported by Paul Sokolovsky:

Some feedback on playing with network API, for a case of TCP:

#1.

Comment in net_context.c:net_context_tcp_init():

{code}
* If we are called by net_receive() first, then
* we are working as a server, if net_send() called
* us first, then we are the client.
*/
{code}

Sorry, but that's quite an arbitrary restriction. There's nothing wrong with a server which speaks to client first on connection, or client which waits data from server. Trying to lump up together who speaks first and server/client side is against established networking practices and can lead only to confusion.

#2:

The same net_context.c:net_context_tcp_init() tries to hide standard socket-like operations LISTEN, CONNECT behind adhoc facade. Why? uIP's interface is more high-level/familiar then than Zephyr's.

#3:
Native uIP API functions like tcp_connect() acquired "struct net_buf *buf" argument. Again, it's unclear why - standard socket API's "connect" operation doesn't involve any (application) data sending.

#4:

net_send()'s operation and return values are confusing. For example, given Zephyr's workflow, a client would create a context, and call net_send(). net_send() will call tcp_connect() eventually, and socket will be guaranteedly in "connection in progress" state, so net_send() will return -EINPROGRESS and put net_buf into the queue to be sent out eventually. So, now an application guaranteedly gets -EINPROGRESS on first call to net_send() and ends up with net_buf it has no idea what to do? Should it be unref'ed? Or not? How much is that useful? Probably not much, what's useful is: a) create socket; b) establish connection (either block until it's done, or be able to query state for non-blocking socket); c) proceed to send data. That's how standard socket paradigm works (as implemented by uIP too), and unfortunately not how Z networking works.

So, I don't know how useful is such feedback. My usecase: be able to write/port networking applications written in standard manner to Zephyr. I understand that a requirement for Zephyr might have been "provide simplified means of network programming", but when dealing with a well-established and already simple area like network programming (with sockets being prevalent paradigm), there's fine order between "simplification" and "deterioration".

So, for my usage, using underlying uIP API is more beneficial than Z's facade on top of it. But it's no going to easy due to Z's patching of classical uIP API. Now I'll be looking how to "undo" Z layer and get back to uIP classics.

Thanks for listening.

(Imported from Jira ZEP-741)

@zephyrbot
Copy link
Collaborator Author

zephyrbot commented Aug 26, 2016

by Flavio Santes:

I would like to add the following information:

#1 is partially covered by GH-2065. The use-case you describe is required to write the NATS client application. See http://goo.gl/3EUMmN (link to Gerrit). Currently, only this use-case is supported:

Client connects and sends something
Server receives and replies

The one you describe is:
Client connects
Server accepts connection and sends something back
Client receives and perhaps sends some data

#2 #3 are partially covered by GH-1987 and GH-2138
#4 GH-1981

Regards,
Flavio

@zephyrbot
Copy link
Collaborator Author

by Paul Sokolovsky:

Thanks for the response and references, Flavio. I have to say that I wrote this down because I knew that work on a new IP stack is in progress, and it would be quite unfortunate if "simplified" networking model was put into its foundation. Protocols where server speaks something first aren't uncommon indeed, e.g.:

$ telnet gerrit.zephyrproject.org 29418
Trying 199.19.213.246...
Connected to dev.zephyrproject.org.
Escape character is '^]'.
SSH-2.0-GerritCodeReview_2.12.3 (SSHD-CORE-0.14.0)

I.e. SSH protocol (as spoken by Zephyr's own Gerrit for example) starts with outputting server version.

I'll review the links provides, thanks!

@zephyrbot
Copy link
Collaborator Author

by Flavio Santes:

That's right, so far it is complicated to mimic that behavior.

AFAIK the new IP stack is not tied the same constrained collection of use-cases. It seems that now connect, listen and similar functions will be supported [1]. I am also interested in the situation you describe because I am working on high-level protocols, so I am facing the same issues you are reporting :)

Regards,
Flavio

[1] https://gerrit.zephyrproject.org/r/#/c/3753/23/include/net/yaip/net_context.h

@zephyrbot
Copy link
Collaborator Author

by Paul Sokolovsky:

Flavio Santes , thanks for all the comments. Still in my queue to review as try to make my first steps thru Z networking API. And to not open another ticket - any hint how to detect normal "peer closed connection" condition (EOF in terms of BSD/POSIX API)? Grepping thru codebase, uip_closed() is called 2 times: in net_context.c, where it's on write path (and meaning connection reset by peer), and in zperf_tcp_receiver.c, which gives a hint that using uIP API directly is indeed a good idea. Am I missing something?

@zephyrbot
Copy link
Collaborator Author

by Jukka Rissanen:

The existing network API is indeed quite bad for applications needing TCP support as you have noticed. There is nothing preventing you from using uIP API if that helps and fixes the issues you are seeing.
As you noticed, we are planning a new IP stack with BSD socket like API for interfacing the rest of the stack. The new API will not have these issues.

See [include/net/net_context.h|https://gerrit.zephyrproject.org/r/gitweb?p=zephyr.git;a=blob;f=include/net/yaip/net_context.h;h=f4b3295e74f624c58c008e75f3a9ee9e0fc2bf4b;hb=refs/heads/net] for API details.
The implementation can be found at [net_context.c|https://gerrit.zephyrproject.org/r/gitweb?p=zephyr.git;a=blob;f=net/yaip/net_context.c;h=cc75fa78ec095ba8b83b45fbc8c08b7e50a37f3e;hb=refs/heads/net]
Example code for echo-server can be found [here|https://gerrit.zephyrproject.org/r/gitweb?p=zephyr.git;a=blob;f=samples/net/echo_server/src/echo-server-yaip.c;h=1c4c857f426e7dc7dda8fad2593945c4b206fceb;hb=refs/heads/net]

The TCP support for new IP stack is currently work in progress but UDP works if you want to try it.

@zephyrbot
Copy link
Collaborator Author

by Sharron LIU:

Anas Nashif , could you review and decide the priority of this issue detected on existing IP stack? Thanks.

@zephyrbot
Copy link
Collaborator Author

by Paul Sokolovsky:

Just to clarify, I submitted this issue to record some concerns and ask for clarifications on usage of Zephyr networking API. I wouldn't expect that it's possible to go and and "fix" it, but hope that it may provide some feedback on networking API, to help better design its successor based on IP stack, and/or add some missing calls/arguments to the current uIP-based implementation.

I already find the answer above helpful, and was able to make rough implementation of client side BSD-like sockets API on top of Z networking (I can't report 100% success yet, as myself and other folks still looking what may cause deeper networking issues like only first packet being sent out in case of TCP/IPv4, etc.)

@zephyrbot
Copy link
Collaborator Author

by Jukka Rissanen:

This is for legacy stack in 1.6 and won't be fixed there.
The 1.7 will have a new IP stack that will have a proper TCP support.

@zephyrbot
Copy link
Collaborator Author

by Sharron LIU:

Paul Sokolovsky , could you review comments from Jukka? the issue will be fixed in new IP stack target V1.7. Thanks.

@zephyrbot
Copy link
Collaborator Author

by Paul Sokolovsky:

I definitely agree with such resolution, and to that end, also abandoned half-finished patches I submitted for the old stack: https://gerrit.zephyrproject.org/r/#/c/4823/ , https://gerrit.zephyrproject.org/r/#/c/5004/ . It makes sense to concentrate only on the native stack.

Sharron LIU , please feel free to set that as Verified if that's what's needed. Thanks.

@zephyrbot
Copy link
Collaborator Author

by Mark Linkmeyer:

Fixing incorrect priority

@zephyrbot zephyrbot added priority: low Low impact/importance bug area: Networking bug The issue is a bug, or the PR is fixing a bug labels Sep 23, 2017
@zephyrbot zephyrbot mentioned this issue Sep 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

2 participants