[ovs-dev] [netlink v4 40/52] datapath: Convert ODP_FLOW_* and ODP_EXECUTE to put dp_idx into message.

Ben Pfaff blp at nicira.com
Tue Jan 18 22:18:48 UTC 2011


On Mon, Jan 17, 2011 at 04:42:37PM -0800, Jesse Gross wrote:
> On Mon, Jan 17, 2011 at 4:15 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Mon, Jan 17, 2011 at 12:22:22PM -0500, Jesse Gross wrote:
> >> On Wed, Jan 12, 2011 at 12:49 AM, Ben Pfaff <blp at nicira.com> wrote:
> >> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> >> > index 4456ec8..ff2a6b8 100644
> >> > --- a/datapath/datapath.c
> >> > +++ b/datapath/datapath.c
> >> > @@ -1619,6 +1662,30 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd,
> >> > + ? ? ? case ODP_FLOW_FLUSH:
> >> > + ? ? ? ? ? ? ? err = flush_flows(argp);
> >> > + ? ? ? ? ? ? ? goto exit;
> >>
> >> I don't see the corresponding userspace changes to support this. ?It
> >> looks like argp will always be NULL.
> >
> > You're talking about ODP_FLOW_FLUSH? ?There's no userspace interface
> > change for this. ?argp in this case is a dp_idx, not a pointer. ?Yes,
> > that's not like any of the other flow functions, but I didn't see a
> > reason to change its interface yet.
> 
> It doesn't look like that is actually true:
> 
> static int
> dpif_linux_flow_flush(struct dpif *dpif_)
> {
>     return do_ioctl(dpif_, ODP_FLOW_FLUSH, NULL);
> }
> 
> Previously dp_idx was coming off the minor number, now it's coming
> from argp which is always NULL, aka datapath 0.

Oh.  Sorry for spouting nonsense, you're right.

I applied the following incremental to the patch:

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index ba69dc0..90f7bd0 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -377,7 +377,8 @@ dpif_linux_get_max_ports(const struct dpif *dpif OVS_UNUSED)
 static int
 dpif_linux_flow_flush(struct dpif *dpif_)
 {
-    return do_ioctl(dpif_, ODP_FLOW_FLUSH, NULL);
+    struct dpif_linux *dpif = dpif_linux_cast(dpif_);
+    return ioctl(dpif->fd, ODP_FLOW_FLUSH, dpif->minor) ? errno : 0;
 }
 
 struct dpif_linux_port_state {

Thanks for the other comments too.




More information about the dev mailing list