Obviously I'm leaning toward having it fixed in v1.2 as well.
Reasoning is as below:
1.The fix does not affect wire protocol as the value of the error code
has always been correct. OF agent talking to Ryu shall not notice the
change.
2.A Ryu application directly handles the error code need update but this
is simply replacing OFPQCFC_EPERM with OFPSCFC_EPERM. Sound and safe.
3.Most importantly, having the fix in all OF protocols allows Ryu
applications to treat the error code consistently without exceptional
handling for OF v1.2.
Thanks,
Shu
On Wed, Feb 11, 2015 at 8:11 PM, Yusuke Iwase <[email protected]>
wrote:
> Hi,
>
> Thank you for your patch.
>
> As you say, EXT-208 was included in Openflow v1.3.1,
> but Openflow v1.2 dose not include this change.
> So, I wonder if ofproto_v1_2.py need to be fixed like OF1.3+ ...
>
> Thanks
>
> On 2015年02月12日 09:05, Shu Shen wrote:
> > Invalid error code OFPQCFC_EPERM should be OFPSCFC_EPERM instead. This
> > is EXT-208 that was included in Openflow v1.3.1
> >
> > Signed-off-by: Shu Shen <[email protected]>
> > ---
> > ryu/ofproto/ofproto_v1_2.py | 2 +-
> > ryu/ofproto/ofproto_v1_3.py | 2 +-
> > ryu/ofproto/ofproto_v1_4.py | 2 +-
> > ryu/tests/unit/ofproto/test_ofproto_v12.py | 2 +-
> > ryu/tests/unit/ofproto/test_parser_v12.py | 4 ++--
> > 5 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/ryu/ofproto/ofproto_v1_2.py b/ryu/ofproto/ofproto_v1_2.py
> > index fd5f1cf..8063689 100644
> > --- a/ryu/ofproto/ofproto_v1_2.py
> > +++ b/ryu/ofproto/ofproto_v1_2.py
> > @@ -738,7 +738,7 @@ OFPQOFC_EPERM = 2 # Permissions error.
> > # enum ofp_switch_config_failed_code
> > OFPSCFC_BAD_FLAGS = 0 # Specified flags is invalid.
> > OFPSCFC_BAD_LEN = 1 # Specified len is invalid.
> > -OFPQCFC_EPERM = 2 # Permissions error.
> > +OFPSCFC_EPERM = 2 # Permissions error.
> >
> > # enum ofp_role_request_failed_code
> > OFPRRFC_STALE = 0 # Stale Message: old generation_id.
> > diff --git a/ryu/ofproto/ofproto_v1_3.py b/ryu/ofproto/ofproto_v1_3.py
> > index d07bad9..e532d09 100644
> > --- a/ryu/ofproto/ofproto_v1_3.py
> > +++ b/ryu/ofproto/ofproto_v1_3.py
> > @@ -1021,7 +1021,7 @@ OFPQOFC_EPERM = 2 # Permissions error.
> > # enum ofp_switch_config_failed_code
> > OFPSCFC_BAD_FLAGS = 0 # Specified flags is invalid.
> > OFPSCFC_BAD_LEN = 1 # Specified len is invalid.
> > -OFPQCFC_EPERM = 2 # Permissions error.
> > +OFPSCFC_EPERM = 2 # Permissions error.
> >
> > # enum ofp_role_request_failed_code
> > OFPRRFC_STALE = 0 # Stale Message: old generation_id.
> > diff --git a/ryu/ofproto/ofproto_v1_4.py b/ryu/ofproto/ofproto_v1_4.py
> > index bb045d8..6542b6f 100644
> > --- a/ryu/ofproto/ofproto_v1_4.py
> > +++ b/ryu/ofproto/ofproto_v1_4.py
> > @@ -850,7 +850,7 @@ OFPQOFC_EPERM = 2 # Permissions error.
> > # enum ofp_switch_config_failed_code
> > OFPSCFC_BAD_FLAGS = 0 # Specified flags is invalid.
> > OFPSCFC_BAD_LEN = 1 # Specified len is invalid.
> > -OFPQCFC_EPERM = 2 # Permissions error.
> > +OFPSCFC_EPERM = 2 # Permissions error.
> >
> > # enum ofp_role_request_failed_code
> > OFPRRFC_STALE = 0 # Stale Message: old generation_id.
> > diff --git a/ryu/tests/unit/ofproto/test_ofproto_v12.py
> b/ryu/tests/unit/ofproto/test_ofproto_v12.py
> > index d6280a6..c1228a0 100644
> > --- a/ryu/tests/unit/ofproto/test_ofproto_v12.py
> > +++ b/ryu/tests/unit/ofproto/test_ofproto_v12.py
> > @@ -630,7 +630,7 @@ class TestOfprot12(unittest.TestCase):
> > def test_enum_ofp_switch_config_failed_code(self):
> > eq_(OFPSCFC_BAD_FLAGS, 0)
> > eq_(OFPSCFC_BAD_LEN, 1)
> > - eq_(OFPQCFC_EPERM, 2)
> > + eq_(OFPSCFC_EPERM, 2)
> >
> > def test_enum_ofp_role_request_failed_code(self):
> > eq_(OFPRRFC_STALE, 0)
> > diff --git a/ryu/tests/unit/ofproto/test_parser_v12.py
> b/ryu/tests/unit/ofproto/test_parser_v12.py
> > index 2560fba..13caa52 100644
> > --- a/ryu/tests/unit/ofproto/test_parser_v12.py
> > +++ b/ryu/tests/unit/ofproto/test_parser_v12.py
> > @@ -742,7 +742,7 @@ class TestOFPErrorMsg(unittest.TestCase):
> >
> > def test_parser_p10_2(self):
> > type_ = ofproto.OFPET_SWITCH_CONFIG_FAILED
> > - code = ofproto.OFPQCFC_EPERM
> > + code = ofproto.OFPSCFC_EPERM
> > data = 'Error Message.'
> > self._test_parser(type_, code, data)
> >
> > @@ -1196,7 +1196,7 @@ class TestOFPErrorMsg(unittest.TestCase):
> >
> > def test_serialize_p10_2(self):
> > self._test_serialize_p(ofproto.OFPET_SWITCH_CONFIG_FAILED,
> > - ofproto.OFPQCFC_EPERM)
> > + ofproto.OFPSCFC_EPERM)
> >
> > def test_serialize_p11_0(self):
> > self._test_serialize_p(ofproto.OFPET_ROLE_REQUEST_FAILED,
> >
>
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel