Skip to content

Swift IPv6 Outbound Fragmentation support - #68

Open
agnosticdev wants to merge 3 commits into
mainfrom
agnosticdev/OutboundIPv6Fragmentation
Open

Swift IPv6 Outbound Fragmentation support#68
agnosticdev wants to merge 3 commits into
mainfrom
agnosticdev/OutboundIPv6Fragmentation

Conversation

@agnosticdev

Copy link
Copy Markdown
Collaborator

Adds outbound fragmentation support for Swift IPv6.
IPv6 takes the same approach that IPv4 takes in that it will only attempt to handle fragmentation if needed, otherwise it will use the standard path.

let fragmentLength = UInt16(chunkLength + IPv6Instance.fragmentExtensionHeaderLength)
// Fragment offset flags
let offsetFlags = UInt16(cursor) | (isLast ? 0 : UInt16(IPv6Instance.ip6fMoreFragmentMask))
var fragmentFrame = Frame(count: ipv6CompleteHeaderLength + chunkLength)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hm, this seems concerning — I don't think we can just do this, since we would need to get the frame from the protocol below in cases where the frames aren't allocated to just be unique arrays, but have other backing.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yep, that makes sense. It was a bit more complex than I expected to adopt getDatagramsToSend because IPv4 and IPv6 instances cannot call this function directly on the instance. I added a callback though that gave writeOutboundFrames access to this function. Adopted in 212cfbb

let selfReference = self.effectiveSelfReference
IPInstance.processOutbound(
&self.instanceType,
getDatagramsToSend: { maxCount, minSize in

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If this is being marked on every IP packet send, I worry that passing a new block every time is too heavy. Why not just pass the reference to lower and self reference?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We should not incur any performance hit here unless the block is actually called. Switching to using lower though is a straightforward change so addressed in e3cf0f6

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.

2 participants