Hi Bill,

>> A little difference though, how about doing "encode" in parser classes as the
>> following? If encoding is done in parser, the differences of Python2 and
>> Python3 can be obscured against Ryu users.
>
> Yes, this change looks good to me. It makes sense to handle the ascii
> encoding in the OFPErrorMsg class.

Thanks, I've just posted patches for this issue:
[Ryu-devel] [PATCH 0/3] ofproto: Encode data field on OFPErrorMsg


>> +                if close_socket:
>> +                    # Forces to call _deactivate()
>> +                    raise CloseSocketRequest
>> +        except CloseSocketRequest:
>> +            pass
>
> I didn't understand this part of your change. You throw a
> CloseSocketRequest exception but immediately catch it. This seems like
> the same operation as a "break" or "return" which will drop you down
> to the finally clause. I see that _deactivate is a decorator that
> calls shutdown() on the socket for wrapped methods, but I don't see
> how it can ever be called in this case. _send_loop() is not protected
> by _deactivate.

Oops, you are right. Here should do "break". Thanks a lot!
I used "break" in the above patches.


Thanks,
Iwase


On 2018年02月28日 15:46, William Fisher wrote:
Hi Iwase,

On Mon, Feb 26, 2018 at 10:55 PM, Iwase Yusuke <iwase.yusu...@gmail.com> wrote:

A little difference though, how about doing "encode" in parser classes as
the
following? If encoding is done in parser, the differences of Python2 and
Python3
can be obscured against Ryu users.

Yes, this change looks good to me. It makes sense to handle the ascii
encoding in the OFPErrorMsg class.


Also: Ryu leaves the OF connection open after responding to the hello
failed message. The connection appears to remain in the
HANDSHAKE_DISPATCHER state. What is the best way to close the
connection?

Thanks for your pointing out.
I found that Ryu does not automatically close the OF connection when the
failure
of the version negotiation, so Ryu application need to close it manually.
I think it can be often overlooked and dangerous.

What do you think about adding "close_socket" option to
"Datapath.send_msg()"
method, which will close socket after sending the given message?

The send_msg api looks okay.  I guess there is no easier way to close
and flush the connection.

+                if close_socket:
+                    # Forces to call _deactivate()
+                    raise CloseSocketRequest
+        except CloseSocketRequest:
+            pass

I didn't understand this part of your change. You throw a
CloseSocketRequest exception but immediately catch it. This seems like
the same operation as a "break" or "return" which will drop you down
to the finally clause. I see that _deactivate is a decorator that
calls shutdown() on the socket for wrapped methods, but I don't see
how it can ever be called in this case. _send_loop() is not protected
by _deactivate.

Regards,

-Bill

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Ryu-devel mailing list
Ryu-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ryu-devel


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Ryu-devel mailing list
Ryu-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to