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

Process DiscardingFlowOp correctly in FastDatapath send() #2008

Closed
wants to merge 1 commit into from

Conversation

dpw
Copy link
Contributor

@dpw dpw commented Feb 29, 2016

The nil FlowOp check in collectFlowOps was not addressed with the
introduction of DiscardingFlowOp. This meant that DiscardingFlowOps
produced in the router core would get handled by fastdp as if they were
foreign FlowOps, preventing flow creation and so leading to unwarranted
ODP misses.

To fix this, collectFlowOps cannot simply eliminate FlowOps for which
Discards() is true, because it's true for odpFlowKeyFlowOp. So the
Discards() case has to be handled in FastDatapath send().

The nil FlowOp check in collectFlowOps was not addressed with the
introduction of DiscardingFlowOp.  This meant that DiscardingFlowOps
produced in the router core would get handled by fastdp as if they were
foreign FlowOps, preventing flow creation and so leading to unwarranted
ODP misses.

To fix this, collectFlowOps cannot simply eliminate FlowOps for which
Discards() is true, because it's true for odpFlowKeyFlowOp.  So the
Discards() case has to be handled in FastDatapath send().
@dpw
Copy link
Contributor Author

dpw commented Feb 29, 2016

Fixes #2003.

@bboreham
Copy link
Contributor

LGTM, but I think we may prefer to create a 1.4 patch release.

@dpw
Copy link
Contributor Author

dpw commented Feb 29, 2016

LGTM, but I think we may prefer to create a 1.4 patch release.

If I tried to follow the formalities, I'd probably forget something important and embarrass myself, so I'll leave that part to your team. This PR is just a way to present the fix.

@bboreham
Copy link
Contributor

Thanks; I'll do the merge

@bboreham bboreham closed this Feb 29, 2016
@rade rade added this to the 1.4.5 milestone Mar 19, 2016
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