| Message ID | 1444944484-117214-2-git-send-email-jrajahalme@nicira.com |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Oct 16, 2015 at 6:28 AM, Jarno Rajahalme <jrajahalme@nicira.com> wrote: > While the presence of the '--bundle' option implicitly added the > OpenFlow 1.4 to the allowed protocols, it failed to remove OpenFlow > 1.0 from the allowed protocols. This is changed so that '--bundle' > option now also implicitly removes versions lesser than 1.4 from the > allowed protocols. This has no behavioral difference when ovs-ofctl > is paired with OVS that supports OpenFlow 1.4, as the greatest common > version is negotiated, but prevents negotiation of OpenFlow 1.0 when > OVS does not support OpenFlow 1.4. > > Found by inspection. > > Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com> Acked-by: YAMAMOTO Takashi <yamamoto@midokura.com> > --- > tests/ofproto.at | 12 ++++++------ > tests/ovs-ofctl.at | 8 ++++---- > utilities/ovs-ofctl.c | 3 +++ > 3 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/tests/ofproto.at b/tests/ofproto.at > index 5e4441c..4c6dd29 100644 > --- a/tests/ofproto.at > +++ b/tests/ofproto.at > @@ -3962,8 +3962,8 @@ vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > @@ -4014,8 +4014,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > @@ -4045,8 +4045,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at > index 6f03adb..7375cad 100644 > --- a/tests/ovs-ofctl.at > +++ b/tests/ovs-ofctl.at > @@ -2889,8 +2889,8 @@ AT_CHECK([print_vconn_debug | vconn_windows_sub | ofctl_strip], [0], [dnl > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > @@ -2926,8 +2926,8 @@ vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): > vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): > importance=1, dl_vlan=1 actions=drop > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c > index fbc9da4..ee15e1a 100644 > --- a/utilities/ovs-ofctl.c > +++ b/utilities/ovs-ofctl.c > @@ -312,6 +312,9 @@ parse_options(int argc, char *argv[]) > /* Add implicit allowance for OpenFlow 1.4. */ > add_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( > OFPUTIL_P_OF14_OXM)); > + /* Remove all prior versions. */ > + mask_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( > + OFPUTIL_P_OF14_UP)); > } > versions = get_allowed_ofp_versions(); > version_protocols = ofputil_protocols_from_version_bitmap(versions); > -- > 2.1.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
> On Oct 15, 2015, at 8:32 PM, Takashi Yamamoto <yamamoto@midokura.com> wrote: > > On Fri, Oct 16, 2015 at 6:28 AM, Jarno Rajahalme <jrajahalme@nicira.com <mailto:jrajahalme@nicira.com>> wrote: >> While the presence of the '--bundle' option implicitly added the >> OpenFlow 1.4 to the allowed protocols, it failed to remove OpenFlow >> 1.0 from the allowed protocols. This is changed so that '--bundle' >> option now also implicitly removes versions lesser than 1.4 from the >> allowed protocols. This has no behavioral difference when ovs-ofctl >> is paired with OVS that supports OpenFlow 1.4, as the greatest common >> version is negotiated, but prevents negotiation of OpenFlow 1.0 when >> OVS does not support OpenFlow 1.4. >> >> Found by inspection. >> >> Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com <mailto:jrajahalme@nicira.com>> > > Acked-by: YAMAMOTO Takashi <yamamoto@midokura.com <mailto:yamamoto@midokura.com>> > Thanks for the review, pushed to master. Jarno >> --- >> tests/ofproto.at | 12 ++++++------ >> tests/ovs-ofctl.at | 8 ++++---- >> utilities/ovs-ofctl.c | 3 +++ >> 3 files changed, 13 insertions(+), 10 deletions(-) >> >> diff --git a/tests/ofproto.at b/tests/ofproto.at >> index 5e4441c..4c6dd29 100644 >> --- a/tests/ofproto.at >> +++ b/tests/ofproto.at >> @@ -3962,8 +3962,8 @@ vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: >> vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> - version bitmap: 0x01, 0x05 >> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) >> + version bitmap: 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) >> vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> bundle_id=0 type=OPEN_REQUEST flags=atomic ordered >> vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> @@ -4014,8 +4014,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: >> vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> - version bitmap: 0x01, 0x05 >> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) >> + version bitmap: 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) >> vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> bundle_id=0 type=OPEN_REQUEST flags=atomic ordered >> vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> @@ -4045,8 +4045,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: >> vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> - version bitmap: 0x01, 0x05 >> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) >> + version bitmap: 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) >> vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> bundle_id=0 type=OPEN_REQUEST flags=atomic ordered >> vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at >> index 6f03adb..7375cad 100644 >> --- a/tests/ovs-ofctl.at >> +++ b/tests/ovs-ofctl.at >> @@ -2889,8 +2889,8 @@ AT_CHECK([print_vconn_debug | vconn_windows_sub | ofctl_strip], [0], [dnl >> vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> - version bitmap: 0x01, 0x05 >> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) >> + version bitmap: 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) >> vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> bundle_id=0 type=OPEN_REQUEST flags=atomic ordered >> vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> @@ -2926,8 +2926,8 @@ vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> - version bitmap: 0x01, 0x05 >> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) >> + version bitmap: 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) >> vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): >> vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): >> importance=1, dl_vlan=1 actions=drop >> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c >> index fbc9da4..ee15e1a 100644 >> --- a/utilities/ovs-ofctl.c >> +++ b/utilities/ovs-ofctl.c >> @@ -312,6 +312,9 @@ parse_options(int argc, char *argv[]) >> /* Add implicit allowance for OpenFlow 1.4. */ >> add_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( >> OFPUTIL_P_OF14_OXM)); >> + /* Remove all prior versions. */ >> + mask_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( >> + OFPUTIL_P_OF14_UP)); >> } >> versions = get_allowed_ofp_versions(); >> version_protocols = ofputil_protocols_from_version_bitmap(versions); >> -- >> 2.1.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> http://openvswitch.org/mailman/listinfo/dev <http://openvswitch.org/mailman/listinfo/dev>
diff --git a/tests/ofproto.at b/tests/ofproto.at index 5e4441c..4c6dd29 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -3962,8 +3962,8 @@ vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -4014,8 +4014,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -4045,8 +4045,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 6f03adb..7375cad 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -2889,8 +2889,8 @@ AT_CHECK([print_vconn_debug | vconn_windows_sub | ofctl_strip], [0], [dnl vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -2926,8 +2926,8 @@ vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): importance=1, dl_vlan=1 actions=drop diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index fbc9da4..ee15e1a 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -312,6 +312,9 @@ parse_options(int argc, char *argv[]) /* Add implicit allowance for OpenFlow 1.4. */ add_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( OFPUTIL_P_OF14_OXM)); + /* Remove all prior versions. */ + mask_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( + OFPUTIL_P_OF14_UP)); } versions = get_allowed_ofp_versions(); version_protocols = ofputil_protocols_from_version_bitmap(versions);
While the presence of the '--bundle' option implicitly added the OpenFlow 1.4 to the allowed protocols, it failed to remove OpenFlow 1.0 from the allowed protocols. This is changed so that '--bundle' option now also implicitly removes versions lesser than 1.4 from the allowed protocols. This has no behavioral difference when ovs-ofctl is paired with OVS that supports OpenFlow 1.4, as the greatest common version is negotiated, but prevents negotiation of OpenFlow 1.0 when OVS does not support OpenFlow 1.4. Found by inspection. Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com> --- tests/ofproto.at | 12 ++++++------ tests/ovs-ofctl.at | 8 ++++---- utilities/ovs-ofctl.c | 3 +++ 3 files changed, 13 insertions(+), 10 deletions(-)