Skip to content

Conversation

@pfalcon
Copy link
Contributor

@pfalcon pfalcon commented Sep 6, 2018

net_tcp_queue_pkt() is a single point where packets are added to
tcp->sent_list, and this happens from application thread. This
list is also accessed (iterated on, items removed) from a number
of delayed work handlers. These handlers are serialized among
themselves. Thus, the only issue is serializing access to this
list between net_tcp_queue_pkt() running in app thread and
delayes works running in own cooperatively scheduled thread.

This patch does this, and as adding to the list is very quick
operation, it just uses irq_lock(), as interrupt jitter added
is very minimal and usage of mutex would likely give the same
amount of jitter anyway.

Signed-off-by: Paul Sokolovsky [email protected]

net_tcp_queue_pkt() is a single point where packets are added to
tcp->sent_list, and this happens from application thread. This
list is also accessed (iterated on, items removed) from a number
of delayed work handlers. These handlers are serialized among
themselves. Thus, the only issue is serializing access to this
list between net_tcp_queue_pkt() running in app thread and
delayes works running in own cooperatively scheduled thread.

This patch does this, and as adding to the list is very quick
operation, it just uses irq_lock(), as interrupt jitter added
is very minimal and usage of mutex would likely give the same
amount of jitter anyway.

Signed-off-by: Paul Sokolovsky <[email protected]>
@pfalcon
Copy link
Contributor Author

pfalcon commented Sep 6, 2018

This is resubmit of code which initially appeared in #9592, but it was suggested to split it off.

Besides description in the commit message, see also previous discussion in #9592 (comment)

@codecov-io
Copy link

Codecov Report

Merging #9819 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9819      +/-   ##
==========================================
+ Coverage   52.24%   52.25%   +<.01%     
==========================================
  Files         212      212              
  Lines       25949    25951       +2     
  Branches     5593     5593              
==========================================
+ Hits        13558    13560       +2     
  Misses      10155    10155              
  Partials     2236     2236
Impacted Files Coverage Δ
subsys/net/ip/tcp.c 61.11% <100%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19845a7...e763d6f. Read the comment docs.

@pfalcon
Copy link
Contributor Author

pfalcon commented Dec 5, 2018

@jukkar, @tbursztyka: ping

Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

I think it is wrong to use irq_lock() here, it is just too heavy operation to be done here.
The #8674 tried to implement locking in net_context but it requires still a bit more TLC and testing.

@pfalcon
Copy link
Contributor Author

pfalcon commented Feb 7, 2019

Assumed to be superseded by #8674.

@pfalcon pfalcon closed this Feb 7, 2019
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.

3 participants