Hi,
(2013/02/20 11:37), Isaku Yamahata wrote:
> General comment. Some of logic could be generic, not specific to
> datapath tracking.
>
Thanks for your comment.
>> +class DatapathDiscovery(app_manager.RyuApp):
>> + _EVENTS = [EventDPEnter, EventDPLeave,
>> + EventDPPortAdd, EventDPPortDelete, EventDPPortModify]
>> +
>> + def __init__(self):
>> + super(DatapathDiscovery, self).__init__()
>> +
>> + self.dpids = [] # datapath_id
>> + self.ports = {} # datapath_id => ports
>> + # OFPP_MAX might be different when using multiple version of
>> OpenFlow?
>> + self._OFPP_MAX = {} # datapath_id => OFPP_MAX
>
> Why not whole ofproto? How about
> self.dpids = dict: dpid -> the pair (ofproto, ports)?
> Thus the logic of _unregister() will be simplified.
> Possibly we'd like to introduce another class to describe the pair.
>
sounds better.
I'll fix and post it.
>> + def _add_port(self, dpid, port_no):
>> + if port_no > self._OFPP_MAX[dpid]:
>> + # LOG.debug('port %s is reserved', port_no)
>> + return
>> +
>> + self.ports.setdefault(dpid, [])
>> + self.ports[dpid].append(port_no)
>
> self.ports.setdefault(dpid, []).append(port_no)
> dict.setdefault() returns value.
>
I see.
>> + @handler.set_ev_cls(ofp_event.EventOFPPortStatus,
>> handler.MAIN_DISPATCHER)
>> + def port_status_handler(self, ev):
>> + msg = ev.msg
>> + reason = msg.reason
>> + dpid = msg.datapath.id
>> + port_no = msg.desc.port_no
>> + ofproto = msg.datapath.ofproto
>> +
>> + if reason == ofproto.OFPPR_ADD:
>> + LOG.debug('port was added.' +
>> + '(datapath id = %s, port number = %s)',
>> + dpid, port_no)
>> + self._add_port(dpid, port_no)
>> + self.send_event_to_observers(EventDPPortAdd(dpid, port_no))
>> +
>> + elif reason == ofproto.OFPPR_DELETE:
>> + LOG.debug('port was deleted.' +
>> + '(datapath id = %s, port number = %s)',
>> + dpid, port_no)
>> + self._del_port(dpid, port_no)
>> + self.send_event_to_observers(EventDPPortDelete(dpid, port_no))
>> +
>> + else:
>> + assert reason == ofproto.OFPPR_MODIFY
>> + LOG.debug('port was modified.' +
>> + '(datapath id = %s, port number = %s)',
>> + dpid, port_no)
>> + self.send_event_to_observers(EventDPPortModify(dpid, port_no))
>
> Doesn't this track port state like link up/down.
>
OK, I'll try to track modify state.
Has OFPPR_MODIFY some parameters?
In dpset.py, PortState.add and PortState.modify are same code.
>> + @handler.set_ev_cls(EventDPListRequest)
>> + def request_handler(self, ev):
>> + dpid = ev.dpid
>> + # LOG.debug(ev)
>> + if dpid is None:
>> + dpids = self.dpids
>> + ports = self.ports
>> + elif dpid in self.dpids:
>> + dpids = [dpid]
>> + ports = [self.ports[dpid]]
>> + else:
>> + dpids = []
>> + ports = {}
>> +
>> + reply = EventDPListReply(self.name, ev, dpids, ports)
>> + self.send_event(ev.src, reply)
>
> This logic can be generalized and can be a method in AppManager or RyuApp.
OK, I'll try to generalize it.
> This needs to be blocked and wait request.
> gevent.event (or equivalent primitive) can be used to blocking.
>
Could you explain it in more detail?
What number of line should be blocked?
>> + def _get_dp_list(self, dpid=None):
>> + request = EventDPListRequest(self.name, dpid)
>> + self.send_event(datapath.DatapathDiscovery.__name__, request)
>> + # assume that replys are in right order
>> + reply = self.replys.get(timeout=self._REQUEST_TIMEOUT)
>> + assert reply.request == request
>> + return reply
>> +
>> + def _request_loop(self):
>> + while self.is_active:
>> + dp_list_all = self._get_dp_list()
>> + LOG.debug('ALL DPList: %s', dp_list_all)
>> + dp1 = self._get_dp_list(1)
>> + LOG.debug('DP1: %s', dp1)
>> + gevent.sleep(10)
>
> I thinks this is just for debug/showing how to use, so it's just ok for now.
> But we need more generic way to snoop event queuing.
> Probably registering observer with the list of (brick name, event class).
>
That's right.
>> +class EventRequestBase(event.EventBase):
>> + def __init__(self, src):
>> + self.src = src # brick name of event source
>
> EventRequestBase and EventReplyBase is generic enough to be in event.py
>
I see.
>> +class EventReplyBase(event.EventBase):
>> + def __init__(self, src, request):
>> + self.src = src # brick name of event source
>> + assert issubclass(request.__class__, EventRequestBase)
> isinstance(request, EventRequestBase)
>
Oh, thanks.
>> + self.request = request
>
> request is an object which is not suitable to be encoded in event.
> How about Event{Request, Reply}Base.xid and use id(request)?
> And more we'd like to a framework to snoop into request/reply.
>
That sounds better.
I'd like to just identify the corresponding request.
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_feb
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel
