Skip to content
This repository has been archived by the owner on Aug 26, 2024. It is now read-only.

make TX and RX could be in seperate go routines #12

Merged
merged 1 commit into from
Jul 12, 2021
Merged

make TX and RX could be in seperate go routines #12

merged 1 commit into from
Jul 12, 2021

Conversation

hujun-open
Copy link
Contributor

split Socket.freeDescs and Socket.getDescs into two instances for TX and RX, so the TX and RX could be in separate go routine

…and RX, so the TX and RX could be in separate go routine
// if rx is true, return desc in first half of umem, 2nd half otherwise
func (xsk *Socket) GetDescs(n int, rx bool) []Desc {
if n > cap(xsk.getRXDescs) {
n = cap(xsk.getRXDescs)
Copy link
Contributor

@crandles crandles Jun 21, 2021

Choose a reason for hiding this comment

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

i notice you're still accessing the RXDesc fields if rx == false (here and in the following code) before referencing the TX vars.

I think it might read better to only reference the appropriate fields (depending on rx):

  • if rx { ... } else { .... }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume you mean line 558

start = cap(xsk.getRXDescs)

this is to determine where is start index of TX, the idea is both freeTXDescs and freeRXDescs has length of options.NumFrames, but freeRXDescs only uses index 0 to options.FillRingNumDescs and freeTXDescs only use options.FillRingNumDesc onward;
the upside of this approach is not introducing too much changes to the code, the downside is of course part of freeTXDescs and freeRXDescs are never used; but given these twojust bool slice, so memory waste here is quite small

xsk.freeTXDescs[i] = true
}
xsk.getTXDescs = make([]Desc, 0, options.CompletionRingNumDescs)
xsk.getRXDescs = make([]Desc, 0, options.FillRingNumDescs)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change to use FillRing / CompletionRing when creating the desc slices is interesting.

Still trying to reason about what it changes -- for instance, should NumFrames == CompletionRingNumDescs + FillRingNumDescs / or are they different? Do we still need a separate NumFrames option, ... ? Need to review AF_XDP docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, this PR is based on assumption that NumFrames == CompletionRingNumDescs + FillRingNumDescs; if NumFrames < CompletionRingNumDescs + FillRingNumDescs, then I don't think it gona work since NumFrames is the max number of available decs for both TX and RX; on theory NumFrames could > CompletionRingNumDescs + FillRingNumDescs, but I don't see the use case of that

@slavc slavc merged commit b4f19b9 into asavie:master Jul 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants