sockets: Fix IPv6 raw packet checksum in IPv6-only configuration #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Issue: In an IPv6-only setup, raw IPv6 packets were being sent with an incorrect checksum, whereas they worked correctly in a dual-stack configuration (both IPv4 and IPv6). Analysis: * The checksum is calculated in `raw.c` when the `pcb->chksum_reqd` flag is set. * This flag is enabled for all IPv6 raw pcbs in `api_msg.c`, but only when `NETCONNTYPE_ISIPV6(msg->conn->type)` evaluates to true. * The issue arises because the macro `NETCONNTYPE_ISIPV6()` checks if the `NETCONN_TYPE_IPV6` flag is set in the netconn type. This flag wasn’t being set when creating the socket, as the `DOMAIN_TO_NETCONN_TYPE()` macro returned the original type without adding the IPv6 flag. Fix: * Made enum netconn_type reflect protocol family also in IPv6-only configs, updating the macro DOMAIN_TO_NETCONN_TYPE() to set the NETCONN_TYPE_IPV6 flag. Alternative Approach: * Another potential solution would be to modify the `enum netconn_type` to reflect both the connection type and protocol family (as of now) in a dual-stack setup, while reflect connection type only in a single-stack setup (IPv4 only or IPv6 only)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue:
In an IPv6-only setup, raw IPv6 packets were being sent with an incorrect checksum, whereas they worked correctly in a dual-stack configuration (both IPv4 and IPv6).
Analysis:
raw.cwhen thepcb->chksum_reqdflag is set.api_msg.c, but only whenNETCONNTYPE_ISIPV6(msg->conn->type)evaluates to true.NETCONNTYPE_ISIPV6()checks if theNETCONN_TYPE_IPV6flag is set in the netconn type. This flag wasn’t being set when creating the socket, as theDOMAIN_TO_NETCONN_TYPE()macro returned the original type without adding the IPv6 flag.Fix:
DOMAIN_TO_NETCONN_TYPE()to set theNETCONN_TYPE_IPV6flag.Alternative Approach:
enum netconn_typeto reflect both the connection type and protocol family (as of now) in a dual-stack setup, while reflect connection type only in a single-stack setup (IPv4 only or IPv6 only).(i.e. setting
NETCONNTYPE_ISIPV6()totruein IPv6 only mode)PS: saw that you enabled CI runs for new PRs, so maybe you'll accept something from Github, too.
PPS: the main reason for choosing this way of fixing is that the alternative would require semantic changes to the API, but open to suggestions.