Thanks a lot for the info. It would be nice to have it in the initial cover letter just to know the broader idea about the intent :). From the code I could see the direction you want to go with it, but I wanted some confirmation.
Acked-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com> > -----Mesaj original----- > De la: Sairam Venugopal [mailto:vsai...@vmware.com] > Trimis: Monday, July 25, 2016 11:19 PM > Către: Alin Serdean <aserd...@cloudbasesolutions.com>; > dev@openvswitch.org > Subiect: Re: [ovs-dev] [PATCH 7/9] datapath-windows: Add support for > multiple event queue in Event.c > > I added the padding to keep the sizeof(OVS_CT_EVENT_ENTRY) == > sizeof(OVS_VPORT_EVENT_ENTRY) > > > I should have sent this as part of Patch 0. This is the general idea: > > 1. Currently we only support subscribing/unsubscribing to Vport related > events. Even for VPORT, we only care about the VPORT-Down event. > 2. Event.c maintains a single queue of Event entries that are then read by > user space. This worked because we only supported 1 event type. > 3. In order to add support for multiple events, I modified the event queue > into an array (size 2). The size of the event queue array is driven by value > of > enum (we explicitly add new event types here). > 4. I decided to make Event Queue an array to avoid creating new ones for > every event type. This also meant smaller code changes and not having > multiple if-else in the code. > 5. Each event queue can be uniquely identified based on the mcast > EventType and hence the requirement for multiple locks based on the event > id. > 6. Though we may not have multiple event queues subscribed for 1 socket, I > still wanted to ensure that we have support for it if it were ever requested. [Alin Gabriel Serdean: ] That was my main concern because it can be added in the future. Good call :). > 7. Each event type will be added to the underlying enum and subtypes can > be represent by means of masks (eg: vport up/down, ct delete/add/update > etc.,) > /* Supported mcast event groups */ > enum OVS_MCAST_EVENT_ENTRIES { > OVS_MCAST_VPORT_EVENT, > OVS_MCAST_CT_EVENT, > __OVS_MCAST_EVENT_ENTRIES_MAX > }; > > > I hope this clarifies the questions. > > Thanks, > Sairam > > On 7/22/16, 6:10 PM, "Alin Serdean" <aserd...@cloudbasesolutions.com> > wrote: > > >Looks good. Just a few questions from my side so I get up to speed with > >the changes in events. > >Why do we need a lock per eventid ? > > > >> +typedef struct _OVS_CT_EVENT_ENTRY { > >> + OVS_CT_ENTRY entry; > >> + UINT8 type; > >> + UINT64 pad[10]; > >> +} OVS_CT_EVENT_ENTRY, *POVS_CT_EVENT_ENTRY; > > > >Why such a big pad? > > > >Thanks, > >Alin. > > > >> -----Mesaj original----- > >> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Sairam > >>Venugopal > >> Trimis: Thursday, July 14, 2016 2:39 AM > >> Către: dev@openvswitch.org > >> Subiect: [ovs-dev] [PATCH 7/9] datapath-windows: Add support for > >>multiple event queue in Event.c > >> > >> Update Event.c to have multiple event queues and mechanism to > >> retrieve the associated queue. Introduce OvsPostCtEvent and > >> OvsRemoveCtEventEntry similar to OvsPostVportEvent and > >> OvsRemoveVportEventEntry. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
