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

feat: allow for larger incoming NGC packets #2380

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

zoff99
Copy link

@zoff99 zoff99 commented Apr 4, 2023

This change is Reviewable

@JFreegman
Copy link
Member

What's the purpose of this and when would it be applicable?

@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (8792672) 74.51% compared to head (d9095eb) 74.43%.
Report is 1 commits behind head on master.

❗ Current head d9095eb differs from pull request most recent head cd34b60. Consider uploading reports for the commit cd34b60 to get more accurate results

Files Patch % Lines
toxcore/tox.c 0.00% 4 Missing ⚠️
toxcore/group_chats.c 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2380      +/-   ##
==========================================
- Coverage   74.51%   74.43%   -0.08%     
==========================================
  Files          87       87              
  Lines       26241    26243       +2     
==========================================
- Hits        19554    19535      -19     
- Misses       6687     6708      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iphydf
Copy link
Member

iphydf commented Aug 30, 2023

It might make sense to make toxcore generally more accepting of incoming packets up to some size (e.g. 1500 bytes). On Ethernet, we'll never get that kind of packet size, but it's not bad to be lenient on the inbound packet size. We still need some limit to avoid DoS, but 1500 seems reasonable for any packet processor functions.

Copy link
Member

@iphydf iphydf left a comment

Choose a reason for hiding this comment

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

Incoming packet sizes should generally be quite accepting. We should not allow arbitrary sizes (because DoS), but something upwards of MTU makes sense.

@iphydf iphydf added this to the v0.2.19 milestone Nov 10, 2023
@Green-Sky Green-Sky changed the title allow for larger incoming NGC packets feat: allow for larger incoming NGC packets Nov 12, 2023
@zoff99 zoff99 force-pushed the zoff99/allow_incoming_large_ngc_pkts branch 2 times, most recently from d38664f to b7bca61 Compare November 16, 2023 12:59
Copy link
Member

@iphydf iphydf left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 3 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@iphydf iphydf force-pushed the zoff99/allow_incoming_large_ngc_pkts branch from b7bca61 to cd34b60 Compare November 16, 2023 20:47
@iphydf iphydf merged commit cd34b60 into TokTok:master Nov 16, 2023
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants