[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