noxrepo / pox

The POX network software platform
https://noxrepo.github.io/pox-doc/html/
Apache License 2.0
624 stars 472 forks source link

Python3 support #192

Open dschoonwinkel opened 7 years ago

dschoonwinkel commented 7 years ago

Hi

Are there any plans to port the POX controller to python 3?

If not, what alternatives can you recommend?

Thank you!

Davidmeng78 commented 7 years ago

Hi, Daniel! Very glad to receive your email. I am a PhD candiate in China, major in computer science. now I have constructed of a SDN simulation environemnt based on Python2.7+Mininet2.2+POX, and plan to carry out some research in further. Are you a researcher of Standford University? or a developer of POX team? Yunfei Meng

At 2017-08-02 16:45:07, "Daniel Schoonwinkel" notifications@github.com wrote:

Hi

Are there any plans to port the POX controller to python 3?

If not, what alternatives can you recommend?

Thank you!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

MurphyMc commented 6 years ago

I guess I'll finally need to edit the last sentence of the Python 3 question in the POX FAQ.

The answer is that there are still no concrete plans to do it. Some of the roadblocks to doing it have gone away, though I still haven't heard a compelling reason to do it sooner rather than later. Do you have one?

MurphyMc commented 6 years ago

Just a note that I've been considering this question again. Specifically, I'm considering having fangtooth be the last Python 2.7 version, with the gar branch being based on Python 3. Welcoming input on the subject.

dnck commented 5 years ago

Just a note that I've been considering this question again. Specifically, I'm considering having fangtooth be the last Python 2.7 version, with the gar branch being based on Python 3. Welcoming input on the subject.

I'm a complete newb to POX, but I'm interested in learning it by offering help in porting to Python3.

MurphyMc commented 5 years ago

I don't immediately have much time to put into the effort myself, but my last statement still stands: I'm thinking Python 2 POX may be coming to an end.

If you wanted to put some work in on it, the first thing to do would be to take the current fangtooth branch from my fork, and run it through 2to3.

IIRC, one of the first things that needs fixing up is... in the packet library, there are a bunch of .next methods, which 2to3 thinks are iterator-related and changes to __next__, which isn't right. We can probably just change them back, though this may be a good time to have a bigger conversation about those. I think .payload has been preferred over .next for a while now, but there's still probably a lot of places in the POX code itself that uses .next, and there's a nice duality between prev/next but not between prev/payload. Should .prev change too? To what? .container? (Of course, the whole packet library could use a rewrite, but that's a pretty substantial project of its own.)

Another portability question that immediately comes to mind is pxpcap, the C extension for capturing packets. I have a vague memory of doing some work on it a while back that I hoped would make it easier to port to Python 3, but I'm not sure if a Python 3 compile was ever actually attempted.

gtataranni commented 4 years ago

You might want to consider that Python 2 will reach end of life (EOL) on January 1st, 2020. If possible, I would convert it to Python 3 as first thing, and then later also consider rewrite considerations. I have started a conversion on my fork, although I am totally new to pox, so there might still be other leftovers coming up. So far, I manage to bring it up without errors with ./pox.py samples.pretty_log forwarding.l2_learning on Python 3.7.5, but I haven't tested it any further. Feel free to contribute :)

MurphyMc commented 4 years ago

I still can't afford to put time on this, so I appreciate that you are.

I agree with you -- due to the EOL of Python 2, we should go as straightforward as possible. So my statement about the "bigger conversation" above should probably wait. That means that the "next" functions in the packet library should stay as "next". Looking at your PR (MurphyMc/pox#2), though, I think you let 2to3 do its thing with them, which is wrong here. They shouldn't be __next__, since they're not iterators. So all those should get rolled back.

Also, have you given building pxpcap in Python 3 a try?

larabr commented 4 years ago

I've taken over the porting from @gtataranni , and opened a PR on your fork as well https://github.com/MurphyMc/pox/pull/3 . As mentioned there, to confirm that everything is working fine I need a bit of support/ideas on how to test some modules (I am not familiar with POX). Thanks!

MurphyMc commented 4 years ago

Just a note that this is moving on my side. The above note that POX gar will be Python 3 would seem to be coming true. With any luck at all, this will happen within a month.

MurphyMc commented 4 years ago

I've just pushed a first pass at the POX gar branch, which will be the start of Python 3 support.

Thanks go to @gtataranni and @larabr for getting this started. I tried to incorporate all your changes, though I made a bunch of changes, and also laboriously fixed lots of the dumb things that 2to3 does (and also made a bunch of fixes that 2to3 doesn't). I'm not sure how best to credit you. Maybe a note of thanks for starting the Python 3 port in the README, referencing your github names? Or maybe it's time to start an AUTHORS file?

MurphyMc commented 4 years ago

Any additional fixes / bug reports / PRs to address outstanding issues with the port are heavily encouraged!

gtataranni commented 4 years ago

I guess I'll be happy with a mention in whatever format you deem most appropriate :) Contributions won't be missing if bugs will pop up. Thank you also for your help!

gtataranni commented 4 years ago

@MurphyMc do you think is going to take long before we see this merged upstream?

MurphyMc commented 4 years ago

There are two issues.

The first is just sort of finishing a couple things off and moving it to the main repo. This shouldn't actually take much time/work, but I'm completely overloaded at the moment and POX isn't currently on my critical path, so I haven't been able to spare the time for it. It's possible it'll enter my critical path again somewhere between November and January, which would certainly give me an excuse to take care of this.

The second part is when it should become the default branch. The default POX branch is supposed to be relatively stable, and I'm not sure the Python 3 version is there yet. There's an argument that maybe the Python 3 version should be put up front ASAP given the end of CPython 2. But PyPy continues to support Python 2.

gtataranni commented 4 years ago

What exactly needs to be finished up? Maybe I can help you out there Also, the sooner this becomes the default branch, the sooner we'll get pull request for bugfixes

reyreaud-l commented 4 years ago

Hello @MurphyMc

Do you know what needs to be done ? I can have a look at fixing some python 2 to 3 problems. Looking at the state of the "gar" branch the only things that need change to make all tests pass is the following patch.

Looked like the factory function was not generating the Object it was supposed to and only returned bytes.

From 5be769c122021915e7668259f9410ac314b54fab Mon Sep 17 00:00:00 2001
From: Loic Reyreaud <lre@open.ch>
Date: Tue, 22 Sep 2020 12:07:30 +0200
Subject: [PATCH] Fix factory class method on IPAddr6 to return the object

Signed-off-by: Loic Reyreaud <lre@open.ch>
---
 pox/lib/addresses.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pox/lib/addresses.py b/pox/lib/addresses.py
index 054b29c..f68bd1a 100644
--- a/pox/lib/addresses.py
+++ b/pox/lib/addresses.py
@@ -446,7 +446,7 @@ class IPAddr6 (_AddrBase):
     """
     Factory that creates an IPAddr6 from a large integer
     """
-    return bytes( (num >> i) & 0xff for i in range(120,-8,-8) )
+    return cls( bytes((num >> i) & 0xff for i in range(120,-8,-8) ), raw=True)

   def __init__ (self, addr = None, raw = False, network_order = False):
     """
--
2.28.0

Do you know of other issues ?

MurphyMc commented 4 years ago

@reyreaud-l : Thanks for that patch; it should get incorporated. Can you post it on its own issue or a PR on the MurphyMc fork to make sure it doesn't get lost?

@reyreaud-l and @gtataranni : I think the remaining issue I know of is that I went in and did my own fairly careful passes of my own in quite a rush, and this meant that other people that contributed to the Python 3 port don't show up in the commit log or (as of now) anywhere at all really -- even though some of their changes may have been used (or at least served to point out places where I ended up fixing things differently). This somehow needs to get rectified so that people that contributed are credited, but I have less than zero time to spend on it at the moment.

reyreaud-l commented 4 years ago

I opened a PR here https://github.com/MurphyMc/pox/pull/4 with the patch.

I understand the lack of time to work on such things, we all have different obligations. If it is just a matter of updating the list of people who contributed, we can use the gar branch for now https://github.com/MurphyMc/pox/tree/gar

BTW if there are tasks (coding tasks) where you still need help related to python2 to python3, point them so we can see if we can alleviate the burden of maintaining that and get it done ;)

Thanks for the time you take for answering!

MurphyMc commented 4 years ago

I've taken the current MurphyMc/gar branch and added it here as gar-experimental. Just so it's clear it may not be quite ready for prime time.

I've also made it the default branch, which goes against the POX policy of the default branch being the stable branch, but it seemed warranted because of the death of Python 2.

(I also filed #249, which was something else I'd hoped to do for gar but won't have time for -- this has been the case since carp, I think. 😢 )

MurphyMc commented 4 years ago

Also, #242 is tagged gar.

MurphyMc commented 4 years ago

See also: #250