[openflowplugin-dev] sanity check of flows to OF1.3 Spec
|
Brent Salisbury <brent.salisbury@...>
Hey Chris, I did a quick audit last weekend and started patching a couple of them. I will clean the list up tonight and reply with them this evening. Fwiw it's a good low hanging fruit for folks that are new to Java, ODL or dev in general to get their feet wet. Will include instructions if anyone is interested. Thanks Chris! Brent On May 30, 2014, at 4:57 PM, Christopher O'SHEA <christopher.o.shea@...> wrote:
|
|
|
|
Abhijit Kumbhare
Sounds a good idea Brent! On Fri, May 30, 2014 at 2:21 PM, Brent Salisbury <brent.salisbury@...> wrote:
|
|
|
|
Christopher O'SHEA <christopher.o.shea@...>
Yea Brent awesome idea.
Would love to get my hands dirty, so +1 to the instructions.
Chris
From: Abhijit Kumbhare <abhijitkoss@...>
Date: Friday, May 30, 2014 at 2:41 PM To: Brent Salisbury <brent.salisbury@...> Cc: Ericsson <christopher.o.shea@...>, "integration-dev@..." <integration-dev@...>, "openflowplugin-dev@..." <openflowplugin-dev@...> Subject: Re: [integration-dev] [openflowplugin-dev] sanity check of flows to OF1.3 Spec Sounds a good idea Brent!
|
|
|
|
Ed Henry <networkn3rd@...>
+1 as well. I'd love to tackle low hanging fruit like this as well. -Ed On May 30, 2014 6:18 PM, "Christopher O'SHEA" <christopher.o.shea@...> wrote:
|
|
|
|
Brent Salisbury <brent.salisbury@...>
Excellent, I feel a vHackfest coming on! Let’s get some content together and get rolling! -B From: Ed Henry <networkn3rd@...> Date: Friday, May 30, 2014 at 7:07 PM To: Christopher O'SHEA <christopher.o.shea@...> Cc: Abhijit Kumbhare <abhijitkoss@...>, "openflowplugin-dev@..." <openflowplugin-dev@...>, Brent Salisbury <brent.salisbury@...>, "integration-dev@..." <integration-dev@...> Subject: Re: [openflowplugin-dev] [integration-dev] sanity check of flows to OF1.3 Spec +1 as well. I'd love to tackle low hanging fruit like this as well. -Ed On May 30, 2014 6:18 PM, "Christopher O'SHEA" <christopher.o.shea@...> wrote:
|
|
|
|
Zoltán Lajos Kis <zoltan.lajos.kis@...>
Hi,
Sanity checking is indeed a great idea. But I think it would be important to clearly understand what needs to be sanity checked and what rather needs to be sanitized, or treated otherwise.
In my understanding the whole point of SAL (and MD-SAL in particular) is to provide an abstract interface to the user, hiding internals and low-level details of southbound protocols. So the user program the abstraction, and some ODL magic will turn that into an actual southbound implementation.
In this specific case we program a FlowCapable node, and not an OpenFlow 1.3 switch. If ODL is configured so, our FlowCapable flow entries will be routed to the OpenFlow plugin, which will transform them to OF 1.3 flows. I know this might sound awkward, as the current ODL abstractions tend to map 1:1 to southbound protocols. But it is quite important to understand the long term implications of forcing such a detail of a specific southbound protocol on the abstract model (especially w.r.t. expanding the scope of the abstraction later).
For example, considering the prerequisite rules of OF 1.3 (see the “IPv6 ND target but not matching on ICMPv6 type code of 135/136” example below), forcing the same prerequisite rules on the abstract FlowCapable nodes is only one possibility. But it is also possible not to enforce those, but rather have OpenFlow Plugin sanitize flow entries by adding the missing match fields.
All I wanted to point out (and perhaps I did this in an overtly long way) is that we have all the freedom of defining the semantics of the FlowCapable node abstraction. We don’t have to blindly copy those of OpenFlow 1.3 – as it might even hurt in the long run. All we need to consider is being able to transform from our abstract semantics to those of the southbounds’ in a sane way.
BR, Zoltan
From: integration-dev-bounces@... [mailto:integration-dev-bounces@...]
On Behalf Of Brent Salisbury
Sent: Saturday, May 31, 2014 1:15 AM To: Ed Henry; Christopher O'SHEA Cc: integration-dev@...; openflowplugin-dev@... Subject: Re: [integration-dev] [openflowplugin-dev] sanity check of flows to OF1.3 Spec
Excellent, I feel a vHackfest coming on! Let’s get some content together and get rolling! -B
From:
Ed Henry <networkn3rd@...>
+1 as well. I'd love to tackle low hanging fruit like this as well. -Ed On May 30, 2014 6:18 PM, "Christopher O'SHEA" <christopher.o.shea@...> wrote: Yea Brent awesome idea.
Would love to get my hands dirty, so +1 to the instructions.
Chris
From:
Abhijit Kumbhare <abhijitkoss@...>
Sounds a good idea Brent!
On Fri, May 30, 2014 at 2:21 PM, Brent Salisbury <brent.salisbury@...> wrote: Hey Chris, I did a quick audit last weekend and started patching a couple of them. I will clean the list up tonight and reply with them this evening. Fwiw it's a good low hanging fruit for folks that are new to Java, ODL or dev in general to get their feet wet. Will include instructions if anyone is interested.
Thanks Chris! Brent
|
|
|
|
Abhijit Kumbhare
Zoltan, That's a fair point - especially the following: forcing the same prerequisite rules on the abstract FlowCapable nodes is only one possibility. But it is also possible not to enforce those, but rather have OpenFlow Plugin sanitize flow entries by adding the missing match fields. Having the OF Plugin sanitize the flow entries by adding missing match fields sounds a good idea but we will also need very careful consideration (as you say) - as we do not want to be auto-completing the match fields that the user did not quite intend & has no easy way of overriding the auto-completion. In general - we may want to do the autocompletion only for the flow match field prerequisites (see 7.2.3.6 Flow Match Field Prerequisite from the 1.3 spec).
Thanks, Abhijit On Sat, May 31, 2014 at 5:58 AM, Zoltán Lajos Kis <zoltan.lajos.kis@...> wrote:
|
|
|
|
Brent Salisbury <brent.salisbury@...>
Sorry for the delay, meant to get this done Friday night, I’ve been grinding through some sticking points this weekend on trying to extend OXMs. Here is a rough draft for a start hacking on the tests. I will add a couple posts and a couple screencasts as soon as time clears up this week. ### Setup your ODL username in Gerrit ### ---------------------------------------------------------- 1. Setup your ODL account https://identity.opendaylight.org/carbon/admin/login.jsp 2. Log into https://git.opendaylight.org/gerrit/#/admin/projects/ 3. Setup your ssh key with your credentials. - Links to get you going: * https://wiki.opendaylight.org/view/OpenDaylight_Controller:Gerrit_Setup * https://help.github.com/articles/generating-ssh-keys * https://help.github.com/articles/set-up-git ### Clone the openflowplugin project ### ------------------------------------------------------- 1. Clone the project: git clone ssh://<odl_gerrit_username>@git.opendaylight.org:29418/openflowplugin.git 2. cd openflowplugin 3. download the SCP hooks. This isn't required but it will make it easier for you in tracking Gerrit-IDs. In the openflowplugin project directory run: scp -p -P 29418 <odl_gerrit_username>@git.opendaylight.org:hooks/commit-msg .git/hooks/ 4. Build the project using Maven (v3.04+ or greater): mvn clean install -DskipTests -Dmaven.compile.fork=true 5. Import the project into your IDE (I personally build and run from the CLI rather then trying to do that within an IDE). 6. In your IDE find the class OpenflowpluginTestCommandProvider and look around. org.opendaylight.openflowplugin.test.OpenflowpluginTestCommandProvider located in: ~/openflowplugin/test-provider/src/main/java/org/opendaylight/openflowplugin/test/OpenflowpluginTestCommandProvider.java 7. Each test function in the class are referenced in the case statements with an 'f' followed by a unique number like so: case "f14": id += 14; flow.setMatch(createMatch1().build()); flow.setInstructions(createAppyActionInstruction7().build()); break; case "f15": id += 15; flow.setMatch(createMatch1().build()); flow.setInstructions(createAppyActionInstruction8().build()); break; 8. Each method has a match and instruction (list of actions in OF v1.1+) call. E.g. "flow.setMatch(...)" and "flow.setInstructions(...)" ### Run OpenFlow TestCommandProvider ### --------------------------------------------------------------- 1. cd ~/openflowplugin/distribution/base/target/distributions-openflowplugin-base-0.0.3-SNAPSHOT-osgipackage/opendaylight 2. ./run.sh ### Test FooNodes() ### 1. This will test the OpenFlow builders in the project. It doesn't however test installing a flowmod into a datapath, it merely tests that the ofplugin as able to build the pseudo json structure that would eventually be serialzed into a real vswitch or physwich. 2. In the OSGI CLI, run the following: addMDFlow foo:node:14 3. The output should look something like the following Gist https://gist.github.com/24a0d7a7ca9f5dc78142 ** you can copy the output and use your favorite Json editor to fancy it into a readable structure or head to http://jsonlint.com and run it through the validator to format it. It will fail JSON validation of course but will still format it into a readable tree. 4. When you find on that fails, then you can dig in and patch it! ### Test on a real datapath ### ----------------------------------------- 1. You can also test on a real datapath. Install OVS and setup the bridge and define the IP address where your controller is running. $ sudo yum install openvswitch || sudo apt-get install openvswitch $ (only needed after a reboot, install will start the services magicaly) service openvswitch start $ sudo ovs-vsctl add-br br-int $ sudo ovs-vsctl set bridge br-int protocols=OpenFlow13 $ sudo ovs-vsctl set-controller br-int tcp:172.16.86.1:6633 $ sudo ovs-vsctl show *Should look something like this: Bridge br-int Controller "tcp:172.16.86.1:6633" is_connected: true Port br-int Interface br-int type: internal 2. Now you can test the CommandProvider methods but first you need to get the DPID of your OVS datapath. 3. An easy way is to start the controller, verify it is attached in the OVS "sudo ovs-vsctl show" like above. 4. Type "printNodes" in the OSGI CLI like so: osgi> printNodes Nodes connected to this controller : [MD_SAL|openflow:108448937000521] 5. 108448937000521 is the datapath ID (DPID) of your ovs instance. 6. with the DPID you can now craft the test command like so: addMDFlow openflow:108448937000521 f14 7. Run that and check OVS to see if the flow installed with: sudo ovs-ofctl -O OpenFlow13 dump-flows br-int 8. I setup a doc and started adding some of the results from when I went through the Foo() methods last week. I don't know if it makes sense tracking them somewhere or not. http://goo.gl/BlTCYm ### Patch a bug ### --------------------------- 1. Add the change you want to make to fix an issue. An example being this patch. https://git.opendaylight.org/gerrit/#/c/7371/1 2. I am replaying that patch as an example. From the openflowplugin directory, type 'git diff' to show your local changes and the difference from the ODL upstream repository branch you checked out, likely 'HEAD'/'master'. openflowplugin # git diff diff --git a/test-provider/src/main/java/org/opendaylight/openflowplugin/test/OpenflowpluginTestCommandProvider.java b/test-provider/src/main/java/org/opendaylight/openflowplugin/test/OpenflowpluginTestComma index 620efed..aaf96b0 100644 --- a/test-provider/src/main/java/org/opendaylight/openflowplugin/test/OpenflowpluginTestCommandProvider.java +++ b/test-provider/src/main/java/org/opendaylight/openflowplugin/test/OpenflowpluginTestCommandProvider.java @@ -1262,8 +1262,7 @@ public class OpenflowpluginTestCommandProvider implements CommandProvider { ActionBuilder ab = new ActionBuilder(); SetVlanPcpActionBuilder pcp = new SetVlanPcpActionBuilder(); - // the code point is a 3-bit(0-7) field representing the frame priority level - VlanPcp pcp1 = new VlanPcp((short) 4); + VlanPcp pcp1 = new VlanPcp((short) 9); pcp.setVlanPcp(pcp1); ab.setAction(new SetVlanPcpActionCaseBuilder().setSetVlanPcpAction(pcp.build()).build()); ab.setKey(new ActionKey(0)); 3. From the openflowplugin directory type 'git status' to view what files have changed and what is not yet committed. openflowplugin # git status HEAD detached at FETCH_HEAD Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git checkout -- <file>..." to discard changes in working directory) modified: test-provider/src/main/java/org/opendaylight/openflowplugin/test/OpenflowpluginTestCommandProvider.java no changes added to commit (use "git add" and/or "git commit -a") 4. Once you are ready to submit the patch, you need to add what files you want committed with 'git add /path/to/changed/file/foobar.class'. openflowplugin # git add test-provider/src/main/java/org/opendaylight/openflowplugin/test/OpenflowpluginTestCommandProvider.java 5. You now need to commit the change. openflowplugin # git commit --signoff 6. This will take you to a vim editor. Here type in a relevant first line that is what is visible in Gerrit and then relevant info in the body. When complete save and exit. If you make a typo, you can restart with :q! (vi commands) ------------------------------------------------------------------------------- Bug 1071: Invalid value used for frame priority code point in OFCommandProvider -Adjusted to a magic number in bounds for PCP (0-7) to enable the test to stop failing for the function f80 in OpenFlowPluginTestCommandProvider. Change-Id: Ib146f228e174b6785dfe9fecf93fa7cabba79f4f Signed-off-by: Brent Salisbury <brent.salisbury@...> ------------------------------------------------------------------------------- [detached HEAD 58f622d] Bug 1071: Invalid value used for frame priority code point in OFCommandProvider 1 file changed, 1 insertion(+), 2 deletions(-) // view your local git branch status after committing the changes: openflowplugin # git status HEAD detached from FETCH_HEAD nothing to commit, working directory clean //Git logs are very handy also: openflowplugin # git status HEAD detached from FETCH_HEAD nothing to commit, working directory clean openflowplugin # git log commit 58f622d142e36deae5c8b8d303de4c01a30d8678 Author: Brent Salisbury <brent.salisbury@...> Date: Mon Jun 2 03:02:38 2014 -0400 Bug 1071: Invalid value used for frame priority code point in OFCommandProvider -Adjusted to a magic number in bounds for PCP (0-7) to enable the test to stop failing for the function f80 in OpenFlowPluginTestCommandProvider. Change-Id: Ib146f228e174b6785dfe9fecf93fa7cabba79f4f 7. Now you can push the patch. Gerrit has a super handy feature which is "drafts" that you will see a link for in the web ui. https://git.opendaylight.org/gerrit/#/q/is:draft,n,z When starting out I recommend getting used to using the ODL git/gerrit repository there. You are the only one that can see it unless you explicitly add a reviewer. From drafts you can use the 'publish draft' button to submit to the master for that branch. This and Github are your friends for learning Git/Gerrit workflows. -Drafts: git push ssh://brentsalisbury@...:29418/openflowplugin.git HEAD:refs/drafts/master -or if your patch is ready for a merge review push directly to master: git push ssh://<username>@git.opendaylight.org:29418/controller.git HEAD:refs/for/master 8. Lastly, if you need to patch your already pushed commit and you need to modify if for example a reviewer asked you to change something, you can do that with the following: git commit --signoff --amend 9. --amend will reopen the commit message. This is where the "scp" command above was useful, if you added commit hooks your change-id will be in that message automatically. If it isn't, don't freak out, we can simply go to the initial commit in the Gerrit web page and look for: Change-Id: Ib146f228e174b6785dfe9fecf93fa7cabba79f4f Screencap: https://www.dropbox.com/s/r4ua9eqzdqwsdad/Screenshot%202014-06-02%2003.14.39.png 10. Simply copy and paste that into your commit amendment, optionally, but useful, add a brief description of the new changes with the apatch-setmend and save the patch. 11. Push the commit again, and as long as you are using the same Change-ID, you will see a new patch-set on the original commit in Gerrit like so: https://www.dropbox.com/s/zsezzw8je6wshf5/Screenshot%202014-06-02%2003.16.11.png haven't 12. Lastly if your patch has been waiting on review for too long, or you haven't had time to get back and patch it, your commit may come out of sync with the upstream branch. If this happens, there are a number of ways to solve this ranging in varying levels of insanity, but the easiest is to simply rebase using 'git pull --rebase origin master' (or some variant of rebase depending on interactivity desired). openflowplugin # git pull --rebase origin master From ssh://git.opendaylight.org:29418/openflowplugin * branch master -> FETCH_HEAD First, rewinding head to replay your work on top of it... Applying: Bug 1071: Invalid value used for frame priority code point in OFCommandProvider 13. Git applies an algorithm that combines the latest upstream code and your code to synchronize them magicaly. Thats it. Please don't hesitate to hit me or any of the dozens of awesome ODL devs in the IRC channels up for assistance if you get stuck. Cheers, -Brent From: Brent Salisbury <brent.salisbury@...> Date: Friday, May 30, 2014 at 7:14 PM To: Ed Henry <networkn3rd@...>, Christopher O'SHEA <christopher.o.shea@...> Cc: Abhijit Kumbhare <abhijitkoss@...>, "openflowplugin-dev@..." <openflowplugin-dev@...>, "integration-dev@..." <integration-dev@...> Subject: Re: [openflowplugin-dev] [integration-dev] sanity check of flows to OF1.3 Spec Excellent, I feel a vHackfest coming on! Let’s get some content together and get rolling! -B From: Ed Henry <networkn3rd@...> Date: Friday, May 30, 2014 at 7:07 PM To: Christopher O'SHEA <christopher.o.shea@...> Cc: Abhijit Kumbhare <abhijitkoss@...>, "openflowplugin-dev@..." <openflowplugin-dev@...>, Brent Salisbury <brent.salisbury@...>, "integration-dev@..." <integration-dev@...> Subject: Re: [openflowplugin-dev] [integration-dev] sanity check of flows to OF1.3 Spec +1 as well. I'd love to tackle low hanging fruit like this as well. -Ed On May 30, 2014 6:18 PM, "Christopher O'SHEA" <christopher.o.shea@...> wrote:
|
|
|
|
Brent Salisbury <brent.salisbury@...>
Totally agree. Only issue I have heard from some is that the implementations can vary between hardware implementations. I tend to regard OVS as the reference implementation purely due to the significant adoption and production usage. The ofplugin APIs are pretty verbose with boilerplate builders. They are also pretty raw with regard to defaults/constructors adding missing values. We ran into that with controller punts last week expecting setMax not being defined to default to a usable value but it defaulted to (0). In the Neutron plugin we added some abstractions for the overlay operations and passed relevant details as method parameters. Nothing fancy but it helps a bit. I have barely used the AD_SAL since we dove straight into OFv1.3 but I thought the AD_SAL FRM did some of the obvious things for you. I submitted a patch on a pre-req in this vein to fix a pre-req that OVS with throwing missing pre-req on in this patch I need to revisit to understand Michal’s comments https://git.opendaylight.org/gerrit/#/c/7373/ This is the only reference for the OFv1.3 Java APIs we have in ODL. I think they should be pristine. Count me in. -Brent From: Abhijit Kumbhare <abhijitkoss@...> Date: Sunday, June 1, 2014 at 4:21 PM To: Zoltán Lajos Kis <zoltan.lajos.kis@...> Cc: Brent Salisbury <brent.salisbury@...>, Ed Henry <networkn3rd@...>, Christopher O'SHEA <christopher.o.shea@...>, "integration-dev@..." <integration-dev@...>, "openflowplugin-dev@..." <openflowplugin-dev@...> Subject: Re: [integration-dev] [openflowplugin-dev] sanity check of flows to OF1.3 Spec Zoltan, That's a fair point - especially the following: forcing the same prerequisite rules on the abstract FlowCapable nodes is only one possibility. But it is also possible not to enforce those, but rather have OpenFlow Plugin sanitize flow entries by adding the missing match fields. Having the OF Plugin sanitize the flow entries by adding missing match fields sounds a good idea but we will also need very careful consideration (as you say) - as we do not want to be auto-completing the match fields that the user did not quite intend & has no easy way of overriding the auto-completion. In general - we may want to do the autocompletion only for the flow match field prerequisites (see 7.2.3.6 Flow Match Field Prerequisite from the 1.3 spec). Thanks, Abhijit On Sat, May 31, 2014 at 5:58 AM, Zoltán Lajos Kis <zoltan.lajos.kis@...> wrote:
|
|
|
|
Ed Warnicke (eaw) <eaw@...>
OMG yes! This!
toggle quoted message
Show quoted text
This is an extension of ‘provide good defaults’. Currently the OpenFlow plugin provides defaults for many parameters (very open to discussion
about what the correct defaults are).
I would tend to say that in the event that the sanitization is protocol specific (for example, OF requiring certain L2 level matches as prerequisites to
IP matches… I suspect your are favoring having the OpenFlow plugin add that if missing… which strikes me as reasonable and also ultra friendly to
consumers).
My tendency would be that we should only be doing sanitization/checking at the FRM level that are actually generically applicable to flow programming
and handling the protocol specific sanitization down to the OpenFlow plugin. I think there’s a *very* interesting discussion to be had around which is which :)
Ed
On May 31, 2014, at 7:58 AM, Zoltán Lajos Kis <zoltan.lajos.kis@...> wrote:
|
|
|
|
Zoltán Lajos Kis <zoltan.lajos.kis@...>
I’m talking about a lot more than just setting defaults. To give a simple example, let’s say I want to match on all HTTP traffic.
With OpenFlow today I need to add two separate rules: one for IPv4 and one for IPv6 (thanks to the prerequisite stuff*). Which to me, as a developer, smells like SB details creeping into my control logic.
At the FlowCapable level however I could imagine that I install a single rule matching on “tcp_dst=80”, and have some *ODL magic* that cleverly installs two OF 1.3 for this. Of course if we start to look closer, we might find a lot of details here that might render the whole idea impossible (e.g., how do you estimate the table size); but I think this is something worth investigating before artificially limiting the FlowCapable scope to OF 1.3.
I haven’t delved into the code enough to have an opinion on where sanitization should be. I guess the top of FRM should still be on FlowCapable level, and the bottom of OpenFlowPlugin on the OpenFlow level. Then the rest is a long religious war between the strategy and dispatcher patterns :o)
Btw, I’m not sure how you can do sanitization at the FRM level. I would assume the same level of sanitization can already be done on the model level (i.e., by specifying constraints on the FlowCapable node yang model).
Also, another interesting aspect is the operational state. Should it reflect the config state (so the user can see that the rules he wanted are installed), or should it reflect the actual SB state (which might come handy for debugging)?
Best, Zoltan
* I believe the OXM match syntax could have been a lot more descriptive then what it turned out to be. It could potentially describe arbitrary deep matches. For example in an MPLS L3VPN I could just say “eth_type=0x8847, mpls_bos=1, mpls_nibble=4, ipv4_dst=10.0.0.1”, et voila: I’m matching on the IPv4 fields of a packet below the MPLS layer. Of course this train is long gone, because the prerequisite rules break the possibility of chaining fields of the same layer. Also by now the ODL FlowCapable model assumes that there’s only one layer of a kind, so that can’t be retrofitted to this either.
From: Ed Warnicke (eaw) [mailto:eaw@...]
Sent: Monday, June 02, 2014 6:01 PM To: Zoltán Lajos Kis Cc: Brent Salisbury; Ed Henry; Christopher O'SHEA; integration-dev@...; openflowplugin-dev@... Subject: Re: [integration-dev] [openflowplugin-dev] sanity check of flows to OF1.3 Spec
OMG yes! This!
This is an extension of ‘provide good defaults’. Currently the OpenFlow plugin provides defaults for many parameters (very open to discussion about what the correct defaults are).
I would tend to say that in the event that the sanitization is protocol specific (for example, OF requiring certain L2 level matches as prerequisites to IP matches… I suspect your are favoring having the OpenFlow plugin add that if missing… which strikes me as reasonable and also ultra friendly to consumers).
My tendency would be that we should only be doing sanitization/checking at the FRM level that are actually generically applicable to flow programming and handling the protocol specific sanitization down to the OpenFlow plugin. I think there’s a *very* interesting discussion to be had around which is which :)
Ed On May 31, 2014, at 7:58 AM, Zoltán Lajos Kis <zoltan.lajos.kis@...> wrote:
Hi, Sanity checking is indeed a great idea. But I think it would be important to clearly understand what needs to be sanity checked and what rather needs to be sanitized, or treated otherwise.
In my understanding the whole point of SAL (and MD-SAL in particular) is to provide an abstract interface to the user, hiding internals and low-level details of southbound protocols. So the user program the abstraction, and some ODL magic will turn that into an actual southbound implementation.
In this specific case we program a FlowCapable node, and not an OpenFlow 1.3 switch. If ODL is configured so, our FlowCapable flow entries will be routed to the OpenFlow plugin, which will transform them to OF 1.3 flows. I know this might sound awkward, as the current ODL abstractions tend to map 1:1 to southbound protocols. But it is quite important to understand the long term implications of forcing such a detail of a specific southbound protocol on the abstract model (especially w.r.t. expanding the scope of the abstraction later).
For example, considering the prerequisite rules of OF 1.3 (see the “IPv6 ND target but not matching on ICMPv6 type code of 135/136” example below), forcing the same prerequisite rules on the abstract FlowCapable nodes is only one possibility. But it is also possible not to enforce those, but rather have OpenFlow Plugin sanitize flow entries by adding the missing match fields.
All I wanted to point out (and perhaps I did this in an overtly long way) is that we have all the freedom of defining the semantics of the FlowCapable node abstraction. We don’t have to blindly copy those of OpenFlow 1.3 – as it might even hurt in the long run. All we need to consider is being able to transform from our abstract semantics to those of the southbounds’ in a sane way.
BR, Zoltan
From: integration-dev-bounces@...
[mailto:integration-dev-bounces@...] On Behalf Of Brent Salisbury
Excellent, I feel a vHackfest coming on! Let’s get some content together and get rolling! -B
From: Ed Henry <networkn3rd@...>
+1 as well. I'd love to tackle low hanging fruit like this as well. -Ed On May 30, 2014 6:18 PM, "Christopher O'SHEA" <christopher.o.shea@...> wrote: Yea Brent awesome idea.
Would love to get my hands dirty, so +1 to the instructions.
Chris
From: Abhijit Kumbhare <abhijitkoss@...>
Sounds a good idea Brent!
On Fri, May 30, 2014 at 2:21 PM, Brent Salisbury <brent.salisbury@...> wrote: Hey Chris, I did a quick audit last weekend and started patching a couple of them. I will clean the list up tonight and reply with them this evening. Fwiw it's a good low hanging fruit for folks that are new to Java, ODL or dev in general to get their feet wet. Will include instructions if anyone is interested.
Thanks Chris! Brent
_______________________________________________
|
|
|
|
Abhijit Kumbhare
Makes sense (FRM level having a more abstract rule - which is broken down into multiple specific rules in the plugin). At least theoretically - implementation wise how complicated this will be remains to be seen.
On Mon, Jun 2, 2014 at 10:00 AM, Zoltán Lajos Kis <zoltan.lajos.kis@...> wrote:
|
|
|
|
Tony Tkacik -X (ttkacik - Pantheon Technologies SRO@Cisco) <ttkacik@...>
+1
FRM (configuration model) should be more abstract, high-level then plugin level definition of flow, Since capabilities and functionality may yeld different use-cases (e.g. FRM configuration model does not support cookies, bariers, openflowplugin level APIs Does support them).
Tony
From: openflowplugin-dev-bounces@... [mailto:openflowplugin-dev-bounces@...]
On Behalf Of Abhijit Kumbhare
Sent: Tuesday, June 03, 2014 12:38 AM To: Zoltán Lajos Kis Cc: integration-dev@...; openflowplugin-dev@... Subject: Re: [openflowplugin-dev] [integration-dev] sanity check of flows to OF1.3 Spec
Makes sense (FRM level having a more abstract rule - which is broken down into multiple specific rules in the plugin). At least theoretically - implementation wise how complicated this will be remains to be seen.
On Mon, Jun 2, 2014 at 10:00 AM, Zoltán Lajos Kis <zoltan.lajos.kis@...> wrote: I’m talking about a lot more than just setting defaults. To give a simple example, let’s say I want to match on all HTTP traffic.
With OpenFlow today I need to add two separate rules: one for IPv4 and one for IPv6 (thanks to the prerequisite stuff*). Which to me, as a developer, smells like SB details creeping into my control logic.
At the FlowCapable level however I could imagine that I install a single rule matching on “tcp_dst=80”, and have some *ODL magic* that cleverly installs two OF 1.3 for this. Of course if we start to look closer, we might find a lot of details here that might render the whole idea impossible (e.g., how do you estimate the table size); but I think this is something worth investigating before artificially limiting the FlowCapable scope to OF 1.3.
I haven’t delved into the code enough to have an opinion on where sanitization should be. I guess the top of FRM should still be on FlowCapable level, and the bottom of OpenFlowPlugin on the OpenFlow level. Then the rest is a long religious war between the strategy and dispatcher patterns :o)
Btw, I’m not sure how you can do sanitization at the FRM level. I would assume the same level of sanitization can already be done on the model level (i.e., by specifying constraints on the FlowCapable node yang model). Also, another interesting aspect is the operational state. Should it reflect the config state (so the user can see that the rules he wanted are installed), or should it reflect the actual SB state (which might come handy for debugging)?
Best, Zoltan
* I believe the OXM match syntax could have been a lot more descriptive then what it turned out to be. It could potentially describe arbitrary deep matches. For example in an MPLS L3VPN I could just say “eth_type=0x8847, mpls_bos=1, mpls_nibble=4, ipv4_dst=10.0.0.1”, et voila: I’m matching on the IPv4 fields of a packet below the MPLS layer. Of course this train is long gone, because the prerequisite rules break the possibility of chaining fields of the same layer. Also by now the ODL FlowCapable model assumes that there’s only one layer of a kind, so that can’t be retrofitted to this either.
From: Ed
Warnicke (eaw) [mailto:eaw@...]
OMG yes! This!
This is an extension of ‘provide good defaults’. Currently the OpenFlow plugin provides defaults for many parameters (very open to discussion about what the correct defaults are).
I would tend to say that in the event that the sanitization is protocol specific (for example, OF requiring certain L2 level matches as prerequisites to IP matches… I suspect your are favoring having the OpenFlow plugin add that if missing… which strikes me as reasonable and also ultra friendly to consumers).
My tendency would be that we should only be doing sanitization/checking at the FRM level that are actually generically applicable to flow programming and handling the protocol specific sanitization down to the OpenFlow plugin. I think there’s a *very* interesting discussion to be had around which is which :)
Ed On May 31, 2014, at 7:58 AM, Zoltán Lajos Kis <zoltan.lajos.kis@...> wrote:
Hi, Sanity checking is indeed a great idea. But I think it would be important to clearly understand what needs to be sanity checked and what rather needs to be sanitized, or treated otherwise.
In my understanding the whole point of SAL (and MD-SAL in particular) is to provide an abstract interface to the user, hiding internals and low-level details of southbound protocols. So the user program the abstraction, and some ODL magic will turn that into an actual southbound implementation.
In this specific case we program a FlowCapable node, and not an OpenFlow 1.3 switch. If ODL is configured so, our FlowCapable flow entries will be routed to the OpenFlow plugin, which will transform them to OF 1.3 flows. I know this might sound awkward, as the current ODL abstractions tend to map 1:1 to southbound protocols. But it is quite important to understand the long term implications of forcing such a detail of a specific southbound protocol on the abstract model (especially w.r.t. expanding the scope of the abstraction later).
For example, considering the prerequisite rules of OF 1.3 (see the “IPv6 ND target but not matching on ICMPv6 type code of 135/136” example below), forcing the same prerequisite rules on the abstract FlowCapable nodes is only one possibility. But it is also possible not to enforce those, but rather have OpenFlow Plugin sanitize flow entries by adding the missing match fields.
All I wanted to point out (and perhaps I did this in an overtly long way) is that we have all the freedom of defining the semantics of the FlowCapable node abstraction. We don’t have to blindly copy those of OpenFlow 1.3 – as it might even hurt in the long run. All we need to consider is being able to transform from our abstract semantics to those of the southbounds’ in a sane way.
BR, Zoltan
From: integration-dev-bounces@...
[mailto:integration-dev-bounces@...] On Behalf Of Brent Salisbury
Excellent, I feel a vHackfest coming on! Let’s get some content together and get rolling! -B
From: Ed Henry <networkn3rd@...>
+1 as well. I'd love to tackle low hanging fruit like this as well. -Ed On May 30, 2014 6:18 PM, "Christopher O'SHEA" <christopher.o.shea@...> wrote: Yea Brent awesome idea.
Would love to get my hands dirty, so +1 to the instructions.
Chris
From: Abhijit
Kumbhare <abhijitkoss@...>
Sounds a good idea Brent!
On Fri, May 30, 2014 at 2:21 PM, Brent Salisbury <brent.salisbury@...> wrote: Hey Chris, I did a quick audit last weekend and started patching a couple of them. I will clean the list up tonight and reply with them this evening. Fwiw it's a good low hanging fruit for folks that are new to Java, ODL or dev in general to get their feet wet. Will include instructions if anyone is interested.
Thanks Chris! Brent
_______________________________________________
|
|
|