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

Added possibility to create one-switch flow for the same port #1488

Merged
merged 8 commits into from Oct 25, 2018

Conversation

@nikitacherevko
Copy link
Collaborator

nikitacherevko commented Oct 23, 2018

No description provided.

@nikitacherevko nikitacherevko force-pushed the issue/one-switch-port-flow branch from 386ccff to 89ca4a3 Oct 23, 2018
* @param ofFactory OF factory for the switch
* @return {@link OFAction}
*/
private OFAction actionSetOutputInPort(final OFFactory ofFactory) {

This comment has been minimized.

Copy link
@surabujin

surabujin Oct 23, 2018

Collaborator

Too many duplication... I think it will be better to change type of second argument of actionSetOutputPort from int to OFPort. And use it for both cases.

@surabujin

This comment has been minimized.

Copy link
Collaborator

surabujin commented Oct 24, 2018

There is one more issue here - controller do not remove rules from switch on flow remove. Altered rule do not match passed remove criteria.

It bring the question - do we really want to support "full match" on delete operations? We should be able to relly on cookie value only (perhaps we should dedicate several bits 2-4 for some kilda-identifier into cookie value to guarantee "kilda-rules only" match).

Copy link

sonarcloud bot left a comment

SonarQube analysis found issues:
Bug Bugs: 0
Vulnerability Vulnerabilities: 0
Code Smell Code Smells: 1

See all issues in SonarCloud

@nikitacherevko nikitacherevko force-pushed the issue/one-switch-port-flow branch from 7b767a9 to 4bffcb5 Oct 24, 2018
Copy link

sonarcloud bot left a comment

SonarQube analysis found issues:
Bug Bugs: 0
Vulnerability Vulnerabilities: 0
Code Smell Code Smells: 1

See all issues in SonarCloud

@@ -189,15 +189,18 @@ def build_one_switch_flow_from_db(switch, stored_flow, output_action):
return flow


def build_delete_flow(switch, flow_id, cookie, meter_id, in_port, in_vlan, out_port):
def build_delete_flow(node):

This comment has been minimized.

Copy link
@surabujin

surabujin Oct 24, 2018

Collaborator

node bad name... there is no associations for it in our domain model. It should be something like flow_segments or path_segment.

@nikitacherevko nikitacherevko force-pushed the issue/one-switch-port-flow branch from 4bffcb5 to d2df68e Oct 25, 2018
Copy link

sonarcloud bot left a comment

SonarQube analysis found issues:
Bug Bugs: 0
Vulnerability Vulnerabilities: 0
Code Smell Code Smells: 1

See all issues in SonarCloud

@nikitacherevko nikitacherevko force-pushed the issue/one-switch-port-flow branch from d2df68e to ce26283 Oct 25, 2018
Copy link

sonarcloud bot left a comment

SonarQube analysis found issues:
Bug Bugs: 0
Vulnerability Vulnerabilities: 0
Code Smell Code Smells: 1

See all issues in SonarCloud

@nikitacherevko nikitacherevko force-pushed the issue/one-switch-port-flow branch from ce26283 to e6981e8 Oct 25, 2018
Copy link

sonarcloud bot left a comment

SonarQube analysis found issues:
Bug Bugs: 0
Vulnerability Vulnerabilities: 0
Code Smell Code Smells: 1

See all issues in SonarCloud

@@ -595,7 +595,10 @@ public String toString() {
result.add(new PathDiscrepancyDto(expected.toString(), "inVlan",
String.valueOf(expected.inVlan), String.valueOf(matched.inVlan)));
}
if (matched.outPort != expected.outPort) {
// FIXME: let's validate in_port output correctly in case of one-switch-port flow.

This comment has been minimized.

Copy link
@sonarcloud

sonarcloud bot Oct 25, 2018

Code Smell Code Smell: Take the required action to fix the issue indicated by this comment. (squid:S1134)

See it in SonarCloud

@nikitacherevko nikitacherevko merged commit 83ff3f4 into develop Oct 25, 2018
4 checks passed
4 checks passed
SonarCloud Code Quality check passed; 33.0% Est. post-merge coverage
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
telstra-jenkins/kilda-ATDD-runner Build 1960: tests PASSED
@nikitacherevko nikitacherevko deleted the issue/one-switch-port-flow branch Oct 25, 2018
@nikitacherevko

This comment has been minimized.

Copy link
Collaborator Author

nikitacherevko commented Nov 22, 2018

closes #1472

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.