[ovs-dev] [PATCH V2] ofproto-dpif-upcall: Remove the dispatcher thread.
Alex Wang
alexw at nicira.com
Wed Feb 19 07:40:20 UTC 2014
Hey Han,
Again, thanks for your comments, ;D
On Tue, Feb 18, 2014 at 10:46 PM, Zhou, Han <hzhou8 at ebay.com> wrote:
> > I think it is okay that we miss the upcalls for a short period.
> >
> >
> > The way current ovs calls 'port_add()' is that ODPP_NONE is given for
> > '*port_nop'.
> > And the datapath will densely allocate the port number and notify it
> > back to
> > userspace. So, there is no way to specify channel in the same
> > transaction in this
> > case. Also, I don't want to separate socket creation from
> > add_vport_channel().
> >
> >
>
> OK, I see. But then the kernel code need to changed, because:
> ovs_dp_process_received_packet() calls ovs_vport_find_pid(). When
> upcall_pids are not set to datapath, ovs_vport_find_pid() returns 0 as a
> nl pid:
> if (!pids)
> return 0;
>
> and then it invokes ovs_dp_upcall() still. I think we need error
> handling here to drop packets in such race condition.
>
>
Inside the current "ovs_dp_upcall()" we have the following code:
277<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l277>int
ovs_dp_upcall(struct datapath *dp, struct sk_buff *skb,
278<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l278>
const struct dp_upcall_info *upcall_info)
279<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l279>{
280<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l280>
struct dp_stats_percpu *stats;
281<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l281>
int err;
282<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l282>
283<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l283>
if (upcall_info->portid == 0) {
284<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l284>
err = -ENOTCONN;
285<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l285>
goto err;
286<http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=datapath/datapath.c;h=a7839a128cae7a6d481fc2b426282b85d4d70ff2;hb=09350a3de320e83517c943858acc397db784d583#l286>
}
So, we have done that already.
> >
> > The logic of calling dpif_recv_set() here is somehow
> > misleading. I
> > understand that it is only for refreshing dpif->n_handlers,
> > rather than
> > enable/disable receiving. But would it be better to introduce
> > a new
> > interface in dpif to clearly update dpif->n_handler, which can
> > be
> > invoked in udpif_set_threads().
> >
> >
> >
> >
> >
> > I'm okay with either. will ask around then update with you.
> >
>
> May I add that there is one more benefit if invoking a new interface in
> udpif_set_threads(): we can handle the update more gracefully in
> udpif_set_threads() to minimize traffic interruption:
> 1. destroy old threads
> 2. update datapath pids via the new interface
> 3. create new threads
>
> Best regards,
> Han
>
I'll explain this in discussion, thanks,
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140218/f5f11edb/attachment-0005.html>
More information about the dev
mailing list