Skip to content

Conversation

polybassa
Copy link
Contributor

This should reduce packet losses in ISOTPSoftSocket

Copy link
Member

Choose a reason for hiding this comment

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

Do you think you could remove the timeout, then the whole while loop, to simply use sniff ?

The stop_filter will take care of exiting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understood the stop_filter, an exit can only happen, if a message is received.
If this is the case, I can not close the socket, if no further message is received. Sending a "fake message" just to trigger the execution of stop_filter inside close won't work, since I can not inject a message into the CAN socket.
Do you see a possibility to trigger the execution of stop_filter without a receive of a message?

Copy link
Member

Choose a reason for hiding this comment

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

Correct... I had a prototype #1523 of a such implementation, but that wasnt clean enough (now it's outdated).

I'll come back to you if i get any additional options available

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I would be happy, if I could avoid that timeout.

@codecov-io
Copy link

codecov-io commented Dec 22, 2018

Codecov Report

Merging #1756 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1756      +/-   ##
==========================================
+ Coverage   85.51%   85.53%   +0.01%     
==========================================
  Files         181      181              
  Lines       42201    42195       -6     
==========================================
+ Hits        36089    36091       +2     
+ Misses       6112     6104       -8
Impacted Files Coverage Δ
scapy/contrib/isotp.py 88.86% <100%> (-0.21%) ⬇️
scapy/sendrecv.py 83.13% <0%> (+0.17%) ⬆️
scapy/layers/inet6.py 88.26% <0%> (+0.23%) ⬆️
scapy/automaton.py 82.39% <0%> (+0.55%) ⬆️

@polybassa polybassa force-pushed the ISOTPSoftSocketImprovement branch from 789bf55 to 19ab270 Compare December 22, 2018 17:12
This should remove packet losses in ISOTPSoftSocket
@polybassa polybassa force-pushed the ISOTPSoftSocketImprovement branch from 19ab270 to d9ebe12 Compare December 23, 2018 10:15
Copy link
Member

@p-l- p-l- left a comment

Choose a reason for hiding this comment

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

@gpotter2 your call!

@gpotter2
Copy link
Member

It's still ugly, but better than what's in place ATM

@gpotter2 gpotter2 merged commit d2ec55f into secdev:master Dec 23, 2018
@polybassa
Copy link
Contributor Author

I totally agree.

@polybassa polybassa deleted the ISOTPSoftSocketImprovement branch February 11, 2019 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants